fix(otel): Overwrite "trace" context in Sentry error (#580)

getsentry · Feb 17, 2023 · 297d2c2 · 297d2c2
1 parent 80b98ba
commit 297d2c2
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 65 deletions.
diff --git a/otel/event_processor.go b/otel/event_processor.go
@@ -15,6 +15,10 @@ func linkTraceContextToErrorEvent(event *sentry.Event, hint *sentry.EventHint) *
 	if hint == nil || hint.Context == nil {
 		return event
 	}
+	// TODO: what we want here is to compare with the (unexported) sentry.transactionType
+	if event.Type == "transaction" {
+		return event
+	}
 	otelSpanContext := trace.SpanContextFromContext(hint.Context)
 	var sentrySpan *sentry.Span
 	if otelSpanContext.IsValid() {
@@ -24,15 +28,13 @@ func linkTraceContextToErrorEvent(event *sentry.Event, hint *sentry.EventHint) *
 		return event
 	}
 
-	traceContext := event.Contexts["trace"]
-	if len(traceContext) > 0 {
-		// trace context is already set, not touching it
-		return event
-	}
-	event.Contexts["trace"] = map[string]interface{}{
-		"trace_id":       sentrySpan.TraceID.String(),
-		"span_id":        sentrySpan.SpanID.String(),
-		"parent_span_id": sentrySpan.ParentSpanID.String(),
+	traceContext, found := event.Contexts["trace"]
+	if !found {
+		event.Contexts["trace"] = make(map[string]interface{})
+		traceContext = event.Contexts["trace"]
 	}
+	traceContext["trace_id"] = sentrySpan.TraceID.String()
+	traceContext["span_id"] = sentrySpan.SpanID.String()
+	traceContext["parent_span_id"] = sentrySpan.ParentSpanID.String()
 	return event
 }
diff --git a/otel/event_processor_test.go b/otel/event_processor_test.go
@@ -4,65 +4,54 @@ package sentryotel
 
 import (
 	"errors"
+	"fmt"
 	"testing"
 
 	"github.com/getsentry/sentry-go"
 )
 
-func TestLinkTraceContextToErrorEventWithEmptyTraceContext(t *testing.T) {
-	_, _, tracer := setupSpanProcessorTest()
-	ctx, otelSpan := tracer.Start(emptyContextWithSentry(), "spanName")
-	sentrySpan, _ := sentrySpanMap.Get(otelSpan.SpanContext().SpanID())
-
-	hub := sentry.GetHubFromContext(ctx)
-	client := hub.Client()
-	client.CaptureException(
-		errors.New("new sentry exception"),
-		&sentry.EventHint{Context: ctx},
-		nil,
-	)
-
-	transport := client.Transport.(*TransportMock)
-	events := transport.Events()
-	assertEqual(t, len(events), 1)
-	err := events[0]
-	exception := err.Exception[0]
-	assertEqual(t, exception.Type, "*errors.errorString")
-	assertEqual(t, exception.Value, "new sentry exception")
-	assertEqual(t, err.Type, "")
-	assertEqual(t,
-		err.Contexts["trace"],
-		map[string]interface{}{
-			"trace_id":       sentrySpan.TraceID.String(),
-			"span_id":        sentrySpan.SpanID.String(),
-			"parent_span_id": sentrySpan.ParentSpanID.String(),
-		},
-	)
-}
-
-func TestLinkTraceContextToErrorEventDoesNotTouchExistingTraceContext(t *testing.T) {
-	_, _, tracer := setupSpanProcessorTest()
-	ctx, _ := tracer.Start(emptyContextWithSentry(), "spanName")
-
-	hub := sentry.GetHubFromContext(ctx)
-	hub.Scope().SetContext("trace", map[string]interface{}{"trace_id": "123"})
-	client := hub.Client()
-	client.CaptureException(
-		errors.New("new sentry exception with existing trace context"),
-		&sentry.EventHint{Context: ctx},
-		hub.Scope(),
-	)
-
-	transport := client.Transport.(*TransportMock)
-	events := transport.Events()
-	assertEqual(t, len(events), 1)
-	err := events[0]
-	exception := err.Exception[0]
-	assertEqual(t, exception.Type, "*errors.errorString")
-	assertEqual(t, exception.Value, "new sentry exception with existing trace context")
-	assertEqual(t, err.Type, "")
-	assertEqual(t,
-		err.Contexts["trace"],
-		map[string]interface{}{"trace_id": "123"},
-	)
+func TestLinkTraceContextToErrorEventSetsContext(t *testing.T) {
+
+	withExistingContextOptions := []bool{false, true}
+
+	for _, withExistingContext := range withExistingContextOptions {
+		withExistingContext := withExistingContext
+		name := fmt.Sprintf("withExistingContext_%v", withExistingContext)
+
+		t.Run(name, func(t *testing.T) {
+			_, _, tracer := setupSpanProcessorTest()
+			ctx, otelSpan := tracer.Start(emptyContextWithSentry(), "spanName")
+			sentrySpan, _ := sentrySpanMap.Get(otelSpan.SpanContext().SpanID())
+
+			hub := sentry.GetHubFromContext(ctx)
+			client, scope := hub.Client(), hub.Scope()
+
+			if withExistingContext {
+				// The existing "trace" context should be ovewritten by the event processor
+				scope.SetContext("trace", map[string]interface{}{"trace_id": "123"})
+			}
+			client.CaptureException(
+				errors.New("new sentry exception with existing trace context"),
+				&sentry.EventHint{Context: ctx},
+				hub.Scope(),
+			)
+
+			transport := client.Transport.(*TransportMock)
+			events := transport.Events()
+			assertEqual(t, len(events), 1)
+			err := events[0]
+			exception := err.Exception[0]
+			assertEqual(t, exception.Type, "*errors.errorString")
+			assertEqual(t, exception.Value, "new sentry exception with existing trace context")
+			assertEqual(t, err.Type, "")
+			assertEqual(t,
+				err.Contexts["trace"],
+				map[string]interface{}{
+					"trace_id":       sentrySpan.TraceID.String(),
+					"span_id":        sentrySpan.SpanID.String(),
+					"parent_span_id": sentrySpan.ParentSpanID.String(),
+				},
+			)
+		})
+	}
 }