-
Notifications
You must be signed in to change notification settings - Fork 795
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
MessageTemplateParser
refactor incl. experiemental .
support
#2063
MessageTemplateParser
refactor incl. experiemental .
support
#2063
Conversation
…behavior of some invalid templates
(Surprising actually, that this late in the game a couple of simple |
Heads-up also @Mpdreamz - I know that |
Updated the PR to consider |
MessageTemplateParser
refactor incl. experiemental .
/-
supportMessageTemplateParser
refactor incl. experiemental .
support
/lgtm Thanks for taking the first step in enabling easier log schema adherence! I'm hoping I can find a chance soon to test this but no promises. |
Love this! Our ECS field based setters can handle both the original dotted key names as well as PascalCased ones. |
Tested with an example app going through an ILogger:
and got the expected output in DataDog: Not exhaustive testing but at least an initial indication that I'm getting the desired result. |
Also verified simple console and file logging works as expected. Sending to DataDog is using the |
This PR came out of, and includes, #1384. You can read about the reasons for considering
.
in message template property names, and the various caveats/disadvantages on that thread. The PR enables wider investigation and experimentation, and there is currently no guarantee that this will ever become a first-class, supported Serilog feature.By setting the experimental and unsupported
AppContext
switch at program start-up:Placeholders such as
{http.request.method}
will (should!) be accepted in message and output templates. Previously, these templates would have been regarded as invalid.Dotted placeholders will be interpreted as literal property names, e.g.
"http.request.method"
, and not structured object paths like{http: {request: {method: ...}}}
.In the process of investigating this I cleaned up some message template parsing tests; these are so old and could use a lot more work, especially translating long multi-assert tests into data-driven
[Theory]
s.MessageTemplateParser
itself has also been unchanged for a long time. It could benefit from porting to aSpan<T>
-based model, but even improving our use of vanillastring
methods yielded a significant performance boost. Long runs of literal text (very common in real-world logs) now allocate dramatically less temporary memory during parsing.Before:
After:
Results were similar on .NET 6 on ARM, though much slower across the board.
The additional eight-byte allocation in the baseline
EmptyTemplate
case is the change inMessageTemplate
memory layout due to the additionalbool _acceptDottedPropertyNames
member.