otelcolconvert: support converting loki exporter #6505
Conversation
| ) | ||
| diags.Add( | ||
| diag.SeverityLevelInfo, | ||
| fmt.Sprintf("Created a loki.write block with a best-effort conversion of the lokiexporter's confighttp, retry and queue configuration settings. You may want to double check the converted configuration as most fields do not have a 1:1 match"), |
There was a problem hiding this comment.
Should this be a warning instead?
There was a problem hiding this comment.
👍 definitely a warning at minimum
| res := &write.Arguments{} | ||
|
|
||
| if cfg.Endpoint != "" { | ||
| // TODO(@tpaschalis) Wire in auth from auth extension. |
There was a problem hiding this comment.
Not sure if there's a great way to do this yet.
There was a problem hiding this comment.
This is sick! I love seeing a converter which takes an interpretation of the upstream OpenTelemetry Collector components and tries to convert them best-effort into the native equivalents. This is exactly the kind of stuff I thought we'd do to make migrating to Flow easy, and I think we should have this.
That being said, I'm wondering if we should put this on hold until after the more literal components converters are finished (e.g., converters for otelcol components where we have 1:1 equivalents). Especially considering my comment about what will be necessary to handle with auth, it's not clear to me how much remaining dev time it would take to fully finish this, and whether the literal components have a higher priority.
It's up to you what you want to do; I just wanted to raise the idea that maybe this type of converter falls into a different category that may have a different priority.
| if err != nil { | ||
| diags.Add( | ||
| diag.SeverityLevelError, | ||
| fmt.Sprintf("could not build loki.write block: %s", err), | ||
| ) | ||
| } | ||
| block2 := common.NewBlockWithOverride([]string{"loki", "write"}, label, args2) |
There was a problem hiding this comment.
I'm pretty sure common.NewBlockWithOverride will panic if err != nil here, since args2 == nil if err != nil.
| args1 := toOtelcolExporterLoki(lokiWriteComponentID) | ||
| block1 := common.NewBlockWithOverride([]string{"otelcol", "exporter", "loki"}, label, args1) |
There was a problem hiding this comment.
naming nit: can we give thiese more descriptive names than args1/block1/args2/block2 please? 😛
| ) | ||
| diags.Add( | ||
| diag.SeverityLevelInfo, | ||
| fmt.Sprintf("Created a loki.write block with a best-effort conversion of the lokiexporter's confighttp, retry and queue configuration settings. You may want to double check the converted configuration as most fields do not have a 1:1 match"), |
There was a problem hiding this comment.
nit: This makes it sound like a different event from the above diagnostic; can we use diags.AddWithDetail on the first one and add the best-effort notice as the detail line?
| res := &write.Arguments{} | ||
|
|
||
| if cfg.Endpoint != "" { | ||
| // TODO(@tpaschalis) Wire in auth from auth extension. |
There was a problem hiding this comment.
(Just a note)
😬 Handling auth here will be tricky, because we can't pass otelcol.auth.basic to loki.write; we'll have to actually probe the auth extension that lokiexporter was using and copy settings from the definition of the auth extension into loki.write.
Signed-off-by: Paschalis Tsilias <[email protected]>
Signed-off-by: Paschalis Tsilias <[email protected]>
tpaschalis
force-pushed
the
convert-loki-exporter
branch
from
092cb20
to
9616a53
Compare
rfratto
added
variant/flow
|
This PR has not had any activity in the past 30 days, so the |
github-actions
bot
added
the
needs-attention
|
I'm going to close this for now. We can listen to feedback to see if this is still needed and if so, port this to Alloy with feedback addressed. |
github-actions
bot
added
the
frozen-due-to-age
Fixes grafana/alloy#243