Skip to content

Fix batched forceFlush failure and make batching linear#5

Open
zeitlinger wants to merge 3 commits intopsx95:issue-8245from
zeitlinger:gregor/8296-forceflush-failure-propagation
Open

Fix batched forceFlush failure and make batching linear#5
zeitlinger wants to merge 3 commits intopsx95:issue-8245from
zeitlinger:gregor/8296-forceflush-failure-propagation

Conversation

@zeitlinger
Copy link
Copy Markdown

@zeitlinger zeitlinger commented Apr 21, 2026

Summary

  • propagate batched export failure to the outer forceFlush() result in PeriodicMetricReader
  • make MetricExportBatcher track current batch point count incrementally so batching stays linear instead of rescanning the active batch for every metric
  • update the partial-failure test to assert the flush fails when one batch export fails

Testing

  • ./gradlew :sdk:metrics:test --tests io.opentelemetry.sdk.metrics.export.PeriodicMetricReaderTest --tests io.opentelemetry.sdk.metrics.export.MetricExportBatcherTest

This is a small follow-up to open-telemetry#8296 addressing:

  • the forceFlush() success-reporting bug after a failed batch export
  • the batching complexity concern around recomputing active batch size on every metric

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger zeitlinger changed the title Fix batched forceFlush failure propagation Fix batched forceFlush failure and make batching linear Apr 21, 2026
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
assertThat(reader.forceFlush().join(10, TimeUnit.SECONDS).isSuccess()).isTrue();
assertThat(reader.forceFlush().join(10, TimeUnit.SECONDS).isSuccess()).isFalse();
assertThat(reader.forceFlush().join(10, TimeUnit.SECONDS).isSuccess()).isFalse();
assertThat(reader.forceFlush().join(10, TimeUnit.SECONDS).isSuccess()).isTrue();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Now that the forceFlush() change has been reverted, these tests should be updated as well?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants