Metric attribute hashing folds True, 1 and 1.0 into a single stream - #5573
Open
dwin-gharibi wants to merge 2 commits into
Open
Metric attribute hashing folds True, 1 and 1.0 into a single stream#5573dwin-gharibi wants to merge 2 commits into
dwin-gharibi wants to merge 2 commits into
Conversation
Python compares True == 1 == 1.0 and hashes them identically, but OTLP encodes them as bool_value, int_value and double_value: three different values. Assert that _hash_attributes keeps them apart, that a sequence of pairs does not hash like the equivalent mapping, and that key ordering remains irrelevant. Integration coverage asserts a counter and a histogram produce one data point per distinct attribute type, and that identical attributes still aggregate into a single stream. These tests fail against the current implementation.
_hash_attributes returned scalar attribute values unchanged as part of the aggregation key, so key identity inherited Python's equality rather than the OTel data model's. True == 1 == 1.0 and all three hash alike, yet OTLP encodes them as bool_value, int_value and double_value. Distinct attribute sets were therefore folded into one time series, and that series was exported labelled with whichever type arrived first. A sequence of pairs collided with the equivalent mapping for the same reason. Tag every branch with its kind: scalars carry their concrete type name, sequences and mappings carry a container tag. Mapping keys are still sorted, so attribute ordering remains irrelevant.
Pull request dashboard statusWaiting on maintainers · refreshed 2026-08-24 13:11 UTC Merge when ready. Status above doesn't look right?
|
DylanRussell
approved these changes
Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5572.
Description
_hash_attributesreturns scalar attribute values unchanged as part of the aggregation key. Python comparesTrue == 1 == 1.0and hashes all three identically, so attribute sets that are distinct in the OpenTelemetry data model - OTLP encodes them asbool_value,int_valueanddouble_value- collapse into a single time series. The surviving series is labelled with whichever type happened to arrive first.The same flaw makes a sequence of pairs hash identically to the mapping it resembles.
Root cause
opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py:36. The scalar branch returns the value itself, so key identity inherits Python's==/hashsemantics rather than the OTel data model's. TheSequenceandMappingbranches return bare tuples, which is why a list of pairs and a mapping collide.Approach
Tag every branch with its kind. Scalars become
(type(value).__name__, value); sequences and mappings are prefixed with"sequence"and"mapping". The key stays a plain hashable tuple, so nothing downstream changes.Sorting of mapping keys is untouched, so attribute ordering remains irrelevant - the property that actually matters for aggregation, and one the new tests pin down explicitly.
Files changed
opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.pyopentelemetry-sdk/tests/metrics/test_attribute_hashing.py.changelog/5565.fixedTesting
Unit coverage asserts
True/1/1.0yield three distinct keys,Falseand0differ,Noneand"None"differ, a sequence of pairs differs from the equivalent mapping, nested values are distinguished, and the key remains hashable. Two tests guard against over-correcting: key order must still be irrelevant, and equal attributes must still share a key.Integration coverage drives a counter and a histogram end to end and checks the data point count and the attribute types on the exported points, plus that identical attributes still aggregate into one stream.
Six of the eleven fail before the change.
Result: 866 passed in
opentelemetry-sdk(855 baseline plus 11 new).Risk / compatibility
_HashedAttributesis private and the key never leaves the SDK, so the shape change is not observable to users. Series that were previously merged will now appear separately, which is the correction; for a backend this looks like a new series appearing at the point of upgrade.