ddtrace/tracer: dereference pointers to supported types in span.SetTag

.gitlab-ci.yml

@@ -9,7 +9,7 @@ variables:
   INDEX_FILE: index.txt
   KUBERNETES_SERVICE_ACCOUNT_OVERWRITE: dd-trace-go
   FF_USE_LEGACY_KUBERNETES_EXECUTION_STRATEGY: "true"
-  BENCHMARK_TARGETS: "BenchmarkStartRequestSpan|BenchmarkHttpServeTrace|BenchmarkTracerAddSpans|BenchmarkStartSpan|BenchmarkSingleSpanRetention|BenchmarkOTelApiWithCustomTags|BenchmarkInjectW3C|BenchmarkExtractW3C|BenchmarkPartialFlushing|BenchmarkGraphQL|BenchmarkSampleWAFContext|BenchmarkCaptureStackTrace"
+  BENCHMARK_TARGETS: "BenchmarkStartRequestSpan|BenchmarkHttpServeTrace|BenchmarkTracerAddSpans|BenchmarkStartSpan|BenchmarkSingleSpanRetention|BenchmarkOTelApiWithCustomTags|BenchmarkInjectW3C|BenchmarkExtractW3C|BenchmarkPartialFlushing|BenchmarkGraphQL|BenchmarkSampleWAFContext|BenchmarkCaptureStackTrace|BenchmarkSetTagString|BenchmarkSetTagStringPtr|BenchmarkSetTagMetric"
 
 include:
   - ".gitlab/benchmarks.yml"

ddtrace/tracer/span.go

