getsentry / raven-go

Sentry client in Go
https://sentry.io
BSD 3-Clause "New" or "Revised" License
561 stars 147 forks source link

golang (1.10) NewStacktrace yields a trace that service considers invalid. #186

Open toshok opened 6 years ago

toshok commented 6 years ago

we're running 1cc47a9463b90f246a0503d4c2e9a55c9459ced3 (although I don't see any changes that might fix it in the commits after), and we see quite a few errors like:

image

after expanding all stack frames and scanning, the reason is pretty clear:

{
abs_path: ...,
filename: ...,
function: ...,
in_app: ...,
lineno: ...,
module: ...,
}, 
{
in_app: False
}, 
{
abs_path: ...,
filename: ...,
function: ...,
in_app: ...,
lineno: ...,
module: ...,
}, 

That middle frame is missing Function/Filename/Module. We aren't doing anything interesting with the call to NewStacktrace:

        // skip = 1, context = 3, prefixes = []string{"github.com/honeycombio"}
    trace := raven.NewStacktrace(skip, context, prefixes)

running a loop over the StacktraceFrames to remove those problematic frames fixes everything, but probably shouldn't be adding them in the first place.

mattrobenolt commented 6 years ago

Interesting, do you know how those frames are possibly generated?

toshok commented 6 years ago

it'll take a bit to get things set up again, but reading in the golang runtime package yielded this (when talking about runtime.Callers, but I expect it to apply to the pc's returned by runtime.Caller as well):

// To translate these PCs into symbolic information such as function
// names and line numbers, use CallersFrames. CallersFrames accounts
// for inlined functions and adjusts the return program counters into
// call program counters. Iterating over the returned slice of PCs
// directly is discouraged, as is using FuncForPC on any of the
// returned PCs, since these cannot account for inlining or return
// program counter adjustment.
toshok commented 6 years ago

haven't tested it yet outside our service, but I have a patch that seems to fix things. Will open a PR.