datalust / serilog-sinks-seq

A Serilog sink that writes events to the Seq structured log server
https://datalust.co/seq
Apache License 2.0
225 stars 50 forks source link

Accept a non-default `IFormatProvider` and use it for message rendering #228

Closed liammclennan closed 1 month ago

liammclennan commented 2 months ago

See https://github.com/datalust/serilog-sinks-seq/issues/150

This PR allows an IFormatProvider to be supplied during the sinks configuration. If supplied the format provider is used to format message tokens.

The PropertiesFormatCorrectlyForTheFormatProvider test is a reasonable summary of the behavior change.

Note that if an IFormatProvider is specified then the message will be rendered and sent in the payload.

nblumhardt commented 1 month ago

Unfortunately, this won't have the desired effect server-side for tokens that don't carry format specifiers.

E.g. Log.Information("{Date}", DateTime.Today) will collect a culture-specific rendering of the entire message, but the {Date} value shown by the server will still be the underlying ISO-8601 property value. This will make searching the log awkward, because text search matches won't line up with what the Events screen displays.

A similar issue prevents us from using this trick to pre-render messages containing dotted property names (https://github.com/datalust/seq-tickets/issues/2240).

I think until we have a complete end-to-end solution, we should roll back message formatting, but keep format provider support for tokens with explicit format strings. The observable result will be the same, with today's Seq, and although some formatting won't be culture-specific, there won't be a confusing mismatch between what's displayed and what's searchable.

Will send a PR :)