feat(sdk-metrics): align PeriodicMetricReader export timeout semantics by Rajkaran-122 · Pull Request #8684 · open-telemetry/opentelemetry-java

Rajkaran-122 · 2026-08-02T12:19:19Z

Summary

Align PeriodicMetricReader export timeout behavior with the Metrics specification by enforcing an exporter timeout for each export batch.

Changes

Added configurable exporter timeout support to PeriodicMetricReaderBuilder
Added a default exporter timeout of 30s
Enforced the configured timeout for each export batch in PeriodicMetricReader
Preserved existing batching behavior while ensuring timed-out exports are reported as failures

Testing

Ran the relevant Gradle build and tests
Verified the updated timeout behavior

Notes

This change updates export timeout behavior without affecting metric collection or scheduling semantics.

opentelemetry-pr-dashboard · 2026-08-02T12:19:58Z

Pull request dashboard status

Waiting on the author · refreshed 2026-08-22 14:08 UTC

Respond to 4 review items (e.g. link a commit, explain why not, ask a follow-up):

Inline threads: 1, 2, 3, 4

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.
Should this be with reviewers? Comment /dashboard route:reviewers to route it to them.
Anything wrong — including the routing? Report it with what you expected; it helps us improve the dashboard.

…cheduler Replace scheduler-based withTimeout() with CompletableResultCode.join(), matching the established pattern in BatchSpanProcessor and BatchLogRecordProcessor. The previous approach used scheduler.schedule() which throws RejectedExecutionException during shutdown because the scheduler is intentionally shut down before the final export flush.

opentelemetry-pr-dashboard · 2026-08-09T15:13:13Z

Hi @Rajkaran-122 — just a friendly reminder that this pull request is waiting on you. The dashboard status comment has the open items and is kept current.

Replying is enough to hand it off — answer, explain why no change is needed, or ask a follow-up. The dashboard routes it onward once nothing on the list is waiting on you.
To hand it back for any other reason, including the dashboard getting this wrong, comment /dashboard route:reviewers.

jack-berg · 2026-08-20T21:26:39Z

+   * Sets the timeout for the underlying exporter. If unset, defaults to {@value
+   * DEFAULT_EXPORT_TIMEOUT_MILLIS}ms.
+   *
+   * @since 1.40.0


Remove the since annotations. They're wrong and are added as part of the release process anyway

jack-berg · 2026-08-20T21:27:42Z

