KEMBAR78
Propagate flush to PeriodicMetricReader's metricExporter. by tylerbenson · Pull Request #7410 · open-telemetry/opentelemetry-java · GitHub
Skip to content

Conversation

@tylerbenson
Copy link
Member

On a synchronous exporter this doesn't matter, but the API design allows for asynchronous exporters, which requires calling flush after manually exporting.

Fixes #7343

On a synchronous exporter this doesn't matter, but the API design allows for asynchronous exporters, which requires calling flush after manually exporting.

Fixes open-telemetry#7343
@tylerbenson tylerbenson requested a review from a team as a code owner June 9, 2025 23:46
Comment on lines 84 to 98
CompletableResultCode result = new CompletableResultCode();
CompletableResultCode flushResult = scheduled.doRun();
flushResult.whenComplete(
() ->
exporter
.flush()
.whenComplete(
() -> {
if (!flushResult.isSuccess()) {
result.fail();
} else {
result.succeed();
}
}));
return result;
Copy link
Member Author

Choose a reason for hiding this comment

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

I've seen this pattern in enough places I wonder if it would be worth adding CompletableResultCode.whenComplete(Callable<CompletableResultCode>) that handles this logic in one place.

Copy link
Member

Choose a reason for hiding this comment

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

I like this idea

@codecov
Copy link

codecov bot commented Jun 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.76%. Comparing base (d5a2fa2) to head (697d280).
Report is 17 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #7410   +/-   ##
=========================================
  Coverage     89.75%   89.76%           
- Complexity     6981     6985    +4     
=========================================
  Files           797      797           
  Lines         21165    21175   +10     
  Branches       2057     2057           
=========================================
+ Hits          18997    19007   +10     
+ Misses         1505     1504    -1     
- Partials        663      664    +1     

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Contributor

@jkwatson jkwatson left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@jack-berg jack-berg left a comment

Choose a reason for hiding this comment

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

@jack-berg jack-berg merged commit aeb08f4 into open-telemetry:main Jun 24, 2025
28 checks passed
@tylerbenson tylerbenson deleted the tyler/call-flush branch June 24, 2025 19:45
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.

PeriodicMetricReader doesn't call flush on the delegate

3 participants