@@ -111,6 +111,9 @@ func (s *span) BaggageItem(key string) string {
 
 // SetTag adds a set of key/value metadata to the span.
 func (s *span) SetTag(key string, value interface{}) {
+	// To avoid dumping the memory address in case value is a pointer, we dereference it.
+	// Any pointer value that is a pointer to a pointer will be dumped as a string.
+	value = dereference(value)
 	s.Lock()
 	defer s.Unlock()
 	// We don't lock spans when flushing, so we could have a data race when

ddtrace/tracer/span_test.go

@@ -369,6 +369,13 @@ func TestSpanSetTag(t *testing.T) {
 	span.SetTag("struct", sharedinternal.MetaStructValue{Value: testValue})
 	require.Equal(t, testValue, span.MetaStruct["struct"])
 
+	s := "string"
+	span.SetTag("str_ptr", &s)
+	assert.Equal(s, span.Meta["str_ptr"])
+
+	span.SetTag("nil_str_ptr", (*string)(nil))
+	assert.Equal("", span.Meta["nil_str_ptr"])
+
 	assert.Panics(func() {
 		span.SetTag("panicStringer", &panicStringer{})
 	})
@@ -1024,18 +1031,18 @@ func TestSetUserPropagatedUserID(t *testing.T) {
 
 func BenchmarkSetTagMetric(b *testing.B) {
 	span := newBasicSpan("bench.span")
-	keys := "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
+	keys := strings.Split("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ", "")
 
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		k := string(keys[i%len(keys)])
+		k := keys[i%len(keys)]
 		span.SetTag(k, float64(12.34))
 	}
 }
 
 func BenchmarkSetTagString(b *testing.B) {
 	span := newBasicSpan("bench.span")
-	keys := "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
+	keys := strings.Split("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ", "")
 
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
@@ -1044,13 +1051,25 @@ func BenchmarkSetTagString(b *testing.B) {
 	}
 }
 
+func BenchmarkSetTagStringPtr(b *testing.B) {
+	span := newBasicSpan("bench.span")
+	keys := strings.Split("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ", "")
+	v := makePointer("some text")
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		k := keys[i%len(keys)]
+		span.SetTag(k, v)
+	}
+}
+
 func BenchmarkSetTagStringer(b *testing.B) {
 	span := newBasicSpan("bench.span")
-	keys := "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
+	keys := strings.Split("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ", "")
 	value := &stringer{}
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		k := string(keys[i%len(keys)])
+		k := keys[i%len(keys)]
 		span.SetTag(k, value)
 	}
 }

ddtrace/tracer/util.go

@@ -20,6 +20,8 @@ import (
 func toFloat64(value interface{}) (f float64, ok bool) {
 	const max = (int64(1) << 53) - 1
 	const min = -max
+	// If any other type is added here, remember to add it to the type switch in
+	// the `span.SetTag` function to handle pointers to these supported types.
 	switch i := value.(type) {
 	case byte:
 		return float64(i), true
@@ -122,3 +124,53 @@ func parsePropagatableTraceTags(s string) (map[string]string, error) {
 	tags[key] = s[start:]
 	return tags, nil
 }
+
+func dereference(value any) any {
+	// Falling into one of the cases will dereference the pointer and return the
+	// value of the pointer. It adds one allocation due to casting.
+	switch value.(type) {
+	case *bool:
+		return dereferenceGeneric(value.(*bool))
+	case *string:
+		return dereferenceGeneric(value.(*string))
+	// Supported type by toFloat64
+	case *byte:
+		return dereferenceGeneric(value.(*byte))
+	case *float32:
+		return dereferenceGeneric(value.(*float32))
+	case *float64:
+		return dereferenceGeneric(value.(*float64))
+	case *int:
+		return dereferenceGeneric(value.(*int))
+	case *int8:
+		return dereferenceGeneric(value.(*int8))
+	case *int16:
+		return dereferenceGeneric(value.(*int16))
+	case *int32:
+		return dereferenceGeneric(value.(*int32))
+	case *int64:
+		return dereferenceGeneric(value.(*int64))
+	case *uint:
+		return dereferenceGeneric(value.(*uint))
+	case *uint16:
+		return dereferenceGeneric(value.(*uint16))
+	case *uint32:
+		return dereferenceGeneric(value.(*uint32))
+	case *uint64:
+		return dereferenceGeneric(value.(*uint64))
+	case *samplernames.SamplerName:
+		v := value.(*samplernames.SamplerName)
+		if v == nil {
+			return samplernames.Unknown
+		}
+		return *v
+	}
+	return value
+}
+
+func dereferenceGeneric[T any](value *T) T {
+	if value == nil {
+		return *new(T)
+	}
+	return *value
+}

ddtrace/tracer/util_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"gopkg.in/DataDog/dd-trace-go.v1/internal/samplernames"
 )
 
 func TestToFloat64(t *testing.T) {
@@ -115,3 +116,60 @@ func TestParsePropagatableTraceTags(t *testing.T) {
 		})
 	}
 }
+
+func TestDereference(t *testing.T) {
+	for i, tt := range []struct {
+		value    interface{}
+		expected interface{}
+	}{
+		{makePointer(1), 1},
+		{makePointer(byte(1)), byte(1)},
+		{makePointer(int16(1)), int16(1)},
+		{makePointer(int32(1)), int32(1)},
+		{makePointer(int64(1)), int64(1)},
+		{makePointer(uint(1)), uint(1)},
+		{makePointer(uint16(1)), uint16(1)},
+		{makePointer(uint32(1)), uint32(1)},
+		{makePointer(uint64(1)), uint64(1)},
+		{makePointer("a"), "a"},
+		{makePointer(float32(1.25)), float32(1.25)},
+		{makePointer(float64(1.25)), float64(1.25)},
+		{makePointer(true), true},
+		{makePointer(false), false},
+		{makePointer(samplernames.SingleSpan), samplernames.SingleSpan},
+		{(*int)(nil), 0},
+		{(*byte)(nil), byte(0)},
+		{(*int16)(nil), int16(0)},
+		{(*int32)(nil), int32(0)},
+		{(*int64)(nil), int64(0)},
+		{(*uint)(nil), uint(0)},
+		{(*uint16)(nil), uint16(0)},
+		{(*uint32)(nil), uint32(0)},
+		{(*uint64)(nil), uint64(0)},
+		{(*string)(nil), ""},
+		{(*float32)(nil), float32(0)},
+		{(*float64)(nil), float64(0)},
+		{(*bool)(nil), false},
+		{(*samplernames.SamplerName)(nil), samplernames.Unknown},
+		{newSpan("test", "service", "resource", 1, 2, 0), "itself"}, // This test uses a value which type is not supported by dereference.
+	} {
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+			actual := dereference(tt.value)
+			// This is a special case where we want to compare the value itself
+			// because the dereference function returns the given value.
+			if tt.expected == "itself" {
+				if actual != tt.value {
+					t.Fatalf("expected: %#v, got: %#v", tt.value, actual)
+				}
+				return
+			}
+			if actual != tt.expected {
+				t.Fatalf("expected: %#v, got: %#v", tt.expected, actual)
+			}
+		})
+	}
+}
+
+func makePointer[T any](value T) *T {
+	return &value
+}