You've already forked opentelemetry-go
mirror of
https://github.com/open-telemetry/opentelemetry-go.git
synced 2026-06-03 18:35:08 +02:00
sdk/trace: propagate SpanExporter.Shutdown error from BatchSpanProcessor (#8197)
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 <alliasgher123@gmail.com> Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> Co-authored-by: Robert Pająk <pellared@hotmail.com>
This commit is contained in:
@@ -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)
|
||||
|
||||
<!-- Released section -->
|
||||
<!-- Don't change this section unless doing release -->
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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{}
|
||||
|
||||
Reference in New Issue
Block a user