Skip to content

fix: enforce last-value-wins in AttributesMap for duplicate keys of different types#8420

Open
kyyril wants to merge 2 commits into
open-telemetry:mainfrom
kyyril:fix/attributesmap-last-value-wins
Open

fix: enforce last-value-wins in AttributesMap for duplicate keys of different types#8420
kyyril wants to merge 2 commits into
open-telemetry:mainfrom
kyyril:fix/attributesmap-last-value-wins

Conversation

@kyyril
Copy link
Copy Markdown

@kyyril kyyril commented May 20, 2026

Resolves #7897

Changes

Fixes inconsistent last-value-wins behavior in AttributesMap.

Previously, AttributesMap stored attributes directly in a HashMap<AttributeKey<?>, Object>. Since AttributeKey equals/hashCode includes the type, adding two attributes with the same raw string name but different types would result in both being kept, violating the OpenTelemetry specification requirement that setting the same key SHOULD overwrite the existing value.

This PR modifies AttributesMap.put to search for any existing key with the same string representation and remove it before inserting the new key/value. It also overrides putAll to ensure all entries are processed through put to correctly deduplicate and respect the capacity limits.

Checklist

  • Added unit tests verifying last-value-wins behavior with different types in AttributesMapTest
  • Code formatted using spotlessApply

@kyyril kyyril requested a review from a team as a code owner May 20, 2026 23:32
@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.14%. Comparing base (32440c1) to head (4c3817a).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8420      +/-   ##
============================================
- Coverage     91.14%   91.14%   -0.01%     
- Complexity     7761     7766       +5     
============================================
  Files           881      881              
  Lines         23409    23421      +12     
  Branches       2331     2334       +3     
============================================
+ Hits          21337    21346       +9     
- Misses         1377     1378       +1     
- Partials        695      697       +2     

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Copy Markdown
Contributor

@psx95 psx95 left a comment

Choose a reason for hiding this comment

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

Based on the the previous discussion around this issue in this comment, I would recommend running benchmark tests to ensure that this implementation is at least as performant as existing code.

Additionally, it looks like this issue warranted a larger change about replacing extends HashMap with a custom implementation that uses a custom map that stores attributes by raw string key.

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.

AttributeMap does not respect last-value-wins semantics for duplicate keys (SpanBuilder.setAttribute vs Attributes.builder)

2 participants