From 18125a1a8ab3dafff0b410540a8f28c928c60c62 Mon Sep 17 00:00:00 2001 From: Anton Ovchinnikov Date: Tue, 14 Feb 2023 19:10:00 +0100 Subject: [PATCH 1/7] Overwrite "trace" context in Sentry error --- otel/event_processor.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/otel/event_processor.go b/otel/event_processor.go index bf9661611..a5e7bc037 100644 --- a/otel/event_processor.go +++ b/otel/event_processor.go @@ -25,14 +25,8 @@ func linkTraceContextToErrorEvent(event *sentry.Event, hint *sentry.EventHint) * } 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["trace_id"] = sentrySpan.TraceID.String() + traceContext["span_id"] = sentrySpan.SpanID.String() + traceContext["parent_span_id"] = sentrySpan.ParentSpanID.String() return event } From 8945d3e4c9cb6dc7a2c1e6f2c0ff4c09a6de74d9 Mon Sep 17 00:00:00 2001 From: Anton Ovchinnikov Date: Tue, 14 Feb 2023 19:26:16 +0100 Subject: [PATCH 2/7] handle map creation --- otel/event_processor.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/otel/event_processor.go b/otel/event_processor.go index a5e7bc037..5ae2e48ef 100644 --- a/otel/event_processor.go +++ b/otel/event_processor.go @@ -24,7 +24,11 @@ func linkTraceContextToErrorEvent(event *sentry.Event, hint *sentry.EventHint) * return event } - traceContext := event.Contexts["trace"] + 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() From 7c23d82eb137037d5da774353121a1ff550cc412 Mon Sep 17 00:00:00 2001 From: Anton Ovchinnikov Date: Tue, 14 Feb 2023 21:45:24 +0100 Subject: [PATCH 3/7] add logging --- otel/event_processor.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/otel/event_processor.go b/otel/event_processor.go index 5ae2e48ef..fd0a76447 100644 --- a/otel/event_processor.go +++ b/otel/event_processor.go @@ -12,6 +12,8 @@ import ( // // Caveat: hint.Context should contain a valid context populated by OpenTelemetry's span context. func linkTraceContextToErrorEvent(event *sentry.Event, hint *sentry.EventHint) *sentry.Event { + sentry.Logger.Printf("\nEvent Processor\nEvent: %#v\nHint: %#v", event, hint) + if hint == nil || hint.Context == nil { return event } From 5b414e12fe9431f3a6564dd3e417fc5bec362f7e Mon Sep 17 00:00:00 2001 From: Anton Ovchinnikov Date: Tue, 14 Feb 2023 22:27:39 +0100 Subject: [PATCH 4/7] log in transport --- transport.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/transport.go b/transport.go index 3722217e6..e8aca4b9c 100644 --- a/transport.go +++ b/transport.go @@ -544,6 +544,8 @@ func (t *HTTPSyncTransport) SendEvent(event *Event) { Logger.Printf("There was an issue with sending an event: %v", err) return } + Logger.Printf("Response status: %q", response.Status) + t.mu.Lock() t.limits.Merge(ratelimit.FromResponse(response)) t.mu.Unlock() From 28d9c3fbe54f5d3969bc25899b499ce280664c9f Mon Sep 17 00:00:00 2001 From: Anton Ovchinnikov Date: Wed, 15 Feb 2023 16:16:16 +0100 Subject: [PATCH 5/7] revert transport --- transport.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/transport.go b/transport.go index e8aca4b9c..3722217e6 100644 --- a/transport.go +++ b/transport.go @@ -544,8 +544,6 @@ func (t *HTTPSyncTransport) SendEvent(event *Event) { Logger.Printf("There was an issue with sending an event: %v", err) return } - Logger.Printf("Response status: %q", response.Status) - t.mu.Lock() t.limits.Merge(ratelimit.FromResponse(response)) t.mu.Unlock() From fddcee656d818aaf6a162c7694ba7eea6c0774b6 Mon Sep 17 00:00:00 2001 From: Anton Ovchinnikov Date: Wed, 15 Feb 2023 16:16:38 +0100 Subject: [PATCH 6/7] Update tests --- otel/event_processor.go | 6 ++- otel/event_processor_test.go | 101 ++++++++++++++++------------------- 2 files changed, 49 insertions(+), 58 deletions(-) diff --git a/otel/event_processor.go b/otel/event_processor.go index fd0a76447..811da0a6b 100644 --- a/otel/event_processor.go +++ b/otel/event_processor.go @@ -12,11 +12,13 @@ import ( // // Caveat: hint.Context should contain a valid context populated by OpenTelemetry's span context. func linkTraceContextToErrorEvent(event *sentry.Event, hint *sentry.EventHint) *sentry.Event { - sentry.Logger.Printf("\nEvent Processor\nEvent: %#v\nHint: %#v", event, hint) - if hint == nil || hint.Context == nil { return event } + // TODO: what we want here is to compare with the (unexported) transactionType + if event.Type == "transaction" { + return event + } otelSpanContext := trace.SpanContextFromContext(hint.Context) var sentrySpan *sentry.Span if otelSpanContext.IsValid() { diff --git a/otel/event_processor_test.go b/otel/event_processor_test.go index 6fd3956a9..692f462fd 100644 --- 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(), + }, + ) + }) + } } From 7d7236ad3b32de760c0ad79664e54ff6efcb30ac Mon Sep 17 00:00:00 2001 From: Anton Ovchinnikov Date: Wed, 15 Feb 2023 16:17:28 +0100 Subject: [PATCH 7/7] clarify comment --- otel/event_processor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/otel/event_processor.go b/otel/event_processor.go index 811da0a6b..d171b7d6b 100644 --- a/otel/event_processor.go +++ b/otel/event_processor.go @@ -15,7 +15,7 @@ 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) transactionType + // TODO: what we want here is to compare with the (unexported) sentry.transactionType if event.Type == "transaction" { return event }