-
Notifications
You must be signed in to change notification settings - Fork 65
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
Stop relying on pkg/errors
to handle stack traces
#70
Comments
If we want to just add stack traces this package is pretty good in my opinion https://github.com/go-stack/stack @knz What do you think ? |
No it is too expensive. The expense to translate the list of PC addresses to metadata should only be paid when the error is formatted. |
@ChrisHines i think you are working on improvements using the newer golang features. Would you like to pitch in ? |
What kind of timeframe are you hoping to resolve this issue? |
The plans in the errors library is to take over the principles from pkg/errors. There is no time frame. |
How about cribbing from uber-zap's function: https://github.com/uber-go/zap/blob/master/stacktrace.go#L38-L77 uber-go/zap is licensed under the MIT License, while cockroachdb/errors is licensed under the Apache License 2.0 |
I have made 2 years ago another errors package to provide stack traces gitlab.com/tozd/go/errors, as a spiritual successor to archived |
Today the stack trace formatting code delegates to
pkg/errors/stack.go
.However, that code uses
runtime.FuncForPC
and emits file paths in the stack trace derived from the raw file name of the source file.This method is outdated, as the on-disk file name of a given package's source may be different from the package path (due to go mod versioning, reproducible build sandboxes, etc).
The "modern" equivalent is
runtime.CallersFrame
, which populatesruntime.Frame
structs with package path-qualified function names.We want to use that instead.
The text was updated successfully, but these errors were encountered: