ddtrace/tracer: fix tracer.StartSpanFromContext race with opts argume…

…nt slice (#1127)

In concurrent environments, the opts slice may be re-used and modified for subsequent concurrent calls,
which can cause a race and/or unexpected options being passed to undesired calls.

Co-authored-by: Gabriel Aszalos <[email protected]>

ddtrace/tracer/context.go

@@ -39,14 +39,19 @@ func SpanFromContext(ctx context.Context) (Span, bool) {
 // is found in the context, it will be used as the parent of the resulting span. If the ChildOf
 // option is passed, the span from context will take precedence over it as the parent span.
 func StartSpanFromContext(ctx context.Context, operationName string, opts ...StartSpanOption) (Span, context.Context) {
+	// copy opts in case the caller reuses the slice in parallel
+	// we will add at least 1, at most 2 items
+	optsLocal := make([]StartSpanOption, len(opts), len(opts)+2)
+	copy(optsLocal, opts)
+
 	if ctx == nil {
 		// default to context.Background() to avoid panics on Go >= 1.15
 		ctx = context.Background()
 	} else if s, ok := SpanFromContext(ctx); ok {
-		opts = append(opts, ChildOf(s.Context()))
+		optsLocal = append(optsLocal, ChildOf(s.Context()))
 	}
-	opts = append(opts, withContext(ctx))
-	s := StartSpan(operationName, opts...)
+	optsLocal = append(optsLocal, withContext(ctx))
+	s := StartSpan(operationName, optsLocal...)
 	if span, ok := s.(*span); ok && span.pprofCtxActive != nil {
 		// If pprof labels were applied for this span, use the derived ctx that
 		// includes them. Otherwise a child of this span wouldn't be able to

ddtrace/tracer/context_test.go

@@ -76,6 +76,37 @@ func TestStartSpanFromContext(t *testing.T) {
 	assert.Equal("/", got.Resource)
 }
 
+func TestStartSpanFromContextRace(t *testing.T) {
+	_, _, _, stop := startTestTracer(t)
+	defer stop()
+
+	// Start 100 goroutines that create child spans with StartSpanFromContext in parallel,
+	// with a shared options slice. The child spans should get parented to the correct spans
+	const contextKey = "key"
+	const numContexts = 100
+	options := make([]StartSpanOption, 0, 3)
+	outputValues := make(chan uint64, numContexts)
+	var expectedTraceIDs []uint64
+	for i := 0; i < numContexts; i++ {
+		parent, childCtx := StartSpanFromContext(context.Background(), "parent")
+		expectedTraceIDs = append(expectedTraceIDs, parent.Context().TraceID())
+		go func() {
+			span, _ := StartSpanFromContext(childCtx, "testoperation", options...)
+			defer span.Finish()
+			outputValues <- span.Context().TraceID()
+		}()
+		parent.Finish()
+	}
+
+	// collect the outputs
+	var outputs []uint64
+	for i := 0; i < numContexts; i++ {
+		outputs = append(outputs, <-outputValues)
+	}
+	assert.Len(t, outputs, numContexts)
+	assert.ElementsMatch(t, outputs, expectedTraceIDs)
+}
+
 func TestStartSpanFromNilContext(t *testing.T) {
 	_, _, _, stop := startTestTracer(t)
 	defer stop()