Skip to content

Refator PMR to use dedicated thread and add export timeout setting - #8827

Merged
jack-berg merged 3 commits into
open-telemetry:mainfrom
jack-berg:pmr-timeout-redesign
Oct 21, 2026
Merged

Refator PMR to use dedicated thread and add export timeout setting#8827
jack-berg merged 3 commits into
open-telemetry:mainfrom
jack-berg:pmr-timeout-redesign

Conversation

@jack-berg

@jack-berg jack-berg commented Sep 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes #8311.

Alternative to #8684. Motivation for this described here: #8684 (comment)

Summary:

  • Refactor PMR to use a dedicated worker thread for collect / export, matching the shape of BSP and BLRP
  • Add new PeriodicMetricReaderBuilder#setExporterTimeout(..) on top of the new shape, the implementation of which is much simpler after the refactor.
  • The timeout defaults to the configured interval (as opposed to the spec which states it should be 30s) so as to no break users by applying a more restrictive default than they would be getting today
  • When maxExportBatchSize is set, timeout applies per export (aligned with spec)

@otelbot otelbot Bot added the api-change Changes to public API surface area label Sep 15, 2026
@otelbot

otelbot Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

⚠️ 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.

@jack-berg jack-berg mentioned this pull request Sep 15, 2026
Closed
@jack-berg
jack-berg marked this pull request as ready for review September 15, 2026 21:59
@jack-berg
jack-berg requested a review from a team as a code owner September 15, 2026 21:59
@codecov

codecov Bot commented Sep 15, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.90826% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.42%. Comparing base (7647e49) to head (bf1e377).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...metry/sdk/metrics/export/PeriodicMetricReader.java 90.19% 6 Missing and 4 partials ⚠️
...dk/metrics/export/PeriodicMetricReaderBuilder.java 85.71% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #8827      +/-   ##
============================================
- Coverage     90.43%   90.42%   -0.02%     
- Complexity    10908    10930      +22     
============================================
  Files          1015     1015              
  Lines         29854    29875      +21     
  Branches       3901     3905       +4     
============================================
+ Hits          26998    27013      +15     
- Misses         1907     1911       +4     
- Partials        949      951       +2

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

Rajkaran-122 added a commit to Rajkaran-122/opentelemetry-java that referenced this pull request Sep 16, 2026
@Rajkaran-122
Refactor PeriodicMetricReader to use dedicated worker thread for exports
1926415 
- Replace scheduler-thread exports with dedicated worker thread architecture
- Scheduler now only emits periodic tick signals; worker owns export execution
- Use blocking signal queue for serial processing of ticks, flushes, and shutdown
- Implement per-batch timeout enforcement that waits for actual completion
- Add regression test proving no concurrent exporter invocations after timeout
- Prevents concurrent Export calls as required by metrics specification

The timeout behavior now correctly:
- Applies timeout to each individual export(batch) invocation
- Reports timeout after configured duration elapses
- Waits for actual export completion before proceeding to next batch
- Prevents concurrent exporter operations when timeout expires

This addresses issue open-telemetry#8311 and aligns with PR open-telemetry#8827's dedicated-worker design.
@Rajkaran-122

Rajkaran-122 commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

@jack-berg sir Would it be okay if I add the exporterTimeout_waitsForCompletionBeforeNextBatch regression test from #8684 to #8827? I’ll keep it limited to the test and won’t make any implementation changes.

@jack-berg

jack-berg commented Sep 17, 2026

Copy link
Copy Markdown
Member Author

Sure go for it. Open a the source branch of this PR in my fork. I'll review / merge it, and then the commit will be added to the batch.

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Sep 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Pull request dashboard status

Merged · refreshed 2026-10-21 20:38 UTC

Status above doesn't look right?
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

@opentelemetry-pr-dashboard opentelemetry-pr-dashboard Bot mentioned this pull request Sep 17, 2026
Open
@Rajkaran-122 Rajkaran-122 mentioned this pull request Sep 18, 2026
Open
psx95
psx95 reviewed Sep 18, 2026
View reviewed changes
Comment thread sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReaderBuilder.java
Comment thread sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReader.java
Comment thread sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReader.java Outdated
Comment thread sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReader.java
@Rajkaran-122

Rajkaran-122 commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

Sure go for it. Open a the source branch of this PR in my fork. I'll review / merge it, and then the commit will be added to the batch.

@jack-berg sir PTAL jack-berg#29 .

@opentelemetry-pr-dashboard

This comment has been minimized.

Sign in to view
jkwatson
jkwatson approved these changes Oct 20, 2026
View reviewed changes

@jkwatson jkwatson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

API changes look fine to me.

@jack-berg

jack-berg commented Oct 21, 2026

Copy link
Copy Markdown
Member Author

Hey @psx95 - sorry for delayed response to your review. I missed it originally and it got lost in the shuffle. Noticed it after the John's approval popped this to the top of my inbox. I've responded to your feedback in 5930524

@jack-berg
jack-berg merged commit 3a6bfef into open-telemetry:main Oct 21, 2026
34 checks passed
@jack-berg jack-berg mentioned this pull request Oct 21, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkwatson jkwatson jkwatson approved these changes
@psx95 psx95 psx95 approved these changes

Assignees

No one assigned

Labels

api-change Changes to public API surface area

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Metrics: clarify and align PeriodicMetricReader export timeout semantics with batching spec

4 participants

@jack-berg @Rajkaran-122 @jkwatson @psx95