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

Add an option for setting stack skip #727

Closed
segevfiner opened this issue Jun 30, 2019 · 4 comments · Fixed by #843
Closed

Add an option for setting stack skip #727

segevfiner opened this issue Jun 30, 2019 · 4 comments · Fixed by #843

Comments

@segevfiner
Copy link
Contributor

Sometimes you log from internal functions and want the stack trace to point to their caller, for example: a panic recovery middleware/wrapper. This is especially important when directing errors logged to zap to an error collection service such as Sentry/Rollbar/Bugsnag/etc as they rely on the stack trace to identify and merge similar errors.

I'm proposing a new field factory func StackSkip(key string, skip int) Field that also allows controlling the stack skip, that is the number of frames to skip from the top of the stack. And a new option func SetStackSkip(skip int) Option that can be used to set the stack skip for the automatic stack traces.

@mfridman
Copy link

@segevfiner Would the AddCallerSkip option be useful for you in this case?

https://godoc.org/go.uber.org/zap#AddCallerSkip

@segevfiner
Copy link
Contributor Author

That's only for the caller, we need something similar for the stack trace. Those two will have similar functionality so we might want to consider that it's likely the user will want to set both to the same value. So maybe we should also have an AddSkip options which sets both.

@westwin
Copy link

westwin commented Mar 17, 2020

Yes, I need the same feature as how java does, find maxDepthPerThrowable in https://github.com/logstash/logstash-logback-encoder/tree/logstash-logback-encoder-6.3#customizing-stack-traces

@prashantv
Copy link
Collaborator

This is similar to the request in #512

I agree that aligning caller skip and stack traces makes sense, but there's also some concerns on backwards compatibility. I think adding an option to align the caller skip and stack trace pruning might be a good way to solve this problem.

segevfiner added a commit to segevfiner/zap that referenced this issue Jun 23, 2020
* UseCallerSkipForStacktrace configures the Logger to honor CallerSkip
when taking stacktraces.
* StackSkip is similar to Stack but allows
skipping frames from the top of the stack

Fixes uber-go#512, fixes uber-go#727
prashantv pushed a commit that referenced this issue Aug 18, 2020
…843)

* Honor `CallerSkip` when taking a stack trace. Both the caller and stack trace will now point to the same frame.
* Add `StackSkip` which is similar to `Stack` but allows skipping frames from the top of the stack.

This removes the internal behavior of skipping zap specific stack frames when taking a stack trace, and instead relies on the same behavior used to calculate the number of frames to skip for getting the caller to also skip frames in the stack trace.

Fixes #512, fixes #727
cgxxv pushed a commit to cgxxv/zap that referenced this issue Mar 25, 2022
…ber-go#843)

* Honor `CallerSkip` when taking a stack trace. Both the caller and stack trace will now point to the same frame.
* Add `StackSkip` which is similar to `Stack` but allows skipping frames from the top of the stack.

This removes the internal behavior of skipping zap specific stack frames when taking a stack trace, and instead relies on the same behavior used to calculate the number of frames to skip for getting the caller to also skip frames in the stack trace.

Fixes uber-go#512, fixes uber-go#727
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants