Use MappedDiagnosticsLogicalContext in NLog provider

[andrey-noskov]

Mapped Diagnostic Logical Context is the async version of the Mapped Diagnostics Context - a logical call context structure that keeps a dictionary of strings and provides methods to output them in layouts. Allows for maintaining state across asynchronous tasks and call contexts.

The PR changes OpenMDC method to use the NLog MDLC instead of non-async friendly MDC restoring feature parity with Log4Net provider.
Note: it is a breaking change as it requires app owners to update NLog config

@damianh please have a look

[snakefoot]

Would prefer usage of NDLC + MDLC to be introduced when NLog 4.6 is ready. Includes better handling of non-string-objects: NLog/NLog#2857

[andrey-noskov]

@snakefoot can you elaborate a bit more on why you want to postpone the change? nothing blocks us from using Nlog MDLC piece today, right?

[snakefoot]

nothing blocks us from using Nlog MDLC piece today, right?

Well right now objects are saved to thread-local-storage. With this change it will be CallContext. Different behavior there. Try to read the link I provided just before.

[snakefoot]

Created #181 for you

[andrey-noskov]

yes, it is a breaking change as I noted above.
what exactly do you suggest? NLog MDLC interface is already there and we need it - what is the easiest way to expose it to end customers? apply the patch and change the behaviour? expose another set of API in liblog?

[snakefoot]

I guess it is for @damianh do decide. Since I'm not sure of the release date for NLog 4.6.

But AppDomains on IIS can throw random exception for complex objects saved to CallContext without ObjectHandle.

[andrey-noskov]

@damianh what do you think?

[damianh]

To be honest, I don't know. I need to study this and #179 in more detail. If feels like there be devils-in-the-details and my position it be conservative with changes in LibLog (breaks will have far reaching impact).

Will take a look at in due course as I don't feel (argue if I am wrong) that this is high priority / critical fix / etc.

[snakefoot]

#181 includes this change. But it is only activated when running NLog 4.6 (Has protection from complex objects being transfered between Appdomain using CallContext)

[andrey-noskov]

wouldn't it make sense to add support for NLog < 4.6 as inability to use MappedDiagnosticsLogicalContext from liblog affects all my projects :-\

[snakefoot]

@AndreyNS I don't know how restrictive the LibLog is against breaking changes. But suddenly changing from MDC to MDLC will probably cause some headache for host applications. Especially when having multiple libraries that have different versions of LibLog.

That is why I wanted to protect the host applications against this change, but at the same time I want it to be the new default.

Not sure if it is possible to implement a parameter for the NLog-provider registration, so one can override the behavior for those who wants to kick-start using MDLC.

[ericnewton76]

@snakefoot what issues do you think come about from switching from MDC to MDLC? After pondering this for a while, I can't think of any.

[snakefoot]

@ericnewton76 When making calls from one AppDomain to another then all objects in the MDLC must be serializable. This PR wants to add random objects into the MDLC. Having non-serializable objects in the MDLC will cause "spurious" exceptions when one AppDomain calls into another AppDomain (Ex. .NET ReportGenerator)

NLog 4.6 will add extra protection to the MDLC prevent random objects from raising exceptions. Allowing random objects in the MDLC. PR #181 will make LibLog use the MDLC when it detects NLog 4.6.

Also changing from MDC to MDLC will be a breaking change, and if having multiple libraries that uses different versions of LibLog then you will get different behavior (Some logging to MDC, some logging to MDLC). By adding the NLog 4.6 requirement, then host-applications can continue working by not upgrading to NLog 4.6 before all their required libraries has upgraded to the latest LibLog.

[snakefoot]

It is on purpose that NDLC + MDLC is not used for NLog 4.5 (Will work with Nlog 4.6). See also #228 (comment)

@damianh Guess this PR can be closed

[snakefoot]

@AndreyNS NLog 4.6 has now been released: https://www.nuget.org/packages/NLog.

Created #233 that updates the LibLog Test project.

So just update to NLog 4.6 and all your worries are over