BatchLogRecordProcessor: make export_timeout_millis / OTEL_BLRP_EXPORT_TIMEOUT take effect through cooperative exporter deadlines #4747

Is your feature request related to a problem?

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

• sdk/include/opentelemetry/sdk/logs/batch_log_record_processor_options.h:69-75: the option's own documentation says it is ignored.
• sdk/src/logs/batch_log_record_processor.cc:57-87: the constructors do not store it.
• sdk/src/logs/batch_log_record_processor.cc:296-297: Export() calls exporter_->Export(records).
• sdk/include/opentelemetry/sdk/logs/exporter.h:45-54: LogRecordExporter::Export has no way to receive a deadline.
• sdk/src/configuration/logs_builders.cc:121-124: the option can also be set from declarative configuration.

  /**
   * The maximum time allowed to export data.
   * It is not currently used by the SDK and the parameter is ignored.
   * TODO: Implement the parameter in BatchLogRecordProcessor
   */
  std::chrono::milliseconds export_timeout_millis =
      batch_log_record_processor_options_env::GetExportTimeoutFromEnv();

Problem

BatchLogRecordProcessorOptions::export_timeout_millis can be set from code, from OTEL_BLRP_EXPORT_TIMEOUT, and from declarative configuration, but it has no effect. In practice, the time a slow or unresponsive backend can hold the single worker thread (while the queue fills and records start being dropped) depends only on each exporter's own settings, such as the OTLP exporters' timeout option. Someone who sets OTEL_BLRP_EXPORT_TIMEOUT=5000 will reasonably expect it to bound each export.

The spec (v1.62.0, Logs SDK, Batching processor) defines exportTimeoutMillis as "how long the export can run before it is cancelled", and requires that Export MUST NOT block indefinitely.

Reproduction

A test exporter's Export() sleeps for 2 s. The processor is configured with export_timeout_millis = 100ms, one record is emitted, and ForceFlush(5s) is called. ForceFlush returned after the full sleep:

batch_log_record_processor_export_timeout_test.cc:139: Failure
Expected: (elapsed.count()) < (500), actual: 2001 vs 500
ForceFlush took 2001 ms, which indicates export_timeout_millis (100 ms) was not enforced. ...

This only shows that the option is not used. A sleeping exporter cannot be interrupted, and I am not asking for that.

Environment:

• Commit: e854d29. The relevant files (options header, processor, exporter interface) are unchanged on current main 3ee38aa.
• Toolchain: GCC 11.5.0

Describe the solution you'd like

Make export_timeout_millis reach exporters that can use it, as a cooperative deadline.

Not proposed:

• Fake timeouts built by running Export() on a detached or std::async thread. Those raise the lifetime and resource concerns already discussed in [SDK] Fix crash in PeriodicExportingMetricReader. #2983 and [BUG] BatchSpanProcessor::Shutdown ignores its timeout when the worker thread is blocked in Export() #4532.
• Any change to the default value or to ForceFlush/Shutdown timeout semantics.
• Aborting or detaching a blocked Export(). An exporter that ignores the deadline still runs to completion.

Already implemented in open-telemetry/opentelemetry-go

PR: open-telemetry/opentelemetry-go#5118 — "Add timeoutExporter" (merged 2024-04-01T16:17:07Z, merge commit bddfbc68caa). It adds a timeoutExporter wrapper that gives every Export call a context limited to the configured timeout; the PR description points to #5093 for its use in the batching processor. This is also cooperative, because the exporter has to honor the context.

How opentelemetry-go fixed it:

--- a/sdk/log/exporter.go
+++ b/sdk/log/exporter.go
@@ -53,6 +54,32 @@ func (noopExporter) Shutdown(context.Context) error { return nil }
 
 func (noopExporter) ForceFlush(context.Context) error { return nil }
 
+// timeoutExporter wraps an Exporter and ensures any call to Export will have a
+// timeout for the context.
+type timeoutExporter struct {
+	Exporter
+
+	// timeout is the maximum time an export is attempted.
+	timeout time.Duration
+}
+
+// newTimeoutExporter wraps exporter with an Exporter that limits the context
+// lifetime passed to Export to be timeout. If timeout is less than or equal to
+// zero, exporter will be returned directly.
+func newTimeoutExporter(exp Exporter, timeout time.Duration) Exporter {
+	if timeout <= 0 {
+		return exp
+	}
+	return &timeoutExporter{Exporter: exp, timeout: timeout}
+}
+
+// Export sets the timeout of ctx before calling the Exporter e wraps.
+func (e *timeoutExporter) Export(ctx context.Context, records []Record) error {
+	ctx, cancel := context.WithTimeout(ctx, e.timeout)
+	defer cancel()
+	return e.Exporter.Export(ctx, records)
+}
+
 // exportSync exports all data from input using exporter in a spawned
 // goroutine. The returned chan will be closed when the spawned goroutine
 // completes.

Suggested fix

Keep export_timeout_millis in BatchLogRecordProcessor, compute a per-Export deadline, and pass it through an additional virtual Export overload or option on LogRecordExporter whose default forwards to the existing Export(records) (ABI and source compatibility would need to be checked). Have the in-tree OTLP HTTP/gRPC log exporters use the smaller of that deadline and their own timeout, and update the option documentation to say what is and is not enforced. Not yet tested against this repository.

Describe alternatives you've considered

• Configuring the exporter's own timeout (e.g. OtlpHttpLogRecordExporterOptions::timeout). This works for OTLP, but it is a second knob, and OTEL_BLRP_EXPORT_TIMEOUT silently does nothing.
• Only clarifying the documentation, e.g. noting in the env-var docs that OTEL_BLRP_EXPORT_TIMEOUT is not enforced. This is better than nothing, and it could be a first step.

Additional context

BatchSpanProcessorOptions::export_timeout carries the same "not currently used" note (sdk/include/opentelemetry/sdk/trace/batch_span_processor_options.h:76-82); if the design applies to both processors, this can cover both.

Checked: #4532 (untimed worker join() in processor shutdown), #3687 and #2085 (reading options from env variables), #4498 (drain loop) and #4681 (shutdown timeout adjustment) are related but none makes the export timeout take effect.