Add support for batching in PeriodicMetricReader by psx95 · Pull Request #8296 · open-telemetry/opentelemetry-java

psx95 · 2026-04-15T01:18:20Z

This PR adds support for configuring setMaxExportBatchSize on the PeriodicMetricReader that sets the maximum number of metric data points to be sent in a single export call.

If the number of metric data points (across Collection<MetricData>) scheduled to be export exceeds the maxExportBatchSize, the data points organized in batches and sent over multiple export calls.

These changes do not modify the current timeout behavior, which is still per export call.

Fix #8245

otelbot · 2026-04-15T14:00:24Z

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

codecov · 2026-04-15T14:19:24Z

Codecov Report

❌ Patch coverage is 98.47328% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.31%. Comparing base (a6fadff) to head (34ca25a).
⚠️ Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
...emetry/sdk/metrics/export/MetricExportBatcher.java	98.26%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8296      +/-   ##
============================================
+ Coverage     90.27%   90.31%   +0.04%     
- Complexity     7684     7708      +24     
============================================
  Files           850      851       +1     
  Lines         23186    23318     +132     
  Branches       2352     2363      +11     
============================================
+ Hits          20931    21060     +129     
- Misses         1530     1532       +2     
- Partials        725      726       +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.

dashpole · 2026-04-15T19:42:07Z

+    Collection<MetricData> currentBatch = new ArrayList<>(maxExportBatchSize);
+
+    // Fill active batch and split overlapping metric points if needed
+    for (MetricData metricData : metrics) {


nit: i expected prepareExportBatches to take the Collection so that all of the batching logic is contained within the batcher (and we don't have to pass + update currentBatch).

But we are not technically updating the currentBatch in this loop - the contents within the currentBatch is updated by the prepareExportBatches.

My intention behind the prepareExportBatches was a function that splits a given MetricData into appropriate export batches. Since the batches need to be completely filled (except the last one), this function requires the currentBatch as well.

Maybe if the function was renamed to splitMetricData it would make more sense?

dashpole · 2026-04-15T19:43:36Z

+              batches = metricsBatcher.batchMetrics(metricData);
+              List<CompletableResultCode> results = new ArrayList<>(batches.size());
+              for (Collection<MetricData> batch : batches) {
+                results.add(exporter.export(batch));


I don't see exportTimeoutMillis implemented in the periodic reader anywhere. https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#periodic-exporting-metricreader. I mostly want to make sure that the timeout, if it exists, is applied to each export, and not to all exports together.

Yes, it's set at the exporter level and I think it's on a per export call basis.

I was not 100% sure about this as well, which is why I called it out in the PR description. Maybe other reviewers can confirm/guide here.

Copilot

Pull request overview

Adds configurable export batching to PeriodicMetricReader so large metric collections can be split into multiple export calls based on a new setMaxExportBatchSize(int) builder option (per spec / #8245).

Changes:

Add PeriodicMetricReaderBuilder#setMaxExportBatchSize(int) and plumb a MetricExportBatcher into PeriodicMetricReader.
Implement MetricExportBatcher to split MetricData point collections into bounded batches.
Add/expand unit tests for batching behavior and builder validation.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReaderBuilder.java	Adds `setMaxExportBatchSize` configuration and passes batcher into reader.
sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReader.java	Uses `MetricExportBatcher` to split exports into multiple calls when configured.
sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/export/MetricExportBatcher.java	New batching/splitting implementation for metric point data.
sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/export/PeriodicMetricReaderTest.java	Adds validation and integration-style tests for periodic batching behavior.
sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/export/MetricExportBatcherTest.java	New focused unit tests for batching and splitting across metric types.
docs/apidiffs/current_vs_latest/opentelemetry-sdk-metrics.txt	Records new public builder method in API diffs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-16T00:52:04Z

+      while (currentIndex < totalPointsInMetricData) {
+        int pointsToTake =
+            Math.min(totalPointsInMetricData - currentIndex, remainingCapacityInCurrentBatch);
+        currentBatch.add(
+            copyMetricData(metricData, originalPointsList, currentIndex, pointsToTake));


prepareExportBatches can enter an infinite loop when remainingCapacityInCurrentBatch is 0 (i.e., the current batch is already full) and the next MetricData has >0 points. In that case pointsToTake becomes 0, currentIndex never advances, and the while loop never terminates. Handle the zero-capacity case by flushing/rolling over to a new batch before the loop (or ensuring pointsToTake is always >0). Please also add a test that covers the scenario where a metric exactly fills a batch and the following metric triggers a split.

I don't think this will happen. When remainingCapacityInCurrentBatch is 0:

First Iteration:

pointsToTake = Math.min(totalPointsInMetricData - 0, 0) evaluates to 0.

copyMetricData(..., 0, 0) executes and creates a MetricData with 0 points, which is added to currentBatch.

currentIndex and remainingCapacityInCurrentBatch remain at 0.

The condition if (remainingCapacityInCurrentBatch == 0) evaluates to true, causing the currentBatch (now containing the extra 0-point MetricData) to be flushed to preparedBatches.

Crucially, remainingCapacityInCurrentBatch is then reset to maxExportBatchSize (which is strictly positive).

Second Iteration:

Since currentIndex is still 0 < totalPointsInMetricData.

The next iteration starts with remainingCapacityInCurrentBatch == maxExportBatchSize.

pointsToTake is now > 0, currentIndex advances normally, and the loop successfully terminates!

Copilot · 2026-04-16T00:52:05Z

+    int currentBatchPoints = 0;
+    for (MetricData m : currentBatch) {
+      currentBatchPoints += m.getData().getPoints().size();
+    }
+    int remainingCapacityInCurrentBatch = maxExportBatchSize - currentBatchPoints;


prepareExportBatches recomputes currentBatchPoints by iterating the entire currentBatch for every incoming MetricData, which makes batching O(n²) in the number of metrics per batch. Consider tracking the current batch point count incrementally (e.g., return it in MetricDataSplitOperationResult or maintain it alongside currentBatch) to keep batching linear.

This is technically correct, but I'm not sure if the performance gain is worth complicating the logic for a loop that will be a reasonably small number of iterations (a reasonable batch size would be < 500 points?).

I'd be curious to hear other reviewers' thoughts on this.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-16T16:35:07Z

+            if (metricsBatcher != null) {
+              batches = metricsBatcher.batchMetrics(metricData);
+              List<CompletableResultCode> results = new ArrayList<>(batches.size());
+              for (Collection<MetricData> batch : batches) {
+                results.add(exporter.export(batch));
+              }
+              result = CompletableResultCode.ofAll(results);
+            } else {


MetricExporter#export documents that the caller (PeriodicMetricReader) will not call export again until the previous call completes. With batching enabled, this loop invokes exporter.export(batch) multiple times back-to-back without waiting for each returned CompletableResultCode to complete, which can lead to concurrent in-flight exports for exporters that are not re-entrant. Please change the batching export logic to export batches sequentially (start the next export only in the previous export's completion callback) and aggregate the final result from that chain.

psx95 added 7 commits April 13, 2026 20:37

WIP: Add metrics batcher in the SDK

08deab0

Allow exporting metricData batches

431af24

Make existing tests compatible with changes

986fd6d

Add docs and update existing tests

78241ed

Fix MetricExportBatcher logic and add tests

2543a51

Add support for EXPONENTIAL_HISTOGRAM and SUMMARY data types

6fadc65

Fix metrics checkstyle issue

8153388

psx95 force-pushed the issue-8245 branch from acad099 to 8153388 Compare April 15, 2026 13:40

Add missing generated diff files

d85e3a2

otelbot bot added the api-change Changes to public API surface area label Apr 15, 2026

psx95 added 6 commits April 15, 2026 15:50

Update unit tests for enhanced coverage

bb86015

Add unit tests for PeriodicMetricReader

1bda08c

Fix checkstyle issues

400981b

Clean up inline code comments

95370fa

Fix bug for miscalculating remaining capacity of a batch

e470a0f

Add missing Javadoc for public facing API

03364b5

psx95 force-pushed the issue-8245 branch from 9d84747 to 03364b5 Compare April 15, 2026 18:17

psx95 marked this pull request as ready for review April 15, 2026 18:42

psx95 requested a review from a team as a code owner April 15, 2026 18:42

dashpole reviewed Apr 15, 2026

View reviewed changes

Refactor logic in prepareExportBatches to remove redundancy

73c380d

dashpole requested a review from Copilot April 16, 2026 00:47

Copilot started reviewing on behalf of dashpole April 16, 2026 00:48 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

Address comment about defensive copy for original point sublist

67509b5

psx95 requested a review from Copilot April 16, 2026 15:59

Copilot started reviewing on behalf of psx95 April 16, 2026 16:00 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

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

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

Prevent copying MetricData for 0 points

ea6330e

dashpole requested a review from Copilot April 16, 2026 16:30

Copilot started reviewing on behalf of dashpole April 16, 2026 16:31 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

Add test case to verify there are no batches with empty metric points

34ca25a

Conversation

psx95 commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

otelbot bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ API changes detected — additional maintainer review required

Uh oh!

codecov bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dashpole Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

psx95 Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

dashpole Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

psx95 Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

psx95 Apr 16, 2026

Choose a reason for hiding this comment

First Iteration:

Second Iteration:

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

psx95 Apr 16, 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!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

psx95 commented Apr 15, 2026 •

edited

Loading

otelbot bot commented Apr 15, 2026 •

edited

Loading

codecov bot commented Apr 15, 2026 •

edited

Loading

psx95 Apr 16, 2026 •

edited

Loading