serilog / serilog-extensions-logging

Serilog provider for Microsoft.Extensions.Logging
Apache License 2.0
307 stars 97 forks source link

Support `ValueTuple<string, object?>` as state of scope. #232

Closed jimbojim1997 closed 3 months ago

jimbojim1997 commented 7 months ago

Change for issue #186.

When calling ILogger.BeginScope<TState>(TState state) TState can now be a ValueTuple<string, object?>, Item1 is used as the property name and Item2 is used as the property value.

using (_logger.BeginScope(("TransactionId", (object?)12345)) {
    _logger.LogInformation("Transaction completed in {DurationMs}ms...", 30);
}
// Example JSON output:
// {
//  "@t":"2020-10-29T19:05:56.4176816Z",
//  "@m":"Completed in 30ms...",
//  "@i":"51812baa",
//  "DurationMs":30,
//  "SourceContext":"SomeNamespace.SomeService",
//  "TransactionId": 12345
// }
nblumhardt commented 7 months ago

Thanks @jimbojim1997! This looks good, even if the need for the second tuple member to be object? is a bit awkward, it's an improvement on the dictionary approach and we might open it up to arbitrary tuple types later on :+1:

8.0.0 is going out in the next ~24 hours so it's too late for this change to make that release, unfortunately. We'll be able to ship it shortly afterwards, though.

nblumhardt commented 3 months ago

I think the checks for OriginalFormatPropertyName can be eliminated on all of the paths except IEnumerable<...>, since only a specifically-typed state object will carry these. Given I dropped the ball on this PR over Christmas though, I'll merge now to avoid any busywork around conflicts etc. :-)

Sorry about the long delay @jimbojim1997, thanks for this!