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
Uses the configured export interval as the default exporter timeout when no explicit timeout is provided
Enforced the configured timeout for each export batch in PeriodicMetricReader
Preserved existing batching behavior while ensuring timed-out exports are reported as failures
Ensured export backpressure remains active until the underlying exporter operation completes, preventing concurrent exports after a timeout
Added regression tests for timeout behavior and export backpressure

Testing

Ran the relevant Gradle build and tests
Verified the updated timeout and backpressure behavior
Ran Spotless checks
Ran git diff --check

Notes

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

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

Pull request dashboard status

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

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

Inline threads: 1, 2, 3
Top-level threads: 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.

@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

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.30%. Comparing base (f8bd413) to head (7ddd5d8).
⚠️ Report is 60 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8684      +/-   ##
============================================
- Coverage     91.48%   91.30%   -0.19%     
- Complexity    10467    10503      +36     
============================================
  Files          1021     1006      -15     
  Lines         27694    28370     +676     
  Branches       3247     3587     +340     
============================================
+ Hits          25337    25902     +565     
- Misses         1615     1675      +60     
- Partials        742      793      +51

☔ 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:06:01Z

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


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

This is still unaddressed.

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

- Change default timeout from min(interval, 30s) to exactly the configured interval - Remove DEFAULT_EXPORT_TIMEOUT_MILLIS constant (no longer used) - Update Javadoc for both timeout overloads to document new default behavior - Add Javadoc explaining timeout applies to each batch when maxExportBatchSize is configured - Update test to verify default timeout equals interval - Add test to verify explicit timeout is preserved with large intervals

Rajkaran-122 · 2026-08-28T04:03:59Z

@jack-berg sir, please review the pr.

jack-berg · 2026-08-28T05:07:47Z

I'm away I'll get back to this when I can

Rajkaran-122 · 2026-09-08T14:59:34Z

@jack-berg sir PTAL.

jack-berg · 2026-09-08T16:53:47Z

Added a default exporter timeout of 30s

This is from the PR description but is no longer accurate. Can you review the whole PR description and make sure its aligned with the current state of the PR? Thanks.

…test improvements Critical Concurrency Fix: - Fixed critical backpressure issue where timeout firing released exportAvailable while underlying exporter.export(...) was still running - Separated backpressure (tied to raw export result) from timeout reporting (tied to timeout result) in doRun() - Removed applyTimeout() from exportMetrics() to apply timeout only at the top level for reporting, not for backpressure - This prevents concurrent exports from accumulating when exporters are slow Test Improvements: - Replaced per-export Thread spawning with shared ExecutorService in tests - Added deterministic timeout regression test using LogCapturer to verify timeout warning was actually logged - Added concurrency semantics test to prove timeout does not release backpressure - Fixed Error Prone warning about ignored Future return value All tests pass, spotless clean, git diff --check clean.

jack-berg · 2026-09-09T20:13:33Z

https://github.com/open-telemetry/opentelemetry-java/pull/8684/changes#r3960271034

Please don't ignore comments. Also, respond to comments and mark resolved if you feel your changes resolve them.

Rajkaran-122 · 2026-09-09T20:15:34Z

https://github.com/open-telemetry/opentelemetry-java/pull/8684/changes#r3960271034

Please don't ignore comments. Also, respond to comments and mark resolved if you feel your changes resolve them.

Ok sir .

Rajkaran-122 · 2026-09-09T20:23:28Z

Thanks for the review @jack-berg sir. I’ve addressed the requested changes:

Updated the timeout test to deterministically verify that a timeout occurred.
Replaced per-export thread creation with a shared executor in the tests and added proper cleanup.
Fixed the backpressure behavior so exportAvailable remains blocked until the underlying exporter completes, even when the timeout fires.
Added regression coverage for the timeout and concurrent-export behavior.

All relevant tests are passing, and formatting and diff checks are clean.

I’ve also addressed the corresponding review comments and resolved the threads.

Rajkaran-122 · 2026-09-09T20:57:24Z

@jack-berg sir PTAL .

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

Comment thread sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReaderTest.java Outdated

jack-berg reviewed Aug 21, 2026

View reviewed changes

Comment thread sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReader.java

jack-berg reviewed Aug 21, 2026

View reviewed changes

Comment thread sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReader.java Outdated

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

jack-berg reviewed Aug 24, 2026

View reviewed changes

Comment thread sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReaderBuilder.java Outdated

Comment thread sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReaderBuilder.java

Rajkaran-122 requested a review from jack-berg August 26, 2026 20:26

jack-berg reviewed Sep 8, 2026

View reviewed changes

Comment thread sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReaderTest.java

jack-berg reviewed Sep 8, 2026

View reviewed changes

Comment thread sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReaderTest.java Outdated

jack-berg reviewed Sep 8, 2026

View reviewed changes

Comment thread sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReader.java

Rajkaran-122 requested a review from jack-berg September 8, 2026 19:29

Uh oh!

Conversation

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

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

jack-berg Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

jack-berg Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Rajkaran-122 commented Aug 28, 2026

Uh oh!

jack-berg commented Aug 28, 2026

Uh oh!

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

Uh oh!

Uh oh!

jack-berg commented Sep 8, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jack-berg commented Sep 9, 2026

Uh oh!

Rajkaran-122 commented Sep 9, 2026

Uh oh!

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

Uh oh!

Uh oh!

Rajkaran-122 commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Rajkaran-122 commented Aug 2, 2026 •

edited

Loading

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

edited

Loading

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

edited

Loading

codecov Bot commented Aug 21, 2026 •

edited

Loading

Rajkaran-122 commented Aug 22, 2026 •

edited

Loading

Rajkaran-122 commented Sep 8, 2026 •

edited

Loading

Rajkaran-122 commented Sep 9, 2026 •

edited

Loading