Skip to content

fix(metrics): retain delta baseline across missed collection in PrecomputedSum - #3720

Draft
darkraider01 wants to merge 1 commit into
open-telemetry:mainfrom
darkraider01:fix/precomputed-sum-delta-baseline-retention
Draft

darkraider01 wants to merge 1 commit into
open-telemetry:mainfrom
darkraider01:fix/precomputed-sum-delta-baseline-retention

Conversation

@darkraider01

Copy link
Copy Markdown
Member

Fixes #3714

Changes

  • Baseline retention & eviction: Replaced the raw cached value in PrecomputedSum::reported with BaselineEntry<T> tracking last_value and consecutive missed_count.
    • Increments missed_count at the start of each collection cycle.
    • Resets missed_count to 0 whenever an attribute set has an accepted observation.
    • Retains the baseline through exactly one missed collection cycle so temporary gaps do not reset delta calculation.
    • Evicts the baseline after two consecutive missed collection cycles to keep memory bounded.
  • Non-finite rejection: Filter out NaN and infinite values upfront in Measure::call and BoundPrecomputedSumHandle::call, preventing invalid measurements from being recorded and treating those cycles as missed collections.
  • Tests:
    • Updated observable_counter_delta_attribute_set_reappears_after_gap to expect delta 30 (80 - 50) on reappearance after 1 missed cycle.
    • Added unit test delta_rejects_nan_and_retains_baseline for f64 NaN rejection and baseline preservation.
    • Added unit test delta_evicts_baseline_after_two_consecutive_missed_collections.
    • Added integration tests observable_counter_delta_stale_eviction and observable_counter_delta_f64_nan_rejection.
  • Documentation: Added entry in opentelemetry-sdk/CHANGELOG.md under ## vNext.

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

…mputedSum

PrecomputedSum::delta previously replaced its reported map with only attribute sets observed during the current collection cycle. If an observable counter missed a collection or observed a non-finite value, its baseline was lost and the subsequent delta was computed from zero (open-telemetry#3714).

Track a consecutive missed count per attribute set to retain the baseline through exactly one missed collection cycle, and evict after two consecutive missed cycles to keep memory bounded. Filter non-finite measurements upfront in Measure::call.
@darkraider01
darkraider01 requested a review from a team as a code owner September 10, 2026 19:25
@codecov

codecov Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.43066% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.1%. Comparing base (7ed5d1b) to head (04b2079).
⚠️ Report is 46 commits behind head on main.

Files with missing lines Patch % Lines
...emetry-sdk/src/metrics/internal/precomputed_sum.rs 92.6% 5 Missing ⚠️
opentelemetry-sdk/src/metrics/mod.rs 94.2% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #3720     +/-   ##
=======================================
- Coverage   83.2%   83.1%   -0.1%     
=======================================
  Files        131     130      -1     
  Lines      29598   29415    -183     
=======================================
- Hits       24630   24455    -175     
+ Misses      4968    4960      -8     

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

@cijothomas cijothomas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I’d prefer to wait for clarification in [specification(https://github.com/open-telemetry/opentelemetry-specification/issues/4861) before changing this behavior. The proposed one-cycle retention policy is not currently defined by the specification and its better to get a spec blessing to ensure consistency.

@darkraider01

Copy link
Copy Markdown
Member Author

Alright ! I'll fold it into draft PR until then

@darkraider01
darkraider01 marked this pull request as draft September 12, 2026 15:22
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Pull request dashboard status

Waiting on the author · refreshed 2026-10-26 20:04 UTC

Move out of draft to request review.

Status above doesn't look right?
  • Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

Thank you for your contribution! This PR has been automatically marked as stale because it has not had activity in the last 14 days. This may be due to a delay in review on our side or awaiting a response from you; either is fine, and we appreciate your patience.

It will be closed in 14 days if no further activity occurs. Pushing a new commit or leaving a comment will remove the stale label and keep the PR open.

@github-actions github-actions Bot added the Stale label Oct 2, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PrecomputedSum in Delta temporality resets baseline on missed/rejected observations, double-counting values

2 participants