Skip to content

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

Open

Description

@hidingmyname

Is your feature request related to a problem?

Location (open-telemetry/opentelemetry-cpp @ default branch 3ee38aa):

    : exporter_(std::move(exporter)),
      max_queue_size_(options.max_queue_size),
      scheduled_delay_millis_(options.schedule_delay_millis),
      max_export_batch_size_(options.max_export_batch_size),
      buffer_(options.max_queue_size),

Problem

A user who lowers OTEL_BLRP_MAX_QUEUE_SIZE (or max_queue_size) to save memory, but keeps the default max_export_batch_size of 512, gets a configuration the spec calls invalid (v1.62.0, Logs SDK, Batching processor: "maxExportBatchSize … It must be smaller or equal to maxQueueSize"). The SDK gives no feedback about it.

#4727 (merged 2026-10-27) added exactly this check for BatchSpanProcessor. It clamps to max_queue_size with a single warning and leaves a queue size of 0 alone (ClampMaxExportBatchSize). The log processor did not get the same check, so the two processors now handle the same invalid configuration differently.

To be precise about impact: I have not observed lost log records from this configuration on current main. The concern is about the future. The open PR #4743 (fixes #4498) brings the "wait for a full batch" behavior of #4466 to logs. For spans, that same change is what made an oversized batch size start dropping data, as described in #4727. Adding the check before or together with #4743 avoids repeating that for logs.

Reproduction

This is a code-reading report, so there is no failing output. Construct a BatchLogRecordProcessor with max_queue_size smaller than max_export_batch_size: the constructors above keep the oversized value and log nothing.

Environment: checked on main at e854d29. The log processor source and options header are byte-identical on current main 3ee38aa.

Describe the solution you'd like

The same validation as #4727 in the BatchLogRecordProcessor constructors, including the size_t overload.

Out of scope: BatchLogRecordProcessorOptions is an aggregate type that supports designated initializers. I am not asking for it to normalize values when it is built. Only the processor's effective setting changes. Valid configurations behave the same.

Already implemented in open-telemetry/opentelemetry-go

PR: open-telemetry/opentelemetry-go#5157 — "Clamp the BatchingProcessor batch size to be less than or equal to queue size" (merged 2024-04-05T23:02:32Z, merge commit 98d961b1410). Its log BatchingProcessor clamps the export batch size to the queue size when the configuration is resolved, with the PR citing the spec requirement above. It is a silent clamp; #4727 remains the closer precedent for this repository because of the warning and the queue-size-0 handling.

How opentelemetry-go fixed it:

--- a/sdk/log/batch.go
+++ b/sdk/log/batch.go
@@ -198,6 +198,7 @@ func newBatchingConfig(options []BatchingOption) batchingConfig {
 		clearLessThanOne[int](),
 		getenv[int](envarExpMaxBatchSize),
 		clearLessThanOne[int](),
+		clampMax[int](c.maxQSize.Value),
 		fallback[int](dfltExpMaxBatchSize),
 	)
 
--- a/sdk/log/setting.go
+++ b/sdk/log/setting.go
@@ -42,6 +42,17 @@ func (s setting[T]) Resolve(fn ...resolver[T]) setting[T] {
 	return s
 }
 
