Skip to content

[SDK] Clamp BatchLogRecordProcessor batch size to queue size - #4768

Open
Syedowais312 wants to merge 4 commits into
open-telemetry:mainfrom
Syedowais312:fix/log-batch-size-clamp
Open

Syedowais312 wants to merge 4 commits into
open-telemetry:mainfrom
Syedowais312:fix/log-batch-size-clamp

Conversation

@Syedowais312

Copy link
Copy Markdown
Contributor

Fixes #4750

Changes

Clamp max_export_batch_size to a non-zero max_queue_size and emit one warning in all three BatchLogRecordProcessor constructors, matching #4727 for BatchSpanProcessor.

Valid batch sizes and zero-sized queues retain their existing behavior. Update the options documentation and changelog.

Add tests covering oversized batches, valid batches, and zero-sized queues across all three constructors. All 27 tests in batch_log_record_processor_test pass; the three oversized-batch cases fail without the fix.

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed — no public API changes

@Syedowais312
Syedowais312 requested a review from a team as a code owner November 6, 2026 18:49
@codecov

codecov Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.61%. Comparing base (d05c6a8) to head (d20b5b2).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4768      +/-   ##
==========================================
+ Coverage   86.61%   86.61%   +0.01%     
==========================================
  Files         530      530              
  Lines       21169    21176       +7     
==========================================
+ Hits        18333    18340       +7     
  Misses       2836     2836              
Files with missing lines Coverage Δ
sdk/src/logs/batch_log_record_processor.cc 91.33% <100.00%> (+0.33%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread CHANGELOG.md Resolved

@marcalff marcalff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for the fix.

@mateenali66 mateenali66 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for the fix.

At d94b0c19 the three oversized cases fail without the clamp, and the clamp tests pass 30 repeats under TSan.

This branch has not been deployed

No deployments
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.

BatchLogRecordProcessor: validate max_export_batch_size > max_queue_size at construction, as #4727 did for BatchSpanProcessor

4 participants