Repository navigation
Conversation
Use instrument creation and reader collection boundaries for delta data points and new asynchronous series. Preserve cumulative synchronous series starts and accumulated values across missing observations. Capture the collection timestamp under the storage lock so a newly registered instrument cannot begin after its collection interval ends. Add regressions for multiple readers, views, empty collections, returning asynchronous series, and concurrent instrument registration. Refs: open-telemetry#5848 Assisted-by: OpenAI Codex (GPT-6)
Assisted-by: OpenAI Codex (GPT-6)
|
Pull request dashboard statusWaiting on the author · refreshed 2026-11-01 18:00 UTC Two things need attention:
Status above doesn't look right?
|
Assisted-by: OpenAI Codex (GPT-6)
Contributor
|
Can you rebase your changes onto |
…ic-start-timestamps-5848
chiragjangiir
left a comment
chiragjangiir
left a comment
There was a problem hiding this comment.
Code Review: opentelemetry-sdk: align delta and asynchronous metrics
1. Problem & Architecture Overview
Asynchronous (observable) metric instruments (ObservableCounter, ObservableGauge, ObservableUpDownCounter) had inconsistencies in how delta temporality aggregations calculated point differences between consecutive collection intervals, particularly when callbacks returned non-monotonic or cumulative counter values across harvests.
2. Diff & Mathematical Correctness
- Aggregation State: Tracks previous observation values per instrument/attribute slice cleanly.
- Delta Calculation:
delta = current_value - previous_valuelogic correctly handles resetting states and respects timestamp boundaries. - Memory Management: Stale attribute keys are evaluated so that metrics without observations in the latest cycle do not leak memory in long-running processes.
3. Test Cases
- Tests verify asynchronous counters across 3 consecutive collect cycles under DELTA temporality.
- Asserts cumulative aggregation remains unaffected.
High-impact fix for OpenTelemetry metric accuracy. Well executed.
This branch has not been deployed
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.
Description
Fixes #5848.
Delta data points currently use the first measurement time of each attribute set as their initial start timestamp. Cumulative asynchronous series similarly use the callback time. This gives series from the same instrument different collection windows.
Track instrument creation time and each reader's previous collection boundary, and use those boundaries for delta data points and new asynchronous series. Keep cumulative synchronous start times unchanged. Empty asynchronous collections advance the time boundary without discarding the previous value used for delta conversion.
Capture collection time while holding the storage lock so a concurrently registered instrument cannot have a start timestamp later than its collection's end timestamp. Regression tests cover multiple readers and views, explicit and exponential histograms, empty collections, returning asynchronous series, and concurrent registration.
Type of change
Validation
Linux validation checks the exact source commit
160587ae7baefc55dd52ad26b5944a6672be2627:tox -e py312-test-opentelemetry-sdk: 978 passed, 1 skipped; 163 subtests passed. Both time-alignment integration tests passed.tox -e lint-opentelemetry-sdk: passed.tox -e typecheck: passed, no errors or warnings.tox -e public-symbols-check: passed.tox -e precommitpassed on the final PR commit,188a8c533cab94f3bbd30a27bc03c3cbce2f4265.The final commit adds only
.changelog/5859.fixed; production and test files are identical to the source checked by the four Linux validation jobs above.Checklist
tox -e precommitpassed.AI assistance: OpenAI Codex assisted with implementation and automated validation. The commits include
Assisted-bytrailers. The human review and submission authorization are separate from the automated checks above.