Merge colliding Prometheus label values by ADITYA-CODE-SOURCE · Pull Request #8364 · open-telemetry/opentelemetry-java

ADITYA-CODE-SOURCE · 2026-05-03T16:44:23Z

Summary

merge colliding normalized attribute labels by joining values with ';' in original-key order
preserve existing precedence between metric, resource, and synthetic labels while fixing collision handling
add regression tests for both metric attribute and allowed resource attribute normalization conflicts

codecov · 2026-05-03T16:47:16Z

Codecov Report

❌ Patch coverage is 86.90476% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.59%. Comparing base (78d0177) to head (d5e3dcc).

Files with missing lines	Patch %	Lines
.../exporter/prometheus/Otel2PrometheusConverter.java	86.90%	8 Missing and 3 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8364      +/-   ##
============================================
- Coverage     91.61%   91.59%   -0.03%     
- Complexity    10320    10338      +18     
============================================
  Files          1013     1013              
  Lines         27287    27359      +72     
  Branches       3203     3210       +7     
============================================
+ Hits          25000    25059      +59     
- Misses         1559     1573      +14     
+ Partials        728      727       -1

☔ 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.

ADITYA-CODE-SOURCE · 2026-05-06T03:43:30Z

@psx95 Thanks for the thoughtful review
I’ve addressed the suggestions in the latest commits, including broader collision coverage, stronger assertions, a cross-source collision case, and helper naming/documentation cleanup. I also fixed a follow-up test expectation in the new ordering case.
Appreciate the detailed feedback.

zeitlinger · 2026-05-06T10:35:44Z

There's significant conceptual overlap with #8346 - so I'd park this PR for now.

ADITYA-CODE-SOURCE · 2026-05-06T12:00:20Z

There's significant conceptual overlap with #8346 - so I'd park this PR for now.

Understood, thanks for the context. There's definitely conceptual overlap with #8346 in the Otel2PrometheusConverter label translation logic. I'll park this PR for now and revisit once #8346 settles, so the collision-handling fix can be cleanly applied on top of the final translation model.
Let me know if there's anything else I should do in the meantime.

jack-berg · 2026-05-12T18:30:09Z

+                MetricDataType.LONG_SUM,
+                Attributes.empty(),
+                Resource.create(
+                    createMapAttributes("foo_bar", "b", "foo-bar", "c", "foo.bar", "a"))),


I eventually put together that createMapAttributes intentionally creates a non-default Attributes implementation to confirm the sorting logic.

Let's add a comment explaining this because its not immediately apparent why we're going out of the way to add a new attributes implementation

Is it possible / reasonable to avoid the extra allocation / overhead by detecting when the Attributes implementation is default ArrayBackedAttributes?

Thinking about this more, I don't think we should try to check if Attributes is instance of ArrayBackedAttributes. But I am still worried about the excessive new allocation here which occurs regardless of whether a collision occurs. To mitigate, let's adjust to a strategy which assumes that no collision occurs, but if one does occur, triggers the code path with all this additional new machinery.

Rebased this onto current main and addressed this in the latest push. The custom MapAttributes helper now has a short comment explaining that it intentionally uses a non-default Attributes implementation so the ordering-sensitive collision tests do not accidentally rely on the SDK default storage type. I did not add an ArrayBackedAttributes special case in production code.

jack-berg · 2026-05-12T18:34:56Z

-            labelNameToValue.put(
-                convertLabelName(key.getKey()), toLabelValue(key.getType(), value)));
+    Map<String, String> labelNameToValue = new LinkedHashMap<>();
+    normalizeAndMergeAttributeLabels(labelNameToValue, attributes);


This is just looking at attributes, but I think the collisions also have to consider collisions potentially caused by normalization of other sources of attributes: resource, scope, additionalAttributes

Addressed in the rebased branch. Collision merging is now source-local and precedence-preserving: metric attributes are normalized/merged first, synthetic and scope labels keep their current putIfAbsent behavior, and filtered resource attributes are normalized/merged within the resource source before being applied last with putIfAbsent. That fixes normalized collisions without concatenating values across sources.

ADITYA-CODE-SOURCE · 2026-05-14T15:37:59Z

@jack-berg Thanks for the review. Those are good points — I’ll make sure to optimize the allocation with a fast path for the no-collision case, extend the collision check to resource/scope/additionalAttributes sources, and add the comment explaining the sorting logic.\n\nThis PR is currently parked per @zeitlinger’s request pending #8346. Once that settles, I’ll rebase, apply these changes, and re-request review.

zeitlinger · 2026-07-10T16:44:47Z

PR is currently parked per @zeitlinger’s request pending #8346.

@ADITYA-CODE-SOURCE this is unblocked now

Preserve Prometheus compatibility when normalized attribute names collide by joining the values in original-key order instead of letting later entries overwrite earlier ones. Add regression tests for metric and resource attribute collisions. Fixes open-telemetry#8236

ADITYA-CODE-SOURCE · 2026-07-11T13:57:30Z

Rebased this parked PR onto current main now that #8346 has landed.

Latest update:

rebuilt the collision handling on top of the current translation-strategy-aware converter instead of replaying the old patch mechanically
added a fast path that keeps the existing cheap insertion behavior unless a normalized label collision is actually detected
kept collision merging source-local so existing precedence still holds: metric attributes first, synthetic/scope labels unchanged, filtered resource attributes last via putIfAbsent
preserved current label rendering by using toLabelValue(...) in the merged resource path as well
expanded regression coverage for metric collisions, resource collisions, metric-vs-resource precedence, array-valued resource collisions, and non-escaping strategy behavior
added a short comment explaining the custom MapAttributes helper used by the ordering-sensitive tests

