fix(ddtrace/tracer): fix panic in SQLCommentCarrier (#3062)

DataDog · Jan 10, 2025 · d5b23f5 · d5b23f5
1 parent 6a2ad62
commit d5b23f5
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 18 deletions.
diff --git a/ddtrace/tracer/span.go b/ddtrace/tracer/span.go
@@ -722,6 +722,13 @@ func (s *span) Format(f fmt.State, c rune) {
 	}
 }
 
+func getMeta(s *span, key string) (string, bool) {
+	s.RLock()
+	defer s.RUnlock()
+	val, ok := s.Meta[key]
+	return val, ok
+}
+
 const (
 	keySamplingPriority     = "_sampling_priority_v1"
 	keySamplingPriorityRate = "_dd.agent_psr"

diff --git a/ddtrace/tracer/span_test.go b/ddtrace/tracer/span_test.go
@@ -855,7 +855,7 @@ func TestSpanLog(t *testing.T) {
 		span.Finish()
 		expect := fmt.Sprintf(`dd.service=tracer.test dd.env=testenv dd.trace_id=%q dd.span_id="87654321" dd.parent_id="0"`, span.context.TraceID128())
 		assert.Equal(expect, fmt.Sprintf("%v", span))
-		v, _ := span.context.meta(keyTraceID128)
+		v, _ := getMeta(span, keyTraceID128)
 		assert.NotEmpty(v)
 	})
 
@@ -871,7 +871,7 @@ func TestSpanLog(t *testing.T) {
 		span.context.traceID.SetUpper(1)
 		span.Finish()
 		assert.Equal(`dd.service=tracer.test dd.env=testenv dd.trace_id="00000000000000010000000005397fb1" dd.span_id="87654321" dd.parent_id="0"`, fmt.Sprintf("%v", span))
-		v, _ := span.context.meta(keyTraceID128)
+		v, _ := getMeta(span, keyTraceID128)
 		assert.Equal("0000000000000001", v)
 	})
 
@@ -887,7 +887,7 @@ func TestSpanLog(t *testing.T) {
 		span.Finish()
 		assert.False(span.context.traceID.HasUpper()) // it should not have generated upper bits
 		assert.Equal(`dd.service=tracer.test dd.env=testenv dd.trace_id="87654321" dd.span_id="87654321" dd.parent_id="0"`, fmt.Sprintf("%v", span))
-		v, _ := span.context.meta(keyTraceID128)
+		v, _ := getMeta(span, keyTraceID128)
 		assert.Equal("", v)
 	})
 }

diff --git a/ddtrace/tracer/spancontext.go b/ddtrace/tracer/spancontext.go
@@ -241,13 +241,6 @@ func (c *spanContext) baggageItem(key string) string {
 	return c.baggage[key]
 }
 
-func (c *spanContext) meta(key string) (val string, ok bool) {
-	c.span.RLock()
-	defer c.span.RUnlock()
-	val, ok = c.span.Meta[key]
-	return val, ok
-}
-
 // finish marks this span as finished in the trace.
 func (c *spanContext) finish() { c.trace.finishedOne(c.span) }
 

diff --git a/ddtrace/tracer/sqlcomment.go b/ddtrace/tracer/sqlcomment.go
@@ -108,21 +108,24 @@ func (c *SQLCommentCarrier) Inject(spanCtx ddtrace.SpanContext) error {
 		fallthrough
 	case DBMPropagationModeService:
 		if ctx, ok := spanCtx.(*spanContext); ok {
-			if e, ok := ctx.meta(ext.Environment); ok && e != "" {
-				tags[sqlCommentEnv] = e
-			}
-			if v, ok := ctx.meta(ext.Version); ok && v != "" {
-				tags[sqlCommentParentVersion] = v
+			if ctx.span != nil {
+				if e, ok := getMeta(ctx.span, ext.Environment); ok && e != "" {
+					tags[sqlCommentEnv] = e
+				}
+				if v, ok := getMeta(ctx.span, ext.Version); ok && v != "" {
+					tags[sqlCommentParentVersion] = v
+				}
+				if v, ok := getMeta(ctx.span, ext.PeerService); ok && v != "" {
+					tags[sqlCommentPeerService] = v
+				}
 			}
 			if c.PeerDBName != "" {
 				tags[sqlCommentPeerDBName] = c.PeerDBName
 			}
 			if c.PeerDBHostname != "" {
 				tags[sqlCommentPeerHostname] = c.PeerDBHostname
 			}
-			if v, ok := ctx.meta(ext.PeerService); ok && v != "" {
-				tags[sqlCommentPeerService] = v
-			} else if c.PeerService != "" {
+			if tags[sqlCommentPeerService] == "" && c.PeerService != "" {
 				tags[sqlCommentPeerService] = c.PeerService
 			}
 		}

diff --git a/ddtrace/tracer/sqlcomment_test.go b/ddtrace/tracer/sqlcomment_test.go
@@ -213,6 +213,35 @@ func TestSQLCommentCarrier(t *testing.T) {
 	}
 }
 
+// https://github.com/DataDog/dd-trace-go/issues/2837
+func TestSQLCommentCarrierInjectNilSpan(t *testing.T) {
+	tracer := newTracer()
+	defer tracer.Stop()
+
+	headers := TextMapCarrier(map[string]string{
+		DefaultTraceIDHeader:  "4",
+		DefaultParentIDHeader: "1",
+		originHeader:          "synthetics",
+		b3TraceIDHeader:       "0021dc1807524785",
+		traceparentHeader:     "00-00000000000000000000000000000004-2222222222222222-01",
+		tracestateHeader:      "dd=s:2;o:rum;p:0000000000000001;t.tid:1230000000000000~~,othervendor=t61rcWkgMzE",
+	})
+
+	spanCtx, err := tracer.Extract(headers)
+	require.NoError(t, err)
+
+	carrier := SQLCommentCarrier{
+		Query:          "SELECT * from FOO",
+		Mode:           DBMPropagationModeFull,
+		DBServiceName:  "whiskey-db",
+		PeerDBHostname: "",
+		PeerDBName:     "",
+		PeerService:    "",
+	}
+	err = carrier.Inject(spanCtx)
+	require.NoError(t, err)
+}
+
 func TestExtractOpenTelemetryTraceInformation(t *testing.T) {
 	// open-telemetry supports 128 bit trace ids
 	traceID := "5bd66ef5095369c7b0d1f8f4bd33716a"