diff --git a/crosstest/http_link_test.go b/crosstest/http_link_test.go index 489a00674..7df9c0354 100644 --- a/crosstest/http_link_test.go +++ b/crosstest/http_link_test.go @@ -26,9 +26,8 @@ import ( "github.com/valyala/fasthttp" ) -func sendSignals(ctx context.Context, identifier string, logger sentry.Logger, meter sentry.Meter) { - hub := sentry.GetHubFromContext(ctx) - hub.CaptureException(errors.New(identifier + " manual error")) +func sendContextSignals(ctx context.Context, identifier string, logger sentry.Logger, meter sentry.Meter) { + sentry.CaptureException(ctx, errors.New(identifier+" manual error")) logger.Info().WithCtx(ctx).Emit(identifier + " linked log") meter.WithCtx(ctx).Count(identifier+".linked.metric", 1) panic(identifier + " panic") @@ -52,15 +51,15 @@ func TestHTTPFamilyIntegrationsLinkManualErrorsLogsMetricsAndPanicsToOTel(t *tes t.Parallel() sentrytest.Run(t, func(t *testing.T, f *sentrytest.Fixture) { const identifier = "http" - baseCtx := sentry.SetHubOnContext(context.Background(), f.Hub) + baseCtx := f.NewContext(context.Background()) logger := sentry.NewLogger(baseCtx) meter := sentry.NewMeter(baseCtx) handler := sentryhttp.New(sentryhttp.Options{WaitForDelivery: true}).HandleFunc(func(_ http.ResponseWriter, r *http.Request) { - sendSignals(r.Context(), identifier, logger, meter) + sendContextSignals(r.Context(), identifier, logger, meter) }) req := httptest.NewRequest(http.MethodGet, "http://example.com/test", nil) - req = req.WithContext(sentry.SetHubOnContext(otelCtx, f.Hub)) + req = req.WithContext(f.NewContext(otelCtx)) handler.ServeHTTP(httptest.NewRecorder(), req) f.Flush() @@ -83,7 +82,7 @@ func TestHTTPFamilyIntegrationsLinkManualErrorsLogsMetricsAndPanicsToOTel(t *tes }) router.Use(sentrygin.New(sentrygin.Options{WaitForDelivery: true})) router.GET("/test", func(c *gin.Context) { - sendSignals(c.Request.Context(), identifier, logger, meter) + sendContextSignals(c.Request.Context(), identifier, logger, meter) }) req := httptest.NewRequest(http.MethodGet, "/test", nil) @@ -111,7 +110,7 @@ func TestHTTPFamilyIntegrationsLinkManualErrorsLogsMetricsAndPanicsToOTel(t *tes }) e.Use(sentryecho.New(sentryecho.Options{WaitForDelivery: true})) e.GET("/test", func(c *echo.Context) error { - sendSignals(c.Request().Context(), identifier, logger, meter) + sendContextSignals(c.Request().Context(), identifier, logger, meter) return nil }) @@ -136,7 +135,7 @@ func TestHTTPFamilyIntegrationsLinkManualErrorsLogsMetricsAndPanicsToOTel(t *tes })) n.Use(sentrynegroni.New(sentrynegroni.Options{WaitForDelivery: true})) n.UseHandler(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - sendSignals(r.Context(), identifier, logger, meter) + sendContextSignals(r.Context(), identifier, logger, meter) })) req := httptest.NewRequest(http.MethodGet, "/test", nil) @@ -161,7 +160,7 @@ func TestHTTPFamilyIntegrationsLinkManualErrorsLogsMetricsAndPanicsToOTel(t *tes }) app.Use(sentryiris.New(sentryiris.Options{WaitForDelivery: true})) app.Get("/test", func(ctx iris.Context) { - sendSignals(ctx.Request().Context(), identifier, logger, meter) + sendContextSignals(ctx.Request().Context(), identifier, logger, meter) }) if err := app.Build(); err != nil { @@ -193,7 +192,7 @@ func TestHTTPFamilyIntegrationsLinkManualErrorsLogsMetricsAndPanicsToOTel(t *tes }) app.Use(sentryfiber.New(sentryfiber.Options{WaitForDelivery: true})) app.Get("/test", func(c *fiber.Ctx) error { - sendSignals(c.UserContext(), identifier, logger, meter) + sendContextSignals(c.UserContext(), identifier, logger, meter) return nil }) @@ -224,7 +223,7 @@ func TestHTTPFamilyIntegrationsLinkManualErrorsLogsMetricsAndPanicsToOTel(t *tes }) app.Use(sentryfiberv3.New(sentryfiberv3.Options{WaitForDelivery: true})) app.Get("/test", func(c fiberv3.Ctx) error { - sendSignals(c.Context(), identifier, logger, meter) + sendContextSignals(c.Context(), identifier, logger, meter) return nil }) diff --git a/http/README.md b/http/README.md index 62b29cfea..6af16e28a 100644 --- a/http/README.md +++ b/http/README.md @@ -69,30 +69,25 @@ Timeout time.Duration ## Usage -`sentryhttp` attaches an instance of `*sentry.Hub` (https://pkg.go.dev/github.com/getsentry/sentry-go#Hub) to the request's context, which makes it available throughout the rest of the request's lifetime. -You can access it by using the `sentry.GetHubFromContext()` method on the request itself in any of your proceeding middleware and routes. -And it should be used instead of the global `sentry.CaptureMessage`, `sentry.CaptureException`, or any other calls, as it keeps the separation of data between the requests. +`sentryhttp` attaches a request-specific `*sentry.Scope` and transaction to the request context. Pass `r.Context()` to capture functions such as `sentry.CaptureMessage` and `sentry.CaptureException` so request data, custom scope data, and trace information are applied to the event. +Use `sentry.ScopeFromContext(r.Context())` when you need to add data that should be available to captures made during the request. -**Keep in mind that `*sentry.Hub` won't be available in middleware attached before to `sentryhttp`!** +**Keep in mind that the request scope won't be available in middleware attached before `sentryhttp`!** ```go type handler struct{} func (h *handler) ServeHTTP(rw http.ResponseWriter, r *http.Request) { - if hub := sentry.GetHubFromContext(r.Context()); hub != nil { - hub.WithScope(func(scope *sentry.Scope) { - scope.SetTag("unwantedQuery", "someQueryDataMaybe") - hub.CaptureMessage("User provided unwanted query string, but we recovered just fine") - }) - } + ctx := r.Context() + scope := sentry.ScopeFromContext(ctx) + scope.SetTag("unwantedQuery", "someQueryDataMaybe") + sentry.CaptureMessage(ctx, "User provided unwanted query string, but we recovered just fine") rw.WriteHeader(http.StatusOK) } func enhanceSentryEvent(handler http.HandlerFunc) http.HandlerFunc { return func(rw http.ResponseWriter, r *http.Request) { - if hub := sentry.GetHubFromContext(r.Context()); hub != nil { - hub.Scope().SetTag("someRandomTag", "maybeYouNeedIt") - } + sentry.ScopeFromContext(r.Context()).SetTag("someRandomTag", "maybeYouNeedIt") handler(rw, r) } } diff --git a/http/example_test.go b/http/example_test.go index bf184b482..0ef4f0a99 100644 --- a/http/example_test.go +++ b/http/example_test.go @@ -16,13 +16,11 @@ func Example() { // sentry.Init(...) http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - // Use GetHubFromContext to get a hub associated with the current - // request. Hubs provide data isolation, such that tags, breadcrumbs - // and other attributes are never mixed up across requests. - hub := sentry.GetHubFromContext(r.Context()) + // Capture with the request context so the request-specific scope and + // trace are applied to the event. _, err := http.Get("example.com") if err != nil { - hub.CaptureException(err) + sentry.CaptureException(r.Context(), err) } }) diff --git a/http/sentryhttp.go b/http/sentryhttp.go index a7c4852d9..fb6170a07 100644 --- a/http/sentryhttp.go +++ b/http/sentryhttp.go @@ -66,7 +66,7 @@ func New(options Options) *Handler { // Handle works as a middleware that wraps an existing http.Handler. A wrapped // handler will recover from and report panics to Sentry, and provide access to -// a request-specific hub to report messages and errors. +// a request-specific scope through the request context. func (h *Handler) Handle(handler http.Handler) http.Handler { return h.handle(handler) } @@ -84,20 +84,12 @@ 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 { - client.SetSDKIdentifier(sdkIdentifier) - } - ctx = sentry.SetHubOnContext(ctx, hub) + created := sentry.SpanFromContext(r.Context()) == nil + ctx, scope := sentry.WithIsolationScope(r.Context()) + sentry.ClientFromContext(ctx).SetSDKIdentifier(sdkIdentifier) options := []sentry.SpanOption{ - sentry.ContinueTrace(r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), + traceutils.ContinueFromRequest(r), sentry.WithOpName("http.server"), sentry.WithTransactionSource(sentry.SourceURL), sentry.WithSpanOrigin(sentry.SpanOriginStdLib), @@ -107,39 +99,38 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc { traceutils.GetHTTPSpanName(r), options..., ) - transaction.SetData("http.request.method", r.Method) - rw := httputils.NewWrapResponseWriter(w, r.ProtoMajor) + if created { + ctx = transaction.Context() + transaction.SetData("http.request.method", r.Method) + defer func() { + // r.Pattern is populated by ServeMux after routing, so we + // read it here in the defer after the handler has run. + if r.Pattern != "" { + transaction.Name = traceutils.GetHTTPSpanName(r) + transaction.Source = sentry.SourceRoute + } + status := rw.Status() + transaction.Status = sentry.HTTPtoSpanStatus(status) + transaction.SetData("http.response.status_code", status) + transaction.Finish() + }() + } - defer func() { - // r.Pattern is populated by ServeMux after routing, so we - // read it here in the defer after the handler has run. - if r.Pattern != "" { - transaction.Name = traceutils.GetHTTPSpanName(r) - transaction.Source = sentry.SourceRoute - } - status := rw.Status() - transaction.Status = sentry.HTTPtoSpanStatus(status) - transaction.SetData("http.response.status_code", status) - transaction.Finish() - }() - - hub.Scope().SetRequest(r) - r = r.WithContext(transaction.Context()) - defer h.recoverWithSentry(hub, r) + r = r.WithContext(ctx) + scope.SetRequest(r) + defer h.recoverWithSentry(r) handler.ServeHTTP(rw, r) } } -func (h *Handler) recoverWithSentry(hub *sentry.Hub, r *http.Request) { +func (h *Handler) recoverWithSentry(r *http.Request) { if err := recover(); err != nil { - eventID := hub.RecoverWithContext( - context.WithValue(r.Context(), sentry.RequestContextKey, r), - err, - ) + ctx := context.WithValue(r.Context(), sentry.RequestContextKey, r) + eventID := sentry.Recover(ctx, err) if eventID != nil && h.waitForDelivery { - hub.Flush(h.timeout) + sentry.ClientFromContext(ctx).Flush(h.timeout) } if h.repanic { panic(err) diff --git a/http/sentryhttp_test.go b/http/sentryhttp_test.go index cedae4904..84a7482ae 100644 --- a/http/sentryhttp_test.go +++ b/http/sentryhttp_test.go @@ -1,6 +1,7 @@ package sentryhttp_test import ( + "context" "fmt" "io" "net/http" @@ -11,11 +12,63 @@ import ( "github.com/getsentry/sentry-go" sentryhttp "github.com/getsentry/sentry-go/http" + "github.com/getsentry/sentry-go/internal/sentrytest" "github.com/getsentry/sentry-go/internal/testutils" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/stretchr/testify/require" ) +type contextKey struct{} + +func TestNestedMiddlewarePreservesActiveTransaction(t *testing.T) { + t.Parallel() + + fixture := sentrytest.NewFixture(t, sentrytest.WithClientOptions(sentry.ClientOptions{ + EnableTracing: true, TracesSampleRate: 1, + })) + outer := sentry.StartTransaction(fixture.NewContext(context.Background()), "outer") + child := sentry.StartSpan(outer.Context(), "child") + key := contextKey{} + request := httptest.NewRequest(http.MethodGet, "http://example.test/nested", nil) + request = request.WithContext(context.WithValue(child.Context(), key, "preserved")) + + handler := sentryhttp.New(sentryhttp.Options{}).Handle(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + require.Equal(t, "preserved", r.Context().Value(key)) + require.Same(t, child, sentry.SpanFromContext(r.Context())) + require.Same(t, outer, sentry.TransactionFromContext(r.Context())) + })) + handler.ServeHTTP(httptest.NewRecorder(), request) + + require.Equal(t, "outer", outer.Name) + require.True(t, outer.EndTime.IsZero(), "nested middleware finished the outer transaction") + child.Finish() + outer.Finish() + fixture.Flush() + require.Len(t, fixture.Events(), 1) + require.Equal(t, "transaction", fixture.Events()[0].Type) +} + +func TestIncomingBaggageUsesAllHeaderLines(t *testing.T) { + t.Parallel() + + fixture := sentrytest.NewFixture(t, sentrytest.WithClientOptions(sentry.ClientOptions{ + EnableTracing: true, TracesSampleRate: 1, + })) + request := httptest.NewRequest(http.MethodGet, "http://example.test/baggage", nil) + request.Header.Set(sentry.SentryTraceHeader, "11111111111111111111111111111111-2222222222222222-1") + request.Header.Add(sentry.SentryBaggageHeader, "sentry-release=from-first-line") + request.Header.Add(sentry.SentryBaggageHeader, "sentry-trace_id=11111111111111111111111111111111") + request = request.WithContext(fixture.NewContext(request.Context())) + + handler := sentryhttp.New(sentryhttp.Options{}).Handle(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + baggage := sentry.TransactionFromContext(r.Context()).ToBaggage() + require.Contains(t, baggage, "sentry-release=from-first-line") + require.Contains(t, baggage, "sentry-trace_id=11111111111111111111111111111111") + })) + handler.ServeHTTP(httptest.NewRecorder(), request) +} + func TestIntegration(t *testing.T) { largePayload := strings.Repeat("Large", 3*1024) // 15 KB @@ -80,12 +133,11 @@ func TestIntegration(t *testing.T) { Body: `{"safe":"value"}`, ContentType: "application/json", Handler: http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - hub := sentry.GetHubFromContext(r.Context()) body, err := io.ReadAll(r.Body) if err != nil { t.Error(err) } - hub.CaptureMessage("post: " + string(body)) + sentry.CaptureMessage(r.Context(), "post: "+string(body)) }), WantStatus: http.StatusOK, @@ -135,8 +187,7 @@ func TestIntegration(t *testing.T) { { Path: "/get", Handler: http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - hub := sentry.GetHubFromContext(r.Context()) - hub.CaptureMessage("get") + sentry.CaptureMessage(r.Context(), "get") }), WantStatus: http.StatusOK, @@ -182,12 +233,11 @@ func TestIntegration(t *testing.T) { Method: "POST", Body: largePayload, Handler: http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - hub := sentry.GetHubFromContext(r.Context()) body, err := io.ReadAll(r.Body) if err != nil { t.Error(err) } - hub.CaptureMessage(fmt.Sprintf("post: %d KB", len(body)/1024)) + sentry.CaptureMessage(r.Context(), fmt.Sprintf("post: %d KB", len(body)/1024)) }), WantStatus: http.StatusOK, @@ -239,8 +289,7 @@ func TestIntegration(t *testing.T) { Method: "POST", Body: "client sends, server ignores, SDK doesn't read", Handler: http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - hub := sentry.GetHubFromContext(r.Context()) - hub.CaptureMessage("body ignored") + sentry.CaptureMessage(r.Context(), "body ignored") }), WantStatus: http.StatusOK, @@ -309,6 +358,12 @@ func TestIntegration(t *testing.T) { sentryHandler := sentryhttp.New(sentryhttp.Options{}) handler := func(w http.ResponseWriter, r *http.Request) { + if sentry.ScopeFromContext(r.Context()) == nil { + t.Error("request context does not carry an isolation scope") + } + if sentry.TransactionFromContext(r.Context()) == nil { + t.Error("request context does not carry a transaction") + } for _, tt := range tests { if r.URL.Path == tt.Path { tt.Handler.ServeHTTP(w, r) diff --git a/internal/traceutils/continuation.go b/internal/traceutils/continuation.go new file mode 100644 index 000000000..bece2443f --- /dev/null +++ b/internal/traceutils/continuation.go @@ -0,0 +1,13 @@ +package traceutils + +import ( + "net/http" + "strings" + + "github.com/getsentry/sentry-go" +) + +// ContinueFromRequest extracts all baggage values for a new root span. +func ContinueFromRequest(r *http.Request) sentry.SpanOption { + return sentry.ContinueTrace(r.Header.Get(sentry.SentryTraceHeader), strings.Join(r.Header.Values(sentry.SentryBaggageHeader), ",")) +} diff --git a/internal/traceutils/continuation_test.go b/internal/traceutils/continuation_test.go new file mode 100644 index 000000000..3673926dd --- /dev/null +++ b/internal/traceutils/continuation_test.go @@ -0,0 +1,59 @@ +package traceutils_test + +import ( + "context" + "net/http/httptest" + "testing" + + "github.com/getsentry/sentry-go" + "github.com/getsentry/sentry-go/internal/traceutils" + "github.com/stretchr/testify/require" +) + +func TestContinueFromRequest(t *testing.T) { + t.Parallel() + traceID, parentID := sentry.TraceID{1}, sentry.SpanID{2} + for _, test := range []struct { + name, suffix string + sampled sentry.Sampled + baggage bool + }{ + {name: "sampled with multiple baggage lines", suffix: "-1", sampled: sentry.SampledTrue, baggage: true}, + {name: "unsampled", suffix: "-0", sampled: sentry.SampledFalse}, + {name: "deferred"}, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + r := httptest.NewRequest("GET", "/", nil) + r.Header.Set(sentry.SentryTraceHeader, traceID.String()+"-"+parentID.String()+test.suffix) + if test.baggage { + r.Header.Add(sentry.SentryBaggageHeader, "othervendor=value") + r.Header.Add(sentry.SentryBaggageHeader, "sentry-trace_id="+traceID.String()) + r.Header.Add(sentry.SentryBaggageHeader, "sentry-public_key=upstream") + } + ctx, _ := sentry.WithIsolationScope(sentry.ContextWithClient(context.Background(), sentry.NewNoopClient())) + root := sentry.StartTransaction(ctx, "request", traceutils.ContinueFromRequest(r)) + require.Equal(t, traceID, root.TraceID) + require.Equal(t, parentID, root.ParentSpanID) + require.Equal(t, test.sampled, root.Sampled) + if test.baggage { + baggage := sentry.GetBaggage(root.Context()) + require.Contains(t, baggage, "sentry-trace_id="+traceID.String()) + require.Contains(t, baggage, "sentry-public_key=upstream") + } + root.Finish() + }) + } + + t.Run("active child ignores incoming parent", func(t *testing.T) { + ctx, _ := sentry.WithIsolationScope(sentry.ContextWithClient(context.Background(), sentry.NewNoopClient())) + root := sentry.StartTransaction(ctx, "root") + r := httptest.NewRequest("GET", "/", nil) + r.Header.Set(sentry.SentryTraceHeader, traceID.String()+"-"+parentID.String()+"-1") + child := sentry.StartSpan(root.Context(), "child", traceutils.ContinueFromRequest(r)) + require.Equal(t, root.TraceID, child.TraceID) + require.Equal(t, root.SpanID, child.ParentSpanID) + child.Finish() + root.Finish() + }) +}