Skip to content

Use failExceptionally in PeriodicMetricReader when exporter is busy#8525

Open
vivekp14 wants to merge 10 commits into
open-telemetry:mainfrom
vivekp14:main
Open

Use failExceptionally in PeriodicMetricReader when exporter is busy#8525
vivekp14 wants to merge 10 commits into
open-telemetry:mainfrom
vivekp14:main

Conversation

@vivekp14

Copy link
Copy Markdown

Fixes #8433

When forceFlush() is called while a periodic export is already in
progress, PeriodicMetricReader was silently failing with plain fail().

Updated to use failExceptionally(IllegalStateException) so callers
can inspect the cause via CompletableResultCode#getFailureThrowable().

Also added JavaDoc to forceFlush() documenting this edge case.

@vivekp14 vivekp14 requested a review from a team as a code owner June 23, 2026 09:55
@linux-foundation-easycla

linux-foundation-easycla Bot commented Jun 23, 2026

Copy link
Copy Markdown

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

  • ✅ login: vivekp14 / name: vivekp14 (d4511d3)

@kropptrevor

Copy link
Copy Markdown

As it says next to the spotless violations:

Run './gradlew spotlessApply' to fix all violations.

@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.51%. Comparing base (a5cd87f) to head (34fee7e).
⚠️ Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
...metry/sdk/metrics/export/PeriodicMetricReader.java 50.00% 1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (50.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8525      +/-   ##
============================================
- Coverage     78.54%   78.51%   -0.03%     
- Complexity     8468     8602     +134     
============================================
  Files          1008     1013       +5     
  Lines         28824    29148     +324     
  Branches       3569     3631      +62     
============================================
+ Hits          22639    22887     +248     
- Misses         5342     5419      +77     
+ Partials        843      842       -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.

@vivekp14

Copy link
Copy Markdown
Author

Build is now passing. I noticed the patch coverage is at 50% — happy to add a test covering the forceFlush failure path if that would help get this merged.

@kropptrevor

Copy link
Copy Markdown

@vivekp14 Yes, please add/modify tests. Make sure to test the tests by breaking it to see the test fail.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Force flush conflicts with periodic background exporting

2 participants