Skip to content

PoC for batching in PeriodicReader#7930

Closed
dashpole wants to merge 1 commit intoopen-telemetry:mainfrom
dashpole:metric_batch_poc
Closed

PoC for batching in PeriodicReader#7930
dashpole wants to merge 1 commit intoopen-telemetry:mainfrom
dashpole:metric_batch_poc

Conversation

@dashpole
Copy link
Contributor

Batching logic is based on the collector's batchprocessor: https://github.com/open-telemetry/opentelemetry-collector/blob/587b90b9ecc1db959ee9104d5bf993591f80ca43/processor/batchprocessor/splitmetrics.go

PoC for open-telemetry/opentelemetry-specification#4895

This PR adds metric.WithMaxExportBatchSize to go.opentelemetry.io/sdk/metric, and causes the SDK to split batches before passing them to the exporter.

One potential issue is that I was hoping the export timeout could be applied individually to each batch, rather than to multiple serial export calls. But currently, we apply the timeout to collect + export. I've changed it to apply the timeout individually to collect, and to each export, but I'm curious how acceptable other maintainers think this kind of change would be. In practice, I suspect Collect() is not a notable source of latency or timeouts.

The spec for the timeout says:

exportTimeoutMillis - how long the export can run before it is cancelled. The default value is 30000 (milliseconds).

It would strike me as a bit odd if we did split metrics into batches to have the timeout still applied to the entire collect + multiple exports based on the spec.

@dashpole dashpole added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Feb 20, 2026
@codecov
Copy link

codecov bot commented Feb 20, 2026

Codecov Report

❌ Patch coverage is 57.59494% with 67 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.5%. Comparing base (64f28b0) to head (a93066b).
⚠️ Report is 64 commits behind head on main.

Files with missing lines Patch % Lines
sdk/metric/splitmetrics.go 48.8% 64 Missing and 1 partial ⚠️
sdk/metric/periodic_reader.go 93.5% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #7930     +/-   ##
=======================================
- Coverage   81.7%   81.5%   -0.2%     
=======================================
  Files        304     305      +1     
  Lines      23283   23430    +147     
=======================================
+ Hits       19032   19116     +84     
- Misses      3864    3926     +62     
- Partials     387     388      +1     
Files with missing lines Coverage Δ
sdk/metric/periodic_reader.go 86.9% <93.5%> (+1.5%) ⬆️
sdk/metric/splitmetrics.go 48.8% <48.8%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip Changelog PRs that do not require a CHANGELOG.md entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant