Skip to content

Commit

Permalink
Function grouping prefix fix (#877)
Browse files Browse the repository at this point in the history
  • Loading branch information
ribice authored Sep 3, 2024
1 parent 72f3bff commit bff876d
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
- Add trace origin to span data ([#849](https://github.com/getsentry/sentry-go/pull/849))
- 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))
- Fix closure functions name grouping ([#877](https://github.com/getsentry/sentry-go/pull/877))

## 0.28.1

Expand Down
7 changes: 6 additions & 1 deletion stacktrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
7 changes: 7 additions & 0 deletions stacktrace_below_go1.21.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//go:build !go1.21

package sentry

func cleanupFunctionNamePrefix(f []Frame) []Frame {
return f
}
17 changes: 17 additions & 0 deletions stacktrace_below_go1.21_test.go
Original file line number Diff line number Diff line change
@@ -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)

}
25 changes: 25 additions & 0 deletions stacktrace_go1.21.go
Original file line number Diff line number Diff line change
@@ -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
}
81 changes: 81 additions & 0 deletions stacktrace_go1.21_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}

0 comments on commit bff876d

Please sign in to comment.