rs / zerolog

Zero Allocation JSON Logger
MIT License
10.63k stars 572 forks source link

Support logger-level ErrorMarshalFunc #329

Open Deiz opened 3 years ago

Deiz commented 3 years ago

I have a use case around dispatching errors with additional context, e.g. request IDs.

Currently, I'm accumulating context in HTTP middleware via the usual With().Str("foo", "bar") mechanism, which goes into the logger's context []byte.

I dispatch errors via a custom ErrorMarshalFunc, however the signature for that function, func(err error) interface{}, prevents obtaining any context from the logger or the event, which is fine.

What I'm proposing is to add an additional field, errorMarshalFunc func(err error) interface{} onto Logger, and an additional method, func (l Logger) ErrorMarshalFunc(func(err error) interface{}) Logger akin to Logger's Output method.

That would support a closure-based flow like so:

// Middleware
attributes := map[string]string{"foo": "bar"}

baseLogger := hlog.FromRequest(r)

errorMarshalFunc := func(err error) interface{} {
    // attributes captured from outer scope; this isn't possible with the global ErrorMarshalFunc
    externalerrors.Submit(err, attributes)

    return err
}

logger := baseLogger.With().Str("foo", "bar").Logger().ErrorMarshalFunc(errorMarshalFunc)

// Much later, in application code. Because of the request context's logger's embedded errorMarshalFunc, it retains the
// request-scoped metadata.
hlog.FromRequest(r).Err(err).Msg("Whoops")

This is probably a somewhat obtuse use case, but I'll be doing this work regardless and would like to see if there's consensus to merge this here.

rs commented 3 years ago

I have the feeling you are trying to use this logging library as an event dispatcher to do other things. How is externalerrors.Submit related to the JSON logging?

Deiz commented 3 years ago

Cheers for the quick response - You're spot on. For full real-world context:

You can see how a few others have built this as an io.Writer, e.g. https://github.com/archdx/zerolog-sentry and https://gist.github.com/asdine/f821abe6189a04250ae61b77a3048bd9 - though I strongly prefer to avoid marshalling an error as a JSON string only to parse it back into an error-like struct that's meaningful to a third-party error reporting library.

What I would actually like is to be able to define a hook where the passed-in zerolog.Event contains a map[string]interface{} of every piece of context ever added to it - but of course that's antithetical to how zerolog was designed.

The other option, of course, is to wrap the Zerolog invocation in a closure of my own, but that entails updating thousands of call sites and breaking muscle memory for a lot of people:

func LogError(r *http.Request, err error) {
    hlog.FromRequest(r).Err(err).Send()

    attributes := someHelper.GetTrace(r)
    externalerrors.Submit(err, attributes)
}
flexoid commented 3 years ago

We have nearly the same use case as @Deiz - we want to send logged errors to an external error tracking system.

Currently we are using work-around solution by implementing custom io.Writer implementation for zerolog.MultiLevelWriter. The problem with this approach is that writer has only access to raw log string, so we need to parse all information back from it. And something can be easily lost, like chain of errors or a stack trace (of course if we are not going to put everything in a log just to be able to parse again).

Having original error along with a log context available somewhere would allow more straightforward integrations. Feels like zerolog lacks a mechanism for such kind of extensions.

Just for example, zap allows to create zapcore.Core implementations, which has a way to capture any information from log fields and use it for later processing. See how elastic-apm implements it to send errors to APM