-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Expose the ability to disable errorVerbose via logger config #650
Comments
I'm also running into this issue. |
Line 45 in 9a9fa7d
I'm no expert here, but it seems that the You could use your own Field factory like this:
And use it like
|
Really, what I would like is for the cause trace provided by |
In a custom field factory like |
This. 10000% This. |
@MichaHoffmann do you have an example of how this would look and be printed? Is this how Zap currently prints the stacktrace in dev mode? |
@MichaHoffmann I see what you are saying, but I think the original issue that the author is asking for is that this is an extra field that gets forced upon the user, even if it is not desired. My concern is that I actually want this data, but in a different format. The only thing I see that I can do is to both |
I've forked this issue, as I believe the concerns are related but different. I would imagine that implementation of #732 would print the error cause trace like it would the stack trace instead of putting it in the |
There is a difference between the stacktrace you are refering of and the zap cannot possibly know that |
I see, that makes sense, Zap cannot know this. And I suppose hard-coding special treatment for pkg/errors would be a bad idea in terms of future proof and design. So it seems the only way to really solve this issue is to wrap errors at log-time using either a custom field or a custom error type like in your example. Kinda awkward and I'm absolutely going to forget to do this extra layer of conversion. Oh well, I'll just live with the 40+ lines of stack on each log I guess ¯\_(ツ)_/¯ |
I have something like, to minimize conversion pain:
used like this
|
That's pretty much what I had in mind, the pain I was referring to was forgetting to use |
I'm sorry, then i'm out of ideas! edit: I just had the idea that one can offload that task to the encoder maybe.. something like this could work
basically you look for errors in fields that fulfill the edit2: there are some bugs in that code; one should take more care in plugging the stack traces together, ( if there are more than one error fields i.e.) but the idea should basically work |
@MichaHoffmann is it possible to write a "composite" stacktracer? One that will print the real stack trace on the error cause trace? |
the stacktrace at the time of logging is saved in |
This addresses uber-go/zap#650 by changing errors to strings for Info and Debug calls. With that in place only Error logs produce errorVerbose and errorCause.
It should probably be made configurable (Probably via the For convenience for users that don't like There is a problem implementing this though. |
I guess this is still not "fixed" right? |
Maybe you can use |
That's not ideal if you have any extension to zap that looks at this field (e.g. Reporting to error collection systems), moreso it requires changes to every single line of code doing this, and doesn't really support doing any other encoding of the error unless you again go and change it at all call sites. |
A possible way to address this issue is by introducing an
If the provided Additionally, if we want |
Do you have any update about it ? |
We welcome any contributions to address the issue, @abhinav's suggestion should be a good starting point for anyone looking to solve this. |
A workaround from here. // MyMessageProducer replace err to string
func MyMessageProducer(ctx context.Context, msg string, level zapcore.Level, code codes.Code, err error, duration zapcore.Field) {
// re-extract logger from newCtx, as it may have extra fields that changed in the holder.
ctxzap.Extract(ctx).Check(level, msg).Write(
zap.String("error", err.Error()), // here it is.
zap.String("grpc.code", code.String()),
duration,
)
}
// use this option to create interceptor
grpc_zap.WithMessageProducer(MyMessageProducer) |
Another workaround sample based on using func noStacktraceError(l *zap.Logger, msg string, e error) {
// increase stacktrace level to zap.DPanicLevel to avoid having stacktrace for errors
l.WithOptions(zap.AddStacktrace(zap.DPanicLevel)).Error(msg, zap.Error(e))
}
func main() {
l, _ := zap.NewDevelopment()
e := fmt.Errorf("foo")
l.Error("normal error", zap.Error(e))
noStacktraceError(l, "no stacktrace error", e)
} |
Any updates on this? |
No updates in 2024 right? |
I eventually stopped using error libraries that include stack traces by default, as I found no real use-case for them and they cluttered logs up and increased log storage costs by a factor of ~x3. Now we use Fault, which only includes stack frames from your own code and not stdlib/external libraries which is exactly what we wanted. May not suit everyone's needs, but Fault is an error wrapping framework so it's versatile enough to build your own error management strategy. It also plays well with Zap via the ErrorFormatter/errorVerbose things mentioned above. It still would be nice to be able to disable errorVerbose but the discussion above outlined why the original problem was not solvable via Zap internally and it's just an attribute of the chosen error library. Given I feel that the original problem was addressed/explained well, I feel this issue could be closed. But as mentioned, disabling errorVerbose may still have values so I'll defer the decision of closing the issue to Zap maintainers. |
I use Zap for basically every service I write, it's great! There's one issue I have though. Since I use pkg/errors, each error satisfies the fmt.Formatter interface and thus, every error log comes with a stack trace. This is great in production when that information is vital, but during development, (i.e. with
NewDevelopmentConfig
) they still show up; which, given the less vital scenario, is a bit annoying.I notice there's
DisableStacktrace
which removes the stack trace that originates from the log call itself but no way to disableerrorVerbose
.In development, stack traces clutter the logs and makes scanning the terminal pretty difficult. I usually don't need to know the full stack trace, only the top-most element (or, the top-most
errors.New
call). And if I do, in a perfect world, I could flip a env var to enable it!The text was updated successfully, but these errors were encountered: