normalize http.response.status_code (#811)

getsentry · Apr 11, 2024 · df20ce6 · df20ce6
1 parent a989948
commit df20ce6
Show file tree

Hide file tree

Showing 8 changed files with 197 additions and 25 deletions.
diff --git a/echo/sentryecho.go b/echo/sentryecho.go
@@ -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()
 		}()
 

diff --git a/echo/sentryecho_test.go b/echo/sentryecho_test.go
@@ -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},
 			},
 		},
 		{
@@ -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},
 			},
 		},
 		{
@@ -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},
 			},
 		},
 		{
@@ -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},
 			},
 		},
 		{
@@ -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},
 			},
 		},
 		{
@@ -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},
 			},
 		},
 		{
@@ -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,
 		},

diff --git a/fasthttp/sentryfasthttp.go b/fasthttp/sentryfasthttp.go
@@ -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()
 		}()
 

diff --git a/fasthttp/sentryfasthttp_test.go b/fasthttp/sentryfasthttp_test.go
@@ -64,7 +64,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},
 			},
 		},
 		{
@@ -103,7 +103,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},
 			},
 		},
 		{
@@ -138,7 +138,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},
 			},
 		},
 		{
@@ -178,7 +178,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},
 			},
 		},
 		{
@@ -219,7 +219,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},
 			},
 		},
 		{

diff --git a/gin/sentrygin.go b/gin/sentrygin.go
@@ -82,7 +82,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()
 	}()
 

diff --git a/gin/sentrygin_test.go b/gin/sentrygin_test.go
@@ -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,
@@ -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,
 		},
@@ -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,
@@ -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,
@@ -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,
@@ -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,
@@ -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,
 		},
@@ -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
 		},

diff --git a/http/sentryhttp.go b/http/sentryhttp.go
@@ -63,6 +63,22 @@ func New(options Options) *Handler {
 	}
 }
 
+// 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)
+}
+
+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.
@@ -83,6 +99,7 @@ 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)
 		if hub == nil {
@@ -99,24 +116,29 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc {
 			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)
 	}
 }