Skip to content
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

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

Merged
merged 6 commits into from
Jul 21, 2024

Conversation

liammclennan
Copy link
Contributor

See #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.

Copy link
Member

@nblumhardt nblumhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a binary breaking change, we'll need to rev the major version of the sink.

Seeing more use of JsonValueFormatter.WriteQuotedJsonString suggests it'll be worth switching to use Utf8JsonWriter now - perhaps by first adding support upstream in Serilog's JsonValueFormatter - I'll dig in when I have a chance :)

@nblumhardt nblumhardt merged commit 198499c into datalust:dev Jul 21, 2024
1 check passed
@nblumhardt nblumhardt changed the title Custom format provider Accept a non-default IFormatProvider and use it for message rendering Jul 21, 2024
@liammclennan liammclennan deleted the custom-format-provider branch July 21, 2024 22:48
@nblumhardt
Copy link
Member

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 (datalust/seq-tickets#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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants