Fix InApp detection for stack traces

[grongor]

Summary

All frames in my stack traces look like they are "in app", no matter where they are from. Currently, the detection is done by this function:

func isInAppFrame(frame Frame) bool {
	if strings.HasPrefix(frame.AbsPath, build.Default.GOROOT) ||
		strings.Contains(frame.Module, "vendor") ||
		strings.Contains(frame.Module, "third_party") {
		return false
	}

	return true
}

There are two issues:

• build.Default.GOROOT seems to be a bad choice there, because at least on my system it has a value of /usr/local/go (or just go with -trimpath), which will never match the path of any files since all src/modules are under (in my case) /home/grongor/projects/go (contains subfolders src, pkg, ...). This path can be found in build.Default.GOPATH. That would fix "outside of app" detection for not vendored dependencies.
• checks for vendor and third_party (I think this one is weird/makes no sense, I would remove it) are based on frame.Module, which is filled from runtime.Frame.Function and it will never contain (at least in all kinds of experiments I tried, please correct me if I'm wrong) these two keywords. vendor is present in the path runtime.Frame.File, but to just use strings.Contains on the whole path isn't, in my opinion, a good way to detect if a frame is "in app". To safely detect that the frame is truly in the vendor directory, we would need to know the path to the application (main module), and check runetime.Frame.File for prefix of of that path + /vendor/

Detecting this correctly when using -trimpath is quite simple, we just need to retrieve the modules' information and check runtime.Frame.File for the prefix of the main module. Without -trimpath it's more complicated, but I think I've found a reliable way to detect the main module path using runtime.Stack(buf, true), where the true flag is the key. It will fill the buffer with all Goroutines. We could then search this buffer, either for goroutine 1 (which should always be main), or/and for main.main() and extract the path from there.

var AppModule, AppPath string

// somewhere "up", just once...
func detectAppModuleOrPath() {
	buf := make([]byte, 1<<20)
	for {
		n := runtime.Stack(buf, true)
		if n < len(buf) {
			buf = buf[:n]

			break
		}
		buf = make([]byte, 2*len(buf))
	}

	var foundMain bool
	scanner := bufio.NewScanner(bytes.NewReader(buf))

	for scanner.Scan() {
		if !foundMain {
			if bytes.HasPrefix(scanner.Bytes(), []byte("main.main")) {
				foundMain = true
			}
			
			continue
		}

		line := bytes.TrimSpace(scanner.Bytes())
		lastColonIndex := bytes.LastIndexByte(line, ':')

		path := string(line[:lastColonIndex])

		if !filepath.IsAbs(path) {
			if buildInfo, ok := debug.ReadBuildInfo(); ok {
				AppModule = buildInfo.Main.Path + "/"
			}

			return
		}

		if cmdIndex := strings.LastIndex(path, "/cmd/"); cmdIndex != -1 {
			path = path[0 : cmdIndex+1]
		} else if binIndex := strings.LastIndex(path, "/bin/"); binIndex != -1 {
			path = path[0 : binIndex+1]
		} else {
			path = filepath.Dir(path) + "/"
		}

		if filepath.IsAbs(path) {
			AppPath = path
		} else {
			AppModule = path
		}

		return
	}
}

// ... and then later when processing stack trace frames:
for i, frame := range stacktrace.Frames {
	if frame.AbsPath == "" {
		frame.InApp = strings.HasPrefix(frame.Filename, AppModule)
	} else {
		frame.InApp = strings.HasPrefix(frame.AbsPath, AppPath)
	}

	stacktrace.Frames[i] = frame
}

We could make these two variables public so that the user has an option to specify them manually (and then skip the detection) but, to be honest, I don't think that it's necessary. If path detection doesn't work for some "obscure" projects, I'd just recommend them to use -trimpath.

Also, if we manage to do this, we could easily trim the paths in the frames to make them relative to the project, eg.:

frame.Filename = strings.TrimPrefix(frame.Filename, AppModule)
frame.AbsPath = strings.TrimPrefix(frame.AbsPath, AppPath)

Project paths would be relative, paths from modules would still be absolute (no chance for conflicts/ambiguous stack traces).

I managed to write an extension for sentry-go to do exactly this (see here https://github.com/grongor/go-bootstrap/blob/master/sentry.go#L78 ), but I think this is something that should be present in this library.

If you wish I can send a pull request with these changes, just let me know.

Environment

SDK

• sentry-go version: latest (0.11.0)
• Go version: 1.17
• Using Go Modules? yes

Sentry

• Using hosted Sentry in sentry.io? no
• Using your own Sentry installation? Version: 21.8.0

[grongor]

Hey @rhcarvalho (I'm tagging you because we interacted earlier, hope it's ok), could you take a quick look at this? :) I can prepare a PR to fix this, I just want a confirmation before "wasting my time" :) Thanks!

[avgeorge]

All frames in my stack traces look like they are "in app", no matter where they are from.

Using -trimpath now has the opposite problem where all frames are marked as not in app. With -trimpath set, build.Default.GOROOT is blank so the strings.HasPrefix(frame.AbsPath, goRoot) check is always true.
See some discussion here: golang/go#51461
and the go1.19 release notes

The GOROOT function now returns the empty string (instead of "go") when the binary was built with the -trimpath flag set and the GOROOT variable is not set in the process environment.

It's possible to get the module path using debug.ReadBuildInfo, but I haven't checked if there's any complications when not using -trimpath or not using go modules.

[eric]

I'm also not seeing my code being marked as in-app using -trimpath. Is there a solution to this?

[avgeorge]

A quick fix in this library would be to change the check here

sentry-go/stacktrace.go

Line 336 in 8f8897d

if strings.HasPrefix(frame.AbsPath, goRoot) ||

to change

strings.HasPrefix(frame.AbsPath, goRoot)

to

(goRoot != "" && strings.HasPrefix(frame.AbsPath, goRoot))