fix(stacktrace): Filter out internal SDK methods #125
Conversation
The frame.Module field in its current form may contain entries like: github.com/getsentry/sentry-go.(*Hub) github.com/getsentry/sentry-go.(*Client) github.com/getsentry/sentry-go.(*Client) Taking those into account now.
| @@ -206,7 +206,8 @@ func filterFrames(frames []Frame) []Frame { | |||
| continue | |||
| } | |||
| // sentry internal frames | |||
| if frame.Module == "github.com/getsentry/sentry-go" { | |||
| if frame.Module == "github.com/getsentry/sentry-go" || | |||
There was a problem hiding this comment.
We can probably just unify this to a single strings.HasPrefix and remove ending dot
There was a problem hiding this comment.
Or strings.Contains. The chance of false positive is very unlikely.
There was a problem hiding this comment.
I considered using only strings.HasPrefix. Eventually, what we need to do is define what frame.Module is supposed to mean.
Quite likely it should be github.com/getsentry/sentry-go, and never github.com/getsentry/sentry-go.(*Hub). Then the direct string comparison with == would be the condition here. What do you think?
There was a problem hiding this comment.
Double-checked it and agree. It should be direct comparison.
But why is deconstructFunctionName putting .(Hub) in frame.Module? It should be a part of a function name. And I'm pretty sure it works correctly, at least for the input that's supposedly incorrect - https://play.golang.org/p/U8SfYCLeyun
There was a problem hiding this comment.
The input is different, see https://play.golang.org/p/bA6FSn1qG8C
It returns the Method name without the type prefix, and the type goes into the package import path.
There was a problem hiding this comment.
That's actually good, as we can reproduce it now. So what should change is the way to determine which parts belong to the module and which to the function name. Buuut it'll probably require a regexp instead of simple . index search 😢
There was a problem hiding this comment.
We didn't touch deconstructFunctionName recently. I can add some new test cases and fix it/change the behavior in a separate PR. Then we can drop the || HasPrefix part :)
There was a problem hiding this comment.
Quite likely it should be github.com/getsentry/sentry-go, and never github.com/getsentry/sentry-go.(*Hub). Then the direct string comparison with == would be the condition here.
Except that we have "subpackages": github.com/getsentry/sentry-go/http, github.com/getsentry/sentry-go/....
I was wrong, forgot about those for a moment :(
The frame.Module field in its current form may contain entries like:
github.com/getsentry/sentry-go.(*Hub)
github.com/getsentry/sentry-go.(*Client)
github.com/getsentry/sentry-go.(*Client)
Taking those into account now.
Fixes up #123.