serilog-extensions-logging
serilog-extensions-logging copied to clipboard
serilog
Log values corresponding to empty bracers
When HelixToolkit switched to Microsoft.Extensions.Logging.Abstractions I noticed that the debug sink handled message templates with empty bracers while Serilog didn't.
I.e. the following logging statement
logger.LogInformation("Adapter Index = {}", adapterIndex);
becomes
[10:17:35][INF][HelixToolkit.SharpDX.Core.EffectsManager] Adapter Index = {}
when logging through Serilog but when formatting the string with the Func<TState, Exception, string> formatter (passed to ILogger.Log<TState>()) it becomes
Adapter Index = 0
This merge request adds integers in the empty braces to fix this. I suspect that Helix Toolkit isn't the only library to use Microsoft.Extensions.Logging.Abstractions with this kind of templates, hence the pull request here instead of in Helix Toolkit.
Howdy! This seems like it would be a bug in Microsoft.Extensions.Logging; the {} syntax isn't valid in either .NET format strings, or message templates as far as I'm aware. Do you have any idea where this usage might have come from? Thanks!
Howdy! This seems like it would be a bug in Microsoft.Extensions.Logging; the
{}syntax isn't valid in either .NET format strings, or message templates as far as I'm aware. Do you have any idea where this usage might have come from? Thanks!
I have no idea. First saw it in Helix Toolkit. I also checked the tests for formatting in Microsoft.Extensions.Logging and there are no tests for this case at all. @holance do you know the origin of the syntax with {} instead of {0} or {variableName}?
@eriove I tested it with NLog and {} is working fine. Also I am writing too many c++ recently and probably got confused with the fmt in c++. If you feels like {} is an incorrect usage, I can definitely change them in helixtoolkit.
I do not have a strong opinion either way. If the MS implementation supports it is isn't unlikely that people will expect it to work with {}. At the same time this it isn't in the documentation and adds an overhead (at least in the way I implemented it here).
I think we'd just be reproducing a MEL template parser quirk/bug by implementing this here. Helix Toolkit will definitely get better compatibility with the rest of the ecosystem by using named (or at worst, positional) parameters for message template holes, so I'd recommend making the change over there.
HelixToolkit has now added arguments between the bracers so I'm closing this here.
Tanks @holance!