feat(outputs.opentelemetry): Add a config option for specifying name/field separator#19156
Conversation
telegraf-tiger
Bot
added
feat
|
@srebhan Hi, could you take a look at this? Thanks. |
skartikey
left a comment
skartikey
left a comment
There was a problem hiding this comment.
Docs: the new otel_name_separator option isn't in sample.conf or README.md. Please add it to sample.conf and run make docs to regenerate the README.
|
|
||
| godebug x509negativeserial=1 | ||
|
|
||
| replace github.com/influxdata/influxdb-observability/influx2otel => github.com/mchrome/influxdb-observability/influx2otel v0.0.0-20260623155732-db200540d054 |
There was a problem hiding this comment.
This can't land on master. The real behavior lives in influxdb-observability#341, so that one needs to be reviewed, merged, and released first; then bump influx2otel here and drop the replace. Note influx2otel, common, and otel2influx are versioned together (currently v0.5.12), so the bump will likely move all three.
There was a problem hiding this comment.
Got you. Is there anything i can do to get that PR reviewed and merged?
| Coralogix *CoralogixConfig `toml:"coralogix"` | ||
| Timeout config.Duration `toml:"timeout"` | ||
| Compression string `toml:"compression"` | ||
| OtelNameSeparator string `toml:"otel_name_separator"` |
There was a problem hiding this comment.
otel_name_separator repeats "otel". name_separator (or just separator) reads better. Config is user-facing API and renaming after release is breaking, so worth settling now.
| require.JSONEq(t, string(expectJSON), string(gotJSON)) | ||
| } | ||
|
|
||
| func TestOpenTelemetrySeparator(t *testing.T) { |
There was a problem hiding this comment.
the new tests only cover .. Backward compatibility relies on the default _ path through Connect() producing the same output as before, but nothing asserts that (the existing default tests inject the converter directly / use the old constructor). A test that calls Connect() with the default and asserts mem_used_percent would lock that in.
There was a problem hiding this comment.
| require.JSONEq(t, string(expectJSON), string(gotJSON)) | ||
| } | ||
|
|
||
| func TestOpenTelemetrySeparatorHTTPJSON(t *testing.T) { |
There was a problem hiding this comment.
the io.ReadAll error branch returns without w.WriteHeader(http.StatusInternalServerError), unlike the protobuf variant.
…in HTTPJson tests
|
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
2 participants
Summary
This PR adds an option to specify a separator for LineProtocol's name/field when converting to OpenTelemtry format, similar to how it is done in graphite's output plugin with graphite_separator option.
This PR relies on changes in a dependency: influxdata/influxdb-observability#341. From my understanding, the team behind telegraf has write access there too, so it would be great if you could review that one also. Until that PR is merged, i replaced the dependency "infuxdb-observability/influx2otel" in go.mod with my fork of it, so tests could pass.
For context: there was another attempt by me to fix this issue here #18680, but after the review i realized it was not the right approach. Sorry for not getting back at you there.
Checklist
Related issues
resolves #18251