From c56c84380c4569ae465dcfc32064883172f86c66 Mon Sep 17 00:00:00 2001 From: Ali Asghar <98263017+alliasgher@users.noreply.github.com> Date: Wed, 15 Apr 2026 21:04:18 +0500 Subject: [PATCH] sdk/trace: propagate SpanExporter.Shutdown error from BatchSpanProcessor (#8197) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #6878. `bsp.e.Shutdown(ctx)` was called with `:=` inside an `if` statement, creating a new variable that shadowed the outer `err`. The error was handled via `otel.Handle` but never returned to the caller of `BatchSpanProcessor.Shutdown`. ### Fix Use a dedicated `exportErr` variable. The goroutine writes it before closing the `wait` channel; the caller reads it only in the `<-wait` select case (where the goroutine is guaranteed done). This is race-free — addressing the concern raised by @seh in #6878. All existing `TestBatchSpanProcessor*` tests pass. --------- Signed-off-by: alliasgher Co-authored-by: Tyler Yahn Co-authored-by: Robert Pająk --- CHANGELOG.md | 1 + sdk/trace/batch_span_processor.go | 10 ++++++---- sdk/trace/batch_span_processor_test.go | 20 ++++++++++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad589817f..446b1643f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Fix gzipped request body replay on redirect in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#8135) - Fix gzipped request body replay on redirect in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#8152) - `go.opentelemetry.io/otel/exporters/prometheus` now uses `Value.String` formatting for label values following the [OpenTelemetry AnyValue representation for non-OTLP protocols](https://opentelemetry.io/docs/specs/otel/common/#anyvalue). (#8170) +- Propagate errors from the exporter when calling `Shutdown` on `BatchSpanProcessor` in `go.opentelemetry.io/otel/sdk/trace`. (#8197) diff --git a/sdk/trace/batch_span_processor.go b/sdk/trace/batch_span_processor.go index 32854b14a..f9f2a6c2c 100644 --- a/sdk/trace/batch_span_processor.go +++ b/sdk/trace/batch_span_processor.go @@ -163,19 +163,21 @@ func (bsp *batchSpanProcessor) Shutdown(ctx context.Context) error { bsp.stopOnce.Do(func() { bsp.stopped.Store(true) wait := make(chan struct{}) + // exportErr is written by the goroutine before closing wait. + // It is only read in the <-wait case, so there is no race. + var exportErr error go func() { close(bsp.stopCh) bsp.stopWait.Wait() if bsp.e != nil { - if err := bsp.e.Shutdown(ctx); err != nil { - otel.Handle(err) - } + exportErr = bsp.e.Shutdown(ctx) } close(wait) }() - // Wait until the wait group is done or the context is cancelled + // Wait until the channel is ready or the context is canceled. select { case <-wait: + err = exportErr case <-ctx.Done(): err = ctx.Err() } diff --git a/sdk/trace/batch_span_processor_test.go b/sdk/trace/batch_span_processor_test.go index 217eef1f6..7f45886e2 100644 --- a/sdk/trace/batch_span_processor_test.go +++ b/sdk/trace/batch_span_processor_test.go @@ -396,6 +396,26 @@ func TestBatchSpanProcessorShutdown(t *testing.T) { assert.Equal(t, 1, bp.shutdownCount) } +// TestBatchSpanProcessorShutdownErrorPropagated verifies that an error +// returned by SpanExporter.Shutdown is propagated to the caller of +// BatchSpanProcessor.Shutdown. Regression test for #6878. +func TestBatchSpanProcessorShutdownErrorPropagated(t *testing.T) { + wantErr := errors.New("exporter shutdown failed") + exporter := &errorShutdownExporter{err: wantErr} + bsp := NewBatchSpanProcessor(exporter) + + err := bsp.Shutdown(t.Context()) + assert.ErrorIs(t, err, wantErr) +} + +// errorShutdownExporter is a SpanExporter whose Shutdown always returns err. +type errorShutdownExporter struct { + err error +} + +func (*errorShutdownExporter) ExportSpans(context.Context, []ReadOnlySpan) error { return nil } +func (e *errorShutdownExporter) Shutdown(context.Context) error { return e.err } + func TestBatchSpanProcessorPostShutdown(t *testing.T) { tp := basicTracerProvider(t) be := testBatchExporter{}