From e91a65d80751e2ea86a8056dac623837ec9e30ee Mon Sep 17 00:00:00 2001 From: Giannis Gkiortzis <58184179+giortzisg@users.noreply.github.com> Date: Thu, 3 Sep 2026 15:24:11 +0200 Subject: [PATCH] ref(http)!: migrate Fiber and FastHTTP to context scopes Carry isolated scopes through native Fiber contexts and an explicit FastHTTP context bridge, including the framework error-handler lifecycle. Preserve active caller traces and let only the transaction creator update or finish them. Keep outer request cleanup registered across nested wrappers, distinguish active FastHTTP nesting from pooled request reuse, and read all incoming baggage values. Fiber v2, Fiber v3, and FastHTTP GetHubFromContext, SetHubOnContext, and GetSpanFromContext helpers are removed. Use the integrations' GetContext helpers and the root sentry context accessors, plus FastHTTP SetContext when supplying a parent context. --- crosstest/http_link_test.go | 36 ++++++--- fasthttp/README.md | 20 ++--- fasthttp/go.mod | 5 ++ fasthttp/go.sum | 10 +++ fasthttp/sentryfasthttp.go | 126 ++++++++++++++++++------------- fasthttp/sentryfasthttp_test.go | 109 ++++++++++++++++++++------- fiber/README.md | 20 ++--- fiber/sentryfiber.go | 125 ++++++++++++++++--------------- fiber/sentryfiber_test.go | 89 ++++++++++++---------- fiberv3/README.md | 19 ++--- fiberv3/sentryfiber.go | 127 ++++++++++++++++---------------- fiberv3/sentryfiber_test.go | 90 +++++++++++----------- internal/sentrytest/request.go | 67 +++++++++++++++++ 13 files changed, 514 insertions(+), 329 deletions(-) create mode 100644 internal/sentrytest/request.go diff --git a/crosstest/http_link_test.go b/crosstest/http_link_test.go index abb859ae5..047a874cc 100644 --- a/crosstest/http_link_test.go +++ b/crosstest/http_link_test.go @@ -180,13 +180,12 @@ func TestHTTPFamilyIntegrationsLinkManualErrorsLogsMetricsAndPanicsToOTel(t *tes t.Parallel() f := sentrytest.NewFixture(t, otelOpts()...) const identifier = "fiber" - baseCtx := sentry.SetHubOnContext(context.Background(), f.Hub) + baseCtx := f.NewContext(context.Background()) logger := sentry.NewLogger(baseCtx) meter := sentry.NewMeter(baseCtx) app := fiber.New() app.Use(func(c *fiber.Ctx) error { - c.SetUserContext(sentry.SetHubOnContext(otelCtx, f.Hub)) - sentryfiber.SetHubOnContext(c, f.Hub) + c.SetUserContext(f.NewContext(otelCtx)) return c.Next() }) app.Use(sentryfiber.New(sentryfiber.Options{WaitForDelivery: true})) @@ -211,13 +210,12 @@ func TestHTTPFamilyIntegrationsLinkManualErrorsLogsMetricsAndPanicsToOTel(t *tes t.Parallel() f := sentrytest.NewFixture(t, otelOpts()...) const identifier = "fiberv3" - baseCtx := sentry.SetHubOnContext(context.Background(), f.Hub) + baseCtx := f.NewContext(context.Background()) logger := sentry.NewLogger(baseCtx) meter := sentry.NewMeter(baseCtx) app := fiberv3.New() app.Use(func(c fiberv3.Ctx) error { - c.SetContext(sentry.SetHubOnContext(otelCtx, f.Hub)) - sentryfiberv3.SetHubOnContext(c, f.Hub) + c.SetContext(f.NewContext(otelCtx)) return c.Next() }) app.Use(sentryfiberv3.New(sentryfiberv3.Options{WaitForDelivery: true})) @@ -237,10 +235,26 @@ func TestHTTPFamilyIntegrationsLinkManualErrorsLogsMetricsAndPanicsToOTel(t *tes f.Flush() requireRequestSignalsLinked(t, f.Events(), traceID, spanID, identifier) }) -} -func TestFastHTTPOTelValidationGap(t *testing.T) { - _ = sentryfasthttp.New - _ = fasthttp.RequestCtx{} - t.Skip("fasthttp does not preserve a standard request context that the OTel integration can resolve automatically today") + t.Run("fasthttp", func(t *testing.T) { + t.Parallel() + sentrytest.Run(t, func(t *testing.T, f *sentrytest.Fixture) { + const identifier = "fasthttp" + baseCtx := f.NewContext(context.Background()) + logger := sentry.NewLogger(baseCtx) + meter := sentry.NewMeter(baseCtx) + handler := sentryfasthttp.New(sentryfasthttp.Options{WaitForDelivery: true}).Handle(func(ctx *fasthttp.RequestCtx) { + sendContextSignals(sentryfasthttp.GetContext(ctx), identifier, logger, meter) + }) + + ctx := &fasthttp.RequestCtx{} + ctx.Request.SetRequestURI("http://example.com/test") + ctx.Request.Header.SetMethod(http.MethodGet) + sentryfasthttp.SetContext(f.NewContext(otelCtx), ctx) + handler(ctx) + + f.Flush() + requireRequestSignalsLinked(t, f.Events(), traceID, spanID, identifier) + }, otelOpts()...) + }) } diff --git a/fasthttp/README.md b/fasthttp/README.md index aac1f0b39..014766e3e 100644 --- a/fasthttp/README.md +++ b/fasthttp/README.md @@ -69,18 +69,15 @@ Timeout time.Duration ## Usage -`sentryfasthttp` 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 `sentryfasthttp.GetHubFromContext()` method on the context 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. +`sentryfasthttp` stores a request-specific standard Go context containing a `*sentry.Scope` and transaction on `fasthttp.RequestCtx`. Pass `sentryfasthttp.GetContext(ctx)` 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(sentryfasthttp.GetContext(ctx))` 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 `sentryfasthttp`!** +**Keep in mind that the request scope won't be available in middleware attached before `sentryfasthttp`!** ```go func enhanceSentryEvent(handler fasthttp.RequestHandler) fasthttp.RequestHandler { return func(ctx *fasthttp.RequestCtx) { - if hub := sentryfasthttp.GetHubFromContext(ctx); hub != nil { - hub.Scope().SetTag("someRandomTag", "maybeYouNeedIt") - } + sentry.ScopeFromContext(sentryfasthttp.GetContext(ctx)).SetTag("someRandomTag", "maybeYouNeedIt") handler(ctx) } } @@ -92,12 +89,9 @@ sentryHandler := sentryfasthttp.New(sentryfasthttp.Options{ }) defaultHandler := func(ctx *fasthttp.RequestCtx) { - if hub := sentryfasthttp.GetHubFromContext(ctx); hub != nil { - hub.WithScope(func(scope *sentry.Scope) { - scope.SetTag("unwantedQuery", "someQueryDataMaybe") - hub.CaptureMessage("User provided unwanted query string, but we recovered just fine") - }) - } + scope := sentry.ScopeFromContext(sentryfasthttp.GetContext(ctx)) + scope.SetTag("unwantedQuery", "someQueryDataMaybe") + sentry.CaptureMessage(sentryfasthttp.GetContext(ctx), "User provided unwanted query string, but we recovered just fine") ctx.SetStatusCode(fasthttp.StatusOK) } diff --git a/fasthttp/go.mod b/fasthttp/go.mod index ed92b801f..3c47d1fb4 100644 --- a/fasthttp/go.mod +++ b/fasthttp/go.mod @@ -7,13 +7,18 @@ replace github.com/getsentry/sentry-go => ../ require ( github.com/getsentry/sentry-go v0.49.0 github.com/google/go-cmp v0.7.0 + github.com/stretchr/testify v1.11.1 github.com/valyala/fasthttp v1.71.0 ) require ( github.com/andybalholm/brotli v1.2.1 // indirect + github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/klauspost/compress v1.18.6 // indirect + github.com/kr/text v0.2.0 // indirect + github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect golang.org/x/sys v0.47.0 // indirect golang.org/x/text v0.41.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/fasthttp/go.sum b/fasthttp/go.sum index 7156d22a0..d67fd3d88 100644 --- a/fasthttp/go.sum +++ b/fasthttp/go.sum @@ -1,5 +1,6 @@ github.com/andybalholm/brotli v1.2.1 h1:R+f5xP285VArJDRgowrfb9DqL18yVK0gKAW/F+eTWro= github.com/andybalholm/brotli v1.2.1/go.mod h1:rzTDkvFWvIrjDXZHkuS16NPggd91W3kUSvPlQ1pLaKY= +github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA= @@ -8,12 +9,18 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/klauspost/compress v1.18.6 h1:2jupLlAwFm95+YDR+NwD2MEfFO9d4z4Prjl1XXDjuao= github.com/klauspost/compress v1.18.6/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/pingcap/errors v0.11.4 h1:lFuQV/oaUMGcD2tqt+01ROSmJs75VG1ToEOkZIZ4nE4= github.com/pingcap/errors v0.11.4/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= +github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= @@ -28,5 +35,8 @@ golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/text v0.41.0 h1:vz/seA0lnX87Othu2f/0L24RcgrXD9/YFTSuGjj3rH8= golang.org/x/text v0.41.0/go.mod h1:jvf1O8ajNzZqhSrQBPbutR/EB83Cc0CFrezNQIwbb5M= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/fasthttp/sentryfasthttp.go b/fasthttp/sentryfasthttp.go index 63a1a5c12..557f1b35c 100644 --- a/fasthttp/sentryfasthttp.go +++ b/fasthttp/sentryfasthttp.go @@ -10,19 +10,26 @@ import ( "github.com/getsentry/sentry-go" "github.com/getsentry/sentry-go/internal/debuglog" + "github.com/getsentry/sentry-go/internal/traceutils" "github.com/valyala/fasthttp" ) -const ( - // sdkIdentifier is the identifier of the FastHTTP SDK. - sdkIdentifier = "sentry.go.fasthttp" +// sdkIdentifier is the identifier of the FastHTTP SDK. +const sdkIdentifier = "sentry.go.fasthttp" - // valuesKey is used as a key to store the Sentry Hub instance on the fasthttp.RequestCtx. - valuesKey = "sentry" +type contextKey struct{} - // transactionKey is used as a key to store the Sentry transaction on the fasthttp.RequestCtx. - transactionKey = "sentry_transaction" -) +type storedContext struct { + ctx context.Context + cancel context.CancelFunc + active bool +} + +// Close lets fasthttp release the request context when it resets user values. +func (ctx *storedContext) Close() error { + ctx.cancel() + return nil +} type Handler struct { repanic bool @@ -59,85 +66,102 @@ 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 := GetHubFromContext(ctx) - if hub == nil { - hub = sentry.CurrentHub().Clone() + parentCtx := GetContext(ctx) + created := sentry.SpanFromContext(parentCtx) == nil + previous, _ := ctx.UserValue(contextKey{}).(*storedContext) + if previous != nil && !previous.active { + previous.cancel() + parentCtx = context.Background() + created = true } + requestCtx, cancel := context.WithCancel(parentCtx) + storedCtx := &storedContext{ctx: requestCtx, cancel: cancel, active: true} + ctx.SetUserValue(contextKey{}, storedCtx) + defer func() { + storedCtx.active = false + if previous != nil && previous.active { + ctx.SetUserValue(contextKey{}, previous) + } + }() + defer func() { + if ctx.LastTimeoutErrorResponse() != nil { + cancel() + } + }() + requestCtx, scope := sentry.WithIsolationScope(requestCtx) - if client := hub.Client(); client != nil { - client.SetSDKIdentifier(sdkIdentifier) - } + sentry.ClientFromContext(requestCtx).SetSDKIdentifier(sdkIdentifier) r := convert(ctx) 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.SpanOriginFastHTTP), } transaction := sentry.StartTransaction( - sentry.SetHubOnContext(ctx, hub), + requestCtx, fmt.Sprintf("%s %s", r.Method, string(ctx.Path())), options..., ) - defer func() { - status := ctx.Response.StatusCode() - transaction.Status = sentry.HTTPtoSpanStatus(status) - transaction.SetData("http.response.status_code", status) - transaction.Finish() - }() - - transaction.SetData("http.request.method", r.Method) + if created { + requestCtx = transaction.Context() + transaction.SetData("http.request.method", r.Method) + defer func() { + status := ctx.Response.StatusCode() + transaction.Status = sentry.HTTPtoSpanStatus(status) + transaction.SetData("http.response.status_code", status) + transaction.Finish() + }() + } + storedCtx.ctx = requestCtx + r = r.WithContext(requestCtx) - scope := hub.Scope() scope.SetRequest(r) scope.SetRequestBody(bytes.Clone(ctx.Request.Body())) - ctx.SetUserValue(valuesKey, hub) - ctx.SetUserValue(transactionKey, transaction) - defer h.recoverWithSentry(hub, ctx) + defer h.recoverWithSentry(requestCtx, ctx, cancel) handler(ctx) } } -func (h *Handler) recoverWithSentry(hub *sentry.Hub, ctx *fasthttp.RequestCtx) { +func (h *Handler) recoverWithSentry(requestCtx context.Context, ctx *fasthttp.RequestCtx, cancel context.CancelFunc) { if err := recover(); err != nil { - eventID := hub.RecoverWithContext( - context.WithValue(context.Background(), sentry.RequestContextKey, ctx), - err, - ) + requestCtx = context.WithValue(requestCtx, sentry.RequestContextKey, ctx) + eventID := sentry.Recover(requestCtx, err) if eventID != nil && h.waitForDelivery { - hub.Flush(h.timeout) + sentry.ClientFromContext(requestCtx).Flush(h.timeout) } if h.repanic { + cancel() panic(err) } } } -// GetHubFromContext retrieves attached *sentry.Hub instance from fasthttp.RequestCtx. -func GetHubFromContext(ctx *fasthttp.RequestCtx) *sentry.Hub { - hub := ctx.UserValue(valuesKey) - if hub, ok := hub.(*sentry.Hub); ok { - return hub +// GetContext retrieves the request context from fasthttp.RequestCtx. +func GetContext(ctx *fasthttp.RequestCtx) context.Context { + if storedCtx, ok := ctx.UserValue(contextKey{}).(*storedContext); ok { + return storedCtx.ctx } - return nil -} - -// SetHubOnContext attaches the *sentry.Hub instance to the fasthttp.RequestCtx. -func SetHubOnContext(ctx *fasthttp.RequestCtx, hub *sentry.Hub) { - ctx.SetUserValue(valuesKey, hub) + if requestCtx, ok := ctx.UserValue(contextKey{}).(context.Context); ok { + return requestCtx + } + return context.Background() } -// GetSpanFromContext retrieves attached *sentry.Span instance from *fasthttp.RequestCtx. -// If there is no transaction on *fasthttp.RequestCtx, it will return nil. -func GetSpanFromContext(ctx *fasthttp.RequestCtx) *sentry.Span { - if span, ok := ctx.UserValue(transactionKey).(*sentry.Span); ok { - return span +// SetContext attaches a request context to fasthttp.RequestCtx. +func SetContext(requestCtx context.Context, ctx *fasthttp.RequestCtx) { + if storedCtx, ok := ctx.UserValue(contextKey{}).(*storedContext); ok { + if storedCtx.active { + storedCtx.ctx = requestCtx + return + } + storedCtx.cancel() } - return nil + ctx.SetUserValue(contextKey{}, requestCtx) } func convert(ctx *fasthttp.RequestCtx) *http.Request { diff --git a/fasthttp/sentryfasthttp_test.go b/fasthttp/sentryfasthttp_test.go index caceb1ac2..0f91e39a2 100644 --- a/fasthttp/sentryfasthttp_test.go +++ b/fasthttp/sentryfasthttp_test.go @@ -1,6 +1,7 @@ package sentryfasthttp_test import ( + "context" "errors" "fmt" "net" @@ -12,9 +13,11 @@ import ( "github.com/getsentry/sentry-go" sentryfasthttp "github.com/getsentry/sentry-go/fasthttp" + "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" "github.com/valyala/fasthttp" "github.com/valyala/fasthttp/fasthttputil" ) @@ -36,7 +39,8 @@ func TestIntegration(t *testing.T) { }{ { Path: "/panic", - Handler: func(*fasthttp.RequestCtx) { + Handler: func(ctx *fasthttp.RequestCtx) { + sentryfasthttp.SetContext(context.Background(), ctx) panic("test") }, WantStatus: 200, @@ -84,8 +88,7 @@ func TestIntegration(t *testing.T) { Body: `{"safe":"value"}`, ContentType: "application/json", Handler: func(ctx *fasthttp.RequestCtx) { - hub := sentryfasthttp.GetHubFromContext(ctx) - hub.CaptureMessage("post: " + string(ctx.Request.Body())) + sentry.CaptureMessage(sentryfasthttp.GetContext(ctx), "post: "+string(ctx.Request.Body())) }, WantEvent: &sentry.Event{ Level: sentry.LevelInfo, @@ -131,8 +134,7 @@ func TestIntegration(t *testing.T) { { Path: "/get", Handler: func(ctx *fasthttp.RequestCtx) { - hub := sentryfasthttp.GetHubFromContext(ctx) - hub.CaptureMessage(http.MethodGet) + sentry.CaptureMessage(sentryfasthttp.GetContext(ctx), http.MethodGet) }, WantStatus: 200, WantEvent: &sentry.Event{ @@ -178,8 +180,7 @@ func TestIntegration(t *testing.T) { Body: largePayload, WantStatus: 200, Handler: func(ctx *fasthttp.RequestCtx) { - hub := sentryfasthttp.GetHubFromContext(ctx) - hub.CaptureMessage(fmt.Sprintf("post: %d KB", len(ctx.Request.Body())/1024)) + sentry.CaptureMessage(sentryfasthttp.GetContext(ctx), fmt.Sprintf("post: %d KB", len(ctx.Request.Body())/1024)) }, WantEvent: &sentry.Event{ Level: sentry.LevelInfo, @@ -228,8 +229,7 @@ func TestIntegration(t *testing.T) { ContentType: "application/json", WantStatus: 200, Handler: func(ctx *fasthttp.RequestCtx) { - hub := sentryfasthttp.GetHubFromContext(ctx) - hub.CaptureMessage("body ignored") + sentry.CaptureMessage(sentryfasthttp.GetContext(ctx), "body ignored") }, WantEvent: &sentry.Event{ Level: sentry.LevelInfo, @@ -278,8 +278,7 @@ func TestIntegration(t *testing.T) { Path: "/post/error-handler", Method: "POST", Handler: func(ctx *fasthttp.RequestCtx) { - hub := sentryfasthttp.GetHubFromContext(ctx) - hub.CaptureException(exception) + sentry.CaptureException(sentryfasthttp.GetContext(ctx), exception) }, WantStatus: 200, WantEvent: &sentry.Event{ @@ -517,7 +516,11 @@ func TestGetTransactionFromContext(t *testing.T) { defer ln.Close() handler := func(ctx *fasthttp.RequestCtx) { - span := sentryfasthttp.GetSpanFromContext(ctx) + scope := sentry.ScopeFromContext(sentryfasthttp.GetContext(ctx)) + if tc.useSentry && scope == nil { + t.Error("expecting scope not to be nil") + } + span := sentry.SpanFromContext(sentryfasthttp.GetContext(ctx)) if tc.useSentry && span == nil { t.Error("expecting span not to be nil") } @@ -573,20 +576,76 @@ func TestGetTransactionFromContext(t *testing.T) { } } -func TestSetHubOnContext(t *testing.T) { - hub := sentry.NewHub(sentry.CurrentHub().Client(), sentry.NewScope()) - ctx := &fasthttp.RequestCtx{} - - sentryfasthttp.SetHubOnContext(ctx, hub) - - retrievedHub := sentryfasthttp.GetHubFromContext(ctx) - if retrievedHub == nil { - t.Fatal("expected hub to be set on context, but got nil") - } - - if !reflect.DeepEqual(hub, retrievedHub) { - t.Fatalf("expected hub to be %v, but got %v", hub, retrievedHub) +func TestRequestIsolation(t *testing.T) { + t.Parallel() + fixture := sentrytest.NewFixture(t) + prepare := func(ctx *fasthttp.RequestCtx) { + ctx.Request.SetRequestURI("http://example.com/test") + ctx.Request.Header.SetMethod(http.MethodGet) } + handler := sentryfasthttp.New(sentryfasthttp.Options{}).Handle(func(*fasthttp.RequestCtx) {}) + + reused := &fasthttp.RequestCtx{} + prepare(reused) + sentryfasthttp.SetContext(fixture.NewContext(context.Background()), reused) + handler(reused) + firstCtx := sentryfasthttp.GetContext(reused) + + handler(reused) + secondCtx := sentryfasthttp.GetContext(reused) + require.NotEqual(t, sentry.SpanFromContext(firstCtx).TraceID, sentry.SpanFromContext(secondCtx).TraceID) + require.ErrorIs(t, firstCtx.Err(), context.Canceled) + require.NoError(t, secondCtx.Err()) + + ctx, scope := sentry.WithIsolationScope(context.Background()) + scope.SetTag("parent", "injected") + parent := sentry.StartTransaction(sentry.ContextWithClient(ctx, sentry.NewNoopClient()), "injected") + defer parent.Finish() + sentryfasthttp.SetContext(parent.Context(), reused) + handler(reused) + thirdCtx := sentryfasthttp.GetContext(reused) + require.ErrorIs(t, secondCtx.Err(), context.Canceled) + require.Same(t, parent, sentry.TransactionFromContext(thirdCtx)) + require.Same(t, sentry.NewNoopClient(), sentry.ClientFromContext(thirdCtx)) + require.NoError(t, thirdCtx.Err()) + fixture.Client.CaptureMessage(thirdCtx, "injected parent") + fixture.Flush() + require.Equal(t, "injected", fixture.Events()[0].Tags["parent"]) + reused.ResetUserValues() + require.ErrorIs(t, thirdCtx.Err(), context.Canceled) + + type nestedKey struct{} + nestedRequest := &fasthttp.RequestCtx{} + prepare(nestedRequest) + sentryfasthttp.SetContext(context.WithValue(fixture.NewContext(context.Background()), nestedKey{}, "preserved"), nestedRequest) + var outerCtx, innerCtx context.Context + inner := sentryfasthttp.New(sentryfasthttp.Options{}).Handle(func(ctx *fasthttp.RequestCtx) { + innerCtx = sentryfasthttp.GetContext(ctx) + }) + outer := sentryfasthttp.New(sentryfasthttp.Options{}).Handle(func(ctx *fasthttp.RequestCtx) { + outerCtx = sentryfasthttp.GetContext(ctx) + sentryfasthttp.SetContext(outerCtx, ctx) + inner(ctx) + }) + outer(nestedRequest) + require.Equal(t, "preserved", outerCtx.Value(nestedKey{})) + require.Equal(t, "preserved", innerCtx.Value(nestedKey{})) + require.Equal(t, sentry.SpanFromContext(outerCtx).TraceID, sentry.SpanFromContext(innerCtx).TraceID) + require.NoError(t, outerCtx.Err()) + require.NoError(t, innerCtx.Err()) + nestedRequest.ResetUserValues() + require.ErrorIs(t, outerCtx.Err(), context.Canceled) + require.ErrorIs(t, innerCtx.Err(), context.Canceled) + + sentrytest.CheckRequestIsolation(t, func() (context.Context, context.Context, error) { + ctx := &fasthttp.RequestCtx{} + prepare(ctx) + sentryfasthttp.SetContext(fixture.NewContext(context.Background()), ctx) + handler(ctx) + requestCtx := sentryfasthttp.GetContext(ctx) + ctx.ResetUserValues() + return requestCtx, nil, nil + }) } // TestMalformedURLNoPanic verifies that malformed URLs don't cause panics diff --git a/fiber/README.md b/fiber/README.md index c94456938..9fd6105ef 100644 --- a/fiber/README.md +++ b/fiber/README.md @@ -68,11 +68,10 @@ Timeout time.Duration ## Usage -`sentryfiber` attaches an instance of `*sentry.Hub` (https://godoc.org/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 `sentryfiber.GetHubFromContext()` method on the context 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. +`sentryfiber` attaches a request-specific `*sentry.Scope` and transaction to Fiber's user context. Pass `ctx.UserContext()` to capture functions such as `sentry.CaptureMessage` and `sentry.CaptureException` so request data, custom scope data, and trace information are applied to the event. In outer middleware or a custom error handler, use `sentryfiber.GetContext(ctx)` after the Sentry middleware has returned. +Use `sentry.ScopeFromContext(ctx.UserContext())` 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 `sentryfiber`!** +**Keep in mind that the request scope won't be available in middleware attached before `sentryfiber`!** ```go // Later in the code @@ -82,9 +81,7 @@ sentryHandler := sentryfiber.New(sentryfiber.Options{ }) enhanceSentryEvent := func(ctx *fiber.Ctx) { - if hub := sentryfiber.GetHubFromContext(ctx); hub != nil { - hub.Scope().SetTag("someRandomTag", "maybeYouNeedIt") - } + sentry.ScopeFromContext(ctx.UserContext()).SetTag("someRandomTag", "maybeYouNeedIt") ctx.Next() } @@ -97,12 +94,9 @@ app.All("/foo", enhanceSentryEvent, func(ctx *fiber.Ctx) { }) app.All("/", func(ctx *fiber.Ctx) { - if hub := sentryfiber.GetHubFromContext(ctx); hub != nil { - hub.WithScope(func(scope *sentry.Scope) { - scope.SetTag("unwantedQuery", "someQueryDataMaybe") - hub.CaptureMessage("User provided unwanted query string, but we recovered just fine") - }) - } + scope := sentry.ScopeFromContext(ctx.UserContext()) + scope.SetTag("unwantedQuery", "someQueryDataMaybe") + sentry.CaptureMessage(ctx.UserContext(), "User provided unwanted query string, but we recovered just fine") ctx.Status(fiber.StatusOK) }) diff --git a/fiber/sentryfiber.go b/fiber/sentryfiber.go index 379fea527..a3086ffb8 100644 --- a/fiber/sentryfiber.go +++ b/fiber/sentryfiber.go @@ -13,18 +13,26 @@ import ( "github.com/getsentry/sentry-go" "github.com/getsentry/sentry-go/internal/debuglog" + "github.com/getsentry/sentry-go/internal/traceutils" ) const ( // sdkIdentifier is the identifier of the FastHTTP SDK. sdkIdentifier = "sentry.go.fiber" +) - // valuesKey is used as a key to store the Sentry Hub instance on the fasthttp.RequestCtx. - valuesKey = "sentry" +type contextKey struct{} - // transactionKey is used as a key to store the Sentry transaction on the fasthttp.RequestCtx. - transactionKey = "sentry_transaction" -) +type storedContext struct { + ctx context.Context + cancel context.CancelFunc +} + +// Close lets Fiber release the request context after its error handler has run. +func (ctx *storedContext) Close() error { + ctx.cancel() + return nil +} type handler struct { repanic bool @@ -58,95 +66,90 @@ func New(options Options) fiber.Handler { } func (h *handler) handle(ctx *fiber.Ctx) error { - hub := GetHubFromContext(ctx) - if hub == nil { - hub = sentry.CurrentHub().Clone() - } + savedCtx := ctx.UserContext() + previous, _ := ctx.Locals(contextKey{}).(*storedContext) + created := sentry.SpanFromContext(savedCtx) == nil + requestCtx, cancel := context.WithCancel(savedCtx) + storedCtx := &storedContext{ctx: requestCtx, cancel: cancel} + ctx.Locals(contextKey{}, storedCtx) + // Keep the outer closer registered; its cancellation reaches nested contexts. + defer func() { + if previous != nil { + ctx.Locals(contextKey{}, previous) + } + }() + defer func() { + if ctx.Context().LastTimeoutErrorResponse() != nil { + cancel() + } + }() + defer func() { ctx.SetUserContext(savedCtx) }() + requestCtx, scope := sentry.WithIsolationScope(requestCtx) - if client := hub.Client(); client != nil { - client.SetSDKIdentifier(sdkIdentifier) - } + sentry.ClientFromContext(requestCtx).SetSDKIdentifier(sdkIdentifier) r := convert(ctx) transactionName := ctx.Path() transactionSource := sentry.SourceURL options := []sentry.SpanOption{ - sentry.ContinueTrace(r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), + traceutils.ContinueFromRequest(r), sentry.WithOpName("http.server"), sentry.WithTransactionSource(transactionSource), sentry.WithSpanOrigin(sentry.SpanOriginFiber), } - savedCtx := ctx.UserContext() - requestCtx, cancel := context.WithCancel(savedCtx) - defer cancel() - defer func() { ctx.SetUserContext(savedCtx) }() - transaction := sentry.StartTransaction( - sentry.SetHubOnContext(requestCtx, hub), + requestCtx, fmt.Sprintf("%s %s", r.Method, transactionName), options..., ) - ctx.SetUserContext(transaction.Context()) - - defer func() { - // Fiber v2 does not expose whether ctx.Route() originates from a middleware. We keep the - // URL-based name (opposite to v3) because middlewares that short-circuits the handler chain can - // otherwise replace the ctx.Route(). See https://github.com/getsentry/sentry-go/issues/1361. - status := ctx.Response().StatusCode() - transaction.Status = sentry.HTTPtoSpanStatus(status) - transaction.SetData("http.response.status_code", status) - transaction.Finish() - }() - - transaction.SetData("http.request.method", r.Method) - r = r.WithContext(transaction.Context()) + if created { + requestCtx = transaction.Context() + transaction.SetData("http.request.method", r.Method) + defer func() { + // Fiber v2 does not expose whether ctx.Route() originates from a middleware. We keep the + // URL-based name (opposite to v3) because middlewares that short-circuits the handler chain can + // otherwise replace the ctx.Route(). See https://github.com/getsentry/sentry-go/issues/1361. + status := ctx.Response().StatusCode() + transaction.Status = sentry.HTTPtoSpanStatus(status) + transaction.SetData("http.response.status_code", status) + transaction.Finish() + }() + } + storedCtx.ctx = requestCtx + ctx.SetUserContext(requestCtx) + r = r.WithContext(requestCtx) - scope := hub.Scope() scope.SetRequest(r) scope.SetRequestBody(bytes.Clone(ctx.Request().Body())) - ctx.Locals(valuesKey, hub) - ctx.Locals(transactionKey, transaction) - defer h.recoverWithSentry(hub, ctx) + defer h.recoverWithSentry(requestCtx, ctx, cancel) return ctx.Next() } -func (h *handler) recoverWithSentry(hub *sentry.Hub, ctx *fiber.Ctx) { +func (h *handler) recoverWithSentry(requestCtx context.Context, ctx *fiber.Ctx, cancel context.CancelFunc) { if err := recover(); err != nil { - eventID := hub.RecoverWithContext( - context.WithValue(ctx.UserContext(), sentry.RequestContextKey, ctx), - err, - ) + requestCtx = context.WithValue(requestCtx, sentry.RequestContextKey, ctx) + eventID := sentry.Recover(requestCtx, err) if eventID != nil && h.waitForDelivery { - hub.Flush(h.timeout) + sentry.ClientFromContext(requestCtx).Flush(h.timeout) } if h.repanic { + cancel() panic(err) } } } -// GetHubFromContext retrieves the Hub instance from the *fiber.Ctx. -func GetHubFromContext(ctx *fiber.Ctx) *sentry.Hub { - if hub, ok := ctx.Locals(valuesKey).(*sentry.Hub); ok { - return hub - } - return nil -} - -// SetHubOnContext sets the Hub instance on the *fiber.Ctx. -func SetHubOnContext(ctx *fiber.Ctx, hub *sentry.Hub) { - ctx.Locals(valuesKey, hub) -} - -// GetSpanFromContext retrieves the Span instance from the *fiber.Ctx. -func GetSpanFromContext(ctx *fiber.Ctx) *sentry.Span { - if span, ok := ctx.Locals(transactionKey).(*sentry.Span); ok { - return span +// GetContext returns the request's Sentry context. Unlike Ctx.UserContext, it +// remains available to outer middleware and custom error handlers after the +// Sentry middleware restores Fiber's original context. +func GetContext(ctx *fiber.Ctx) context.Context { + if storedCtx, ok := ctx.Locals(contextKey{}).(*storedContext); ok { + return storedCtx.ctx } - return nil + return ctx.UserContext() } func convert(ctx *fiber.Ctx) *http.Request { diff --git a/fiber/sentryfiber_test.go b/fiber/sentryfiber_test.go index b1ab124df..41d3f8813 100644 --- a/fiber/sentryfiber_test.go +++ b/fiber/sentryfiber_test.go @@ -1,6 +1,7 @@ package sentryfiber_test import ( + "context" "errors" "fmt" "net/http" @@ -15,6 +16,7 @@ import ( "github.com/getsentry/sentry-go" sentryfiber "github.com/getsentry/sentry-go/fiber" + "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" @@ -63,43 +65,42 @@ func TestIntegration(t *testing.T) { app := fiber.New(fiber.Config{ ErrorHandler: func(c *fiber.Ctx, e error) error { - hub := sentryfiber.GetHubFromContext(c) - hub.CaptureException(e) + requestCtx := sentryfiber.GetContext(c) + if err := requestCtx.Err(); err != nil { + t.Errorf("request context in error handler: %v", err) + } + sentry.CaptureException(requestCtx, e) return nil }, }) app.Use(sentryHandler) - app.Get("/panic", func(_ *fiber.Ctx) error { + app.Get("/panic", func(c *fiber.Ctx) error { + c.SetUserContext(context.Background()) panic("test") }) app.Post("/post", func(c *fiber.Ctx) error { - hub := sentryfiber.GetHubFromContext(c) - hub.CaptureMessage("post: " + string(c.Body())) + sentry.CaptureMessage(c.UserContext(), "post: "+string(c.Body())) return nil }) app.Get("/get", func(c *fiber.Ctx) error { - hub := sentryfiber.GetHubFromContext(c) - hub.CaptureMessage("get") + sentry.CaptureMessage(c.UserContext(), "get") return nil }) app.Get("/get/:id", func(c *fiber.Ctx) error { - hub := sentryfiber.GetHubFromContext(c) - hub.CaptureMessage(fmt.Sprintf("get: %s", c.Params("id"))) + sentry.CaptureMessage(c.UserContext(), fmt.Sprintf("get: %s", c.Params("id"))) return nil }) app.Post("/post/large", func(c *fiber.Ctx) error { - hub := sentryfiber.GetHubFromContext(c) - hub.CaptureMessage(fmt.Sprintf("post: %d KB", len(c.Body())/1024)) + sentry.CaptureMessage(c.UserContext(), fmt.Sprintf("post: %d KB", len(c.Body())/1024)) return nil }) app.Post("/post/body-ignored", func(c *fiber.Ctx) error { - hub := sentryfiber.GetHubFromContext(c) - hub.CaptureMessage("body ignored") + sentry.CaptureMessage(c.UserContext(), "body ignored") return nil }) app.Post("/post/error-handler", func(_ *fiber.Ctx) error { @@ -577,7 +578,11 @@ func TestHandlers(t *testing.T) { } handler := func(ctx *fiber.Ctx) error { - span := sentryfiber.GetSpanFromContext(ctx) + scope := sentry.ScopeFromContext(ctx.UserContext()) + if tc.useSentry && scope == nil { + t.Error("expecting scope not to be nil") + } + span := sentry.SpanFromContext(ctx.UserContext()) if tc.useSentry && span == nil { t.Error("expecting span not to be nil") } @@ -607,37 +612,41 @@ func TestHandlers(t *testing.T) { } } -func TestSetHubOnContext(t *testing.T) { +func TestRequestIsolation(t *testing.T) { + t.Parallel() + fixture := sentrytest.NewFixture(t) + contexts := make(chan context.Context, 1) + outerContexts := make(chan context.Context, 1) app := fiber.New() - hub := sentry.NewHub(sentry.CurrentHub().Client(), sentry.NewScope()) + app.Use(func(ctx *fiber.Ctx) error { + original := fixture.NewContext(ctx.UserContext()) + ctx.SetUserContext(original) + err := ctx.Next() + if ctx.UserContext() != original { + t.Error("Sentry middleware did not restore the caller context") + } + return err + }) + app.Use(sentryfiber.New(sentryfiber.Options{})) + app.Use(func(ctx *fiber.Ctx) error { + outerContexts <- ctx.UserContext() + return ctx.Next() + }) + app.Use(sentryfiber.New(sentryfiber.Options{})) + app.Get("/test", func(ctx *fiber.Ctx) error { contexts <- ctx.UserContext(); return nil }) - app.Get("/test", func(c *fiber.Ctx) error { - sentryfiber.SetHubOnContext(c, hub) - retrievedHub := sentryfiber.GetHubFromContext(c) - if retrievedHub == nil { - t.Fatal("expected hub to be set on context, but got nil") + sentrytest.CheckRequestIsolation(t, func() (context.Context, context.Context, error) { + req, err := http.NewRequest(http.MethodGet, "http://example.com/test", nil) + if err != nil { + return nil, nil, err } - if !reflect.DeepEqual(hub, retrievedHub) { - t.Fatalf("expected hub to be %v, but got %v", hub, retrievedHub) + resp, err := app.Test(req) + if err != nil { + return nil, nil, err } - return nil + err = resp.Body.Close() + return <-contexts, <-outerContexts, err }) - - req, err := http.NewRequest(http.MethodGet, "http://example.com/test", nil) - if err != nil { - t.Fatal(err) - } - req.Header.Set("User-Agent", "fiber") - - resp, err := app.Test(req) - if err != nil { - t.Fatalf("Request failed: %s", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - t.Fatalf("Expected status code %d, got %d", http.StatusOK, resp.StatusCode) - } } // TestMalformedURLNoPanic verifies that malformed URLs don't cause panics diff --git a/fiberv3/README.md b/fiberv3/README.md index 0b7911e53..32456c691 100644 --- a/fiberv3/README.md +++ b/fiberv3/README.md @@ -58,10 +58,10 @@ Timeout time.Duration ## Usage -`sentryfiber` attaches an instance of `*sentry.Hub` to the request context, which makes it available throughout the rest of the request's lifetime. -You can access it by using `sentryfiber.GetHubFromContext()` in any subsequent middleware and routes. +`sentryfiber` attaches a request-specific `*sentry.Scope` and transaction to Fiber's standard Go context. Pass `ctx.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. In outer middleware or a custom error handler, use `sentryfiber.GetContext(ctx)` after the Sentry middleware has returned. +Use `sentry.ScopeFromContext(ctx.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 `sentryfiber`.** +**Keep in mind that the request scope won't be available in middleware attached before `sentryfiber`.** ```go sentryHandler := sentryfiber.New(sentryfiber.Options{ @@ -70,9 +70,7 @@ sentryHandler := sentryfiber.New(sentryfiber.Options{ }) enhanceSentryEvent := func(ctx fiber.Ctx) error { - if hub := sentryfiber.GetHubFromContext(ctx); hub != nil { - hub.Scope().SetTag("someRandomTag", "maybeYouNeedIt") - } + sentry.ScopeFromContext(ctx.Context()).SetTag("someRandomTag", "maybeYouNeedIt") return ctx.Next() } @@ -84,12 +82,9 @@ app.All("/foo", enhanceSentryEvent, func(ctx fiber.Ctx) error { }) app.All("/", func(ctx fiber.Ctx) error { - if hub := sentryfiber.GetHubFromContext(ctx); hub != nil { - hub.WithScope(func(scope *sentry.Scope) { - scope.SetTag("unwantedQuery", "someQueryDataMaybe") - hub.CaptureMessage("User provided unwanted query string, but we recovered just fine") - }) - } + scope := sentry.ScopeFromContext(ctx.Context()) + scope.SetTag("unwantedQuery", "someQueryDataMaybe") + sentry.CaptureMessage(ctx.Context(), "User provided unwanted query string, but we recovered just fine") return ctx.SendStatus(fiber.StatusOK) }) diff --git a/fiberv3/sentryfiber.go b/fiberv3/sentryfiber.go index 4359583ea..99b33258a 100644 --- a/fiberv3/sentryfiber.go +++ b/fiberv3/sentryfiber.go @@ -13,18 +13,26 @@ import ( "github.com/getsentry/sentry-go" "github.com/getsentry/sentry-go/internal/debuglog" + "github.com/getsentry/sentry-go/internal/traceutils" ) const ( // sdkIdentifier is the identifier of the Fiber SDK. sdkIdentifier = "sentry.go.fiberv3" +) - // valuesKey is used as a key to store the Sentry Hub instance on the fiber.Ctx. - valuesKey = "sentry" +type contextKey struct{} - // transactionKey is used as a key to store the Sentry transaction on the fiber.Ctx. - transactionKey = "sentry_transaction" -) +type storedContext struct { + ctx context.Context + cancel context.CancelFunc +} + +// Close lets Fiber release the request context after its error handler has run. +func (ctx *storedContext) Close() error { + ctx.cancel() + return nil +} type handler struct { repanic bool @@ -58,96 +66,91 @@ func New(options Options) fiber.Handler { } func (h *handler) handle(ctx fiber.Ctx) error { - hub := GetHubFromContext(ctx) - if hub == nil { - hub = sentry.CurrentHub().Clone() - } + savedCtx := ctx.Context() + previous, _ := ctx.Locals(contextKey{}).(*storedContext) + created := sentry.SpanFromContext(savedCtx) == nil + requestCtx, cancel := context.WithCancel(savedCtx) + storedCtx := &storedContext{ctx: requestCtx, cancel: cancel} + ctx.Locals(contextKey{}, storedCtx) + // Keep the outer closer registered; its cancellation reaches nested contexts. + defer func() { + if previous != nil { + ctx.Locals(contextKey{}, previous) + } + }() + defer func() { + if ctx.IsAbandoned() { + cancel() + } + }() + defer ctx.SetContext(savedCtx) + requestCtx, scope := sentry.WithIsolationScope(requestCtx) - if client := hub.Client(); client != nil { - client.SetSDKIdentifier(sdkIdentifier) - } + sentry.ClientFromContext(requestCtx).SetSDKIdentifier(sdkIdentifier) r := convert(ctx) transactionName := ctx.Path() transactionSource := sentry.SourceURL options := []sentry.SpanOption{ - sentry.ContinueTrace(r.Header.Get(sentry.SentryTraceHeader), r.Header.Get(sentry.SentryBaggageHeader)), + traceutils.ContinueFromRequest(r), sentry.WithOpName("http.server"), sentry.WithTransactionSource(transactionSource), sentry.WithSpanOrigin(sentry.SpanOriginFiber), } - savedCtx := ctx.Context() - requestCtx, cancel := context.WithCancel(savedCtx) - defer cancel() - defer ctx.SetContext(savedCtx) - transaction := sentry.StartTransaction( - sentry.SetHubOnContext(requestCtx, hub), + requestCtx, fmt.Sprintf("%s %s", r.Method, transactionName), options..., ) - ctx.SetContext(transaction.Context()) - - defer func() { - if routePath := ctx.Route().Path; routePath != "" && !ctx.IsMiddleware() { - transaction.Name = fmt.Sprintf("%s %s", r.Method, routePath) - transaction.Source = sentry.SourceRoute - } - status := ctx.Response().StatusCode() - transaction.Status = sentry.HTTPtoSpanStatus(status) - transaction.SetData("http.response.status_code", status) - transaction.Finish() - }() - - transaction.SetData("http.request.method", r.Method) - r = r.WithContext(transaction.Context()) + if created { + requestCtx = transaction.Context() + transaction.SetData("http.request.method", r.Method) + defer func() { + if routePath := ctx.Route().Path; routePath != "" && !ctx.IsMiddleware() { + transaction.Name = fmt.Sprintf("%s %s", r.Method, routePath) + transaction.Source = sentry.SourceRoute + } + status := ctx.Response().StatusCode() + transaction.Status = sentry.HTTPtoSpanStatus(status) + transaction.SetData("http.response.status_code", status) + transaction.Finish() + }() + } + storedCtx.ctx = requestCtx + ctx.SetContext(requestCtx) + r = r.WithContext(requestCtx) - scope := hub.Scope() scope.SetRequest(r) scope.SetRequestBody(bytes.Clone(ctx.Body())) - ctx.Locals(valuesKey, hub) - ctx.Locals(transactionKey, transaction) - defer h.recoverWithSentry(hub, ctx) + defer h.recoverWithSentry(requestCtx, ctx, cancel) return ctx.Next() } -func (h *handler) recoverWithSentry(hub *sentry.Hub, ctx fiber.Ctx) { +func (h *handler) recoverWithSentry(requestCtx context.Context, ctx fiber.Ctx, cancel context.CancelFunc) { if err := recover(); err != nil { - eventID := hub.RecoverWithContext( - context.WithValue(ctx.Context(), sentry.RequestContextKey, ctx), - err, - ) + requestCtx = context.WithValue(requestCtx, sentry.RequestContextKey, ctx) + eventID := sentry.Recover(requestCtx, err) if eventID != nil && h.waitForDelivery { - hub.Flush(h.timeout) + sentry.ClientFromContext(requestCtx).Flush(h.timeout) } if h.repanic { + cancel() panic(err) } } } -// GetHubFromContext retrieves the Hub instance from the fiber.Ctx. -func GetHubFromContext(ctx fiber.Ctx) *sentry.Hub { - if hub, ok := ctx.Locals(valuesKey).(*sentry.Hub); ok { - return hub - } - return nil -} - -// SetHubOnContext sets the Hub instance on the fiber.Ctx. -func SetHubOnContext(ctx fiber.Ctx, hub *sentry.Hub) { - ctx.Locals(valuesKey, hub) -} - -// GetSpanFromContext retrieves the Span instance from the fiber.Ctx. -func GetSpanFromContext(ctx fiber.Ctx) *sentry.Span { - if span, ok := ctx.Locals(transactionKey).(*sentry.Span); ok { - return span +// GetContext returns the request's Sentry context. Unlike Ctx.Context, it +// remains available to outer middleware and custom error handlers after the +// Sentry middleware restores Fiber's original context. +func GetContext(ctx fiber.Ctx) context.Context { + if storedCtx, ok := ctx.Locals(contextKey{}).(*storedContext); ok { + return storedCtx.ctx } - return nil + return ctx.Context() } func convert(ctx fiber.Ctx) *http.Request { diff --git a/fiberv3/sentryfiber_test.go b/fiberv3/sentryfiber_test.go index 5de9a9e14..9904f25a6 100644 --- a/fiberv3/sentryfiber_test.go +++ b/fiberv3/sentryfiber_test.go @@ -1,6 +1,7 @@ package sentryfiber_test import ( + "context" "errors" "fmt" "net/http" @@ -15,6 +16,7 @@ import ( "github.com/getsentry/sentry-go" sentryfiber "github.com/getsentry/sentry-go/fiberv3" + "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" @@ -63,43 +65,42 @@ func TestIntegration(t *testing.T) { app := fiber.New(fiber.Config{ ErrorHandler: func(c fiber.Ctx, e error) error { - hub := sentryfiber.GetHubFromContext(c) - hub.CaptureException(e) + requestCtx := sentryfiber.GetContext(c) + if err := requestCtx.Err(); err != nil { + t.Errorf("request context in error handler: %v", err) + } + sentry.CaptureException(requestCtx, e) return nil }, }) app.Use(sentryHandler) - app.Get("/panic", func(_ fiber.Ctx) error { + app.Get("/panic", func(c fiber.Ctx) error { + c.SetContext(context.Background()) panic("test") }) app.Post("/post", func(c fiber.Ctx) error { - hub := sentryfiber.GetHubFromContext(c) - hub.CaptureMessage("post: " + string(c.Body())) + sentry.CaptureMessage(c.Context(), "post: "+string(c.Body())) return nil }) app.Get("/get", func(c fiber.Ctx) error { - hub := sentryfiber.GetHubFromContext(c) - hub.CaptureMessage("get") + sentry.CaptureMessage(c.Context(), "get") return nil }) app.Get("/get/:id", func(c fiber.Ctx) error { - hub := sentryfiber.GetHubFromContext(c) - hub.CaptureMessage(fmt.Sprintf("get: %s", c.Params("id"))) + sentry.CaptureMessage(c.Context(), fmt.Sprintf("get: %s", c.Params("id"))) return nil }) app.Post("/post/large", func(c fiber.Ctx) error { - hub := sentryfiber.GetHubFromContext(c) - hub.CaptureMessage(fmt.Sprintf("post: %d KB", len(c.Body())/1024)) + sentry.CaptureMessage(c.Context(), fmt.Sprintf("post: %d KB", len(c.Body())/1024)) return nil }) app.Post("/post/body-ignored", func(c fiber.Ctx) error { - hub := sentryfiber.GetHubFromContext(c) - hub.CaptureMessage("body ignored") + sentry.CaptureMessage(c.Context(), "body ignored") return nil }) app.Post("/post/error-handler", func(_ fiber.Ctx) error { @@ -575,7 +576,11 @@ func TestHandlers(t *testing.T) { } handler := func(ctx fiber.Ctx) error { - span := sentryfiber.GetSpanFromContext(ctx) + scope := sentry.ScopeFromContext(ctx.Context()) + if tc.useSentry && scope == nil { + t.Error("expecting scope not to be nil") + } + span := sentry.SpanFromContext(ctx.Context()) if tc.useSentry && span == nil { t.Error("expecting span not to be nil") } @@ -606,38 +611,41 @@ func TestHandlers(t *testing.T) { } } -func TestSetHubOnContext(t *testing.T) { +func TestRequestIsolation(t *testing.T) { + t.Parallel() + fixture := sentrytest.NewFixture(t) + contexts := make(chan context.Context, 1) + outerContexts := make(chan context.Context, 1) app := fiber.New() - hub := sentry.NewHub(sentry.CurrentHub().Client(), sentry.NewScope()) + app.Use(func(ctx fiber.Ctx) error { + original := fixture.NewContext(ctx.Context()) + ctx.SetContext(original) + err := ctx.Next() + if ctx.Context() != original { + t.Error("Sentry middleware did not restore the caller context") + } + return err + }) + app.Use(sentryfiber.New(sentryfiber.Options{})) + app.Use(func(ctx fiber.Ctx) error { + outerContexts <- ctx.Context() + return ctx.Next() + }) + app.Use(sentryfiber.New(sentryfiber.Options{})) + app.Get("/test", func(ctx fiber.Ctx) error { contexts <- ctx.Context(); return nil }) - app.Get("/test", func(c fiber.Ctx) error { - sentryfiber.SetHubOnContext(c, hub) - retrievedHub := sentryfiber.GetHubFromContext(c) - if retrievedHub == nil { - t.Fatal("expected hub to be set on context, but got nil") + sentrytest.CheckRequestIsolation(t, func() (context.Context, context.Context, error) { + req, err := http.NewRequest(http.MethodGet, "http://example.com/test", nil) + if err != nil { + return nil, nil, err } - if !reflect.DeepEqual(hub, retrievedHub) { - t.Fatalf("expected hub to be %v, but got %v", hub, retrievedHub) + resp, err := app.Test(req) + if err != nil { + return nil, nil, err } - return nil + err = resp.Body.Close() + return <-contexts, <-outerContexts, err }) - - req, err := http.NewRequest(http.MethodGet, "/test", nil) - if err != nil { - t.Fatal(err) - } - req.Header.Set("User-Agent", "fiber") - req.Host = "example.com" - - resp, err := app.Test(req) - if err != nil { - t.Fatalf("Request failed: %s", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - t.Fatalf("Expected status code %d, got %d", http.StatusOK, resp.StatusCode) - } } func TestMalformedURLNoPanic(t *testing.T) { diff --git a/internal/sentrytest/request.go b/internal/sentrytest/request.go new file mode 100644 index 000000000..521b5d5c4 --- /dev/null +++ b/internal/sentrytest/request.go @@ -0,0 +1,67 @@ +package sentrytest + +import ( + "context" + "errors" + "sync" + "testing" + + "github.com/getsentry/sentry-go" +) + +// CheckRequestIsolation exercises sequential reuse and concurrent requests. +// request returns the completed request context and, optionally, its outer +// middleware context. Both contexts must be canceled when the request ends. +func CheckRequestIsolation(t *testing.T, request func() (context.Context, context.Context, error)) { + t.Helper() + const concurrentRequests = 32 + type result struct{ inner, outer context.Context } + results := make(chan result, concurrentRequests+2) + run := func() { + inner, outer, err := request() + if err != nil { + t.Error(err) + return + } + results <- result{inner, outer} + } + for range 2 { + run() + } + var wg sync.WaitGroup + for range concurrentRequests { + wg.Add(1) + go func() { defer wg.Done(); run() }() + } + wg.Wait() + close(results) + + scopes := make(map[*sentry.Scope]bool) + traces := make(map[sentry.TraceID]bool) + for result := range results { + if result.inner == nil { + t.Fatal("request returned a nil context") + } + if !errors.Is(result.inner.Err(), context.Canceled) { + t.Fatalf("request context error = %v, want context canceled", result.inner.Err()) + } + if result.outer != nil && !errors.Is(result.outer.Err(), context.Canceled) { + t.Fatalf("outer request context error = %v, want context canceled", result.outer.Err()) + } + scope := sentry.ScopeFromContext(result.inner) + span := sentry.SpanFromContext(result.inner) + if scope == nil || span == nil { + t.Fatal("request context is missing its scope or span") + } + if scopes[scope] { + t.Fatal("request reused an isolation scope") + } + if traces[span.TraceID] { + t.Fatal("request reused a trace ID") + } + scopes[scope], traces[span.TraceID] = true, true + } + if len(scopes) != concurrentRequests+2 { + t.Errorf("isolated requests = %d, want %d", len(scopes), concurrentRequests+2) + } +}