IDestructuringPolicy for Exceptions

[alex-y-su]

As improvement it would be nice to run Exception serialization through collection of registered IDestructuringPolicy or special sort of destructing handlers. Often we have business exception with extra fields which should be written: like ticked id or current user identity. Now we have to write this code to run business exception over destructing policies: Logger.Error("An error intro {@exception}", exception); Let me know if this sounds reasonable and I will try to implement it myself as a pull request.

[Jaben]

I :+1: this... often custom exception carry valuable information.

[nblumhardt]

To make the change in the core (e.g. by changing the type of LogEvent.Exception to a LogEventPropertyValue) we'd end up breaking sinks, which (given the number of them) is infeasible right now I think.

The current conversion of an exception to a string happens in OutputProperties.cs which is only used by text-based sinks. The stringification in other sinks happens e.g. when converting to JSON.

That does then open up the possibility of making the change on the sinks themselves, or for those that use it, on the JsonFormatter class. It's not a hugely requested feature, so maybe configuration like this would work?

.WriteTo(new SomeSink(
    ...,
    new JsonFormatter
    {
        ExceptionDestructuring = new SomePolicy()
    })

This approach would also help toggle features like #113 and #107.

Sinks do tend to use JsonFormatter in different ways, so thinking carefully through its API to work out how to fit this in is probably the next step.

• Should sinks accept a JsonFormatter, or is IJsonFormatter a better bet?
• How should sinks control their preference e.g. for flattening the JSON by excluding enclosing braces? (Currently a JsonFormatter constructor param)
• Sinks like the one for Raven use dictionary structures as a mid-point between LogEvents and JSON. Can this be considered in the design? (I.e. is IJsonFormatter the wrong thing, should there be the option of a two-step process?)
• Sinks like Seq's customise the JSON output (with a completely separate formatter) but would do well to be supported consistently by this too - is that where the event -> dictionaries -> JSON pipeline fits in, or should they just mimic the implementation pattern?

Lots to consider, but hopefully part of a worthwhile change...

[Jaben]

What about a simple addition of an exception property that is deconstructed? Exception specific field stays the same, but (maybe optionally?) the whole exception gets dumped e.g.:

Log.Error(exception, "{@exception}", exception);

That brings up another question: any way of configuring an interceptor in Serilog? Take for example the "BufferedAppender" that Log4Net has: if a fatal is logged, X previous logs (including warning, debug, etc) are pushed to the sink leading up to the failure. (ref: https://logging.apache.org/log4net/release/sdk/log4net.Appender.BufferingForwardingAppender.html)

Seems like this feature could be a sort of additional configurable add-on, e.g.:

Log.Logger = new LoggerConfiguration()                
                .InterceptAllWith.ExceptionDeconstructor()                
                .WriteTo.ColoredConsole()
                .WriteTo.InterceptWith.BufferedForwarding(last:50, minLevel: LogEventLevel.Debug).EventLog()
                .CreateLogger();

Not sure about the InterceptWith API, since it would need to be attached to the LoggerSinkConfiguration.

[nblumhardt]

@Jaben I think "@exception" is the workaround the OP is using; in the short term that seems like the best approach.

The ExceptionDeconstructor might be able to be implemented using an ILogEventEnricher today:

 .Enrich.With<DestructuredExceptionEnricher>()

Then, in the enricher, you'd do:

if (event.Exception != null && event.Exception is ...) // Just interesting ones?
  event.AddPropertyIfAbsent("InterestingException",
    propertyValueFactory.Create(event.Exception, destructureObjects: true);

(Forgetting the types and method names here, but seems like a workable direction to go!)

The buffering idea was described as "transactional logging" by @sandcastle when I first heard of it; starting out with a sink-wrapper sounds like a good option (will have to check out @mivano's cut! :))

HTH.

[Jaben]

@nblumhardt The enricher would work good for my needs. Also, yeah, I'm not interested in all exceptions, really just stuff in our namespace.

Sorry about the transactional logging tangent. :)

[nblumhardt]

I think because this one seems to be a bit of a niche case I'll close this for the time being - the enrichment mechanism was designed for "hackability" of this kind so seems to cover it pretty well. Feel free to reopen if there's more to be done. Cheers!

[nblumhardt]

A basic implementation example: https://gist.github.com/nblumhardt/dddaa2139bbf4b561fa7

[nblumhardt]

Reopening this to carry through some of the discussion to "v2" - it turns out there are a lot of interesting things that can be done with exceptions, and Serilog's currently a bit inflexible about them.

[nblumhardt]

Some time on, it seems like the "enrichment"-based approach to this is sufficient and widely adopted.

[RehanSaeed]

There is a very good implementation I discovered here. However, you had to choose between creating custom exception destructurers and a reflection based version.

I improved it slightly so it can use a custom destructurer if one is available or otherwise fallback on the reflection based one. This should give better performance. https://gist.github.com/RehanSaeed/e06374c06d06312b836d

Perhaps this should be a new Serilog Git repository? Ideally, this should be built in and enabled by default.

[nblumhardt]

Great gist! If you are interested in shipping and maintaining it, perhaps we could add a repo under https://github.com/destructurama ?

[RehanSaeed]

Sure, why not. What shall I call it and where shall I put it.

[RehanSaeed]

Started here.

[IanYates]

@RehanSaeed - nice :)