zap
zap copied to clipboard
uber-go
Expose the ability to disable errorVerbose via logger config
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 disable errorVerbose.
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!
https://github.com/uber-go/zap/blob/9a9fa7d4b5f07a9b634983678a65b5525f81e58b/zapcore/error.go#L45
I'm no expert here, but it seems that the errorVerbose should only be present if you use zap.Error(err) and err implements fmt.Formatter. As a hack you could add the error field differently.
You could use your own Field factory like this:
func SilentError(err error) zap.Field {
// you could even flip the encoding depending on the environment here if you want
return zap.Field{Key: "error", Type: zapcore.StringType, String: err.Error()}
}
And use it like
log.Error("something happened", SilentError(err))
Really, what I would like is for the cause trace provided by pkg/errors to be pretty-printed so that I can read it. Right now, the formatting is that it provides several lines, separated by \n which get shoved into the errorVerbose field. Imagine if the stacktrace provided by Zap were to be shoved into a field in development mode. It would be very hard to read.
In a custom field factory like SilentError above you could provide your own formatting logic if you wanted. For example parse the lines into an zapcore.ArrayMarshaler implementation and use the field type Type: zapcore.ArrayMarshalerType.
Right now, the formatting is that it provides several lines, separated by
\nwhich get shoved into the errorVerbose field.
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 log and fmt.Printf the error (in dev mode) so that I can see the cause trace.
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 errorVerbose field (when in dev mode), which does still require (technically) that errorVerbose is not used for all errors.
There is a difference between the stacktrace you are refering of and the errorVerbose field. errorVerbose is a property of the zap.Error(..) combined with pkg/errors. The printing of a stacktrace is a property of the encoder. You could achieve what you want by using an encoder config with AddStacktrace(zapcore.ErrorLevel) and silencing the errorVerbose field in a way like the one above.
zap cannot possibly know that pkg/errors chooses to write a stack trace with the io.Formatter interface. You could provide your own error that implements error verbose differently (e.g https://play.golang.org/p/NrnwBDrBGRW).
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:
type plainError struct {
e error
}
func (pe plainError) Error() string {
return pe.e.Error()
}
func PlainError(err error) zap.Field {
return zap.Error(plainError{err})
}
used like this
log.Error("something happened!", PlainError(err))
That's pretty much what I had in mind, the pain I was referring to was forgetting to use PlainError on every log entry!
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
type errorVerboseToStacktraceEncoder struct {
zapcore.Encoder
}
type stackTracer interface {
StackTrace() errors.StackTrace
}
func (he errorVerboseToStacktraceEncoder) EncodeEntry(ent zapcore.Entry, fields []zapcore.Field) (*buffer.Buffer, error) {
filteredFields := make([]zapcore.Field, 0)
var st stackTracer
for _, f := range fields {
if f.Type != zapcore.ErrorType {
filteredFields = append(filteredFields, f)
continue
}
if t, ok := f.Interface.(stackTracer); ok {
st = t
}
filteredFields = append(filteredFields, PlainError(f.Interface.(error)))
}
if st != nil {
ent.Stack = fmt.Sprintf("%+v", st.StackTrace())
}
return he.Encoder.EncodeEntry(ent, filteredFields)
}
basically you look for errors in fields that fulfill the stackTracer interface ( pkg/errors does) and in that case you add a plain error and override the stack of the entry with the stack from the error. I tried it and it works; but im not sure if that is the best idea
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 ent.Stack you could combine it with the one of the stackTracer.
It should probably be made configurable (Probably via the EncoderConfig?), so you can encode errors in any way you wish. Either to remove errorVerbose, limit its depth, write it out in a different way entirely, support other types of rich error interfaces, and so on.
For convenience for users that don't like errorVerbose, there could be a default error encoder and another one that doesn't write errorVerbose similar to other encoder callbacks. And any other variation of this that users will commonly want/need.
There is a problem implementing this though. encodeError is invoked directly by the Field, not via the encoder, so has no access to such a callback if added to the EncoderConfig as far as I can tell.
Maybe you can use zap.String("error", err.Error()) replace the zap.Error(err)
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 ErrorEnoder interface. Something like,
type ErrorEncoder {
AddError(key string, err error) error
}
If the provided ObjectEncoder implements this interface, Field.AddTo should use it to serialize the error instead of encodeError. We'd have to make sure that the solution also works for the zap.Errors field.
Additionally, if we want zap.Errors to support serializing a list of errors into a list of strings (as opposed to the list of objects they get serialized to today, i.e., "errors": [{"error": "problem"}, ...]), ErrorEncoder may need to support an AddErrors(string, []error) error method as well.
We welcome any contributions to address the issue, @abhinav's suggestion should be a good starting point for anyone looking to solve this.
Maybe you can use
zap.String("error", err.Error())replace thezap.Error(err)
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)
The printing of a stacktrace is a property of the encoder. You could achieve what you want by using an encoder config with
AddStacktrace(zapcore.ErrorLevel)and silencing theerrorVerbosefield in a way like the one above.
Another workaround sample based on using AddStacktrace is to have a wrapper function and use that for logging.
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)
}
