Skip to content

exporter/prometheus: Implement translation strategies#41530

Merged
songy23 merged 7 commits into
open-telemetry:mainfrom
ArthurSens:promexporter-translation-strategies
Aug 1, 2025
Merged

exporter/prometheus: Implement translation strategies#41530
songy23 merged 7 commits into
open-telemetry:mainfrom
ArthurSens:promexporter-translation-strategies

Conversation

@ArthurSens
Copy link
Copy Markdown
Member

@ArthurSens ArthurSens commented Jul 23, 2025

Description

Implements translation_strategy as described in open-telemetry/opentelemetry-specification#4533

Link to tracking issue

Fixes #35459

Testing

Documentation

@ArthurSens
Copy link
Copy Markdown
Member Author

opening as draft since tests are failing and I couldn't figure out why today... I suspect content negotiation

@ywwg
Copy link
Copy Markdown
Contributor

ywwg commented Jul 24, 2025

@ywwg
Copy link
Copy Markdown
Contributor

ywwg commented Jul 24, 2025

Well that link didn't work, but that's the diff I found

@ArthurSens
Copy link
Copy Markdown
Member Author

ArthurSens commented Jul 24, 2025

Well that link didn't work, but that's the diff I found

🤦 ... ok that was a problem too, but the diff also shows the metric name being escaped when it shouldn't

@ArthurSens ArthurSens changed the title WIP/exporter/prometheus: Implement translation strategies exporter/prometheus: Implement translation strategies Jul 25, 2025
@ArthurSens ArthurSens marked this pull request as ready for review July 25, 2025 21:50
@ArthurSens ArthurSens requested review from a team and dashpole as code owners July 25, 2025 21:50
@github-actions github-actions Bot requested a review from Aneurysm9 July 25, 2025 21:50
@ArthurSens
Copy link
Copy Markdown
Member Author

I suspect content negotiation

Indeed it was!!! I was using escaping=allow-utf8 while it was expected escaping=allow-utf-8

Comment thread exporter/prometheusexporter/config.go Outdated
noTranslation translationStrategy = "NoTranslation"
)

var translationStrategyFeatureGate = featuregate.GlobalRegistry().MustRegister(
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.

IMO the feature gate should only control the removal of existing configuration options, and translation_strategy should always take precedence over existing conflicting options. That way someone can just migrate and doesn't need to mess with feature gates.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

hmmm, that's a good point 🤔

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
…tion

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
@ArthurSens ArthurSens force-pushed the promexporter-translation-strategies branch from 879d5e5 to 0a3987a Compare August 1, 2025 12:37
@ArthurSens ArthurSens added the ready to merge Code review completed; ready to merge by maintainers label Aug 1, 2025
@ArthurSens
Copy link
Copy Markdown
Member Author

Following up on #41318 (comment), I was planning on upgrading otlptranslator to latest in this PR as well, but it's probably better to do in a follow-up. It is bringing some API changes that might be confusing to review in this PR that adds a new feature

@songy23 songy23 merged commit afe4f3b into open-telemetry:main Aug 1, 2025
190 of 191 checks passed
Dylan-M pushed a commit to Dylan-M/opentelemetry-collector-contrib that referenced this pull request Aug 5, 2025
…#41530)

#### Description
Implements `translation_strategy` as described in
open-telemetry/opentelemetry-specification#4533

#### Link to tracking issue
Fixes open-telemetry#35459

<!--Describe what testing was performed and which tests were added.-->
#### Testing

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

exporter/prometheus ready to merge Code review completed; ready to merge by maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[exporter/prometheus] Implement Prometheus translation strategies.

5 participants