net-commons / common-logging

A portable logging abstraction for .NET
http://net-commons.github.io/common-logging
Apache License 2.0
649 stars 205 forks source link

Common.Logging.NLog45 with support for structured logging #176

Open snakefoot opened 5 years ago

snakefoot commented 5 years ago

And MDLC / NDLC

Attached custom build of nuget-package Common.Logging.NLog45 (Just remove .zip file extension):

Updated build -> Common.Logging.NLog45.3.4.2.nupkg.zip

It is not built with strong-name. This requires an official build with the "secret" snk-file. But for NetCore / NetStandard then strong-name not required.

You can use it like this in app.config:

<?xml version="1.0" encoding="utf-8"?>
<configuration>
<configSections>
  <section name="nlog" type="NLog.Config.ConfigSectionHandler, NLog" requirePermission="false" />
  <sectionGroup name="common">
    <section name="logging" type="Common.Logging.ConfigurationSectionHandler, Common.Logging" requirePermission="false" />
  </sectionGroup>
</configSections>
<startup><supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.5"/></startup>
<common>
  <logging>
    <factoryAdapter type="Common.Logging.NLog.NLogLoggerFactoryAdapter, Common.Logging.NLog45">
      <arg key="configType" value="INLINE" />
    </factoryAdapter>
  </logging>
</common>
  <nlog>
    <targets>
      <target name="console" type="Console" layout="${message}" />
    </targets>
    <rules>
      <logger name="*" minlevel="Debug" writeTo="console" />
    </rules>
  </nlog>
</configuration>

Or use it like this in NetCore appsettings.json together with binding-logic (And NLog.config):

{
  "LogConfiguration": {
    "factoryAdapter": {
      "type": "Common.Logging.NLog.NLogLoggerFactoryAdapter, Common.Logging.NLog45",
      "arguments": {
        "configType": "INLINE",
      }
    }
  }
}
using Common.Logging;
using Common.Logging.Configuration;
using Microsoft.Extensions.Configuration;

IConfiguration config = new ConfigurationBuilder()
        .AddJsonFile("appsettings.json", optional: true, reloadOnChange: true)
        .Build();

LogConfiguration logConfiguration = new LogConfiguration();
config.GetSection("LogConfiguration").Bind(logConfiguration);
LogManager.Configure(logConfiguration);

Or just do it from code alone without any config-file:

var properties = new System.Collections.Specialized.NameValueCollection();
properties["configType"] = "INLINE";
Common.Logging.LogManager.Adapter = new NLogFactoryAdapter(properties);

Previous Build without proper callsite support -> Common.Logging.NLog45.3.4.1.nupkg.zip

snakefoot commented 5 years ago

@sbohlen Added the project to Common.Logging.2010.sln and cloned from NLog444 (No Visual Studio upgrade)

304NotModified commented 5 years ago

nice @snakefoot !

snakefoot commented 5 years ago

@sbohlen Tried upgrading to VS2017-Solution together with new VS2017-csproj-format (Support NetCore)

Mies75 commented 5 years ago

Please please please, will this make var clientId = 100; Logger.InfoFormat("A test with client id {ClientId}", clientId.ToString());

display

A test with client id 100

instead of

A test with client id {0}

?!

snakefoot commented 5 years ago

Yes it will. But because you have done ClientId.ToString then NLog will put quotes around "100"

Mies75 commented 5 years ago

Great!

Only did the ToString() because I thought I was preventing boxing, but yeah.. I see the quotes now. May I ask for your timeline to publish the new version?

Cheers

snakefoot commented 5 years ago

I can attach a nuget package for the build. That you can upload to your own nuget-server or put in local-package-folder

Not sure when this PR will be consumed and published by the official project. Not the owner of this project.

Mies75 commented 5 years ago

Please, that will help me sort this before my stackify retrace trial ends 🥇 .

Ps, I never saw attachments in GitHub, so, where will this attachment appear?

snakefoot commented 5 years ago

@Mies75 Not able to work on spare-time-projects 24/7. But I have now updated original post with an attachment to a custom build of Common.Logging.NLog45 (Not strong-named)

Mies75 commented 5 years ago

Awesome effort, trying it now!

304NotModified commented 5 years ago

bump @sbohlen

any news @Mies75 ?

Mies75 commented 5 years ago

Must have replied on Stack Overflow instead of GitHub, sorry for that. It works flawlessy, good job!

Mies75 commented 5 years ago

@sbohlen please merge and release this, thanks!

@sbohlen, do you have time to do the merge and publish it? Then we can remove the custom Common.Logging.NLog45 from our own NuGet repository!

snakefoot commented 5 years ago

Updated the PR to include better support for callsite-handling. Thanks to @Defee

Mies75 commented 5 years ago

I am running some perfview tests on our servers, to clear up the exception stack.

Now I run into this: nlog.messagetemplates.templaterenderer,render -> indexoutofrangeexception On a: logger.InfoFormat("End job {QuartzJobName}/{QuartzJobInstance}: ran for {QuartzJobRuntime}ms", context.JobDetail.Key.Name, context.FireInstanceId, context.JobRunTime.Milliseconds)

Is this related to this PR?

snakefoot commented 5 years ago

Are you running the latest build that I have just updated the initial post (ver 3.4.2) ?

Mies75 commented 5 years ago

It's our PRD system, running your initial custom build. Is your new PR production worthy in your eyes? Or are you planning on some additional coding?

Update: Pushed the nwe version to our repo, going to run some tests.

Update 2: Tests ran successful, but mind you: wasn't able to reproduce the problem on my dev box.

snakefoot commented 5 years ago

What version of NLog? Remember never to use throwExceptions=true in production.

The InfoFormat call looks valid. So not sure if you have found the one that actually failed.

Mies75 commented 5 years ago

NLog 4.5.11 for the affected system.

I am 100% sure that is the line that was failing, and I cannot fathom why. Also tried to make those arguments NULL, didn't fail..

We never user throwExceptions in the nlog.config. When we are experiencing problems we use internal logging.

snakefoot commented 5 years ago

That version of NLog should work just fine. If you are able to reproduce the case then please let me know.

santiagoIT commented 5 years ago

Any time frame when this will be merged?

anreton commented 3 years ago

Very important and useful changes, is there any news?