Skip to content

fix(prometheus-exporter): Concatenate metric attributes with Scope during collisions#3549

Merged
cijothomas merged 1 commit into
open-telemetry:mainfrom
ArthurSens:prevent-collisions
Jun 20, 2026
Merged

fix(prometheus-exporter): Concatenate metric attributes with Scope during collisions#3549
cijothomas merged 1 commit into
open-telemetry:mainfrom
ArthurSens:prevent-collisions

Conversation

@ArthurSens

Copy link
Copy Markdown
Member

Follow up from #3503 (comment)

Changes

Small change so extra labels always have preference over metric attributes in get_attributes. From my understanding, this will make sure Scope Name/Version/SchemaURL and attributes will always superseed metric attributes

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

@ArthurSens ArthurSens requested a review from a team as a code owner June 10, 2026 17:34
@ArthurSens

Copy link
Copy Markdown
Member Author

Do we need a changelog entry if the bug wasn't released 🤔 ?

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.8%. Comparing base (2571776) to head (341ef5d).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
opentelemetry-prometheus/src/lib.rs 0.0% 39 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #3549     +/-   ##
=======================================
- Coverage   82.9%   82.8%   -0.1%     
=======================================
  Files        130     130             
  Lines      27350   27510    +160     
=======================================
+ Hits       22675   22800    +125     
- Misses      4675    4710     +35     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread opentelemetry-prometheus/src/lib.rs Outdated
let mut keys_map = BTreeMap::<String, Vec<String>>::new();
for (key, value) in kvs {
let key = utils::sanitize_prom_kv(key.as_str());
if extra.iter().any(|label| label.name() == key) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The spec says this must be concatenated, not overridden.
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/compatibility/prometheus_and_openmetrics.md#metric-attributes

In such cases, the values MUST be concatenated together, separated by ;, and ordered by the lexicographical order of the original keys.

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.

whoops, my bad

@ArthurSens ArthurSens changed the title bugfix(prometheus-exporter): Ensure metric attributes don't override Scope during collisions fix(prometheus-exporter): Ensure metric attributes don't override Scope during collisions Jun 12, 2026
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
@ArthurSens ArthurSens force-pushed the prevent-collisions branch from 6a02bef to 341ef5d Compare June 12, 2026 19:45
@ArthurSens ArthurSens changed the title fix(prometheus-exporter): Ensure metric attributes don't override Scope during collisions fix(prometheus-exporter): Concatenate metric attributes with Scope during collisions Jun 12, 2026

@cijothomas cijothomas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. no changelog needed

@cijothomas cijothomas added this pull request to the merge queue Jun 20, 2026
Merged via the queue into open-telemetry:main with commit 69213f2 Jun 20, 2026
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants