LogManager.Setup() - LoadConfigurationFromAppSettings

[snakefoot]

Now you can do this in NetCore3:

var logger = NLog.LogManager.Setup().LoadConfigurationFromAppSettings().GetCurrentClassLogger();

And it replaces this for NetCore3 (Could consider making it obsolete):

var logger = NLog.Web.NLogBuilder.ConfigureNLog("nlog.config").GetCurrentClassLogger();

And also configures ${configsetting} and also checks appsetting.json if it contains a NLog-section with NLog config. But if none found, then NLog automatic load of NLog.config takes over.

For NetCore2 then one can do this instead of NLogBuilder.ConfigureNLog:

var builder = new ConfigurationBuilder()
                .SetBasePath(basePath ?? Directory.GetCurrentDirectory())
                .AddJsonFile("appsettings.json", optional: false, reloadOnChange: true)
                .AddJsonFile($"appsettings.{environment}.json", optional: true)
                .AddEnvironmentVariables();
var config = builder.Build();
var logger = LogManager.Setup()
                       .RegisterNLogWeb(config)
                       .LoadConfigurationFromFile("nlog.config")
                       .GetCurrentClassLogger();

This PR is a proof-of-concept, and should be delayed until NLog.Extension.Logging has been released with NLog 4.7 (Or maybe NLog 4.7.1)

[snakefoot]

Notice this is proof-on-concept and should not be merged, since it will bump to NLog 4.7

NLog.Extensions.Logging ver. 1.6.3 should be released first. Maybe after NLog 4.7.1 has been released. And then NLog.Web.AspNetCore can update its dependency, and then this can be merged.

[snakefoot]

I have my doubts about having two extension methods with the same name but different namespaces:

• NLog.Web.LoadNLogConfigFromSection
• NLog.Extensions.Logging.LoadNLogConfigFromSection

Both methods does the same, but NLog.Web has the additional logic that it automatically registers NLog.Web-extensions before loading the NLogConfig.

But I think confusion in tutorials may occur, and also confusion when right-clicking the method to fix missing usings (Will show two possible namespaces to include).

But the same issue is also present with AddNLog and UseNLog, so I guess it is ok.

[304NotModified]

Have to think about it. It could be confusing, maybe there is a better solution.

[304NotModified]

What about LoadNLogConfigFromSectionWithWebIntegration? (so something like that?)

[snakefoot]

What about LoadNLogConfigFromSectionWithWebIntegration? (so something like that?)

Sounds like an awful name. Then prefer the name collission that also exists with AddNLog and UseNLog

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@ef56917). Click here to learn what that means.
The diff coverage is n/a.

[snakefoot]

What about doing this in NetCore2 (Introducing SetupNLogWeb() and instead reuse LoadNLogConfigFromSection from NLog.Extension.Logging)

var logger = NLog.LogManager.Setup().SetupNLogWeb().LoadNLogConfigFromSection(config).LogFactory.GetCurrentClassLogger();

Of course this means people have to remember two method-calls when in "legacy-mode". Or just want it flexible. Could also consider removing SetupNLogWeb and just tell people they should do this (Official "wordy" approach):

var logger = NLog.LogManager.Setup().SetupExtensions(e => e.RegisterNLogWeb()).LoadNLogConfigFromSection(config).LogFactory.GetCurrentClassLogger();

When using NetCore3 (where all bells and whistles are available):

var logger = NLog.LogManager.Setup().LoadNLogConfigFromAppSettings().LogFactory.GetCurrentClassLogger();

[304NotModified]

Sounds like an awful name. Then prefer the name collission that also exists with AddNLog and UseNLog

Agree, but that even better to me than a collision ;) (no joke).

[304NotModified]

What about doing this in NetCore2 (Introducing SetupNLogWeb() and instead reuse LoadNLogConfigFromSection from NLog.Extension.Logging)

Sounds good! (update: or maybe AddNLogWeb, to get rid of 2 setups?)

PS: for readability, I prefer newlines in those examples

So instead of

var logger = NLog.LogManager.Setup(). SetupNLogWeb().LoadNLogConfigFromSection(config).LogFactory.GetCurrentClassLogger();

this:

var logger = NLog.LogManager
                 .Setup()
                 .SetupNLogWeb()
                 .LoadNLogConfigFromSection(config)
                 .LogFactory
                 .GetCurrentClassLogger();

PS2:

maybe stupid question, but we can't get rid if Setup() and .LogFactory? (it's a bit hard for an end-user I guess)

[snakefoot]

Sounds good! (update: or maybe AddNLogWeb, to get rid of 2 setups?)

AddNLogWeb also works for me. Unsure if it the name collides too much with AddNLog for ILoggerBuilder. Could also call it EnableNLogWeb or IncludeNLogWeb.

maybe stupid question, but we can't get rid if Setup() and .LogFactory? (it's a bit hard for an end-user I guess)

We can easily add a GetCurrentClassLogger()-extension-method to the ISetupBuilder-interface.

Not sure what you mean about removing Setup(), can you show some code for implementing this?

[snakefoot]

@304NotModified Have added unit-tests for the new method LoadConfigurationFromAppSettings(), but it seems code-coverage is not seeing them.

[304NotModified]

Thanks

I think that coverage is only for one platform (merge same code base for multiple platforms is always difficult, also in terms of performance)

[304NotModified]

Is this ready to review? (Unexpected force pushes :))

[snakefoot]

Is this ready to review? (Unexpected force pushes :))

Yes ready now. Just wanted to add AutoLoadAssemblies(false) to RegisterNLogWeb

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

100.0% Coverage
0.0% Duplication

[304NotModified]

Thanks! Merged!