+   *
+   * @since 1.40.0
+   */
+  public PeriodicMetricReaderBuilder setExporterTimeout(long timeout, TimeUnit unit) {


This is new public API surface area, which comes with new content checks into /docs/apidiffs. Please run the build to generate this.

Also, you'll see failing tests if you run the build. You'll want to fix those failing tests, and add new tests for this specific feature.

jack-berg · 2026-08-20T21:35:54Z

      if (maxExportBatchSize == 0) {
-        return exporter.export(metricData);
+        CompletableResultCode result = exporter.export(metricData);
+        result.join(exporterTimeoutNanos, TimeUnit.NANOSECONDS);


This change in semantic (here and below) causes callers to be blocked waiting for the timeout. I don't think this is a desirable or necessary to add a timeout.

jack-berg · 2026-08-20T21:42:12Z

 public final class PeriodicMetricReaderBuilder {

  static final long DEFAULT_SCHEDULE_DELAY_MINUTES = 1;
+  static final int DEFAULT_EXPORT_TIMEOUT_MILLIS = 30_000;


Adding a default timeout when none exists today is problematic, since it can change behavior that has been otherwise working. We either have to call the lack of a timeout a bug, or be more conservative and set the default timeout to the interval when the user doesn't explicitly set it.

Thanks @jack-berg sir or the feedback. I’ve addressed the API review comments, generated the API diff, and added tests for the new timeout API.

I also updated the timeout implementation to avoid blocking the PeriodicMetricReader scheduler while preserving asynchronous batching and handling shutdown/final export safely.

The full CI matrix is now passing. I’d appreciate your review of the updated implementation.

This comment is not addressed. To address, you would need to leave exporterTimeoutNanos as null, and resolve the value conditionally in build().

@SInCE

…ication - Added setExporterTimeout() API to PeriodicMetricReaderBuilder with 30-second default per spec - Removed @SInCE 1.40.0 annotations as requested by reviewer - Updated API-diff to reflect new public API - Added comprehensive tests for timeout behavior - Implemented conditional timeout enforcement (only when explicitly configured) - Addressed blocking concern by making timeout opt-in via setExporterTimeout()

otelbot · 2026-08-21T05:42:27Z

⚠️ API changes detected — additional maintainer review required

@jack-berg @jkwatson

This PR modifies the public API surface area of the following module(s):

opentelemetry-sdk-metrics

Please review the changes in docs/apidiffs/current_vs_latest/ carefully before approving.

…lder

…Reader Add asynchronous timeout enforcement for MetricExporter operations in PeriodicMetricReader using a dedicated timeout executor. This preserves the existing asynchronous scheduling and batching behavior while enforcing the spec-required 30-second default export timeout. Changes: - Add dedicated ScheduledExecutorService for timeout scheduling - Implement applyTimeout() method with asynchronous timeout enforcement - Preserve async batch processing with Iterator-based sequential execution - Fix Error Prone warnings (UnusedVariable, PreferJavaTimeOverload) - Add timeout enforcement test The timeout executor is shut down after final export completes to avoid RejectedExecutionException during shutdown. Timeout enforcement fails the result when timeout expires without blocking the periodic scheduler. Resolves CI compilation failures in PR open-telemetry#8684.

codecov · 2026-08-21T08:04:29Z

Codecov Report

❌ Patch coverage is 97.43590% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.29%. Comparing base (f8bd413) to head (acca5b7).
⚠️ Report is 44 commits behind head on main.

Files with missing lines	Patch %	Lines
...metry/sdk/metrics/export/PeriodicMetricReader.java	96.55%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8684      +/-   ##
============================================
- Coverage     91.48%   91.29%   -0.20%     
- Complexity    10467    10479      +12     
============================================
  Files          1021     1006      -15     
  Lines         27694    28310     +616     
  Branches       3247     3575     +328     
============================================
+ Hits          25337    25846     +509     
- Misses         1615     1674      +59     
- Partials        742      790      +48

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

jack-berg · 2026-08-21T18:04:15Z

+    }
+  }
+
+  private static class SlowMetricExporter implements MetricExporter {


Combine this with FastMetricExporter and just set the sleep time to 0 for the fast case.

jack-berg · 2026-08-21T18:06:01Z

+  }
+
+  @Test
+  void explicitTimeout_exporterCompletesBeforeTimeout() throws Exception {


This and the test below is (and maybe all the validation cases could be parameterized tests

jack-berg · 2026-08-21T18:19:00Z

+    private CompletableResultCode applyTimeout(CompletableResultCode result) {
+      if (exporterTimeoutNanos == Long.MAX_VALUE) {
+        return result;
+      }
+      CompletableResultCode timeoutResult = new CompletableResultCode();
+      AtomicBoolean timedOut = new AtomicBoolean(false);
+      ScheduledFuture<?> timeoutFuture =
+          timeoutExecutor.schedule(
+              () -> {
+                if (!result.isDone()) {
+                  timedOut.set(true);
+                  logger.log(
+                      Level.WARNING, "Export timed out after " + exporterTimeoutNanos + "ns");
+                  timeoutResult.fail();
+                }
+              },
+              exporterTimeoutNanos,
+              TimeUnit.NANOSECONDS);
+      result.whenComplete(
+          () -> {
+            if (timeoutFuture != null) {
+              timeoutFuture.cancel(false);
+            }
+            if (!timedOut.get()) {
+              if (result.isSuccess()) {
+                timeoutResult.succeed();
+              } else {
+                timeoutResult.fail();
+              }
+            }
+          });
+      return timeoutResult;
+    }


Suggested change

private CompletableResultCode applyTimeout(CompletableResultCode result) {

if (exporterTimeoutNanos == Long.MAX_VALUE) {

return result;

}

CompletableResultCode timeoutResult = new CompletableResultCode();

AtomicBoolean timedOut = new AtomicBoolean(false);

ScheduledFuture<?> timeoutFuture =

timeoutExecutor.schedule(

() -> {

if (!result.isDone()) {

timedOut.set(true);

logger.log(

Level.WARNING, "Export timed out after " + exporterTimeoutNanos + "ns");

timeoutResult.fail();

}

},

exporterTimeoutNanos,

TimeUnit.NANOSECONDS);

result.whenComplete(

() -> {

if (timeoutFuture != null) {

timeoutFuture.cancel(false);

}

if (!timedOut.get()) {

if (result.isSuccess()) {

timeoutResult.succeed();

} else {

timeoutResult.fail();

}

}

});

return timeoutResult;

}

private CompletableResultCode applyTimeout(CompletableResultCode result) {

if (exporterTimeoutNanos == Long.MAX_VALUE) {

return result;

}

CompletableResultCode timeoutResult = new CompletableResultCode();

ScheduledFuture<?> timeoutFuture =

scheduler.schedule(

() -> {

logger.log(Level.WARNING, "Export timed out after " + exporterTimeoutNanos + "ns");

timeoutResult.fail();

},

exporterTimeoutNanos,

TimeUnit.NANOSECONDS);

result.whenComplete(

() -> {

timeoutFuture.cancel(false);

if (result.isSuccess()) {

timeoutResult.succeed();

} else {

timeoutResult.fail();

}

});

return timeoutResult;

}

jack-berg · 2026-08-21T18:20:05Z

    this.intervalNanos = intervalNanos;
+    this.exporterTimeoutNanos = exporterTimeoutNanos;
    this.scheduler = scheduler;
+    this.timeoutExecutor =


Instead of a new schedule executor and the logic to shut it down, can expand the existing scheduler initialized in the builder to have 2 threads.

… jack-berg review - Remove separate timeout executor and reuse existing scheduler - Increase scheduler from 1 to 2 threads to handle both periodic exports and timeout tasks - Simplify applyTimeout() by removing AtomicBoolean/timedOut state tracking - Combine SlowMetricExporter and FastMetricExporter into single DelayingMetricExporter - Add RejectedExecutionException handling for shutdown race condition - Fix BooleanParameter warnings in tests

Rajkaran-122 · 2026-08-21T19:12:29Z

Thanks, @jack-berg sir. I've updated the implementation based on your feedback:

Reused the existing ScheduledExecutorService and increased it to 2 threads.
Removed the separate timeout executor and its shutdown lifecycle.
Simplified applyTimeout() by removing the AtomicBoolean state.
Combined SlowMetricExporter and FastMetricExporter into a single configurable DelayingMetricExporter.
Kept the timeout tests focused on the existing behavior.

I also handled the scheduler-shutdown case in applyTimeout() because the final shutdown export can race with scheduler shutdown when scheduling the timeout task.

All relevant sdk:metrics formatting, compilation, and PeriodicMetricReaderTest checks pass.

Rajkaran-122 · 2026-08-22T09:43:32Z

@jack-berg sir , please review this pr.

skrcode · 2026-08-22T11:20:19Z

I ran JAIPilot Cloud against this exact PR head. It found one deterministic follow-up: when an export is already complete, skip creating and immediately cancelling the timeout task.

The focused path changed from 1 schedule/cancel pair to 0. Baseline passed 31 focused tests, candidate passed 32 including the new regression test, and both clean sdk:metrics builds completed 161 tasks. No wall-clock speed claim is being made.

Cloud-generated draft and evidence: skrcode#2
PR directly onto this source branch: Rajkaran-122#1

Feel free to merge or cherry-pick if it fits the intended timeout semantics.

…ptimization

Rajkaran-122 · 2026-08-22T13:19:05Z

@jack-berg sir, please review the pr.

feat: align PeriodicMetricReader export timeout semantics

41910f3

Rajkaran-122 requested a review from a team as a code owner August 2, 2026 12:19

opentelemetry-pr-dashboard Bot mentioned this pull request Aug 2, 2026

Pull Request Dashboard #8439

Open

Rajkaran-122 added 3 commits August 2, 2026 17:57

fix: resolve test failures by implementing export timeout asynchronously

e371867

fix: resolve errorprone warnings

b78cae9

jack-berg reviewed Aug 20, 2026

View reviewed changes

otelbot Bot added the api-change Changes to public API surface area label Aug 21, 2026

Rajkaran-122 added 3 commits August 21, 2026 11:23

fix: resolve JavaDoc formatting violations in PeriodicMetricReaderBui…

c3b8e7f

…lder

fix: apply Spotless formatting changes

7d07ef8

Rajkaran-122 requested a review from jack-berg August 21, 2026 18:03

jack-berg reviewed Aug 21, 2026

View reviewed changes

Rajkaran-122 requested a review from jack-berg August 21, 2026 19:10

This was referenced Aug 22, 2026

Mirror: align PeriodicMetricReader export timeout semantics skrcode/opentelemetry-java#1

Open

Skip redundant timeout scheduling for completed exports Rajkaran-122/opentelemetry-java#1

Open

Address review comments for exporter timeout defaults and fast-path o…

acca5b7

…ptimization

Uh oh!

Conversation

Rajkaran-122 commented Aug 2, 2026

Summary

Changes

Testing

Notes

Uh oh!

opentelemetry-pr-dashboard Bot commented Aug 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request dashboard status

Uh oh!

opentelemetry-pr-dashboard Bot commented Aug 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rajkaran-122 Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

otelbot Bot commented Aug 21, 2026

⚠️ API changes detected — additional maintainer review required

Uh oh!

codecov Bot commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rajkaran-122 commented Aug 21, 2026

Uh oh!

Rajkaran-122 commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skrcode commented Aug 22, 2026

Uh oh!

Rajkaran-122 commented Aug 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

opentelemetry-pr-dashboard Bot commented Aug 2, 2026 •

edited

Loading

opentelemetry-pr-dashboard Bot commented Aug 9, 2026 •

edited

Loading

Rajkaran-122 Aug 21, 2026 •

edited

Loading

codecov Bot commented Aug 21, 2026 •

edited

Loading

Rajkaran-122 commented Aug 22, 2026 •

edited

Loading