Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Set either Frame.Filename or Frame.AbsPath #233

Merged
merged 1 commit into from
Jul 6, 2020

Conversation

rhcarvalho
Copy link
Contributor

@rhcarvalho rhcarvalho commented May 27, 2020

Despite the name "filename", the field is supposed to contain "The path to the source file relative to the project root directory." [1]

Setting Frame.Filename to filepath.Base(FrameAbsPath) is a problem because it creates ambiguous paths and may affect the "Suspect Commits" feature.

Depending on how the binary is built, the Go runtime knows either the absolute path or the relative path of source files.

In the general case, it can be difficult to derive a correct "project-relative" path. Code built using Go Modules can be anywhere in the file system (not only under GOPATH). GOPATH can be multiple paths.

There was an attempt to trim paths in raven-go [2], but the approach there relies on GOROOT and GOPATH as configured for "future builds", and does not reflect how the running binary has been built, thus not being a good general case solution.

For now we set either Filename or AbsPath depending on what is known, and we leave figuring out a relative path from an absolute path for a later improvement.

[1] https://develop.sentry.dev/sdk/event-payloads/stacktrace#frame-attributes
[2] https://github.com/getsentry/raven-go/blob/5c24d5110e0e198d9ae16f1f3465366085001d92/stacktrace.go#L141

Fixes #216.

Despite the name "filename", the field is supposed to contain "The path
to the source file relative to the project root directory." [1]

Setting Frame.Filename to filepath.Base(FrameAbsPath) is a problem
because it creates ambiguous paths and may affect the "Suspect Commits"
feature.

Depending on how the binary is built, the Go runtime knows either the
absolute path or the relative path of source files.

In the general case, it can be difficult to derive a correct
"project-relative" path. Code built using Go Modules can be anywhere in
the file system (not only under GOPATH). GOPATH can be multiple paths.

There was an attempt to trim paths in raven-go [2], but the approach
there relies on GOROOT and GOPATH as configured for "future builds", and
does not reflect how the running binary has been built, thus not being a
good general case solution.

For now we set either Filename or AbsPath depending on what is known,
and we leave figuring out a relative path from an absolute path for
a later improvement.

[1] https://develop.sentry.dev/sdk/event-payloads/stacktrace#frame-attributes
[2] https://github.com/getsentry/raven-go/blob/5c24d5110e0e198d9ae16f1f3465366085001d92/stacktrace.go#L141
@rhcarvalho rhcarvalho marked this pull request as ready for review July 6, 2020 11:19
@rhcarvalho rhcarvalho changed the title fix: Set Frame.Filename to project-relative path fix: Set either Frame.Filename or Frame.AbsPath Jul 6, 2020
Copy link
Contributor

@kamilogorek kamilogorek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@rhcarvalho
Copy link
Contributor Author

Travis failures are related to downloading github.com/CloudyKit/jet with GO111MODULE=off.

@rhcarvalho
Copy link
Contributor Author

The failure is due to fetching github.com/kataras/iris from master with modules turned off. Since Iris only supports GO111MODULE=on, we need to disable those tests accordingly.

@rhcarvalho rhcarvalho merged commit 691b7df into getsentry:master Jul 6, 2020
@rhcarvalho rhcarvalho deleted the stacktrace-filename branch July 6, 2020 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Relative path in stack traces
2 participants