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
Fix observable counter cumulative aggregation #1644
base: main
Are you sure you want to change the base?
Fix observable counter cumulative aggregation #1644
Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1644 +/- ##
=====================================
Coverage 69.3% 69.3%
=====================================
Files 136 136
Lines 19637 19554 -83
=====================================
- Hits 13610 13567 -43
+ Misses 6027 5987 -40 ☔ View full report in Codecov by Sentry. |
@stormshield-fabs Could you follow the EasyCLA process and sign it? It is a pre-req for any contributions to OpenTelemetry! |
The CLA is still under review at my company, thanks for the heads up. |
a6d6c0c
to
c2f39a3
Compare
The CLA has been signed. I've rebased this PR and updated the changelog. |
c2f39a3
to
12e0a03
Compare
Fixes #1517 (incorrect cumulative aggregation for observable counters).
Changes
The observable counters rely on
PrecomputedSum
that was using the sameValueMap
implementation asSum
(its counterpart for synchronous counters), effectively adding all observations (also drastically reducing the counter range).This PR specializes
ValueMap
with anO
(operation) parameter so that observations can be performed with the correct semantics (AddAssign
for synchronous counters,Assign
for observable ones). It also updates thePrecomputedSum::cumulative
aggregation to return the current value.The tests (
observable_counter_delta
andobservable_counter_cumulative
) are from #1516 with tiny fixes.Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes