Skip to content

Commit

Permalink
fix(stacktrace): Correctly report package names
Browse files Browse the repository at this point in the history
Makes function receivers part of the function name and not part of the
package name.

Consequently, filteredFrames needs only to compare the frame.Module
value (however confusing it is, since it holds a package name).
  • Loading branch information
rhcarvalho committed Dec 18, 2019
1 parent d743e93 commit 068b461
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 38 deletions.
63 changes: 46 additions & 17 deletions stacktrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func NewFrame(f runtime.Frame) Frame {
abspath := f.File
filename := f.File
function := f.Function
var module string
var pkg string

if filename != "" {
filename = filepath.Base(filename)
Expand All @@ -162,14 +162,14 @@ func NewFrame(f runtime.Frame) Frame {
}

if function != "" {
module, function = deconstructFunctionName(function)
pkg, function = splitQualifiedFunctionName(function)
}

frame := Frame{
AbsPath: abspath,
Filename: filename,
Lineno: f.Line,
Module: module,
Module: pkg,
Function: function,
}

Expand All @@ -178,6 +178,15 @@ func NewFrame(f runtime.Frame) Frame {
return frame
}

// splitQualifiedFunctionName splits a package path-qualified function name into
// package name and function name. Such qualified names are found in
// runtime.Frame.Function values.
func splitQualifiedFunctionName(name string) (pkg string, fun string) {
pkg = packageName(name)
fun = strings.TrimPrefix(name, pkg+".")
return
}

func extractFrames(pcs []uintptr) []Frame {
var frames []Frame
callersFrames := runtime.CallersFrames(pcs)
Expand Down Expand Up @@ -206,8 +215,7 @@ func filterFrames(frames []Frame) []Frame {
continue
}
// sentry internal frames
if frame.Module == "github.com/getsentry/sentry-go" ||
strings.HasPrefix(frame.Module, "github.com/getsentry/sentry-go.") {
if frame.Module == "github.com/getsentry/sentry-go" {
continue
}
filteredFrames = append(filteredFrames, frame)
Expand All @@ -226,21 +234,42 @@ func isInAppFrame(frame Frame) bool {
return true
}

// Transform `runtime/debug.*T·ptrmethod` into `{ module: runtime/debug, function: *T.ptrmethod }`
func deconstructFunctionName(name string) (module string, function string) {
if idx := strings.LastIndex(name, "."); idx != -1 {
module = name[:idx]
function = name[idx+1:]
}
function = strings.Replace(function, "·", ".", -1)
return module, function
}

func callerFunctionName() string {
pcs := make([]uintptr, 1)
runtime.Callers(3, pcs)
callersFrames := runtime.CallersFrames(pcs)
callerFrame, _ := callersFrames.Next()
_, function := deconstructFunctionName(callerFrame.Function)
return function
return baseName(callerFrame.Function)
}

// packageName returns the package part of the symbol name, or the empty string
// if there is none.
// It replicates https://golang.org/pkg/debug/gosym/#Sym.PackageName, avoiding a
// dependency on debug/gosym.
func packageName(name string) string {
// A prefix of "type." and "go." is a compiler-generated symbol that doesn't belong to any package.
// See variable reservedimports in cmd/compile/internal/gc/subr.go
if strings.HasPrefix(name, "go.") || strings.HasPrefix(name, "type.") {
return ""
}

pathend := strings.LastIndex(name, "/")
if pathend < 0 {
pathend = 0
}

if i := strings.Index(name[pathend:], "."); i != -1 {
return name[:pathend+i]
}
return ""
}

// baseName returns the symbol name without the package or receiver name.
// It replicates https://golang.org/pkg/debug/gosym/#Sym.BaseName, avoiding a
// dependency on debug/gosym.
func baseName(name string) string {
if i := strings.LastIndex(name, "."); i != -1 {
return name[i+1:]
}
return name
}
67 changes: 46 additions & 21 deletions stacktrace_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package sentry

import (
"runtime"
"testing"

"github.com/google/go-cmp/cmp"
)

func NewStacktraceForTest() *Stacktrace {
Expand All @@ -15,28 +16,52 @@ func (StacktraceTestHelper) NewStacktrace() *Stacktrace {
return NewStacktrace()
}

func TestFunctionName(t *testing.T) {
for _, test := range []struct {
skip int
pack string
name string
}{
{0, "sentry-go", "TestFunctionName"},
{1, "testing", "tRunner"},
{2, "runtime", "goexit"},
{100, "", ""},
} {
pc, _, _, _ := runtime.Caller(test.skip)
pack, name := deconstructFunctionName(runtime.FuncForPC(pc).Name())

// Go1.10 reports different paths than Modules aware versions (>=1.11)
assertStringContains(t, pack, test.pack)
assertEqual(t, name, test.name)
}
}

func BenchmarkNewStacktrace(b *testing.B) {
for i := 0; i < b.N; i++ {
Trace()
}
}

//nolint: scopelint // false positive https://github.com/kyoh86/scopelint/issues/4
func TestSplitQualifiedFunctionName(t *testing.T) {
tests := []struct {
in string
pkg string
fun string
}{
{"", "", ""},
{"runtime.Callers", "runtime", "Callers"},
{"main.main.func1", "main", "main.func1"},
{
"github.com/getsentry/sentry-go.Init",
"github.com/getsentry/sentry-go",
"Init",
},
{
"github.com/getsentry/sentry-go.(*Hub).Flush",
"github.com/getsentry/sentry-go",
"(*Hub).Flush",
},
{
"github.com/getsentry/sentry-go.Test.func2.1.1",
"github.com/getsentry/sentry-go",
"Test.func2.1.1",
},
{
"github.com/getsentry/confusing%2epkg%2ewith%2edots.Test.func1",
"github.com/getsentry/confusing%2epkg%2ewith%2edots",
"Test.func1",
},
}
for _, tt := range tests {
t.Run(tt.in, func(t *testing.T) {
pkg, fun := splitQualifiedFunctionName(tt.in)
if diff := cmp.Diff(tt.pkg, pkg); diff != "" {
t.Errorf("Package name mismatch (-want +got):\n%s", diff)
}
if diff := cmp.Diff(tt.fun, fun); diff != "" {
t.Errorf("Function name mismatch (-want +got):\n%s", diff)
}
})
}
}

0 comments on commit 068b461

Please sign in to comment.