feat(influx2otel): add an option to set name separator - #341
Conversation
To other reviewers: this PR alone doesn't fix the issue, someone needs to bump the dependency in the telegraf repo to pull this in after merging. |
|
@helenosheaa Hi, could you take a look at this? Thanks |
|
@skartikey Hi, could you take a look at this? In relation to influxdata/telegraf#19156 |
| }, nil | ||
| } | ||
|
|
||
| func NewLineProtocolToOtelMetricsWithSeparator(logger common.Logger, nameSeparator string) (*LineProtocolToOtelMetrics, error) { |
There was a problem hiding this comment.
[question] Could this take a config struct instead of a separator arg? otel2influx hit the same thing and went with OtelMetricsToLineProtocolConfig plus DefaultOtelMetricsToLineProtocolConfig() (metrics.go:14, same shape in logs.go and traces.go), so there's a pattern to copy. Same number of new exported names either way, since the old constructor has to stay regardless, but the next knob becomes a struct field instead of another constructor name, and contrib and telegraf both import this so those names are permanent. It'd also handle "" for free, which today silently gives you measurementfield. Not blocking if you'd rather ship it as is.
There was a problem hiding this comment.
Good point, i got rid of the new constructor in 6ad72fb in favor of a config struct
|
|
||
| assertMetricsEqual(t, expect, b.GetMetrics()) | ||
| } | ||
| func TestAddPoint_v1_sum_separator(t *testing.T) { |
There was a problem hiding this comment.
[nit] The three new test funcs have no blank line between them, 595 and 660 butt straight up against the previous closing brace. gofmt doesn't care, just easier to scan with one. Ignore if you like.
|
@skartikey Hi, could you take a look? |
3 participants
Adds nameSeparator field to LineProtocolToOtelMetrics and a new constructor with custom separator as an argument. This is needed in order to preserve metric names when switching from graphite output plugin to OTel output plugin in telegraf.
Fixes telegraf#18251