Skip to content
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

Enable wrapping of an existing NLog.Logger as a Microsoft.Extensions.Logging.ILogger #455

Closed
swlasse opened this issue Nov 16, 2020 · 6 comments

Comments

@swlasse
Copy link

swlasse commented Nov 16, 2020

Type (choose one):

  • Feature request

NLog version: 4.7.5
NLog.Extensions.Logging version: 1.6.5
NLog.Web.AspNetCore version: not used

Platform: .NET Core 3.1

Based on https://github.com/NLog/NLog/wiki/EventProperties-Layout-Renderer#logger-withproperty-or-setproperty I am creating a logger with event properties specified during logger creation:

// Creates an NLog.Logger
NLog.Logger propertyLogger = LogManager.GetCurrentClassLogger(typeof(MyLogger)).WithProperty("key1", "val1");

In my code base, I am already using the Microsoft.Extensions.Logging.ILogger<T> abstraction, and I would like to use this NLog powered propertyLogger as an instance implementing this MEL.ILogger interface (to keep things clean). Example:

// I am looking for something like this
Microsoft.Extensions.Logging.ILogger melLogger = NLogLogger.Wrap(propertyLogger);

I have been looking around in the NLog.Extensions.Logging project to see if I could find a way to wrap an existing instance of an NLog.Logger as an MEL.ILogger, but I have not had any luck. The closest I get is the NLogLogger type which is marked as internal and therefore cannot be instantiated "outside".

Questions:

A) Is is possible to wrap an existing instance of an NLog.Logger as an MEL.ILogger today? If so, how should that be done?

B) If not, would it be an option for you to provide framework support for this? I am not sure if it is as simple as updating the access modifier on NLogLogger from internal to public? Probably not :)

Thanks in advance for your help.

Best regards,
Lasse ( a 10+ years happy user of NLog :)

@snakefoot
Copy link
Contributor

snakefoot commented Nov 16, 2020

Sounds a lot like this thread: dotnet/runtime#35995

Have you read this wiki-page: https://github.com/NLog/NLog.Extensions.Logging/wiki/NLog-properties-with-Microsoft-Extension-Logging

Maybe this is what you are looking for: https://www.nuget.org/packages/XeonApps.Extensions.Logging.WithProperty/

@swlasse
Copy link
Author

swlasse commented Nov 17, 2020

Thanks for your quick response @snakefoot. This is indeed related to the referenced dotnet issue.

I had been reading the article https://github.com/NLog/NLog.Extensions.Logging/wiki/NLog-properties-with-Microsoft-Extension-Logging a couple of times, but I was so focused on trying to understand why NLog did not provide a way of wrapping an NLog.Logger as an MEL.ILogger, that I missed MyLogEvent2 is in fact an MEL.ILogger giving you the option to add event properties. Thanks for pointing me in that direction again.

The work done here looks very good: https://github.com/viktor-nikolaev/XeonApps.Extensions.Logging.WithProperty - I see that @304NotModified has contributed to it as well. Are there any plans for the NLog project to provide a similar abstraction? Would be nice to see in the framework.

Thanks again for your help.

@snakefoot
Copy link
Contributor

@swlasse Since these extension methods are not really NLog-specific then it might not make sense to have them in a NLog-specific-package.

Also dragging random code into the NLog-project, means that someone has to maintain and support it. Rather scarce with developer resources on the NLog-project.

@swlasse
Copy link
Author

swlasse commented Nov 17, 2020

@snakefoot I totally understand your reasoning for not taking this in just out-of-the-box, which is fair :) I guess it is also a question of where you want to draw the line. I mean, why should you invest heavily in supporting extensions on top of the MEL.ILogger when your main project is NLog and the features it delivers. I think that is an honest question, and I understand if your primary focus is on NLog specific functionality.

The reason I initially created this issue is because I am in a project where I am trying to standardize on logging. The MEL.ILogger seems to be a "new standard" supported by Microsoft, which is fine, but there are areas where more rich logging frameworks like NLog is needed to do the job. And it is in this "bridge" that I am looking for solid extensions, that allow me to close the gap between the MEL.ILogger and NLog - where lack of event properties support has been one of them.

On a final note; should I find time some day, would you be interested in accepting a PR with a solution based on https://github.com/viktor-nikolaev/XeonApps.Extensions.Logging.WithProperty or is your previous answer a "clear" no go on this?

@snakefoot
Copy link
Contributor

@swlasse Pull-requests from eager hands are always welcome. There is some dependency issue that might annoy some, as the NLog.Extension.Logging caries some other baggage like configuration-loading. Maybe it should be an isolated package just like NLog.Extension.Hosting? (All business-logic libraries that wants these extension method suddenly all have to depend on this package). Then there is issue about what namespace to put such extension-methods.

@swlasse
Copy link
Author

swlasse commented Nov 18, 2020

@snakefoot Alright, that sounds good. Yeah, naming is hard! If I get the time, I will see what I can come up with. I am glad for the input you have provided on this issue, and I believe I have the information I need to move on. Thanks again.

@swlasse swlasse closed this as completed Nov 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants