Create Test Case with Example of Formatting

[mozts2005]

Example failing test case for what I think may be a bug.

I am trying to setup structured logging. when I add a template parameter to messages and call Nlog directly the log files show the message as I would expect. However when called using NLog.Extensions.Logging the message never gets formatted; only ToString is called.
Expected:

_logger.LogDebug("message with id and {@ObjectParameter} parameters", new TestObject());

{"level": "DEBUG", "message": "message with id and {"TestValue": "This is the test value"}  parameters" }

Actual:

{"level": "DEBUG", "message": "message with id and NLog.Extensions.Logging.Tests.LoggerTests+TestObject parameters" }

I am more then willing to help with a fix if this is really a bug and not me misunderstanding.

[snakefoot]

Think we need a new parameter for NLog.Extensions.Logging, whether it should use the NLog-MesageFormatter instead of the Microsoft.Extension.Logging-MessageFormatter.

Can see that Serilog attempts to take over the formatting if it is able to capture all parameters and the original messagetemplate (Thus supporting @ and $)

[304NotModified]

Think we need a new parameter for NLog.Extensions.Logging, whether it should use the NLog-MesageFormatter instead of the Microsoft.Extension.Logging-MessageFormatter.

I think the root cause is here that we still use NLogLoggerProvider and not NLogLoggerFactory (higher level)

see NLog/NLog.Web#235

[mozts2005]

@snakefoot I would agree that we need to use the NLog-MesageFormatter more like Serilog but I don't think we need a new parameter.

@304NotModified I may not understand your point.

I have an idea on how this could be fixed to work like Serilog and if we have structured logging turned on using the current parameters to then not call the Microsoft formatter but call the Nlog one. I think I could make the change in just a few lines.

[snakefoot]

@304NotModified Well I can fix the issue in the NLogLogger, by extracting from LogState (Just like Serilog). But I guess it would give much better performance if we could skip the Microsoft-Message-Parser, and just forward message and parameters directly to the Nlog-Message-Parser.

[mozts2005]

@304NotModified and @snakefoot please look closely at the commits. I had to update one other test to account for a formatting change that add quotes to the output.

[snakefoot]

@304NotModified Tried looking at the code, and avoiding the Microsoft-Message-Parser means avoid the Microsoft-Log-Extensions (See how they convert to FormattedLogValues)

https://github.com/aspnet/Logging/blob/dev/src/Microsoft.Extensions.Logging.Abstractions/LoggerExtensions.cs

Don't think this is an option, so I guess it means double parsing. But we can skip the double rendering.

[snakefoot]

@mozts2005 Good start. Please make sure not to call the formatter-method, when deciding to use the NLog-renderer:

var message = formatter(state, exception);

Right now the NLog.Extension.Logging will only attempt to perform rendering if non-positional string-format. But actually the Microsoft-Message-Renderer doesn't support position-string-format.

Should the NLog-Message-Template renderer also be used for the following?

Log("My Details:{0}Name={1}{0}EyeColor={2}{0}", Environment.NewLine, "Snakefoot", "Yellow");

[snakefoot]

If I should implement this, then I would probably do it like this:

1. Introduce a new setting call RenderMessageTemplates (default=false), when enabled it always uses the NLog-Message-Formatter (when possible).
2. If one of the captured parameters has a non-default CaptureType, then it automatically uses the NLog-Message-Formatter (Even if RenderMessageTemplates = false).

The final solution should only render the log-message once, not twice with both renderers.

[mozts2005]

@snakefoot I agree with you. With one change as that project is still a release candidate. that would be to make RenderMessageTemplates default to true.
I am working on this set of changes for my next commit;
and working to make sure that the positional example you supplied will still work as well.

[304NotModified]

I think we need some changes for the structured logging in combination with MS's syntax.

Will move that to 2.0

[snakefoot]

Have created PR #213

[mozts2005]

@snakefoot thanks for the work with #213 that is a way better way to do this.

Closing my PR as it is not needed anymore.