Local verification completed with:

./gradlew :exporters:prometheus:test --tests "*Otel2PrometheusConverterTest"
./gradlew :exporters:prometheus:spotlessCheck
./gradlew :exporters:prometheus:check

The branch was force-pushed after rebase.

opentelemetry-pr-dashboard · 2026-07-11T13:57:37Z

This PR has review comments. Review suggestions, whether from maintainers or automated reviewers, aren't always correct or required. Please evaluate each comment on its merits, then make sure each thread has a clear outcome.

For example, link to the commit if you applied a suggestion, explain why it wasn't applied, or ask a follow-up question.

Automation flags a PR for human review once every review thread has a reply or is marked as resolved.

Status across open PRs is visible on the pull request dashboard.

ADITYA-CODE-SOURCE · 2026-07-13T12:40:03Z

Pushed a follow-up for the failing CI. The Linux/macOS Build jobs were not failing in the build/test steps themselves; they were failing in the post-build diff check because ./gradlew build regenerated tracked docs/apidiffs/current_vs_latest/*.txt files after the rebase onto current main.

I regenerated the apidiffs with ./gradlew jApiCmp, committed the updated files, and pushed them in f11595db6.

Local verification:

./gradlew :exporters:prometheus:test --tests "*Otel2PrometheusConverterTest"
./gradlew :exporters:prometheus:spotlessCheck
./gradlew :exporters:prometheus:check
./gradlew jApiCmp

At this point the code-side maintainer feedback is addressed and CI should rerun cleanly on the updated branch. The remaining merge blocker after CI is the stale review state from the earlier requested-changes review, which will still need maintainer re-review or dismissal.

ADITYA-CODE-SOURCE · 2026-07-13T13:07:06Z

@psx95 @jack-berg CI is green now, and I’ve addressed the earlier review feedback in the rebased branch.

The main implementation update is in 6e3a97f, and the follow-up f11595d only refreshes the tracked docs/apidiffs/current_vs_latest/ files after rebasing onto current main.

Could you please take another look

…y-java into issue-8236-prometheus-label-collisions

ADITYA-CODE-SOURCE · 2026-07-14T12:43:22Z

@psx95 @jack-berg All checks are green now, and the branch has been rebased onto current main with the latest follow-ups incorporated.

At this point the remaining merge blocker appears to be the old CHANGES_REQUESTED review state from the earlier version of the PR. Could you please take another look

ADITYA-CODE-SOURCE requested a review from a team as a code owner May 3, 2026 16:44

github-actions Bot mentioned this pull request May 4, 2026

Pull Request Dashboard #8366

Closed

psx95 requested changes May 5, 2026

View reviewed changes

psx95 reviewed May 5, 2026

View reviewed changes

Comment thread .../prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java Outdated

Comment thread .../prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java Outdated

ADITYA-CODE-SOURCE requested a review from psx95 May 7, 2026 04:59

github-actions Bot mentioned this pull request May 7, 2026

Pull Request Dashboard #8375

Closed

jack-berg reviewed May 12, 2026

View reviewed changes

This was referenced May 26, 2026

Pull Request Dashboard #8425

Closed

Pull Request Dashboard #8439

Open

ADITYA-CODE-SOURCE force-pushed the issue-8236-prometheus-label-collisions branch from 5c8f7e1 to 6e3a97f Compare July 11, 2026 13:56

Update API diffs after rebase

f11595d

otelbot Bot added the api-change Changes to public API surface area label Jul 13, 2026

ADITYA-CODE-SOURCE requested a review from jack-berg July 13, 2026 12:46

Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…

d5e3dcc

…y-java into issue-8236-prometheus-label-collisions

otelbot Bot removed the api-change Changes to public API surface area label Jul 13, 2026

Uh oh!

Conversation

ADITYA-CODE-SOURCE commented May 3, 2026

Summary

Uh oh!

codecov Bot commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ADITYA-CODE-SOURCE commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zeitlinger commented May 6, 2026

Uh oh!

ADITYA-CODE-SOURCE commented May 6, 2026

Uh oh!

jack-berg May 12, 2026

Choose a reason for hiding this comment

Uh oh!

jack-berg May 12, 2026

Choose a reason for hiding this comment

Uh oh!

ADITYA-CODE-SOURCE Jul 11, 2026

Choose a reason for hiding this comment

Uh oh!

jack-berg May 12, 2026

Choose a reason for hiding this comment

Uh oh!

ADITYA-CODE-SOURCE Jul 11, 2026

Choose a reason for hiding this comment

Uh oh!

ADITYA-CODE-SOURCE commented May 14, 2026

Uh oh!

zeitlinger commented Jul 10, 2026

Uh oh!

ADITYA-CODE-SOURCE commented Jul 11, 2026

Uh oh!

opentelemetry-pr-dashboard Bot commented Jul 11, 2026

Uh oh!

ADITYA-CODE-SOURCE commented Jul 13, 2026

Uh oh!

ADITYA-CODE-SOURCE commented Jul 13, 2026

Uh oh!

ADITYA-CODE-SOURCE commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented May 3, 2026 •

edited

Loading

ADITYA-CODE-SOURCE commented May 6, 2026 •

edited

Loading