-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Combine precomputed values of filtered attribute sets #3549
Conversation
Fix open-telemetry#3439 When an attribute filter drops a distinguishing attribute during the aggregation of a precomputed sum add that value to existing, instead of just setting the value as an override (current behavior).
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3549 +/- ##
=======================================
+ Coverage 79.0% 79.1% +0.1%
=======================================
Files 170 170
Lines 12585 12654 +69
=======================================
+ Hits 9948 10018 +70
Misses 2424 2424
+ Partials 213 212 -1
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Adds example from supplimental guide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still have trouble understanding the code structure a bit, but the tests help me believe that it works correctly. Comments are just for my understanding, and possibly for additional comments in the code
Tracking link CI failure here: #2957 (comment) |
Fix #3439
When an attribute filter drops a distinguishing attribute during the aggregation of a precomputed sum that value needs to be added to any existing one for an equivalent set1. This changes the current behavior of overriding the equivalent set's value.
Footnotes
https://github.com/open-telemetry/opentelemetry-specification/issues/2905 ↩