Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize http.response.status_code #811

Merged
merged 2 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 7 additions & 3 deletions echo/sentryecho.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,18 @@ func (h *handler) handle(next echo.HandlerFunc) echo.HandlerFunc {
options...,
)

transaction.SetData("http.request.method", ctx.Request().Method)

defer func() {
status := ctx.Response().Status
if err := ctx.Get("error"); err != nil {
if httpError, ok := err.(*echo.HTTPError); ok {
transaction.Status = sentry.HTTPtoSpanStatus(httpError.Code)
status = httpError.Code
}
} else {
transaction.Status = sentry.HTTPtoSpanStatus(ctx.Response().Status)
}

transaction.Status = sentry.HTTPtoSpanStatus(status)
transaction.SetData("http.response.status_code", status)
transaction.Finish()
}()

Expand Down
7 changes: 7 additions & 0 deletions echo/sentryecho_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]any{"http.request.method": "GET", "http.response.status_code": http.StatusOK},
},
},
{
Expand All @@ -87,6 +88,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]any{"http.request.method": "GET", "http.response.status_code": 404},
},
},
{
Expand Down Expand Up @@ -133,6 +135,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]any{"http.request.method": "POST", "http.response.status_code": http.StatusOK},
},
},
{
Expand Down Expand Up @@ -170,6 +173,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]any{"http.request.method": "GET", "http.response.status_code": http.StatusOK},
},
},
{
Expand Down Expand Up @@ -216,6 +220,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]any{"http.request.method": "POST", "http.response.status_code": http.StatusOK},
},
},
{
Expand Down Expand Up @@ -260,6 +265,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]any{"http.request.method": "POST", "http.response.status_code": http.StatusOK},
},
},
{
Expand All @@ -283,6 +289,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]any{"http.request.method": "GET", "http.response.status_code": 400},
},
WantEvent: nil,
},
Expand Down
4 changes: 3 additions & 1 deletion fasthttp/sentryfasthttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ func (h *Handler) Handle(handler fasthttp.RequestHandler) fasthttp.RequestHandle
options...,
)
defer func() {
transaction.Status = sentry.HTTPtoSpanStatus(ctx.Response.StatusCode())
status := ctx.Response.StatusCode()
transaction.Status = sentry.HTTPtoSpanStatus(status)
transaction.SetData("http.response.status_code", status)
transaction.Finish()
}()

Expand Down
10 changes: 5 additions & 5 deletions fasthttp/sentryfasthttp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]any{"http.request.method": http.MethodGet},
Extra: map[string]any{"http.request.method": http.MethodGet, "http.response.status_code": http.StatusOK},
},
},
{
Expand Down Expand Up @@ -96,7 +96,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]any{"http.request.method": http.MethodPost},
Extra: map[string]any{"http.request.method": http.MethodPost, "http.response.status_code": http.StatusOK},
},
},
{
Expand Down Expand Up @@ -130,7 +130,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]any{"http.request.method": http.MethodGet},
Extra: map[string]any{"http.request.method": http.MethodGet, "http.response.status_code": http.StatusOK},
},
},
{
Expand Down Expand Up @@ -169,7 +169,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]any{"http.request.method": http.MethodPost},
Extra: map[string]any{"http.request.method": http.MethodPost, "http.response.status_code": http.StatusOK},
},
},
{
Expand Down Expand Up @@ -209,7 +209,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]any{"http.request.method": http.MethodPost},
Extra: map[string]any{"http.request.method": http.MethodPost, "http.response.status_code": http.StatusOK},
},
},
}
Expand Down
4 changes: 3 additions & 1 deletion gin/sentrygin.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ func (h *handler) handle(c *gin.Context) {
)
transaction.SetData("http.request.method", c.Request.Method)
defer func() {
transaction.Status = sentry.HTTPtoSpanStatus(c.Writer.Status())
status := c.Writer.Status()
transaction.Status = sentry.HTTPtoSpanStatus(status)
transaction.SetData("http.response.status_code", status)
transaction.Finish()
}()

Expand Down
15 changes: 8 additions & 7 deletions gin/sentrygin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]interface{}{"http.request.method": string("GET")},
Extra: map[string]any{"http.request.method": string("GET"), "http.response.status_code": http.StatusOK},
},
WantEvent: &sentry.Event{
Level: sentry.LevelFatal,
Expand Down Expand Up @@ -87,7 +87,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "url"},
Extra: map[string]interface{}{"http.request.method": string("GET")},
Extra: map[string]any{"http.request.method": string("GET"), "http.response.status_code": 404},
},
WantEvent: nil,
},
Expand Down Expand Up @@ -121,7 +121,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]interface{}{"http.request.method": string("POST")},
Extra: map[string]any{"http.request.method": string("POST"), "http.response.status_code": http.StatusOK},
},
WantEvent: &sentry.Event{
Level: sentry.LevelInfo,
Expand Down Expand Up @@ -161,7 +161,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]interface{}{"http.request.method": string("GET")},
Extra: map[string]any{"http.request.method": string("GET"), "http.response.status_code": http.StatusOK},
},
WantEvent: &sentry.Event{
Level: sentry.LevelInfo,
Expand Down Expand Up @@ -204,7 +204,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]interface{}{"http.request.method": string("POST")},
Extra: map[string]any{"http.request.method": string("POST"), "http.response.status_code": http.StatusOK},
},
WantEvent: &sentry.Event{
Level: sentry.LevelInfo,
Expand Down Expand Up @@ -248,7 +248,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]interface{}{"http.request.method": string("POST")},
Extra: map[string]any{"http.request.method": string("POST"), "http.response.status_code": http.StatusOK},
},
WantEvent: &sentry.Event{
Level: sentry.LevelInfo,
Expand Down Expand Up @@ -287,7 +287,7 @@ func TestIntegration(t *testing.T) {
},
},
TransactionInfo: &sentry.TransactionInfo{Source: "route"},
Extra: map[string]interface{}{"http.request.method": string("GET")},
Extra: map[string]any{"http.request.method": string("GET"), "http.response.status_code": 400},
},
WantEvent: nil,
},
Expand All @@ -303,6 +303,7 @@ func TestIntegration(t *testing.T) {
return event
},
BeforeSendTransaction: func(tx *sentry.Event, hint *sentry.EventHint) *sentry.Event {
fmt.Println("BeforeSendTransaction")
transactionsCh <- tx
return tx
},
Expand Down
36 changes: 29 additions & 7 deletions http/sentryhttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,22 @@
}
}

// responseWriter is a wrapper around http.ResponseWriter that captures the status code.
type responseWriter struct {
http.ResponseWriter
statusCode int
}

// WriteHeader captures the status code and calls the original WriteHeader method.
func (rw *responseWriter) WriteHeader(code int) {
rw.statusCode = code
rw.ResponseWriter.WriteHeader(code)

Check warning on line 75 in http/sentryhttp.go

View check run for this annotation

Codecov / codecov/patch

http/sentryhttp.go#L73-L75

Added lines #L73 - L75 were not covered by tests
}

func newResponseWriter(w http.ResponseWriter) *responseWriter {
return &responseWriter{ResponseWriter: w, statusCode: http.StatusOK}
}

// 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.
Expand All @@ -83,6 +99,7 @@

func (h *Handler) handle(handler http.Handler) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {

ctx := r.Context()
hub := sentry.GetHubFromContext(ctx)
if hub == nil {
Expand All @@ -99,24 +116,29 @@
sentry.ContinueFromRequest(r),
sentry.WithTransactionSource(sentry.SourceURL),
}
// We don't mind getting an existing transaction back so we don't need to
// check if it is.

transaction := sentry.StartTransaction(ctx,
fmt.Sprintf("%s %s", r.Method, r.URL.Path),
options...,
)
transaction.SetData("http.request.method", r.Method)
defer transaction.Finish()

rw := newResponseWriter(w)

defer func() {
status := rw.statusCode
transaction.Status = sentry.HTTPtoSpanStatus(status)
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)
defer h.recoverWithSentry(hub, r)
// TODO(tracing): use custom response writer to intercept
// response. Use HTTP status to add tag to transaction; set span
// status.
handler.ServeHTTP(w, r)
handler.ServeHTTP(rw, r)
}
}

Expand Down