From d7cc13aa4296b947c98d533f761024306b30304d Mon Sep 17 00:00:00 2001 From: Mikayla Toffler <46911781+mtoffl01@users.noreply.github.com> Date: Thu, 14 Nov 2024 12:00:57 -0500 Subject: [PATCH 1/2] ddtrace/tracer: Ensure SpanLink access is safe from corruption (#2975) --- ddtrace/ddtrace.go | 5 +---- ddtrace/tracer/context_test.go | 10 ++++------ ddtrace/tracer/spancontext.go | 9 ++++----- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/ddtrace/ddtrace.go b/ddtrace/ddtrace.go index 768146e7ac..928816ba6c 100644 --- a/ddtrace/ddtrace.go +++ b/ddtrace/ddtrace.go @@ -38,11 +38,8 @@ type SpanContextW3C interface { type SpanContextWithLinks interface { SpanContext - // SpanLinks returns the span links on the SpanContext. + // SpanLinks returns a copy of the span links on the SpanContext. SpanLinks() []SpanLink - - // Setlinks takes in a slice of SpanLinks and sets them to the SpanContext. - SetLinks(links []SpanLink) } // Tracer specifies an implementation of the Datadog tracer which allows starting diff --git a/ddtrace/tracer/context_test.go b/ddtrace/tracer/context_test.go index 8a5fd617f0..50c2ec869b 100644 --- a/ddtrace/tracer/context_test.go +++ b/ddtrace/tracer/context_test.go @@ -84,14 +84,12 @@ func TestStartSpanWithSpanLinks(t *testing.T) { _, _, _, stop := startTestTracer(t) defer stop() spanLink := ddtrace.SpanLink{TraceID: 789, TraceIDHigh: 0, SpanID: 789, Attributes: map[string]string{"reason": "terminated_context", "context_headers": "datadog"}, Flags: 0} - var ctx ddtrace.SpanContext - ctx = &spanContext{spanID: 789, traceID: traceIDFrom64Bits(789), spanLinks: []ddtrace.SpanLink{spanLink}} + ctx := &spanContext{spanID: 789, traceID: traceIDFrom64Bits(789), spanLinks: []ddtrace.SpanLink{spanLink}} t.Run("spanContext with spanLinks satisfies SpanContextWithLinks interface", func(t *testing.T) { - ctxLink, ok := ctx.(ddtrace.SpanContextWithLinks) - assert.True(t, ok) - assert.Equal(t, len(ctxLink.SpanLinks()), 1) - assert.Equal(t, ctxLink.SpanLinks()[0], spanLink) + var _ ddtrace.SpanContextWithLinks = ctx + assert.Equal(t, len(ctx.SpanLinks()), 1) + assert.Equal(t, ctx.SpanLinks()[0], spanLink) }) t.Run("create span from spancontext with links", func(t *testing.T) { diff --git a/ddtrace/tracer/spancontext.go b/ddtrace/tracer/spancontext.go index 44419241db..30a791ee8e 100644 --- a/ddtrace/tracer/spancontext.go +++ b/ddtrace/tracer/spancontext.go @@ -183,12 +183,11 @@ func (c *spanContext) TraceID128Bytes() [16]byte { return c.traceID } +// SpanLinks implements ddtrace.SpanContextWithLinks func (c *spanContext) SpanLinks() []ddtrace.SpanLink { - return c.spanLinks -} - -func (c *spanContext) SetLinks(links []ddtrace.SpanLink) { - c.spanLinks = links + cp := make([]ddtrace.SpanLink, len(c.spanLinks)) + copy(cp, c.spanLinks) + return cp } // ForeachBaggageItem implements ddtrace.SpanContext. From 58e1e634e199676dd2ec5ce89600da91a282b253 Mon Sep 17 00:00:00 2001 From: Tony Redondo Date: Tue, 19 Nov 2024 10:28:08 +0100 Subject: [PATCH 2/2] internal/civisibility: fix RWMutex usages (#2982) --- .../integrations/gotesting/instrumentation.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/civisibility/integrations/gotesting/instrumentation.go b/internal/civisibility/integrations/gotesting/instrumentation.go index c7b18a92c0..1c3be06223 100644 --- a/internal/civisibility/integrations/gotesting/instrumentation.go +++ b/internal/civisibility/integrations/gotesting/instrumentation.go @@ -104,15 +104,15 @@ func getInstrumentationMetadata(fn *runtime.Func) *instrumentationMetadata { // setInstrumentationMetadata stores an instrumentation metadata for a given *runtime.Func. func setInstrumentationMetadata(fn *runtime.Func, metadata *instrumentationMetadata) { - instrumentationMapMutex.RLock() - defer instrumentationMapMutex.RUnlock() + instrumentationMapMutex.Lock() + defer instrumentationMapMutex.Unlock() instrumentationMap[fn] = metadata } // createTestMetadata creates the CI visibility test metadata associated with a given *testing.T, *testing.B, *testing.common func createTestMetadata(tb testing.TB) *testExecutionMetadata { - ciVisibilityTestMetadataMutex.RLock() - defer ciVisibilityTestMetadataMutex.RUnlock() + ciVisibilityTestMetadataMutex.Lock() + defer ciVisibilityTestMetadataMutex.Unlock() execMetadata := &testExecutionMetadata{} ciVisibilityTestMetadata[reflect.ValueOf(tb).UnsafePointer()] = execMetadata return execMetadata @@ -135,8 +135,8 @@ func getTestMetadataFromPointer(ptr unsafe.Pointer) *testExecutionMetadata { // deleteTestMetadata delete the CI visibility test metadata associated with a given *testing.T, *testing.B, *testing.common func deleteTestMetadata(tb testing.TB) { - ciVisibilityTestMetadataMutex.RLock() - defer ciVisibilityTestMetadataMutex.RUnlock() + ciVisibilityTestMetadataMutex.Lock() + defer ciVisibilityTestMetadataMutex.Unlock() delete(ciVisibilityTestMetadata, reflect.ValueOf(tb).UnsafePointer()) }