diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 00f8b74e72..c6d7819a1d 100644 --- a/.gitlab-ci.yml +++ b/.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|BenchmarkSetTagStringer" include: - ".gitlab/benchmarks.yml" diff --git a/ddtrace/tracer/span.go b/ddtrace/tracer/span.go index 14f816f9f1..323003e23a 100644 --- a/ddtrace/tracer/span.go +++ b/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 diff --git a/ddtrace/tracer/span_test.go b/ddtrace/tracer/span_test.go index 19e8edb8a4..95b1460ac0 100644 --- a/ddtrace/tracer/span_test.go +++ b/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) } } diff --git a/ddtrace/tracer/util.go b/ddtrace/tracer/util.go index 67ee16149f..bd3c03a845 100644 --- a/ddtrace/tracer/util.go +++ b/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,54 @@ 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 { + var v T + return v + } + return *value +} diff --git a/ddtrace/tracer/util_test.go b/ddtrace/tracer/util_test.go index 87e4d9ef19..9690430ff2 100644 --- a/ddtrace/tracer/util_test.go +++ b/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 +}