+// clampMax returns a resolver that will ensure a setting value is no greater
+// than n. If it is, the value is set to n.
+func clampMax[T ~int | ~int64](n T) resolver[T] {
+	return func(s setting[T]) setting[T] {
+		if s.Value > n {
+			s.Value = n
+		}
+		return s
+	}
+}
+
 // clearLessThanOne returns a resolver that will clear a setting value and
 // change its set state to false if its value is less than 1.
 func clearLessThanOne[T ~int | ~int64]() resolver[T] {

Suggested fix

Clamp max_export_batch_size to max_queue_size with one warning in all three BatchLogRecordProcessor constructors, leaving a queue size of 0 alone, as #4727 does for spans:

diff --git a/sdk/include/opentelemetry/sdk/logs/batch_log_record_processor_options.h b/sdk/include/opentelemetry/sdk/logs/batch_log_record_processor_options.h
--- a/sdk/include/opentelemetry/sdk/logs/batch_log_record_processor_options.h
+++ b/sdk/include/opentelemetry/sdk/logs/batch_log_record_processor_options.h
@@ -76,7 +76,8 @@ struct OPENTELEMETRY_EXPORT BatchLogRecordProcessorOptions
 
   /**
    * The maximum batch size of every export. It must be smaller or
-   * equal to max_queue_size.
+   * equal to max_queue_size. BatchLogRecordProcessor clamps a larger value
+   * to max_queue_size and logs a warning, unless max_queue_size is 0.
    */
   size_t max_export_batch_size =
       batch_log_record_processor_options_env::GetMaxExportBatchSizeFromEnv();
diff --git a/sdk/src/logs/batch_log_record_processor.cc b/sdk/src/logs/batch_log_record_processor.cc
--- a/sdk/src/logs/batch_log_record_processor.cc
+++ b/sdk/src/logs/batch_log_record_processor.cc
@@ -17,6 +17,7 @@
 #include "opentelemetry/sdk/common/atomic_unique_ptr.h"
 #include "opentelemetry/sdk/common/circular_buffer.h"
 #include "opentelemetry/sdk/common/circular_buffer_range.h"
+#include "opentelemetry/sdk/common/global_log_handler.h"
 #include "opentelemetry/sdk/logs/batch_log_record_processor.h"
 #include "opentelemetry/sdk/logs/batch_log_record_processor_options.h"
 #include "opentelemetry/sdk/logs/batch_log_record_processor_runtime_options.h"
@@ -36,6 +37,23 @@ namespace sdk
 {
 namespace logs
 {
+namespace
+{
+// max_export_batch_size must not exceed max_queue_size. A queue size of 0 is left alone: clamping
+// the batch size to 0 would make the worker spin without exporting anything.
+size_t ClampMaxExportBatchSize(size_t max_export_batch_size, size_t max_queue_size)
+{
+  if (max_queue_size > 0 && max_export_batch_size > max_queue_size)
+  {
+    OTEL_INTERNAL_LOG_WARN("[BatchLogRecordProcessor] max_export_batch_size "
+                           << max_export_batch_size << " is greater than max_queue_size "
+                           << max_queue_size << ", using max_queue_size.");
+    return max_queue_size;
+  }
+  return max_export_batch_size;
+}
+}  // namespace
+
 BatchLogRecordProcessor::BatchLogRecordProcessor(
     std::unique_ptr<LogRecordExporter> &&exporter,
     const size_t max_queue_size,
@@ -44,7 +62,7 @@ BatchLogRecordProcessor::BatchLogRecordProcessor(
     : exporter_(std::move(exporter)),
       max_queue_size_(max_queue_size),
       scheduled_delay_millis_(scheduled_delay_millis),
-      max_export_batch_size_(max_export_batch_size),
+      max_export_batch_size_(ClampMaxExportBatchSize(max_export_batch_size, max_queue_size)),
       buffer_(max_queue_size_),
       synchronization_data_(std::make_shared<SynchronizationData>()),
       worker_thread_instrumentation_(nullptr),
@@ -59,7 +77,8 @@ BatchLogRecordProcessor::BatchLogRecordProcessor(std::unique_ptr<LogRecordExport
     : exporter_(std::move(exporter)),
       max_queue_size_(options.max_queue_size),
       scheduled_delay_millis_(options.schedule_delay_millis),
-      max_export_batch_size_(options.max_export_batch_size),
+      max_export_batch_size_(
+          ClampMaxExportBatchSize(options.max_export_batch_size, options.max_queue_size)),
       buffer_(options.max_queue_size),
       synchronization_data_(std::make_shared<SynchronizationData>()),
       worker_thread_instrumentation_(nullptr),
@@ -76,7 +95,8 @@ BatchLogRecordProcessor::BatchLogRecordProcessor(
     : exporter_(std::move(exporter)),
       max_queue_size_(options.max_queue_size),
       scheduled_delay_millis_(options.schedule_delay_millis),
-      max_export_batch_size_(options.max_export_batch_size),
+      max_export_batch_size_(
+          ClampMaxExportBatchSize(options.max_export_batch_size, options.max_queue_size)),
       buffer_(options.max_queue_size),
       synchronization_data_(std::make_shared<SynchronizationData>()),
       worker_thread_instrumentation_(runtime_options.thread_instrumentation),

Verified on e854d29 (GCC 11.4.0, googletest 1.14.0, SDK and tests compiled directly with g++): a regression test that counts the warning fails without this patch and passes with it; no other test results change (batch_log_record_processor_test).

I can submit a PR for this.

Describe alternatives you've considered

Additional context

Checked: #4727 is span-only (merged), and #4498/#4743 are about log batching/flush and do not add this check. The review discussion on #4727 and #4743 shows no planned follow-up for logs.

Activity

  1. added
    needs-triageIndicates an issue or PR lacks a `triage/foo` label and requires one.
    on Oct 30, 2026
  2. adityamukesh commented on Oct 30, 2026

    @adityamukesh

    I'd like to help with this validation. I'd cover each constructor path, valid and oversized batch sizes, and the queue-size-zero exception, following the span processor precedent. I checked #4743: it changes batching and flushing but does not add the constructor check, so I'd keep this work scoped to that gap and coordinate with it.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageIndicates an issue or PR lacks a `triage/foo` label and requires one.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions