Skip to content

Fix start timestamps for dynamically created metric instruments - #8992

Open
DevyanshMalhotra wants to merge 1 commit into
open-telemetry:mainfrom
DevyanshMalhotra:fix/metrics-dynamic-instrument-start-time
Open

DevyanshMalhotra wants to merge 1 commit into
open-telemetry:mainfrom
DevyanshMalhotra:fix/metrics-dynamic-instrument-start-time

Conversation

@DevyanshMalhotra

Copy link
Copy Markdown

Description

Fixes #8980.

Corrects start timestamps for metric instruments created after a reader has already performed a collection.

Previously, newly created instruments could use the reader's previous collection timestamp even when it was earlier than the instrument's creation time.

The fix uses Math.max to select the later of the reader's previous collection timestamp and the instrument creation time.

Changes

  • Fix start timestamps for delta synchronous instruments.
  • Fix start timestamps for delta asynchronous instruments.
  • Fix initial aggregator handle timestamps for cumulative asynchronous instruments.
  • Add regression tests for all three cases, covering both memory modes.
  • Update existing tests to use chronologically consistent timestamps.

Validation

  • Confirmed all six new regression test invocations fail before the fix.
  • Confirmed the regression tests pass after the fix.
  • ./gradlew :sdk:metrics:spotlessApply — passed.
  • ./gradlew :sdk:metrics:check — passed.

@DevyanshMalhotra
DevyanshMalhotra requested a review from a team as a code owner October 31, 2026 08:56
@linux-foundation-easycla

linux-foundation-easycla Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: DevyanshMalhotra / name: Devyansh Malhotra (87d2f50)

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Pull request dashboard status

Waiting on reviewers · refreshed 2026-10-31 11:53 UTC

Review the latest changes.

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.

@codecov

codecov Bot commented Oct 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.38%. Comparing base (dc3019e) to head (87d2f50).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8992      +/-   ##
============================================
+ Coverage     90.36%   90.38%   +0.01%     
  Complexity    10995    10995              
============================================
  Files          1020     1020              
  Lines         30096    30099       +3     
  Branches       3929     3929              
============================================
+ Hits          27197    27204       +7     
+ Misses         1942     1937       -5     
- Partials        957      958       +1     

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

@DevyanshMalhotra

Copy link
Copy Markdown
Author

Hi maintainers,

I checked the failing CI jobs. The macOS Java 8 and Java 11 builds are failing in SynchronousInstrumentStressTest.partialWriteStressTest due to 10-second timeouts.

The remaining platform builds passed, and ./gradlew :sdk:metrics:check passes locally.

These appear potentially environment-related. Could someone please rerun the two failed macOS jobs to confirm?

Thanks!

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

None yet

Projects

None yet

1 participant