Merge branch 'master' into fix/http/use-value-from-pattern

CHANGELOG.md

@@ -4,6 +4,12 @@
 - Add ability to skip frames in stacktrace ([#852](https://github.com/getsentry/sentry-go/pull/852))
 - Remove Martini integration ([#861](https://github.com/getsentry/sentry-go/pull/861))
 - Use value from http.Request.Pattern as HTTP server span name ([#875](https://github.com/getsentry/sentry-go/pull/875))
+- Fix closure functions name grouping ([#877](https://github.com/getsentry/sentry-go/pull/877))
+
+
+### Breaking Changes
+
+- WrapResponseWriter has been moved from sentryhttp to sentry. If you've imported it from sentryhttp, you'll need to update your imports.
 
 ## 0.28.1
 

http/sentryhttp.go

@@ -107,7 +107,7 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc {
 		)
 		transaction.SetData("http.request.method", r.Method)
 
-		rw := NewWrapResponseWriter(w, r.ProtoMajor)
+		rw := sentry.NewWrapResponseWriter(w, r.ProtoMajor)
 
 		defer func() {
 			status := rw.Status()

negroni/sentrynegroni.go

@@ -45,22 +45,6 @@ func New(options Options) negroni.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}
-}
-
 func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
 	ctx := r.Context()
 	hub := sentry.GetHubFromContext(ctx)
@@ -91,10 +75,10 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next http.Ha
 		options...,
 	)
 	transaction.SetData("http.request.method", r.Method)
-	rw := newResponseWriter(w)
+	rw := sentry.NewWrapResponseWriter(w, r.ProtoMajor)
 
 	defer func() {
-		status := rw.statusCode
+		status := rw.Status()
 		transaction.Status = sentry.HTTPtoSpanStatus(status)
 		transaction.SetData("http.response.status_code", status)
 		transaction.Finish()

stacktrace.go

@@ -311,7 +311,12 @@ func createFrames(frames []runtime.Frame, skip int) []Frame {
 		return []Frame{}
 	}
 
-	return result[:len(result)-skip]
+	result = result[:len(result)-skip]
+
+	// Fix issues grouping errors with the new fully qualified function names
+	// introduced from Go 1.21
+	result = cleanupFunctionNamePrefix(result)
+	return result
 }
 
 // TODO ID: why do we want to do this?

stacktrace_below_go1.21.go

@@ -0,0 +1,7 @@
+//go:build !go1.21
+
+package sentry
+
+func cleanupFunctionNamePrefix(f []Frame) []Frame {
+	return f
+}

stacktrace_below_go1.21_test.go

@@ -0,0 +1,17 @@
+//go:build !go1.21
+
+package sentry
+
+import (
+	"testing"
+)
+
+func Test_cleanupFunctionNamePrefix(t *testing.T) {
+	f := []Frame{
+		{Function: "main.main"},
+		{Function: "main.main.func1"},
+	}
+	got := cleanupFunctionNamePrefix(f)
+	assertEqual(t, got, f)
+
+}

stacktrace_go1.21.go

@@ -0,0 +1,25 @@
+//go:build go1.21
+
+package sentry
+
+import "strings"
+
+// Walk backwards through the results and for the current function name
+// remove it's parent function's prefix, leaving only it's actual name. This
+// fixes issues grouping errors with the new fully qualified function names
+// introduced from Go 1.21.
+
+func cleanupFunctionNamePrefix(f []Frame) []Frame {
+	for i := len(f) - 1; i > 0; i-- {
+		name := f[i].Function
+		parentName := f[i-1].Function + "."
+
+		if !strings.HasPrefix(name, parentName) {
+			continue
+		}
+
+		f[i].Function = name[len(parentName):]
+	}
+
+	return f
+}

stacktrace_go1.21_test.go

@@ -0,0 +1,81 @@
+//go:build go1.21
+
+package sentry
+
+import (
+	"testing"
+)
+
+func Test_cleanupFunctionNamePrefix(t *testing.T) {
+	cases := map[string]struct {
+		f    []Frame
+		want []Frame
+	}{
+		"SimpleCase": {
+			f: []Frame{
+				{Function: "main.main"},
+				{Function: "main.main.func1"},
+			},
+			want: []Frame{
+				{Function: "main.main"},
+				{Function: "func1"},
+			},
+		},
+		"MultipleLevels": {
+			f: []Frame{
+				{Function: "main.main"},
+				{Function: "main.main.func1"},
+				{Function: "main.main.func1.func2"},
+			},
+			want: []Frame{
+				{Function: "main.main"},
+				{Function: "func1"},
+				{Function: "func2"},
+			},
+		},
+		"PrefixWithRun": {
+			f: []Frame{
+				{Function: "Run.main"},
+				{Function: "Run.main.func1"},
+			},
+			want: []Frame{
+				{Function: "Run.main"},
+				{Function: "func1"},
+			},
+		},
+		"NoPrefixMatch": {
+			f: []Frame{
+				{Function: "main.main"},
+				{Function: "main.handler"},
+			},
+			want: []Frame{
+				{Function: "main.main"},
+				{Function: "main.handler"},
+			},
+		},
+		"SingleFrame": {
+			f: []Frame{
+				{Function: "main.main"},
+			},
+			want: []Frame{
+				{Function: "main.main"},
+			},
+		},
+		"ComplexPrefix": {
+			f: []Frame{
+				{Function: "app.package.Run"},
+				{Function: "app.package.Run.Logger.func1"},
+			},
+			want: []Frame{
+				{Function: "app.package.Run"},
+				{Function: "Logger.func1"},
+			},
+		},
+	}
+	for name, tt := range cases {
+		t.Run(name, func(t *testing.T) {
+			got := cleanupFunctionNamePrefix(tt.f)
+			assertEqual(t, got, tt.want)
+		})
+	}
+}

http/wrap_writer.go → wrap_writer.go

@@ -1,4 +1,4 @@
-package sentryhttp
+package sentry
 
 import (
 	"bufio"

http/wrap_writer_test.go → wrap_writer_test.go

@@ -1,4 +1,4 @@
-package sentryhttp
+package sentry
 
 import (
 	"bufio"