From c02b5edb1f296e264a67a839991995c9143253e7 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 18 Jul 2024 05:10:45 +0200 Subject: [PATCH 01/14] WIP --- client.go | 2 +- echo/sentryecho.go | 30 +++++---- gin/sentrygin.go | 39 +++++++---- http/sentryhttp.go | 10 +-- hub.go | 46 +++++++++---- hub_test.go | 84 ----------------------- iris/sentryiris.go | 28 ++++---- negroni/sentrynegroni.go | 25 +++---- scope.go | 51 +++++++------- tracing.go | 27 ++------ tracing_test.go | 141 +++++++++++++++++++-------------------- 11 files changed, 207 insertions(+), 276 deletions(-) diff --git a/client.go b/client.go index dd5e98262..1179f762c 100644 --- a/client.go +++ b/client.go @@ -612,7 +612,7 @@ func (client *Client) processEvent(event *Event, hint *EventHint, scope EventMod // Transactions are sampled by options.TracesSampleRate or // options.TracesSampler when they are started. Other events // (errors, messages) are sampled here. Does not apply to check-ins. - if event.Type != transactionType && event.Type != checkInType && !sample(client.options.SampleRate) { + if event.Type == eventType && !sample(client.options.SampleRate) { Logger.Println("Event dropped due to SampleRate hit.") return nil } diff --git a/echo/sentryecho.go b/echo/sentryecho.go index 26972d9f3..fe28ed6c3 100644 --- a/echo/sentryecho.go +++ b/echo/sentryecho.go @@ -49,44 +49,46 @@ func New(options Options) echo.MiddlewareFunc { } func (h *handler) handle(next echo.HandlerFunc) echo.HandlerFunc { - return func(ctx echo.Context) error { - hub := sentry.GetHubFromContext(ctx.Request().Context()) + return func(echoCtx echo.Context) error { + r := echoCtx.Request() + ctx := r.Context() + + hub := sentry.GetHubFromContext(ctx) if hub == nil { hub = sentry.CurrentHub().Clone() + ctx = sentry.SetHubOnContext(ctx, hub) } if client := hub.Client(); client != nil { client.SetSDKIdentifier(sdkIdentifier) } - r := ctx.Request() - transactionName := r.URL.Path transactionSource := sentry.SourceURL - if path := ctx.Path(); path != "" { + if path := echoCtx.Path(); path != "" { transactionName = path transactionSource = sentry.SourceRoute } options := []sentry.SpanOption{ + hub.ContinueTrace(r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), sentry.WithOpName("http.server"), - sentry.ContinueFromRequest(r), sentry.WithTransactionSource(transactionSource), sentry.WithSpanOrigin(sentry.SpanOriginEcho), } transaction := sentry.StartTransaction( - sentry.SetHubOnContext(r.Context(), hub), + ctx, fmt.Sprintf("%s %s", r.Method, transactionName), options..., ) - transaction.SetData("http.request.method", ctx.Request().Method) + transaction.SetData("http.request.method", r.Method) defer func() { - status := ctx.Response().Status - if err := ctx.Get("error"); err != nil { + status := echoCtx.Response().Status + if err := echoCtx.Get("error"); err != nil { if httpError, ok := err.(*echo.HTTPError); ok { status = httpError.Code } @@ -98,14 +100,14 @@ func (h *handler) handle(next echo.HandlerFunc) echo.HandlerFunc { }() hub.Scope().SetRequest(r) - ctx.Set(valuesKey, hub) - ctx.Set(transactionKey, transaction) + echoCtx.Set(valuesKey, hub) + echoCtx.Set(transactionKey, transaction) defer h.recoverWithSentry(hub, r) - err := next(ctx) + err := next(echoCtx) if err != nil { // Store the error so it can be used in the deferred function - ctx.Set("error", err) + echoCtx.Set("error", err) } return err diff --git a/gin/sentrygin.go b/gin/sentrygin.go index 1ccc1d1e6..31c86b48e 100644 --- a/gin/sentrygin.go +++ b/gin/sentrygin.go @@ -17,6 +17,7 @@ import ( const sdkIdentifier = "sentry.go.gin" const valuesKey = "sentry" +const transactionKey = "sentry_transaction" type handler struct { repanic bool @@ -50,8 +51,8 @@ func New(options Options) gin.HandlerFunc { }).handle } -func (h *handler) handle(c *gin.Context) { - ctx := c.Request.Context() +func (h *handler) handle(ginContext *gin.Context) { + ctx := ginContext.Request.Context() hub := sentry.GetHubFromContext(ctx) if hub == nil { hub = sentry.CurrentHub().Clone() @@ -62,38 +63,39 @@ func (h *handler) handle(c *gin.Context) { client.SetSDKIdentifier(sdkIdentifier) } - transactionName := c.Request.URL.Path + transactionName := ginContext.Request.URL.Path transactionSource := sentry.SourceURL - if fp := c.FullPath(); fp != "" { + if fp := ginContext.FullPath(); fp != "" { transactionName = fp transactionSource = sentry.SourceRoute } options := []sentry.SpanOption{ + hub.ContinueTrace(ginContext.GetHeader(sentry.SentryTraceHeader), ginContext.GetHeader(sentry.SentryBaggageHeader)), sentry.WithOpName("http.server"), - sentry.ContinueFromRequest(c.Request), sentry.WithTransactionSource(transactionSource), sentry.WithSpanOrigin(sentry.SpanOriginGin), } transaction := sentry.StartTransaction(ctx, - fmt.Sprintf("%s %s", c.Request.Method, transactionName), + fmt.Sprintf("%s %s", ginContext.Request.Method, transactionName), options..., ) - transaction.SetData("http.request.method", c.Request.Method) + transaction.SetData("http.request.method", ginContext.Request.Method) defer func() { - status := c.Writer.Status() + status := ginContext.Writer.Status() transaction.Status = sentry.HTTPtoSpanStatus(status) transaction.SetData("http.response.status_code", status) transaction.Finish() }() - c.Request = c.Request.WithContext(transaction.Context()) - hub.Scope().SetRequest(c.Request) - c.Set(valuesKey, hub) - defer h.recoverWithSentry(hub, c.Request) - c.Next() + hub.Scope().SetRequest(ginContext.Request) + ginContext.Set(valuesKey, hub) + ginContext.Set(transactionKey, transaction) + defer h.recoverWithSentry(hub, ginContext.Request) + + ginContext.Next() } func (h *handler) recoverWithSentry(hub *sentry.Hub, r *http.Request) { @@ -135,3 +137,14 @@ func GetHubFromContext(ctx *gin.Context) *sentry.Hub { } return nil } + +// GetSpanFromContext retrieves attached *sentry.Span instance from gin.Context. +// If there is no transaction on echo.Context, it will return nil. +func GetSpanFromContext(ctx *gin.Context) *sentry.Span { + if span, ok := ctx.Get(transactionKey); ok { + if span, ok := span.(*sentry.Span); ok { + return span + } + } + return nil +} diff --git a/http/sentryhttp.go b/http/sentryhttp.go index 0eb998c57..aad426886 100644 --- a/http/sentryhttp.go +++ b/http/sentryhttp.go @@ -14,6 +14,9 @@ import ( // The identifier of the HTTP SDK. const sdkIdentifier = "sentry.go.http" +const valuesKey = "sentry" +const transactionKey = "sentry_transaction" + // A Handler is an HTTP middleware factory that provides integration with // Sentry. type Handler struct { @@ -95,8 +98,8 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc { } options := []sentry.SpanOption{ + hub.ContinueTrace(r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), sentry.WithOpName("http.server"), - sentry.ContinueFromRequest(r), sentry.WithTransactionSource(sentry.SourceURL), sentry.WithSpanOrigin(sentry.SpanOriginStdLib), } @@ -116,11 +119,8 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc { transaction.Finish() }() - // TODO(tracing): if the next handler.ServeHTTP panics, store - // information on the transaction accordingly (status, tag, - // level?, ...). - r = r.WithContext(transaction.Context()) hub.Scope().SetRequest(r) + r = r.WithContext(transaction.Context()) defer h.recoverWithSentry(hub, r) handler.ServeHTTP(rw, r) diff --git a/hub.go b/hub.go index 5c9fc800e..ebc7b7fc3 100644 --- a/hub.go +++ b/hub.go @@ -2,6 +2,7 @@ package sentry import ( "context" + "fmt" "sync" "time" ) @@ -223,7 +224,7 @@ func (hub *Hub) CaptureEvent(event *Event) *EventID { } eventID := client.CaptureEvent(event, nil, scope) - if event.Type != transactionType && eventID != nil { + if event.Type == eventType && eventID != nil { hub.mu.Lock() hub.lastEventID = *eventID hub.mu.Unlock() @@ -365,25 +366,44 @@ func (hub *Hub) Flush(timeout time.Duration) bool { return client.Flush(timeout) } -// Continue a trace based on HTTP header values. If performance is enabled this -// returns a SpanOption that can be used to start a transaction, otherwise nil. -func (hub *Hub) ContinueTrace(trace, baggage string) (SpanOption, error) { +// GetTraceparent returns the current Sentry traceparent string, to be used as a HTTP header value +// or HTML meta tag value. +// This function is context aware, as in it either returns the traceparent based +// on the current span, or the scope's propagation context. +func (hub *Hub) GetTraceparent() string { scope := hub.Scope() - propagationContext, err := PropagationContextFromHeaders(trace, baggage) - if err != nil { - return nil, err + + if scope.span != nil { + return scope.span.ToSentryTrace() } - scope.SetPropagationContext(propagationContext) + return fmt.Sprintf("%s-%s", scope.propagationContext.TraceID, scope.propagationContext.SpanID) +} - client := hub.Client() - if client != nil && client.options.EnableTracing { - return ContinueFromHeaders(trace, baggage), nil +// GetBaggage returns the current Sentry baggage string, to be used as a HTTP header value +// or HTML meta tag value. +// This function is context aware, as in it either returns the baggage based +// on the current span or the scope's propagation context. +func (hub *Hub) GetBaggage() string { + scope := hub.Scope() + + if scope.span != nil { + return scope.span.ToBaggage() } - scope.SetContext("trace", propagationContext.Map()) + return scope.propagationContext.DynamicSamplingContext.String() +} + +// Continue a trace based on traceparent and bagge values. +// If the SDK is configured with tracing enabled, +// this function returns populated SpanOption. +// In any other cases, it populates the propagation context on the scope. +func (hub *Hub) ContinueTrace(traceparent, baggage string) SpanOption { + scope := hub.Scope() + propagationContext, _ := PropagationContextFromHeaders(traceparent, baggage) + scope.SetPropagationContext(propagationContext) - return nil, nil + return ContinueFromHeaders(traceparent, baggage) } // HasHubOnContext checks whether Hub instance is bound to a given Context struct. diff --git a/hub_test.go b/hub_test.go index 79ea26717..46b889774 100644 --- a/hub_test.go +++ b/hub_test.go @@ -2,14 +2,12 @@ package sentry import ( "context" - "errors" "fmt" "sync" "testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "github.com/stretchr/testify/assert" ) const testDsn = "http://whatever@example.com/1337" @@ -326,88 +324,6 @@ func TestHasHubOnContextReturnsFalseIfHubIsNotThere(t *testing.T) { assertEqual(t, false, HasHubOnContext(ctx)) } -func TestHub_ContinueTrace(t *testing.T) { - newScope := func() *Scope { - return &Scope{contexts: make(map[string]Context)} - } - - mockClient := &Client{options: ClientOptions{EnableTracing: true}} - - tests := map[string]struct { - hub *Hub - trace string - baggage string - expectedErr error - expectedSpan bool // Whether a SpanOption is expected to be returned - checkScope func(t *testing.T, scope *Scope) // Additional checks on the scope - }{ - "Valid trace and baggage": { - hub: NewHub(mockClient, newScope()), - trace: "4fbfb1b884c8532962a3c0b7b834428e-a9f442f9330b4e09", - baggage: "sentry-release=1.0.0,sentry-environment=production", - expectedErr: nil, - expectedSpan: true, - checkScope: func(t *testing.T, scope *Scope) { - assert.Equal(t, "4fbfb1b884c8532962a3c0b7b834428e", scope.propagationContext.TraceID.String()) - }, - }, - "Invalid trace": { - hub: NewHub(mockClient, newScope()), - trace: "invalid", - baggage: "sentry-release=1.0.0,sentry-environment=production", - expectedErr: nil, - expectedSpan: true, - checkScope: func(t *testing.T, scope *Scope) { - assert.NotEmpty(t, scope.propagationContext.TraceID.String()) - }, - }, - "Invalid baggage": { - hub: NewHub(mockClient, newScope()), - trace: "4fbfb1b884c8532962a3c0b7b834428e-a9f442f9330b4e09", - baggage: "invalid_baggage", - expectedErr: errors.New("invalid baggage list-member: \"invalid_baggage\""), - expectedSpan: false, - checkScope: func(t *testing.T, scope *Scope) { - assert.Equal(t, "00000000000000000000000000000000", scope.propagationContext.TraceID.String()) - }, - }, - "Tracing not enabled": { - hub: NewHub(&Client{options: ClientOptions{EnableTracing: false}}, newScope()), - trace: "4fbfb1b884c8532962a3c0b7b834428e-a9f442f9330b4e09", - baggage: "sentry-release=1.0.0,sentry-environment=production", - expectedErr: nil, - expectedSpan: false, - checkScope: func(t *testing.T, scope *Scope) { - assert.Equal(t, "4fbfb1b884c8532962a3c0b7b834428e", scope.propagationContext.TraceID.String()) - assert.Contains(t, scope.contexts, "trace") - }, - }, - } - - for name, tt := range tests { - t.Run(name, func(t *testing.T) { - opt, err := tt.hub.ContinueTrace(tt.trace, tt.baggage) - - if tt.expectedErr != nil { - assert.Error(t, err, "expected error, got nil") - assert.Equal(t, tt.expectedErr.Error(), err.Error()) - } else { - assert.NoError(t, err, "expected no error, got one") - } - - // Check for expected SpanOption - if tt.expectedSpan { - assert.NotNil(t, opt, "expected SpanOption, got nil") - } else { - assert.Nil(t, opt, "expected no SpanOption, got one") - } - - // Additional checks on the scope - tt.checkScope(t, tt.hub.Scope()) - }) - } -} - func TestGetHubFromContext(t *testing.T) { hub, _, _ := setupHubTest() ctx := context.Background() diff --git a/iris/sentryiris.go b/iris/sentryiris.go index eb00e30ac..e29c64c2b 100644 --- a/iris/sentryiris.go +++ b/iris/sentryiris.go @@ -52,10 +52,14 @@ func New(options Options) iris.Handler { }).handle } -func (h *handler) handle(ctx iris.Context) { - hub := sentry.GetHubFromContext(ctx.Request().Context()) +func (h *handler) handle(irisCtx iris.Context) { + request := irisCtx.Request() + ctx := request.Context() + + hub := sentry.GetHubFromContext(ctx) if hub == nil { hub = sentry.CurrentHub().Clone() + ctx = sentry.SetHubOnContext(ctx, hub) } if client := hub.Client(); client != nil { @@ -63,13 +67,13 @@ func (h *handler) handle(ctx iris.Context) { } options := []sentry.SpanOption{ + hub.ContinueTrace(request.Header.Get(sentry.SentryTraceHeader), request.Header.Get(sentry.SentryBaggageHeader)), sentry.WithOpName("http.server"), - sentry.ContinueFromRequest(ctx.Request()), sentry.WithTransactionSource(sentry.SourceRoute), sentry.WithSpanOrigin(sentry.SpanOriginIris), } - currentRoute := ctx.GetCurrentRoute() + currentRoute := irisCtx.GetCurrentRoute() transaction := sentry.StartTransaction( sentry.SetHubOnContext(ctx, hub), @@ -78,18 +82,18 @@ func (h *handler) handle(ctx iris.Context) { ) defer func() { - transaction.SetData("http.response.status_code", ctx.GetStatusCode()) - transaction.Status = sentry.HTTPtoSpanStatus(ctx.GetStatusCode()) + transaction.SetData("http.response.status_code", irisCtx.GetStatusCode()) + transaction.Status = sentry.HTTPtoSpanStatus(irisCtx.GetStatusCode()) transaction.Finish() }() - transaction.SetData("http.request.method", ctx.Request().Method) + transaction.SetData("http.request.method", request.Method) - hub.Scope().SetRequest(ctx.Request()) - ctx.Values().Set(valuesKey, hub) - ctx.Values().Set(transactionKey, transaction) - defer h.recoverWithSentry(hub, ctx.Request()) - ctx.Next() + hub.Scope().SetRequest(request) + irisCtx.Values().Set(valuesKey, hub) + irisCtx.Values().Set(transactionKey, transaction) + defer h.recoverWithSentry(hub, request) + irisCtx.Next() } func (h *handler) recoverWithSentry(hub *sentry.Hub, r *http.Request) { diff --git a/negroni/sentrynegroni.go b/negroni/sentrynegroni.go index 2a49695d5..b9a81904f 100644 --- a/negroni/sentrynegroni.go +++ b/negroni/sentrynegroni.go @@ -63,34 +63,32 @@ func newResponseWriter(w http.ResponseWriter) *responseWriter { func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) { ctx := r.Context() + hub := sentry.GetHubFromContext(ctx) if hub == nil { hub = sentry.CurrentHub().Clone() + ctx = sentry.SetHubOnContext(ctx, hub) } if client := hub.Client(); client != nil { client.SetSDKIdentifier(sdkIdentifier) } - hub.Scope().SetRequest(r) - ctx = sentry.SetHubOnContext( - context.WithValue(ctx, sentry.RequestContextKey, r), - hub, - ) - options := []sentry.SpanOption{ + hub.ContinueTrace(r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), sentry.WithOpName("http.server"), - sentry.ContinueFromRequest(r), sentry.WithTransactionSource(sentry.SourceURL), sentry.WithSpanOrigin(sentry.SpanOriginNegroni), } - // We don't mind getting an existing transaction back so we don't need to - // check if it is. - transaction := sentry.StartTransaction(ctx, + + transaction := sentry.StartTransaction( + ctx, fmt.Sprintf("%s %s", r.Method, r.URL.Path), options..., ) + transaction.SetData("http.request.method", r.Method) + rw := newResponseWriter(w) defer func() { @@ -99,12 +97,11 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next http.Ha transaction.SetData("http.response.status_code", status) transaction.Finish() }() - // TODO(tracing): if the next handler.ServeHTTP panics, store - // information on the transaction accordingly (status, tag, - // level?, ...). - r = r.WithContext(transaction.Context()) + hub.Scope().SetRequest(r) + r = r.WithContext(transaction.Context()) defer h.recoverWithSentry(hub, r) + next(rw, r.WithContext(ctx)) } diff --git a/scope.go b/scope.go index 74eab95e1..9bf126c3a 100644 --- a/scope.go +++ b/scope.go @@ -383,14 +383,14 @@ func (scope *Scope) ApplyToEvent(event *Event, hint *EventHint, client *Client) } for key, value := range scope.contexts { - if key == "trace" && event.Type == transactionType { - // Do not override trace context of - // transactions, otherwise it breaks the - // transaction event representation. - // For error events, the trace context is used - // to link errors and traces/spans in Sentry. - continue - } + // if key == "trace" && event.Type == transactionType { + // // Do not override trace context of + // // transactions, otherwise it breaks the + // // transaction event representation. + // // For error events, the trace context is used + // // to link errors and traces/spans in Sentry. + // continue + // } // Ensure we are not overwriting event fields if _, ok := event.Contexts[key]; !ok { @@ -399,29 +399,26 @@ func (scope *Scope) ApplyToEvent(event *Event, hint *EventHint, client *Client) } } - // Apply the trace context to errors if there is a Span on the scope. If - // there isn't then fall back to the propagation context. - if event.Type != transactionType { - if event.Contexts == nil { - event.Contexts = make(map[string]Context) - } + if event.Contexts == nil { + event.Contexts = make(map[string]Context) + } - if scope.span != nil { - event.Contexts["trace"] = scope.span.traceContext().Map() + if scope.span != nil { + event.Contexts["trace"] = scope.span.traceContext().Map() - transaction := scope.span.GetTransaction() - if transaction != nil { - event.sdkMetaData.dsc = DynamicSamplingContextFromTransaction(transaction) - } - } else { - event.Contexts["trace"] = scope.propagationContext.Map() + transaction := scope.span.GetTransaction() + if transaction != nil { + event.sdkMetaData.dsc = DynamicSamplingContextFromTransaction(transaction) + } + } else { - dsc := scope.propagationContext.DynamicSamplingContext - if !dsc.HasEntries() && client != nil { - dsc = DynamicSamplingContextFromScope(scope, client) - } - event.sdkMetaData.dsc = dsc + event.Contexts["trace"] = scope.propagationContext.Map() + + dsc := scope.propagationContext.DynamicSamplingContext + if !dsc.HasEntries() && client != nil { + dsc = DynamicSamplingContextFromScope(scope, client) } + event.sdkMetaData.dsc = dsc } if len(scope.extra) > 0 { diff --git a/tracing.go b/tracing.go index 9c5852872..4a84982a1 100644 --- a/tracing.go +++ b/tracing.go @@ -192,11 +192,11 @@ func StartSpan(ctx context.Context, operation string, options ...SpanOption) *Sp span.recorder.record(&span) - hub := hubFromContext(ctx) - - // Update scope so that all events include a trace context, allowing - // Sentry to correlate errors to transactions/spans. - hub.Scope().SetContext("trace", span.traceContext().Map()) + clientOptions := span.clientOptions() + if clientOptions.EnableTracing { + hub := hubFromContext(ctx) + hub.Scope().SetSpan(&span) + } // Start profiling only if it's a sampled root transaction. if span.IsTransaction() && span.Sampled.Bool() { @@ -554,7 +554,6 @@ func (s *Span) toEvent() *Event { for k, v := range s.contexts { contexts[k] = cloneContext(v) } - contexts["trace"] = s.traceContext().Map() // Make sure that the transaction source is valid transactionSource := s.Source @@ -893,22 +892,6 @@ func WithSpanOrigin(origin SpanOrigin) SpanOption { } } -func GetTraceHeader(s *Scope) string { - if s.span != nil { - return s.span.ToSentryTrace() - } - - return fmt.Sprintf("%s-%s", s.propagationContext.TraceID, s.propagationContext.SpanID) -} - -func GetBaggageHeader(s *Scope) string { - if s.span != nil { - return s.span.ToBaggage() - } - - return s.propagationContext.DynamicSamplingContext.String() -} - // ContinueFromRequest returns a span option that updates the span to continue // an existing trace. If it cannot detect an existing trace in the request, the // span will be left unchanged. diff --git a/tracing_test.go b/tracing_test.go index da2980286..47de93161 100644 --- a/tracing_test.go +++ b/tracing_test.go @@ -10,7 +10,6 @@ import ( "math" "net/http" "reflect" - "strings" "sync" "testing" "time" @@ -483,76 +482,76 @@ func TestToSentryTrace(t *testing.T) { } } -func TestGetTraceHeader(t *testing.T) { - tests := map[string]struct { - scope *Scope - expected string - }{ - "With span": { - scope: func() *Scope { - s := NewScope() - s.span = &Span{ - TraceID: TraceIDFromHex("d49d9bf66f13450b81f65bc51cf49c03"), - SpanID: SpanIDFromHex("a9f442f9330b4e09"), - Sampled: SampledTrue, - } - return s - }(), - expected: "d49d9bf66f13450b81f65bc51cf49c03-a9f442f9330b4e09-1", - }, - "Without span": { - scope: func() *Scope { - s := NewScope() - s.propagationContext.TraceID = TraceIDFromHex("d49d9bf66f13450b81f65bc51cf49c03") - s.propagationContext.SpanID = SpanIDFromHex("a9f442f9330b4e09") - return s - }(), - expected: "d49d9bf66f13450b81f65bc51cf49c03-a9f442f9330b4e09", - }, - } - - for name, tt := range tests { - t.Run(name, func(t *testing.T) { - result := GetTraceHeader(tt.scope) - assertEqual(t, tt.expected, result) - }) - } -} - -func TestGetBaggageHeader(t *testing.T) { - tests := map[string]struct { - scope *Scope - expected string - }{ - "With span": { - scope: func() *Scope { - s := NewScope() - s.span = &Span{} - return s - }(), - expected: "", - }, - "Without span": { - scope: func() *Scope { - s := NewScope() - s.propagationContext.DynamicSamplingContext = DynamicSamplingContext{ - Entries: map[string]string{"release": "1.0.0", "environment": "production"}, - } - return s - }(), - expected: "sentry-environment=production,sentry-release=1.0.0", - }, - } - - for name, tt := range tests { - t.Run(name, func(t *testing.T) { - result := GetBaggageHeader(tt.scope) - res := strings.Split(result, ",") - sortSlice(res) - assertEqual(t, tt.expected, strings.Join(res, ",")) - }) - } -} +// func TestGetTraceHeader(t *testing.T) { +// tests := map[string]struct { +// scope *Scope +// expected string +// }{ +// "With span": { +// scope: func() *Scope { +// s := NewScope() +// s.span = &Span{ +// TraceID: TraceIDFromHex("d49d9bf66f13450b81f65bc51cf49c03"), +// SpanID: SpanIDFromHex("a9f442f9330b4e09"), +// Sampled: SampledTrue, +// } +// return s +// }(), +// expected: "d49d9bf66f13450b81f65bc51cf49c03-a9f442f9330b4e09-1", +// }, +// "Without span": { +// scope: func() *Scope { +// s := NewScope() +// s.propagationContext.TraceID = TraceIDFromHex("d49d9bf66f13450b81f65bc51cf49c03") +// s.propagationContext.SpanID = SpanIDFromHex("a9f442f9330b4e09") +// return s +// }(), +// expected: "d49d9bf66f13450b81f65bc51cf49c03-a9f442f9330b4e09", +// }, +// } + +// for name, tt := range tests { +// t.Run(name, func(t *testing.T) { +// result := GetTraceHeader(tt.scope) +// assertEqual(t, tt.expected, result) +// }) +// } +// } + +// func TestGetBaggageHeader(t *testing.T) { +// tests := map[string]struct { +// scope *Scope +// expected string +// }{ +// "With span": { +// scope: func() *Scope { +// s := NewScope() +// s.span = &Span{} +// return s +// }(), +// expected: "", +// }, +// "Without span": { +// scope: func() *Scope { +// s := NewScope() +// s.propagationContext.DynamicSamplingContext = DynamicSamplingContext{ +// Entries: map[string]string{"release": "1.0.0", "environment": "production"}, +// } +// return s +// }(), +// expected: "sentry-environment=production,sentry-release=1.0.0", +// }, +// } + +// for name, tt := range tests { +// t.Run(name, func(t *testing.T) { +// result := GetBaggageHeader(tt.scope) +// res := strings.Split(result, ",") +// sortSlice(res) +// assertEqual(t, tt.expected, strings.Join(res, ",")) +// }) +// } +// } func TestContinueSpanFromRequest(t *testing.T) { traceID := TraceIDFromHex("bc6d53f15eb88f4320054569b8c553d4") From debe320599f601d19afe17088bb3f90026a59bfb Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 18 Jul 2024 12:16:13 +0200 Subject: [PATCH 02/14] Updates --- echo/sentryecho.go | 28 +++++++++++++--------------- fasthttp/sentryfasthttp.go | 12 ++++++++---- fiber/sentryfiber.go | 12 ++++++++---- gin/sentrygin.go | 35 +++++++++++++++++++---------------- http/sentryhttp.go | 14 +++++--------- hub.go | 12 ------------ iris/sentryiris.go | 30 ++++++++++++++---------------- negroni/sentrynegroni.go | 11 ++++------- tracing.go | 12 ++++++++++++ 9 files changed, 83 insertions(+), 83 deletions(-) diff --git a/echo/sentryecho.go b/echo/sentryecho.go index fe28ed6c3..f8226fb4c 100644 --- a/echo/sentryecho.go +++ b/echo/sentryecho.go @@ -49,37 +49,35 @@ func New(options Options) echo.MiddlewareFunc { } func (h *handler) handle(next echo.HandlerFunc) echo.HandlerFunc { - return func(echoCtx echo.Context) error { - r := echoCtx.Request() - ctx := r.Context() - - hub := sentry.GetHubFromContext(ctx) + return func(ctx echo.Context) error { + hub := GetHubFromContext(ctx) if hub == nil { hub = sentry.CurrentHub().Clone() - ctx = sentry.SetHubOnContext(ctx, hub) } if client := hub.Client(); client != nil { client.SetSDKIdentifier(sdkIdentifier) } + r := ctx.Request() + transactionName := r.URL.Path transactionSource := sentry.SourceURL - if path := echoCtx.Path(); path != "" { + if path := ctx.Path(); path != "" { transactionName = path transactionSource = sentry.SourceRoute } options := []sentry.SpanOption{ - hub.ContinueTrace(r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), + sentry.ContinueTrace(hub, r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), sentry.WithOpName("http.server"), sentry.WithTransactionSource(transactionSource), sentry.WithSpanOrigin(sentry.SpanOriginEcho), } transaction := sentry.StartTransaction( - ctx, + sentry.SetHubOnContext(r.Context(), hub), fmt.Sprintf("%s %s", r.Method, transactionName), options..., ) @@ -87,8 +85,8 @@ func (h *handler) handle(next echo.HandlerFunc) echo.HandlerFunc { transaction.SetData("http.request.method", r.Method) defer func() { - status := echoCtx.Response().Status - if err := echoCtx.Get("error"); err != nil { + status := ctx.Response().Status + if err := ctx.Get("error"); err != nil { if httpError, ok := err.(*echo.HTTPError); ok { status = httpError.Code } @@ -100,14 +98,14 @@ func (h *handler) handle(next echo.HandlerFunc) echo.HandlerFunc { }() hub.Scope().SetRequest(r) - echoCtx.Set(valuesKey, hub) - echoCtx.Set(transactionKey, transaction) + ctx.Set(valuesKey, hub) + ctx.Set(transactionKey, transaction) defer h.recoverWithSentry(hub, r) - err := next(echoCtx) + err := next(ctx) if err != nil { // Store the error so it can be used in the deferred function - echoCtx.Set("error", err) + ctx.Set("error", err) } return err diff --git a/fasthttp/sentryfasthttp.go b/fasthttp/sentryfasthttp.go index f8a5bf5c4..bd8cc6261 100644 --- a/fasthttp/sentryfasthttp.go +++ b/fasthttp/sentryfasthttp.go @@ -58,17 +58,20 @@ func New(options Options) *Handler { // Handle wraps fasthttp.RequestHandler and recovers from caught panics. func (h *Handler) Handle(handler fasthttp.RequestHandler) fasthttp.RequestHandler { return func(ctx *fasthttp.RequestCtx) { - hub := sentry.CurrentHub().Clone() + hub := GetHubFromContext(ctx) + if hub == nil { + hub = sentry.CurrentHub().Clone() + } if client := hub.Client(); client != nil { client.SetSDKIdentifier(sdkIdentifier) } - convertedHTTPRequest := convert(ctx) + r := convert(ctx) options := []sentry.SpanOption{ + sentry.ContinueTrace(hub, r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), sentry.WithOpName("http.server"), - sentry.ContinueFromRequest(convertedHTTPRequest), sentry.WithTransactionSource(sentry.SourceRoute), sentry.WithSpanOrigin(sentry.SpanOriginFastHTTP), } @@ -90,11 +93,12 @@ func (h *Handler) Handle(handler fasthttp.RequestHandler) fasthttp.RequestHandle transaction.SetData("http.request.method", method) scope := hub.Scope() - scope.SetRequest(convertedHTTPRequest) + scope.SetRequest(r) scope.SetRequestBody(ctx.Request.Body()) ctx.SetUserValue(valuesKey, hub) ctx.SetUserValue(transactionKey, transaction) defer h.recoverWithSentry(hub, ctx) + handler(ctx) } } diff --git a/fiber/sentryfiber.go b/fiber/sentryfiber.go index 604c5cf12..3586e192d 100644 --- a/fiber/sentryfiber.go +++ b/fiber/sentryfiber.go @@ -54,13 +54,16 @@ func New(options Options) fiber.Handler { } func (h *handler) handle(ctx *fiber.Ctx) error { - hub := sentry.CurrentHub().Clone() + hub := GetHubFromContext(ctx) + if hub == nil { + hub = sentry.CurrentHub().Clone() + } if client := hub.Client(); client != nil { client.SetSDKIdentifier(sdkIdentifier) } - convertedHTTPRequest := convert(ctx) + r := convert(ctx) method := ctx.Method() @@ -68,8 +71,8 @@ func (h *handler) handle(ctx *fiber.Ctx) error { transactionSource := sentry.SourceURL options := []sentry.SpanOption{ + sentry.ContinueTrace(hub, r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), sentry.WithOpName("http.server"), - sentry.ContinueFromRequest(convertedHTTPRequest), sentry.WithTransactionSource(transactionSource), sentry.WithSpanOrigin(sentry.SpanOriginFiber), } @@ -90,11 +93,12 @@ func (h *handler) handle(ctx *fiber.Ctx) error { transaction.SetData("http.request.method", method) scope := hub.Scope() - scope.SetRequest(convertedHTTPRequest) + scope.SetRequest(r) scope.SetRequestBody(ctx.Request().Body()) ctx.Locals(valuesKey, hub) ctx.Locals(transactionKey, transaction) defer h.recoverWithSentry(hub, ctx) + return ctx.Next() } diff --git a/gin/sentrygin.go b/gin/sentrygin.go index 31c86b48e..f50b8b793 100644 --- a/gin/sentrygin.go +++ b/gin/sentrygin.go @@ -51,51 +51,54 @@ func New(options Options) gin.HandlerFunc { }).handle } -func (h *handler) handle(ginContext *gin.Context) { - ctx := ginContext.Request.Context() - hub := sentry.GetHubFromContext(ctx) +func (h *handler) handle(ctx *gin.Context) { + hub := sentry.GetHubFromContext(ctx.Request.Context()) if hub == nil { hub = sentry.CurrentHub().Clone() - ctx = sentry.SetHubOnContext(ctx, hub) } if client := hub.Client(); client != nil { client.SetSDKIdentifier(sdkIdentifier) } - transactionName := ginContext.Request.URL.Path + r := ctx.Request + + transactionName := ctx.Request.URL.Path transactionSource := sentry.SourceURL - if fp := ginContext.FullPath(); fp != "" { + if fp := ctx.FullPath(); fp != "" { transactionName = fp transactionSource = sentry.SourceRoute } options := []sentry.SpanOption{ - hub.ContinueTrace(ginContext.GetHeader(sentry.SentryTraceHeader), ginContext.GetHeader(sentry.SentryBaggageHeader)), + sentry.ContinueTrace(hub, ctx.GetHeader(sentry.SentryTraceHeader), ctx.GetHeader(sentry.SentryBaggageHeader)), sentry.WithOpName("http.server"), sentry.WithTransactionSource(transactionSource), sentry.WithSpanOrigin(sentry.SpanOriginGin), } - transaction := sentry.StartTransaction(ctx, - fmt.Sprintf("%s %s", ginContext.Request.Method, transactionName), + transaction := sentry.StartTransaction( + sentry.SetHubOnContext(r.Context(), hub), + fmt.Sprintf("%s %s", r.Method, transactionName), options..., ) - transaction.SetData("http.request.method", ginContext.Request.Method) + + transaction.SetData("http.request.method", r.Method) + defer func() { - status := ginContext.Writer.Status() + status := ctx.Writer.Status() transaction.Status = sentry.HTTPtoSpanStatus(status) transaction.SetData("http.response.status_code", status) transaction.Finish() }() - hub.Scope().SetRequest(ginContext.Request) - ginContext.Set(valuesKey, hub) - ginContext.Set(transactionKey, transaction) - defer h.recoverWithSentry(hub, ginContext.Request) + hub.Scope().SetRequest(r) + ctx.Set(valuesKey, hub) + ctx.Set(transactionKey, transaction) + defer h.recoverWithSentry(hub, r) - ginContext.Next() + ctx.Next() } func (h *handler) recoverWithSentry(hub *sentry.Hub, r *http.Request) { diff --git a/http/sentryhttp.go b/http/sentryhttp.go index aad426886..e2d1cd2eb 100644 --- a/http/sentryhttp.go +++ b/http/sentryhttp.go @@ -14,9 +14,6 @@ import ( // The identifier of the HTTP SDK. const sdkIdentifier = "sentry.go.http" -const valuesKey = "sentry" -const transactionKey = "sentry_transaction" - // A Handler is an HTTP middleware factory that provides integration with // Sentry. type Handler struct { @@ -86,11 +83,9 @@ func (h *Handler) HandleFunc(handler http.HandlerFunc) http.HandlerFunc { func (h *Handler) handle(handler http.Handler) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() - hub := sentry.GetHubFromContext(ctx) + hub := sentry.GetHubFromContext(r.Context()) if hub == nil { hub = sentry.CurrentHub().Clone() - ctx = sentry.SetHubOnContext(ctx, hub) } if client := hub.Client(); client != nil { @@ -98,13 +93,14 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc { } options := []sentry.SpanOption{ - hub.ContinueTrace(r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), + sentry.ContinueTrace(hub, r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), sentry.WithOpName("http.server"), sentry.WithTransactionSource(sentry.SourceURL), sentry.WithSpanOrigin(sentry.SpanOriginStdLib), } - transaction := sentry.StartTransaction(ctx, + transaction := sentry.StartTransaction( + sentry.SetHubOnContext(r.Context(), hub), fmt.Sprintf("%s %s", r.Method, r.URL.Path), options..., ) @@ -121,8 +117,8 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc { hub.Scope().SetRequest(r) r = r.WithContext(transaction.Context()) - defer h.recoverWithSentry(hub, r) + handler.ServeHTTP(rw, r) } } diff --git a/hub.go b/hub.go index ebc7b7fc3..f38d62f48 100644 --- a/hub.go +++ b/hub.go @@ -394,18 +394,6 @@ func (hub *Hub) GetBaggage() string { return scope.propagationContext.DynamicSamplingContext.String() } -// Continue a trace based on traceparent and bagge values. -// If the SDK is configured with tracing enabled, -// this function returns populated SpanOption. -// In any other cases, it populates the propagation context on the scope. -func (hub *Hub) ContinueTrace(traceparent, baggage string) SpanOption { - scope := hub.Scope() - propagationContext, _ := PropagationContextFromHeaders(traceparent, baggage) - scope.SetPropagationContext(propagationContext) - - return ContinueFromHeaders(traceparent, baggage) -} - // HasHubOnContext checks whether Hub instance is bound to a given Context struct. func HasHubOnContext(ctx context.Context) bool { _, ok := ctx.Value(HubContextKey).(*Hub) diff --git a/iris/sentryiris.go b/iris/sentryiris.go index e29c64c2b..dbf273176 100644 --- a/iris/sentryiris.go +++ b/iris/sentryiris.go @@ -52,28 +52,26 @@ func New(options Options) iris.Handler { }).handle } -func (h *handler) handle(irisCtx iris.Context) { - request := irisCtx.Request() - ctx := request.Context() - - hub := sentry.GetHubFromContext(ctx) +func (h *handler) handle(ctx iris.Context) { + hub := sentry.GetHubFromContext(ctx.Request().Context()) if hub == nil { hub = sentry.CurrentHub().Clone() - ctx = sentry.SetHubOnContext(ctx, hub) } if client := hub.Client(); client != nil { client.SetSDKIdentifier(sdkIdentifier) } + r := ctx.Request() + options := []sentry.SpanOption{ - hub.ContinueTrace(request.Header.Get(sentry.SentryTraceHeader), request.Header.Get(sentry.SentryBaggageHeader)), + sentry.ContinueTrace(hub, r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), sentry.WithOpName("http.server"), sentry.WithTransactionSource(sentry.SourceRoute), sentry.WithSpanOrigin(sentry.SpanOriginIris), } - currentRoute := irisCtx.GetCurrentRoute() + currentRoute := ctx.GetCurrentRoute() transaction := sentry.StartTransaction( sentry.SetHubOnContext(ctx, hub), @@ -82,18 +80,18 @@ func (h *handler) handle(irisCtx iris.Context) { ) defer func() { - transaction.SetData("http.response.status_code", irisCtx.GetStatusCode()) - transaction.Status = sentry.HTTPtoSpanStatus(irisCtx.GetStatusCode()) + transaction.SetData("http.response.status_code", ctx.GetStatusCode()) + transaction.Status = sentry.HTTPtoSpanStatus(ctx.GetStatusCode()) transaction.Finish() }() - transaction.SetData("http.request.method", request.Method) + transaction.SetData("http.request.method", r.Method) - hub.Scope().SetRequest(request) - irisCtx.Values().Set(valuesKey, hub) - irisCtx.Values().Set(transactionKey, transaction) - defer h.recoverWithSentry(hub, request) - irisCtx.Next() + hub.Scope().SetRequest(r) + ctx.Values().Set(valuesKey, hub) + ctx.Values().Set(transactionKey, transaction) + defer h.recoverWithSentry(hub, r) + ctx.Next() } func (h *handler) recoverWithSentry(hub *sentry.Hub, r *http.Request) { diff --git a/negroni/sentrynegroni.go b/negroni/sentrynegroni.go index b9a81904f..b3ed5cc75 100644 --- a/negroni/sentrynegroni.go +++ b/negroni/sentrynegroni.go @@ -62,12 +62,9 @@ func newResponseWriter(w http.ResponseWriter) *responseWriter { } func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) { - ctx := r.Context() - - hub := sentry.GetHubFromContext(ctx) + hub := sentry.GetHubFromContext(r.Context()) if hub == nil { hub = sentry.CurrentHub().Clone() - ctx = sentry.SetHubOnContext(ctx, hub) } if client := hub.Client(); client != nil { @@ -75,14 +72,14 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next http.Ha } options := []sentry.SpanOption{ - hub.ContinueTrace(r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), + sentry.ContinueTrace(hub, r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), sentry.WithOpName("http.server"), sentry.WithTransactionSource(sentry.SourceURL), sentry.WithSpanOrigin(sentry.SpanOriginNegroni), } transaction := sentry.StartTransaction( - ctx, + sentry.SetHubOnContext(r.Context(), hub), fmt.Sprintf("%s %s", r.Method, r.URL.Path), options..., ) @@ -102,7 +99,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next http.Ha r = r.WithContext(transaction.Context()) defer h.recoverWithSentry(hub, r) - next(rw, r.WithContext(ctx)) + next(rw, r.WithContext(r.Context())) } func (h *handler) recoverWithSentry(hub *sentry.Hub, r *http.Request) { diff --git a/tracing.go b/tracing.go index 4a84982a1..40fd722ff 100644 --- a/tracing.go +++ b/tracing.go @@ -892,6 +892,18 @@ func WithSpanOrigin(origin SpanOrigin) SpanOption { } } +// Continue a trace based on traceparent and bagge values. +// If the SDK is configured with tracing enabled, +// this function returns populated SpanOption. +// In any other cases, it populates the propagation context on the scope. +func ContinueTrace(hub *Hub, traceparent, baggage string) SpanOption { + scope := hub.Scope() + propagationContext, _ := PropagationContextFromHeaders(traceparent, baggage) + scope.SetPropagationContext(propagationContext) + + return ContinueFromHeaders(traceparent, baggage) +} + // ContinueFromRequest returns a span option that updates the span to continue // an existing trace. If it cannot detect an existing trace in the request, the // span will be left unchanged. From 20ca5e7df258400c90f73bf5ff0161b286f353fe Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 18 Jul 2024 13:32:53 +0200 Subject: [PATCH 03/14] Revert --- client.go | 2 +- hub.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client.go b/client.go index 1179f762c..dd5e98262 100644 --- a/client.go +++ b/client.go @@ -612,7 +612,7 @@ func (client *Client) processEvent(event *Event, hint *EventHint, scope EventMod // Transactions are sampled by options.TracesSampleRate or // options.TracesSampler when they are started. Other events // (errors, messages) are sampled here. Does not apply to check-ins. - if event.Type == eventType && !sample(client.options.SampleRate) { + if event.Type != transactionType && event.Type != checkInType && !sample(client.options.SampleRate) { Logger.Println("Event dropped due to SampleRate hit.") return nil } diff --git a/hub.go b/hub.go index f38d62f48..c99b6d70d 100644 --- a/hub.go +++ b/hub.go @@ -224,7 +224,7 @@ func (hub *Hub) CaptureEvent(event *Event) *EventID { } eventID := client.CaptureEvent(event, nil, scope) - if event.Type == eventType && eventID != nil { + if event.Type != transactionType && eventID != nil { hub.mu.Lock() hub.lastEventID = *eventID hub.mu.Unlock() From 5beaa5bd1c857fc651daaa0e568018b77d035a11 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 18 Jul 2024 13:35:41 +0200 Subject: [PATCH 04/14] Revert --- hub.go | 22 ++++++++++++++ hub_test.go | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/hub.go b/hub.go index c99b6d70d..7979c68a7 100644 --- a/hub.go +++ b/hub.go @@ -366,6 +366,28 @@ func (hub *Hub) Flush(timeout time.Duration) bool { return client.Flush(timeout) } +// Continue a trace based on HTTP header values. If performance is enabled this +// returns a SpanOption that can be used to start a transaction, otherwise nil. +// TODO: remove - moved to tracing.go +func (hub *Hub) ContinueTrace(trace, baggage string) (SpanOption, error) { + scope := hub.Scope() + propagationContext, err := PropagationContextFromHeaders(trace, baggage) + if err != nil { + return nil, err + } + + scope.SetPropagationContext(propagationContext) + + client := hub.Client() + if client != nil && client.options.EnableTracing { + return ContinueFromHeaders(trace, baggage), nil + } + + scope.SetContext("trace", propagationContext.Map()) + + return nil, nil +} + // GetTraceparent returns the current Sentry traceparent string, to be used as a HTTP header value // or HTML meta tag value. // This function is context aware, as in it either returns the traceparent based diff --git a/hub_test.go b/hub_test.go index 46b889774..79ea26717 100644 --- a/hub_test.go +++ b/hub_test.go @@ -2,12 +2,14 @@ package sentry import ( "context" + "errors" "fmt" "sync" "testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/stretchr/testify/assert" ) const testDsn = "http://whatever@example.com/1337" @@ -324,6 +326,88 @@ func TestHasHubOnContextReturnsFalseIfHubIsNotThere(t *testing.T) { assertEqual(t, false, HasHubOnContext(ctx)) } +func TestHub_ContinueTrace(t *testing.T) { + newScope := func() *Scope { + return &Scope{contexts: make(map[string]Context)} + } + + mockClient := &Client{options: ClientOptions{EnableTracing: true}} + + tests := map[string]struct { + hub *Hub + trace string + baggage string + expectedErr error + expectedSpan bool // Whether a SpanOption is expected to be returned + checkScope func(t *testing.T, scope *Scope) // Additional checks on the scope + }{ + "Valid trace and baggage": { + hub: NewHub(mockClient, newScope()), + trace: "4fbfb1b884c8532962a3c0b7b834428e-a9f442f9330b4e09", + baggage: "sentry-release=1.0.0,sentry-environment=production", + expectedErr: nil, + expectedSpan: true, + checkScope: func(t *testing.T, scope *Scope) { + assert.Equal(t, "4fbfb1b884c8532962a3c0b7b834428e", scope.propagationContext.TraceID.String()) + }, + }, + "Invalid trace": { + hub: NewHub(mockClient, newScope()), + trace: "invalid", + baggage: "sentry-release=1.0.0,sentry-environment=production", + expectedErr: nil, + expectedSpan: true, + checkScope: func(t *testing.T, scope *Scope) { + assert.NotEmpty(t, scope.propagationContext.TraceID.String()) + }, + }, + "Invalid baggage": { + hub: NewHub(mockClient, newScope()), + trace: "4fbfb1b884c8532962a3c0b7b834428e-a9f442f9330b4e09", + baggage: "invalid_baggage", + expectedErr: errors.New("invalid baggage list-member: \"invalid_baggage\""), + expectedSpan: false, + checkScope: func(t *testing.T, scope *Scope) { + assert.Equal(t, "00000000000000000000000000000000", scope.propagationContext.TraceID.String()) + }, + }, + "Tracing not enabled": { + hub: NewHub(&Client{options: ClientOptions{EnableTracing: false}}, newScope()), + trace: "4fbfb1b884c8532962a3c0b7b834428e-a9f442f9330b4e09", + baggage: "sentry-release=1.0.0,sentry-environment=production", + expectedErr: nil, + expectedSpan: false, + checkScope: func(t *testing.T, scope *Scope) { + assert.Equal(t, "4fbfb1b884c8532962a3c0b7b834428e", scope.propagationContext.TraceID.String()) + assert.Contains(t, scope.contexts, "trace") + }, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + opt, err := tt.hub.ContinueTrace(tt.trace, tt.baggage) + + if tt.expectedErr != nil { + assert.Error(t, err, "expected error, got nil") + assert.Equal(t, tt.expectedErr.Error(), err.Error()) + } else { + assert.NoError(t, err, "expected no error, got one") + } + + // Check for expected SpanOption + if tt.expectedSpan { + assert.NotNil(t, opt, "expected SpanOption, got nil") + } else { + assert.Nil(t, opt, "expected no SpanOption, got one") + } + + // Additional checks on the scope + tt.checkScope(t, tt.hub.Scope()) + }) + } +} + func TestGetHubFromContext(t *testing.T) { hub, _, _ := setupHubTest() ctx := context.Background() From 578719a5e9a2707e0e02c11b2d6fdbdda64ae459 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 18 Jul 2024 13:53:45 +0200 Subject: [PATCH 05/14] Moce tests --- hub_test.go | 76 +++++++++++++++++++++++++++++++++++++++++++++++++ tracing_test.go | 71 --------------------------------------------- 2 files changed, 76 insertions(+), 71 deletions(-) diff --git a/hub_test.go b/hub_test.go index 79ea26717..27930732a 100644 --- a/hub_test.go +++ b/hub_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "sync" "testing" @@ -408,6 +409,81 @@ func TestHub_ContinueTrace(t *testing.T) { } } +func TestGetTraceparent(t *testing.T) { + tests := map[string]struct { + hub *Hub + expected string + }{ + "With span": { + hub: func() *Hub { + h, _, s := setupHubTest() + s.span = &Span{ + TraceID: TraceIDFromHex("d49d9bf66f13450b81f65bc51cf49c03"), + SpanID: SpanIDFromHex("a9f442f9330b4e09"), + Sampled: SampledTrue, + } + return h + }(), + expected: "d49d9bf66f13450b81f65bc51cf49c03-a9f442f9330b4e09-1", + }, + "Without span": { + hub: func() *Hub { + h, _, s := setupHubTest() + s.propagationContext.TraceID = TraceIDFromHex("d49d9bf66f13450b81f65bc51cf49c03") + s.propagationContext.SpanID = SpanIDFromHex("a9f442f9330b4e09") + return h + }(), + expected: "d49d9bf66f13450b81f65bc51cf49c03-a9f442f9330b4e09", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + result := tt.hub.GetTraceparent() + assertEqual(t, tt.expected, result) + }) + } +} + +func TestGetBaggageHeader(t *testing.T) { + tests := map[string]struct { + hub *Hub + expected string + }{ + "With span": { + hub: func() *Hub { + h, _, s := setupHubTest() + s.span = &Span{ + dynamicSamplingContext: DynamicSamplingContext{ + Entries: map[string]string{"sample_rate": "1", "release": "1.0.0", "environment": "production"}, + }, + } + return h + }(), + expected: "sentry-sample_rate=1,sentry-environment=production,sentry-release=1.0.0", + }, + "Without span": { + hub: func() *Hub { + h, _, s := setupHubTest() + s.propagationContext.DynamicSamplingContext = DynamicSamplingContext{ + Entries: map[string]string{"release": "1.0.0", "environment": "production"}, + } + return h + }(), + expected: "sentry-environment=production,sentry-release=1.0.0", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + result := tt.hub.GetBaggage() + res := strings.Split(result, ",") + sortSlice(res) + assertEqual(t, tt.expected, strings.Join(res, ",")) + }) + } +} + func TestGetHubFromContext(t *testing.T) { hub, _, _ := setupHubTest() ctx := context.Background() diff --git a/tracing_test.go b/tracing_test.go index 47de93161..795c33d33 100644 --- a/tracing_test.go +++ b/tracing_test.go @@ -482,77 +482,6 @@ func TestToSentryTrace(t *testing.T) { } } -// func TestGetTraceHeader(t *testing.T) { -// tests := map[string]struct { -// scope *Scope -// expected string -// }{ -// "With span": { -// scope: func() *Scope { -// s := NewScope() -// s.span = &Span{ -// TraceID: TraceIDFromHex("d49d9bf66f13450b81f65bc51cf49c03"), -// SpanID: SpanIDFromHex("a9f442f9330b4e09"), -// Sampled: SampledTrue, -// } -// return s -// }(), -// expected: "d49d9bf66f13450b81f65bc51cf49c03-a9f442f9330b4e09-1", -// }, -// "Without span": { -// scope: func() *Scope { -// s := NewScope() -// s.propagationContext.TraceID = TraceIDFromHex("d49d9bf66f13450b81f65bc51cf49c03") -// s.propagationContext.SpanID = SpanIDFromHex("a9f442f9330b4e09") -// return s -// }(), -// expected: "d49d9bf66f13450b81f65bc51cf49c03-a9f442f9330b4e09", -// }, -// } - -// for name, tt := range tests { -// t.Run(name, func(t *testing.T) { -// result := GetTraceHeader(tt.scope) -// assertEqual(t, tt.expected, result) -// }) -// } -// } - -// func TestGetBaggageHeader(t *testing.T) { -// tests := map[string]struct { -// scope *Scope -// expected string -// }{ -// "With span": { -// scope: func() *Scope { -// s := NewScope() -// s.span = &Span{} -// return s -// }(), -// expected: "", -// }, -// "Without span": { -// scope: func() *Scope { -// s := NewScope() -// s.propagationContext.DynamicSamplingContext = DynamicSamplingContext{ -// Entries: map[string]string{"release": "1.0.0", "environment": "production"}, -// } -// return s -// }(), -// expected: "sentry-environment=production,sentry-release=1.0.0", -// }, -// } - -// for name, tt := range tests { -// t.Run(name, func(t *testing.T) { -// result := GetBaggageHeader(tt.scope) -// res := strings.Split(result, ",") -// sortSlice(res) -// assertEqual(t, tt.expected, strings.Join(res, ",")) -// }) -// } -// } - func TestContinueSpanFromRequest(t *testing.T) { traceID := TraceIDFromHex("bc6d53f15eb88f4320054569b8c553d4") spanID := SpanIDFromHex("b72fa28504b07285") From 6395982843649c61b3fedc75b833ebe2e4332900 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 18 Jul 2024 14:01:06 +0200 Subject: [PATCH 06/14] WIP --- gin/sentrygin.go | 2 +- hub_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gin/sentrygin.go b/gin/sentrygin.go index f50b8b793..d404bbcc9 100644 --- a/gin/sentrygin.go +++ b/gin/sentrygin.go @@ -63,7 +63,7 @@ func (h *handler) handle(ctx *gin.Context) { r := ctx.Request - transactionName := ctx.Request.URL.Path + transactionName := r.URL.Path transactionSource := sentry.SourceURL if fp := ctx.FullPath(); fp != "" { diff --git a/hub_test.go b/hub_test.go index 27930732a..00f1e513d 100644 --- a/hub_test.go +++ b/hub_test.go @@ -440,7 +440,7 @@ func TestGetTraceparent(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { result := tt.hub.GetTraceparent() - assertEqual(t, tt.expected, result) + assertEqual(t, result, tt.expected) }) } } @@ -479,7 +479,7 @@ func TestGetBaggageHeader(t *testing.T) { result := tt.hub.GetBaggage() res := strings.Split(result, ",") sortSlice(res) - assertEqual(t, tt.expected, strings.Join(res, ",")) + assertEqual(t, strings.Join(res, ","), tt.expected) }) } } From 4889474a8cb5fd24e0ada70c43b5d2ceab944b9c Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Mon, 22 Jul 2024 13:27:56 +0200 Subject: [PATCH 07/14] Set hub on context in net/http --- http/sentryhttp.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/http/sentryhttp.go b/http/sentryhttp.go index e2d1cd2eb..d58f7bd05 100644 --- a/http/sentryhttp.go +++ b/http/sentryhttp.go @@ -83,9 +83,11 @@ func (h *Handler) HandleFunc(handler http.HandlerFunc) http.HandlerFunc { func (h *Handler) handle(handler http.Handler) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() hub := sentry.GetHubFromContext(r.Context()) if hub == nil { hub = sentry.CurrentHub().Clone() + ctx = sentry.SetHubOnContext(ctx, hub) } if client := hub.Client(); client != nil { @@ -99,8 +101,7 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc { sentry.WithSpanOrigin(sentry.SpanOriginStdLib), } - transaction := sentry.StartTransaction( - sentry.SetHubOnContext(r.Context(), hub), + transaction := sentry.StartTransaction(ctx, fmt.Sprintf("%s %s", r.Method, r.URL.Path), options..., ) From 35092b3eb890aa2584eb6aefc0a61b76d9a0a6ce Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Fri, 26 Jul 2024 14:56:35 +0200 Subject: [PATCH 08/14] WIP --- hub_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hub_test.go b/hub_test.go index 00f1e513d..c1614f5fa 100644 --- a/hub_test.go +++ b/hub_test.go @@ -445,7 +445,7 @@ func TestGetTraceparent(t *testing.T) { } } -func TestGetBaggageHeader(t *testing.T) { +func TestGetBaggage(t *testing.T) { tests := map[string]struct { hub *Hub expected string @@ -457,7 +457,12 @@ func TestGetBaggageHeader(t *testing.T) { dynamicSamplingContext: DynamicSamplingContext{ Entries: map[string]string{"sample_rate": "1", "release": "1.0.0", "environment": "production"}, }, + recorder: &spanRecorder{}, + ctx: context.Background(), + Sampled: SampledTrue, } + s.span.spanRecorder().record(s.span) + return h }(), expected: "sentry-sample_rate=1,sentry-environment=production,sentry-release=1.0.0", From 15b02957e8344e71b349850c1c8167a864ac8c2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emir=20Ribi=C4=87?= Date: Wed, 21 Aug 2024 21:35:39 +0200 Subject: [PATCH 09/14] fix getBaggage test --- dynamic_sampling_context.go | 9 +++------ hub_test.go | 3 ++- tracing.go | 25 +++++++++++++++---------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/dynamic_sampling_context.go b/dynamic_sampling_context.go index 1eba2a28f..6eec95ba1 100644 --- a/dynamic_sampling_context.go +++ b/dynamic_sampling_context.go @@ -39,8 +39,6 @@ func DynamicSamplingContextFromHeader(header []byte) (DynamicSamplingContext, er } func DynamicSamplingContextFromTransaction(span *Span) DynamicSamplingContext { - entries := map[string]string{} - hub := hubFromContext(span.Context()) scope := hub.Scope() client := hub.Client() @@ -52,6 +50,8 @@ func DynamicSamplingContextFromTransaction(span *Span) DynamicSamplingContext { } } + entries := make(map[string]string) + if traceID := span.TraceID.String(); traceID != "" { entries["trace_id"] = traceID } @@ -80,10 +80,7 @@ func DynamicSamplingContextFromTransaction(span *Span) DynamicSamplingContext { entries["sampled"] = strconv.FormatBool(span.Sampled.Bool()) - return DynamicSamplingContext{ - Entries: entries, - Frozen: true, - } + return DynamicSamplingContext{Entries: entries, Frozen: true} } func (d DynamicSamplingContext) HasEntries() bool { diff --git a/hub_test.go b/hub_test.go index c1614f5fa..59c56f447 100644 --- a/hub_test.go +++ b/hub_test.go @@ -461,11 +461,12 @@ func TestGetBaggage(t *testing.T) { ctx: context.Background(), Sampled: SampledTrue, } + s.span.spanRecorder().record(s.span) return h }(), - expected: "sentry-sample_rate=1,sentry-environment=production,sentry-release=1.0.0", + expected: "sentry-environment=production,sentry-release=1.0.0,sentry-sample_rate=1", }, "Without span": { hub: func() *Hub { diff --git a/tracing.go b/tracing.go index 40fd722ff..b45523ebf 100644 --- a/tracing.go +++ b/tracing.go @@ -324,17 +324,22 @@ func (s *Span) ToSentryTrace() string { // Use this function to propagate the DynamicSamplingContext to a downstream SDK, // either as the value of the "baggage" HTTP header, or as an html "baggage" meta tag. func (s *Span) ToBaggage() string { - if containingTransaction := s.GetTransaction(); containingTransaction != nil { - // In case there is currently no frozen DynamicSamplingContext attached to the transaction, - // create one from the properties of the transaction. - if !s.dynamicSamplingContext.IsFrozen() { - // This will return a frozen DynamicSamplingContext. - s.dynamicSamplingContext = DynamicSamplingContextFromTransaction(containingTransaction) - } + t := s.GetTransaction() + if t == nil { + return "" + } - return containingTransaction.dynamicSamplingContext.String() + // In case there is currently no frozen DynamicSamplingContext attached to the transaction, + // create one from the properties of the transaction. + if !s.dynamicSamplingContext.IsFrozen() { + // This will return a frozen DynamicSamplingContext. + if dsc := DynamicSamplingContextFromTransaction(t); dsc.HasEntries() { + t.dynamicSamplingContext = dsc + } } - return "" + + return t.dynamicSamplingContext.String() + } // SetDynamicSamplingContext sets the given dynamic sampling context on the @@ -550,7 +555,7 @@ func (s *Span) toEvent() *Event { s.dynamicSamplingContext = DynamicSamplingContextFromTransaction(s) } - contexts := map[string]Context{} + contexts := make(map[string]Context, len(s.contexts)) for k, v := range s.contexts { contexts[k] = cloneContext(v) } From 27039f2b482a09759ac9ce6001191672911657e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emir=20Ribi=C4=87?= Date: Wed, 21 Aug 2024 21:37:55 +0200 Subject: [PATCH 10/14] linter fixes --- hub.go | 2 +- scope.go | 1 - tracing.go | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/hub.go b/hub.go index 7979c68a7..358e712f3 100644 --- a/hub.go +++ b/hub.go @@ -368,7 +368,7 @@ func (hub *Hub) Flush(timeout time.Duration) bool { // Continue a trace based on HTTP header values. If performance is enabled this // returns a SpanOption that can be used to start a transaction, otherwise nil. -// TODO: remove - moved to tracing.go +// TODO: remove - moved to tracing.go. func (hub *Hub) ContinueTrace(trace, baggage string) (SpanOption, error) { scope := hub.Scope() propagationContext, err := PropagationContextFromHeaders(trace, baggage) diff --git a/scope.go b/scope.go index 9bf126c3a..0219e00f9 100644 --- a/scope.go +++ b/scope.go @@ -411,7 +411,6 @@ func (scope *Scope) ApplyToEvent(event *Event, hint *EventHint, client *Client) event.sdkMetaData.dsc = DynamicSamplingContextFromTransaction(transaction) } } else { - event.Contexts["trace"] = scope.propagationContext.Map() dsc := scope.propagationContext.DynamicSamplingContext diff --git a/tracing.go b/tracing.go index b45523ebf..0f5ade2e8 100644 --- a/tracing.go +++ b/tracing.go @@ -339,7 +339,6 @@ func (s *Span) ToBaggage() string { } return t.dynamicSamplingContext.String() - } // SetDynamicSamplingContext sets the given dynamic sampling context on the From cc2703928fdf50ea997dd80a9d7cc29002b57c67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emir=20Ribi=C4=87?= Date: Tue, 27 Aug 2024 16:10:33 +0200 Subject: [PATCH 11/14] fix gin --- gin/sentrygin.go | 1 + gin/sentrygin_test.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/gin/sentrygin.go b/gin/sentrygin.go index d404bbcc9..c125ef0ca 100644 --- a/gin/sentrygin.go +++ b/gin/sentrygin.go @@ -93,6 +93,7 @@ func (h *handler) handle(ctx *gin.Context) { transaction.Finish() }() + ctx.Request = ctx.Request.WithContext(transaction.Context()) hub.Scope().SetRequest(r) ctx.Set(valuesKey, hub) ctx.Set(transactionKey, transaction) diff --git a/gin/sentrygin_test.go b/gin/sentrygin_test.go index cef5aa228..a72f1f7aa 100644 --- a/gin/sentrygin_test.go +++ b/gin/sentrygin_test.go @@ -96,7 +96,7 @@ func TestIntegration(t *testing.T) { RoutePath: "/post", Method: "POST", WantStatus: 200, - Body: "payload", + Body: "", Handler: func(c *gin.Context) { hub := sentry.GetHubFromContext(c.Request.Context()) body, err := io.ReadAll(c.Request.Body) @@ -113,7 +113,7 @@ func TestIntegration(t *testing.T) { Request: &sentry.Request{ URL: "/post", Method: "POST", - Data: "payload", + Data: "", Headers: map[string]string{ "Content-Length": "7", "Accept-Encoding": "gzip", From cb94b9b3141d8609447565917180551c9685d7d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emir=20Ribi=C4=87?= Date: Wed, 28 Aug 2024 10:57:17 +0200 Subject: [PATCH 12/14] fix gin tests --- gin/sentrygin.go | 33 ++++++++++++++++----------------- gin/sentrygin_test.go | 4 ++-- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/gin/sentrygin.go b/gin/sentrygin.go index c125ef0ca..1b658d351 100644 --- a/gin/sentrygin.go +++ b/gin/sentrygin.go @@ -51,8 +51,9 @@ func New(options Options) gin.HandlerFunc { }).handle } -func (h *handler) handle(ctx *gin.Context) { - hub := sentry.GetHubFromContext(ctx.Request.Context()) +func (h *handler) handle(c *gin.Context) { + ctx := c.Request.Context() + hub := sentry.GetHubFromContext(ctx) if hub == nil { hub = sentry.CurrentHub().Clone() } @@ -61,45 +62,43 @@ func (h *handler) handle(ctx *gin.Context) { client.SetSDKIdentifier(sdkIdentifier) } - r := ctx.Request - - transactionName := r.URL.Path + transactionName := c.Request.URL.Path transactionSource := sentry.SourceURL - if fp := ctx.FullPath(); fp != "" { + if fp := c.FullPath(); fp != "" { transactionName = fp transactionSource = sentry.SourceRoute } options := []sentry.SpanOption{ - sentry.ContinueTrace(hub, ctx.GetHeader(sentry.SentryTraceHeader), ctx.GetHeader(sentry.SentryBaggageHeader)), + sentry.ContinueTrace(hub, c.GetHeader(sentry.SentryTraceHeader), c.GetHeader(sentry.SentryBaggageHeader)), sentry.WithOpName("http.server"), sentry.WithTransactionSource(transactionSource), sentry.WithSpanOrigin(sentry.SpanOriginGin), } transaction := sentry.StartTransaction( - sentry.SetHubOnContext(r.Context(), hub), - fmt.Sprintf("%s %s", r.Method, transactionName), + sentry.SetHubOnContext(ctx, hub), + fmt.Sprintf("%s %s", c.Request.Method, transactionName), options..., ) - transaction.SetData("http.request.method", r.Method) + transaction.SetData("http.request.method", c.Request.Method) defer func() { - status := ctx.Writer.Status() + status := c.Writer.Status() transaction.Status = sentry.HTTPtoSpanStatus(status) transaction.SetData("http.response.status_code", status) transaction.Finish() }() - ctx.Request = ctx.Request.WithContext(transaction.Context()) - hub.Scope().SetRequest(r) - ctx.Set(valuesKey, hub) - ctx.Set(transactionKey, transaction) - defer h.recoverWithSentry(hub, r) + c.Request = c.Request.WithContext(transaction.Context()) + hub.Scope().SetRequest(c.Request) + c.Set(valuesKey, hub) + c.Set(transactionKey, transaction) + defer h.recoverWithSentry(hub, c.Request) - ctx.Next() + c.Next() } func (h *handler) recoverWithSentry(hub *sentry.Hub, r *http.Request) { diff --git a/gin/sentrygin_test.go b/gin/sentrygin_test.go index a72f1f7aa..cef5aa228 100644 --- a/gin/sentrygin_test.go +++ b/gin/sentrygin_test.go @@ -96,7 +96,7 @@ func TestIntegration(t *testing.T) { RoutePath: "/post", Method: "POST", WantStatus: 200, - Body: "", + Body: "payload", Handler: func(c *gin.Context) { hub := sentry.GetHubFromContext(c.Request.Context()) body, err := io.ReadAll(c.Request.Body) @@ -113,7 +113,7 @@ func TestIntegration(t *testing.T) { Request: &sentry.Request{ URL: "/post", Method: "POST", - Data: "", + Data: "payload", Headers: map[string]string{ "Content-Length": "7", "Accept-Encoding": "gzip", From aaf641a8eda91dbe1c1841bfe6720cab4d3da0f5 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 4 Sep 2024 09:19:50 +0200 Subject: [PATCH 13/14] Cleanup --- scope.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/scope.go b/scope.go index 0219e00f9..48621c94f 100644 --- a/scope.go +++ b/scope.go @@ -383,15 +383,6 @@ func (scope *Scope) ApplyToEvent(event *Event, hint *EventHint, client *Client) } for key, value := range scope.contexts { - // if key == "trace" && event.Type == transactionType { - // // Do not override trace context of - // // transactions, otherwise it breaks the - // // transaction event representation. - // // For error events, the trace context is used - // // to link errors and traces/spans in Sentry. - // continue - // } - // Ensure we are not overwriting event fields if _, ok := event.Contexts[key]; !ok { event.Contexts[key] = cloneContext(value) From f13c84aa003407d054476b2c288393f6c730a7fb Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 4 Sep 2024 09:22:59 +0200 Subject: [PATCH 14/14] cleanup --- hub.go | 22 -------------- hub_test.go | 84 ----------------------------------------------------- 2 files changed, 106 deletions(-) diff --git a/hub.go b/hub.go index 358e712f3..c99b6d70d 100644 --- a/hub.go +++ b/hub.go @@ -366,28 +366,6 @@ func (hub *Hub) Flush(timeout time.Duration) bool { return client.Flush(timeout) } -// Continue a trace based on HTTP header values. If performance is enabled this -// returns a SpanOption that can be used to start a transaction, otherwise nil. -// TODO: remove - moved to tracing.go. -func (hub *Hub) ContinueTrace(trace, baggage string) (SpanOption, error) { - scope := hub.Scope() - propagationContext, err := PropagationContextFromHeaders(trace, baggage) - if err != nil { - return nil, err - } - - scope.SetPropagationContext(propagationContext) - - client := hub.Client() - if client != nil && client.options.EnableTracing { - return ContinueFromHeaders(trace, baggage), nil - } - - scope.SetContext("trace", propagationContext.Map()) - - return nil, nil -} - // GetTraceparent returns the current Sentry traceparent string, to be used as a HTTP header value // or HTML meta tag value. // This function is context aware, as in it either returns the traceparent based diff --git a/hub_test.go b/hub_test.go index 59c56f447..0b306a0a3 100644 --- a/hub_test.go +++ b/hub_test.go @@ -2,7 +2,6 @@ package sentry import ( "context" - "errors" "fmt" "strings" "sync" @@ -10,7 +9,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "github.com/stretchr/testify/assert" ) const testDsn = "http://whatever@example.com/1337" @@ -327,88 +325,6 @@ func TestHasHubOnContextReturnsFalseIfHubIsNotThere(t *testing.T) { assertEqual(t, false, HasHubOnContext(ctx)) } -func TestHub_ContinueTrace(t *testing.T) { - newScope := func() *Scope { - return &Scope{contexts: make(map[string]Context)} - } - - mockClient := &Client{options: ClientOptions{EnableTracing: true}} - - tests := map[string]struct { - hub *Hub - trace string - baggage string - expectedErr error - expectedSpan bool // Whether a SpanOption is expected to be returned - checkScope func(t *testing.T, scope *Scope) // Additional checks on the scope - }{ - "Valid trace and baggage": { - hub: NewHub(mockClient, newScope()), - trace: "4fbfb1b884c8532962a3c0b7b834428e-a9f442f9330b4e09", - baggage: "sentry-release=1.0.0,sentry-environment=production", - expectedErr: nil, - expectedSpan: true, - checkScope: func(t *testing.T, scope *Scope) { - assert.Equal(t, "4fbfb1b884c8532962a3c0b7b834428e", scope.propagationContext.TraceID.String()) - }, - }, - "Invalid trace": { - hub: NewHub(mockClient, newScope()), - trace: "invalid", - baggage: "sentry-release=1.0.0,sentry-environment=production", - expectedErr: nil, - expectedSpan: true, - checkScope: func(t *testing.T, scope *Scope) { - assert.NotEmpty(t, scope.propagationContext.TraceID.String()) - }, - }, - "Invalid baggage": { - hub: NewHub(mockClient, newScope()), - trace: "4fbfb1b884c8532962a3c0b7b834428e-a9f442f9330b4e09", - baggage: "invalid_baggage", - expectedErr: errors.New("invalid baggage list-member: \"invalid_baggage\""), - expectedSpan: false, - checkScope: func(t *testing.T, scope *Scope) { - assert.Equal(t, "00000000000000000000000000000000", scope.propagationContext.TraceID.String()) - }, - }, - "Tracing not enabled": { - hub: NewHub(&Client{options: ClientOptions{EnableTracing: false}}, newScope()), - trace: "4fbfb1b884c8532962a3c0b7b834428e-a9f442f9330b4e09", - baggage: "sentry-release=1.0.0,sentry-environment=production", - expectedErr: nil, - expectedSpan: false, - checkScope: func(t *testing.T, scope *Scope) { - assert.Equal(t, "4fbfb1b884c8532962a3c0b7b834428e", scope.propagationContext.TraceID.String()) - assert.Contains(t, scope.contexts, "trace") - }, - }, - } - - for name, tt := range tests { - t.Run(name, func(t *testing.T) { - opt, err := tt.hub.ContinueTrace(tt.trace, tt.baggage) - - if tt.expectedErr != nil { - assert.Error(t, err, "expected error, got nil") - assert.Equal(t, tt.expectedErr.Error(), err.Error()) - } else { - assert.NoError(t, err, "expected no error, got one") - } - - // Check for expected SpanOption - if tt.expectedSpan { - assert.NotNil(t, opt, "expected SpanOption, got nil") - } else { - assert.Nil(t, opt, "expected no SpanOption, got one") - } - - // Additional checks on the scope - tt.checkScope(t, tt.hub.Scope()) - }) - } -} - func TestGetTraceparent(t *testing.T) { tests := map[string]struct { hub *Hub