prometheusexporter: Expose new otlptranslator options#43050
Conversation
Adds underscore_label_sanitization and preserve_multiple_underscores to config options. This allows users who were relying on previous default translation behavior to maintain metric name continuity. Signed-off-by: Owen Williams <owen.williams@grafana.com>
Signed-off-by: Owen Williams <owen.williams@grafana.com>
| // UnderscoreLabelSanitization, if true, enables prepending 'key' to labels | ||
| // starting with '_'. Reserved labels starting with `__` are not modified. | ||
| // Included for compatibility with default previous behavior. | ||
| // | ||
| // Deprecated: This will be removed in a future version. | ||
| UnderscoreLabelSanitization bool `mapstructure:"underscore_label_sanitization"` | ||
|
|
||
| // PreserveMultipleUnderscores enables preserving of multiple | ||
| // consecutive underscores in label names when UTF8Allowed is false. | ||
| // This option is discouraged as it violates the OpenTelemetry to Prometheus | ||
| // specification https://github.com/open-telemetry/opentelemetry-specification/blob/v1.38.0/specification/compatibility/prometheus_and_openmetrics.md#otlp-metric-points-to-prometheus), | ||
| // but may be needed for compatibility with legacy systems that rely on the old behavior. | ||
| PreserveMultipleUnderscores bool `mapstructure:"preserve_multiple_underscores"` |
There was a problem hiding this comment.
This is already controlled by a feature flag, see #40604
We shouldn't have two ways of doing the same thing 🤔
There was a problem hiding this comment.
oh, hm, I forgot we'd already made it a flag -- we still need to expose the underscores setting though, right?
There was a problem hiding this comment.
Well, the OTel collector never exposed multiple underscores, so I don't see why make that possible now 🤔
This multiple-underscore option was created because Mimir needed it
There was a problem hiding this comment.
ok, then would you say nobody should get broken by this upgrade in a way that they can't work around? if so, yay! But we still need to make sure we are loud about the possibility of breakage so that people can debug it quickly
There was a problem hiding this comment.
I have a feeling people will get broken because we haven't stitched together the feature-flag with the code written in otlptranslator.
I feel like it's safer to revert the upgrade of otlptranslator to 1.0.0, revist the code behind that feature-gate and once we're confident that they are working well together... then we upgrade otlptranslator
There was a problem hiding this comment.
I would really love to avoid a configuration option for this, and encourage users to just migrate. As a workaround, we can tell users to use the transform processor to rename metrics prior to reaching the exporter. When we graduate the feature gate to beta, users still have the option to revert, and open issues with feedback. If this change turns out to be too disruptive, we can revisit adding config options at that point.
There was a problem hiding this comment.
that's fine by me -- I was inclined to be more cautious given the amount of pings I get when metric names break.
Where would be a good place to put that workaround recommendation so people find it?
There was a problem hiding this comment.
Probably in the package README under the feature gate?
|
no config needed, will update README in a separate PR |
|
can someone reopen this? I do not have permission |
) Followon from #43050 #### Description Connects up PermissiveLabelSanitization feature gate. <!-- Issue number (e.g. #1234) or full URL to issue, if applicable. --> #### Link to tracking issue First part of #43077 <!--Describe what testing was performed and which tests were added.--> #### Testing New unit tests <!--Describe the documentation added.--> #### Documentation explanatory comments added to new options in code <!--Please delete paragraphs that you did not use before submitting.--> --------- Signed-off-by: Owen Williams <owen.williams@grafana.com>
4 participants
Description
Adds underscore_label_sanitization and preserve_multiple_underscores to config options. This allows users who were relying on previous default translation behavior to maintain metric name continuity.
"underscore_label_sanitization", when true, prepends "key" to labels starting with an underscore. This behavior is not required by Prometheus nor is it done by Prometheus' own otel endpoint. Therefore we have disabled this behavior by default. Users who rely on this can set this option to true.
"preserve_multiple_underscores", when true, does not collapse multiple underscores when translating metric and label names. For instance "my.silly$%^&.metric" would become "my_silly_____metric" instead of "my_silly_metric". This is another inconsistency between different otel translation libraries that have been rectified, but since some users rely on the preservation of multiple underscores, they can set this option to true.
Link to tracking issue
Fixes #43077
Testing
New unit tests
Documentation
explanatory comments added to new options in code