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

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

snakefoot
Copy link

@snakefoot snakefoot commented Jan 21, 2019

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
Copy link
Author

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

@304NotModified
Copy link
Contributor

nice @snakefoot !

@snakefoot
Copy link
Author

snakefoot commented Jan 22, 2019

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

@Mies75
Copy link

Mies75 commented Jan 23, 2019

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
Copy link
Author

snakefoot commented Jan 23, 2019 via email

@Mies75
Copy link

Mies75 commented Jan 23, 2019

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
Copy link
Author

snakefoot commented Jan 23, 2019 via email

@Mies75
Copy link

Mies75 commented Jan 23, 2019

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
Copy link
Author

snakefoot commented Jan 23, 2019

@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
Copy link

Mies75 commented Jan 28, 2019

Awesome effort, trying it now!

@304NotModified
Copy link
Contributor

bump @sbohlen

any news @Mies75 ?

@Mies75
Copy link

Mies75 commented Mar 15, 2019

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

Copy link
Contributor

@304NotModified 304NotModified left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbohlen please merge and release this, thanks!

@Mies75
Copy link

Mies75 commented Jun 20, 2019

@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
Copy link
Author

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

@Mies75
Copy link

Mies75 commented Jun 25, 2019

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
Copy link
Author

snakefoot commented Jun 25, 2019 via email

@Mies75
Copy link

Mies75 commented Jun 25, 2019

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
Copy link
Author

snakefoot commented Jun 25, 2019 via email

…NDLC (Extract exception from message object)
@Mies75
Copy link

Mies75 commented Jun 26, 2019

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
Copy link
Author

snakefoot commented Jun 26, 2019 via email

@santiagoIT
Copy link

Any time frame when this will be merged?

@anreton
Copy link

anreton commented Aug 23, 2021

Very important and useful changes, is there any news?

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

Successfully merging this pull request may close these issues.

5 participants