Honor context deadline or cancellation in SimpleSpanProcessor.Shutdown (#1856)

* Honor context Done in SSP Shutdown

* Update PR number in changelog

* Update comment

* Add tests

* Make tests more concise

* Restructure tests for readability

CHANGELOG.md

@@ -22,6 +22,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ### Fixed
 
 - Only report errors from the `"go.opentelemetry.io/otel/sdk/resource".Environment` function when they are not `nil`. (#1850, #1851)
+- The `Shutdown` method of the simple `SpanProcessor` in the `go.opentelemetry.io/otel/sdk/trace` package now honors the context deadline or cancellation. (#1616, #1856)
 
 ### Security
 

sdk/trace/simple_span_processor.go

@@ -66,16 +66,34 @@ func (ssp *simpleSpanProcessor) OnEnd(s ReadOnlySpan) {
 func (ssp *simpleSpanProcessor) Shutdown(ctx context.Context) error {
 	var err error
 	ssp.stopOnce.Do(func() {
+		stopFunc := func(exp SpanExporter) (<-chan error, func()) {
+			done := make(chan error)
+			return done, func() { done <- exp.Shutdown(ctx) }
+		}
+
+		// The exporter field of the simpleSpanProcessor needs to be zeroed to
+		// signal it is shut down, meaning all subsequent calls to OnEnd will
+		// be gracefully ignored. This needs to be done synchronously to avoid
+		// any race condition.
+		//
+		// A closure is used to keep reference to the exporter and then the
+		// field is zeroed. This ensures the simpleSpanProcessor is shut down
+		// before the exporter. This order is important as it avoids a
+		// potential deadlock. If the exporter shut down operation generates a
+		// span, that span would need to be exported. Meaning, OnEnd would be
+		// called and try acquiring the lock that is held here.
 		ssp.exporterMu.Lock()
-		exporter := ssp.exporter
-		// Set exporter to nil so subsequent calls to OnEnd are ignored
-		// gracefully.
+		done, shutdown := stopFunc(ssp.exporter)
 		ssp.exporter = nil
 		ssp.exporterMu.Unlock()
 
-		// Clear the ssp.exporter prior to shutting it down so if that creates
-		// a span that needs to be exported there is no deadlock.
-		err = exporter.Shutdown(ctx)
+		go shutdown()
+
+		select {
+		case err = <-done:
+		case <-ctx.Done():
+			err = ctx.Err()
+		}
 	})
 	return err
 }

sdk/trace/simple_span_processor_test.go

@@ -16,7 +16,9 @@ package trace_test
 
 import (
 	"context"
+	"errors"
 	"testing"
+	"time"
 
 	"go.opentelemetry.io/otel/trace"
 
@@ -142,3 +144,24 @@ func TestSimpleSpanProcessorShutdownOnEndConcurrency(t *testing.T) {
 	stop <- struct{}{}
 	<-done
 }
+
+func TestSimpleSpanProcessorShutdownHonorsContextDeadline(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), time.Nanosecond)
+	defer cancel()
+	<-ctx.Done()
+
+	ssp := sdktrace.NewSimpleSpanProcessor(&testExporter{})
+	if got, want := ssp.Shutdown(ctx), context.DeadlineExceeded; !errors.Is(got, want) {
+		t.Errorf("SimpleSpanProcessor.Shutdown did not return %v, got %v", want, got)
+	}
+}
+
+func TestSimpleSpanProcessorShutdownHonorsContextCancel(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+
+	ssp := sdktrace.NewSimpleSpanProcessor(&testExporter{})
+	if got, want := ssp.Shutdown(ctx), context.Canceled; !errors.Is(got, want) {
+		t.Errorf("SimpleSpanProcessor.Shutdown did not return %v, got %v", want, got)
+	}
+}