otelconf: fix Prometheus reader converting dot-style labels to underscores#8696
otelconf: fix Prometheus reader converting dot-style labels to underscores#8696dmitryax wants to merge 1 commit intoopen-telemetry:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8696 +/- ##
=====================================
Coverage 82.1% 82.1%
=====================================
Files 183 183
Lines 13790 13790
=====================================
Hits 11332 11332
Misses 2054 2054
Partials 404 404
🚀 New features to boost your workflow:
|
underscores
When both without_type_suffix and without_units are set (which is the
default config for the OTel Collector), prometheusReaderOpts was
selecting
UnderscoreEscapingWithoutSuffixes as the translation strategy. This
strategy
sets LabelNamer{UTF8Allowed: false}, converting OTel dot-style label
names
(e.g. service.name) to underscore-style (service_name) in target_info
and
on all metric datapoints.
Fix by using NoTranslation instead, which preserves dot-style label
names
(ShouldEscape()=false) and suppresses metric name suffixes
(ShouldAddSuffixes()=false).
Related issue:
open-telemetry/opentelemetry-collector-contrib#47011
75d9783 to
a395b37
Compare
| _, err = buf.ReadFrom(resp.Body) | ||
| require.NoError(t, err) | ||
| body := buf.String() | ||
|
|
There was a problem hiding this comment.
Should we verify the body contains target_info and not contains the target.info? This prevents MetricNamer accidentally takes TranslationStrategyOption.NoTranslation seriously in the future.
|
See collector upgrade slack thread: https://cloud-native.slack.com/archives/C02JJK840SH/p1773874400856229 I agree with @codeboten's comments on that thread. When setting up the prometheus exporter, we should follow the prometheus conventions, not enforce the OTel ones. |
|
cc @dashpole |
|
Is the collector able to use NoTranslation if it wants this behavior? Escaping to underscores is the correct default behavior according to the prometheus sdk exporter spec. |
|
This is just an attempt to address a regression on the collector side appeared in 0.148.0 release when instead of getting the following resource for internal collector metrics we started getting with the collector's default configuration. According the @dmathieu this can be resolved by moving collector go sdk config to 1.0. If that better way, I'm happy to close this PR. For now I've marked this issue as a collector release blocker |
codeboten
left a comment
There was a problem hiding this comment.
Since setting the translation strategy via the old configuration options was causing problems for the one known use case of the package, i'm in favour of this fix until the collector can move to 1.0.
Please add the same fix to 0.2.0 for consistency
|
|
||
| ### Fixed | ||
|
|
||
| - Fix `go.opentelemetry.io/contrib/otelconf` Prometheus reader converting OTel dot-style label names (e.g. `service.name`) to underscore-style (`service_name`) in `target_info` when both `without_type_suffix` and `without_units` are set. Use `NoTranslation` instead of `UnderscoreEscapingWithoutSuffixes` to preserve dot-style label names while still suppressing metric name suffixes. (#PLACEHOLDER) |
There was a problem hiding this comment.
| - Fix `go.opentelemetry.io/contrib/otelconf` Prometheus reader converting OTel dot-style label names (e.g. `service.name`) to underscore-style (`service_name`) in `target_info` when both `without_type_suffix` and `without_units` are set. Use `NoTranslation` instead of `UnderscoreEscapingWithoutSuffixes` to preserve dot-style label names while still suppressing metric name suffixes. (#PLACEHOLDER) | |
| - Fix `go.opentelemetry.io/contrib/otelconf` Prometheus reader converting OTel dot-style label names (e.g. `service.name`) to underscore-style (`service_name`) in `target_info` when both `without_type_suffix` and `without_units` are set. Use `NoTranslation` instead of `UnderscoreEscapingWithoutSuffixes` to preserve dot-style label names while still suppressing metric name suffixes. (#8696 ) |
|
Replaced by #8763 |
|
Thanks @dmitryax for the initial pr, apologies for overriding your pr here, was trying to get this merged as soon as possible :) |
When both
without_type_suffixandwithout_unitsare set (which is the default config for the OTel Collector),prometheusReaderOptswas selectingUnderscoreEscapingWithoutSuffixesas the translation strategy. This strategy setsLabelNamer{UTF8Allowed: false}, converting OTel dot-style label names (e.g.service.name) to underscore-style (service_name) in target_info and on all metric datapoints.Fix by using
NoTranslationinstead, which preserves dot-style label names (ShouldEscape()=false) and suppresses metric name suffixes (ShouldAddSuffixes()=false).The restored behavior is close to what we had before #8595.
Related issue: open-telemetry/opentelemetry-collector-contrib#47011
I manually tested it and it solves the issue above for the collector