Skip to content

Commit

Permalink
fix(stacktrace): Ignore SDK subpackage frames
Browse files Browse the repository at this point in the history
Compared to release v0.3.1, now filteredFrames compares the frame.Module
value instead of frame.AbsPath, and ignores SDK "_test" packages instead
of pattern matching test files and examples.

However confusing it might be, frame.Module holds a package name,
unrelated to Go Modules.

This fixes up the regression introduced in
61d7ccc, which caused frames from
integrations/subpackages to be reported to Sentry.
  • Loading branch information
rhcarvalho committed Dec 20, 2019
1 parent 43729e5 commit bf38760
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 4 deletions.
15 changes: 11 additions & 4 deletions stacktrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,17 +206,24 @@ func extractFrames(pcs []uintptr) []Frame {
return frames
}

// filterFrames filters out stack frames that are not meant to be reported to
// Sentry. Those are frames internal to the SDK or Go.
func filterFrames(frames []Frame) []Frame {
if len(frames) == 0 {
return nil
}

filteredFrames := make([]Frame, 0, len(frames))

for _, frame := range frames {
// go runtime frames
// Skip Go internal frames.
if frame.Module == "runtime" || frame.Module == "testing" {
continue
}
// sentry internal frames
if frame.Module == "github.com/getsentry/sentry-go" ||
strings.HasPrefix(frame.Module, "github.com/getsentry/sentry-go.") {
// Skip Sentry internal frames, except for frames in _test packages (for
// testing).
if strings.HasPrefix(frame.Module, "github.com/getsentry/sentry-go") &&
!strings.HasSuffix(frame.Module, "_test") {
continue
}
filteredFrames = append(filteredFrames, frame)
Expand Down
94 changes: 94 additions & 0 deletions stacktrace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,97 @@ func TestSplitQualifiedFunctionName(t *testing.T) {
})
}
}

//nolint: scopelint // false positive https://github.com/kyoh86/scopelint/issues/4
func TestFilterFrames(t *testing.T) {
tests := []struct {
in []Frame
out []Frame
}{
// sanity check
{},
// filter out go internals and SDK internals; "sentry-go_test" is
// considered outside of the SDK and thus included (useful for testing)
{
in: []Frame{
{
Function: "goexit",
Module: "runtime",
AbsPath: "/goroot/src/runtime/asm_amd64.s",
InApp: false,
},
{
Function: "tRunner",
Module: "testing",
AbsPath: "/goroot/src/testing/testing.go",
InApp: false,
},
{
Function: "TestNewStacktrace.func1",
Module: "github.com/getsentry/sentry-go_test",
AbsPath: "/somewhere/sentry/sentry-go/stacktrace_external_test.go",
InApp: true,
},
{
Function: "StacktraceTestHelper.NewStacktrace",
Module: "github.com/getsentry/sentry-go",
AbsPath: "/somewhere/sentry/sentry-go/stacktrace_test.go",
InApp: true,
},
{
Function: "NewStacktrace",
Module: "github.com/getsentry/sentry-go",
AbsPath: "/somewhere/sentry/sentry-go/stacktrace.go",
InApp: true,
},
},
out: []Frame{
{
Function: "TestNewStacktrace.func1",
Module: "github.com/getsentry/sentry-go_test",
AbsPath: "/somewhere/sentry/sentry-go/stacktrace_external_test.go",
InApp: true,
},
},
},
// filter out integrations; SDK subpackages
{
in: []Frame{
{
Function: "Example.Integration",
Module: "github.com/getsentry/sentry-go/http/integration",
AbsPath: "/somewhere/sentry/sentry-go/http/integration/integration.go",
InApp: true,
},
{
Function: "(*Handler).Handle",
Module: "github.com/getsentry/sentry-go/http",
AbsPath: "/somewhere/sentry/sentry-go/http/sentryhttp.go",
InApp: true,
},
{
Function: "main",
Module: "main",
AbsPath: "/somewhere/example.com/pkg/main.go",
InApp: true,
},
},
out: []Frame{
{
Function: "main",
Module: "main",
AbsPath: "/somewhere/example.com/pkg/main.go",
InApp: true,
},
},
},
}
for _, tt := range tests {
t.Run("", func(t *testing.T) {
got := filterFrames(tt.in)
if diff := cmp.Diff(tt.out, got); diff != "" {
t.Errorf("filterFrames() mismatch (-want +got):\n%s", diff)
}
})
}
}

0 comments on commit bf38760

Please sign in to comment.