Skip to content

[pkg/translator/prometheusremotewrite] preserve multiple underscores when permissive sanitization is enabled - #49565

Merged
ChrsMark merged 3 commits into
open-telemetry:mainfrom
s3onghyun:prw-preserve-multiple-underscores
Aug 5, 2026
Merged

[pkg/translator/prometheusremotewrite] preserve multiple underscores when permissive sanitization is enabled#49565
ChrsMark merged 3 commits into
open-telemetry:mainfrom
s3onghyun:prw-preserve-multiple-underscores

Conversation

@s3onghyun

Copy link
Copy Markdown
Contributor

Description

newPrometheusConverter builds its LabelNamer with only UnderscoreLabelSanitization set:

labelNamer: otlptranslator.LabelNamer{UnderscoreLabelSanitization: !prometheus.DropSanitizationGate.IsEnabled(), UTF8Allowed: utf8Allowed},

Because PreserveMultipleUnderscores is never set, label names with consecutive underscores (e.g. a__b) are collapsed to a single underscore (a_b) even when the pkg.translator.prometheus.PermissiveLabelSanitization feature gate is enabled — so enabling the gate does not fully drop sanitization as documented.

This sets PreserveMultipleUnderscores from the same feature gate, so when the gate is on, consecutive underscores are preserved. otlptranslator.LabelNamer already exposes this field (v1.0.0), and reserved __-prefixed labels remain untouched.

Link to tracking issue

Fixes #48991

Testing

Added TestNewPrometheusConverterLabelNamerMultipleUnderscores covering:

  • gate enabled: a__b is preserved
  • gate disabled: a__b is collapsed to a_b (unchanged behavior)
  • reserved __name__ is preserved regardless of the gate

The gate-enabled case fails before this change and passes after; the full package test suite still passes.

Documentation

No user-facing docs change; the feature gate behavior now matches its documented intent.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aligns pkg/translator/prometheusremotewrite label-name handling with the intended behavior of the pkg.translator.prometheus.PermissiveLabelSanitization (DropSanitizationGate) feature gate by preserving consecutive underscores (e.g., a__b) when permissive sanitization is enabled.

Changes:

  • Configure otlptranslator.LabelNamer to set PreserveMultipleUnderscores when DropSanitizationGate is enabled.
  • Add a unit test covering multiple-underscore behavior for gate enabled/disabled and reserved labels.
  • Add module dependency needed for the new test helper and include a changelog entry.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
pkg/translator/prometheusremotewrite/metrics_to_prw.go Updates the converter’s LabelNamer configuration to preserve consecutive underscores when the feature gate is enabled.
pkg/translator/prometheusremotewrite/labelnamer_underscore_test.go Adds test coverage for multiple-underscore label behavior under the feature gate.
pkg/translator/prometheusremotewrite/go.mod Adds the internal/common module dependency used by the new test helper.
.chloggen/prw-preserve-multiple-underscores.yaml Adds a changelog entry for the bug fix in label sanitization behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/translator/prometheusremotewrite/metrics_to_prw.go Outdated
Comment thread pkg/translator/prometheusremotewrite/labelnamer_underscore_test.go Outdated
Comment thread pkg/translator/prometheusremotewrite/metrics_to_prw.go Outdated
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 18, 2026

Copy link
Copy Markdown

Pull request dashboard status

Merged · refreshed 2026-08-05 08:32 UTC

Status above doesn't look right?
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

@singhvibhanshu

singhvibhanshu commented Aug 4, 2026

Copy link
Copy Markdown
Member

@s3onghyun,
Please fix the merge conflicts and we will get this in.

…when permissive sanitization is enabled

The converter built its LabelNamer with only UnderscoreLabelSanitization
set, so label names with consecutive underscores (e.g. a__b) were still
collapsed to a single underscore even when the
pkg.translator.prometheus.PermissiveLabelSanitization feature gate was
enabled. Set PreserveMultipleUnderscores from the same gate so the gate
actually preserves them, and add tests covering the gate on/off behavior
and the reserved __-prefixed label.

Signed-off-by: s3onghyun <s3onghyun@users.noreply.github.com>
Review follow-up:

- newPrometheusConverterV2 built its own LabelNamer without
  PreserveMultipleUnderscores, so FromMetricsV2 kept collapsing consecutive
  underscores even with the permissive sanitization gate on. Same field, same
  gate, now set there too, with a test covering both gate states.
- The reserved __name__ subtest claimed it held 'regardless of gate' but only
  ran the gate-disabled case; it is now table-driven over both.
- Cached DropSanitizationGate.IsEnabled() in a local instead of evaluating it
  twice per struct literal.

Signed-off-by: s3onghyun <s3onghyun@users.noreply.github.com>
@s3onghyun
s3onghyun force-pushed the prw-preserve-multiple-underscores branch from c94750d to f9bf8ec Compare August 5, 2026 00:31
@s3onghyun

Copy link
Copy Markdown
Contributor Author

Thanks @singhvibhanshu — rebased on latest main, conflicts resolved (f9bf8ec). The only conflict was pkg/translator/prometheusremotewrite/go.mod (version bump to v0.158.0); kept the internal/common dependency since the test uses internal/common/testutil. go build ./... and the underscore tests (v1 + v2) pass locally. Should be good to merge now.

@singhvibhanshu singhvibhanshu added ready to merge Code review completed; ready to merge by maintainers and removed waiting-for-code-owners labels Aug 5, 2026
@ChrsMark
ChrsMark merged commit 9d1352c into open-telemetry:main Aug 5, 2026
157 checks passed
@otelbot

otelbot Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution @s3onghyun! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg/translator/prometheusremotewrite ready to merge Code review completed; ready to merge by maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pkg/translator/prometheusremotewrite label multiple underscore sanitization

6 participants