16

Is there any way I can get the context so I can retrieve the loggerName and use LogManager.GetLogger(loggerName) instead of LogManager.GetCurrentClassLogger()?

I noticed container.RegisterConditional() has access to the context.

Also, I want to avoid solutions like SimpleLogging.NLog for now.

Finally, I am willing to accept this is not the right approach. BTW, AOP is an option that I've already explored (Is it a good practice to have logger as a singleton?).

Note: I am aware that GetCurrentClassLogger() gets the same information I'd retrieve with .NET reflection.

using NLog;
using SimpleInjector;

namespace DependencyInjection
{
    class Program
    {
        private static Container _container;
        static void Main(string[] args)
        {
            Bootstrap();
            _container.GetInstance<Greeter>().Greet();
        }

        private static void Bootstrap()
        {
            _container = new Container();

            _container.Register<ILogger>(() => LogManager.GetCurrentClassLogger(), Lifestyle.Transient);
            _container.Register<Greeter>();

            _container.Verify();
        }

        public class Greeter
        {
            private ILogger _logger;

            public Greeter(ILogger logger)
            {
                _logger = logger;
            }

            public void Greet()
            {
                _logger.Log(LogLevel.Info, "Hello world!");
            }
        }
    }
}
Steven
  • 166,672
  • 24
  • 332
  • 435
Gonzalo Contento
  • 857
  • 9
  • 21
  • what would you want the value of `loggerName` to be? – qujck Jan 06 '16 at 23:29
  • 1
    Related: https://stackoverflow.com/q/32952701/264697 – Steven Jan 06 '16 at 23:41
  • 1
    Applying AOP techniques is very useful, but please stay away from code weaving tools. Apply AOP by using decorators. Related: https://stackoverflow.com/a/9915056/264697 – Steven Jan 07 '16 at 07:54
  • @qujck the Class name that has been injected – Gonzalo Contento Jan 07 '16 at 13:39
  • @Steven I guess the answer to the original question is: No, you can't (got it). But in regards to AOP weavers, PostSharp, for example, would save us the burden of creating a wrapper. A wrapper that has to be maintained (NLog.ILogger contract is huge). Of course the struggle here is the compromise... – Gonzalo Contento Jan 07 '16 at 18:18
  • With code weaving, you will have to write an aspect class; it has to be maintained as well. – Steven Jan 07 '16 at 23:40

1 Answers1

17

You need to define a proxy logger which routes the message to the correct Nlog Logger. This proxy is pretty simple:

public class NLogProxy<T> : ILogger
{
    private static readonly NLog.ILogger logger = 
                 LogManager.GetLogger(typeof (T).FullName);

    void ILogger.Log(string message)
    {
        logger.Log(LogLevel.Info, message);
    }
}

You can register this as

container.RegisterConditional(typeof(ILogger), 
     context => typeof(NLogProxy<>).MakeGenericType(context.Consumer.ImplementationType), 
     Lifestyle.Singleton, context => true);

Everywhere you need logging you only will have to inject ILogger.

As for AOP. I'm unsure what you mean by this comment

A wrapper that has to be maintained (NLog.ILogger contract is huge).

Logging is a cross cutting concern and using a decorator is a great way to apply cross cutting concerns. With a decorator it won't be possible to log every (private) function call entrance and exit but why would you want that? As you can read here you probably don't need that. The simple fact that a service is called (with the data passed to this service) and possible exceptions are logged with the complete stacktrace will be more than sufficient in most cases.

So consider this:

public interface ISomeService
{
    void DoSomething(string someParameter);
}

public class SomeServiceDecorator : ISomeService
{
    private readonly ISomeService decoratee;
    private readonly ILogger logger;

    public SomeServiceDecorator(ISomeService decoratee, ILogger logger)
    {
        this.decoratee = decoratee;
        this.logger = logger;
    }

    public void DoSomething(string someParameter)
    {
        try
        {
            this.logger.Log(string.Format("Do something called with {0}", someParameter));
            this.decoratee.DoSomething(someParameter);
        }
        catch (Exception e)
        {
            this.logger.Log(e.ToString());                
            throw;
        }
    }
}

This decorator will log all function calls with the information passed to the service and will also log any exception.

But this approach would increase the number of classes by 2, so not very DRY. This problem is caused because this design is at least suboptimal. Using a design around a single open generic abstraction will totally solve this problem. You can read about this design here and here.

In this case you would have a single `LoggingDecorator' as

public class LoggingCommandHandlerDecorator<T> : ICommandHandler<T>
{
    private readonly ICommandHandler<T> decoratee;
    private readonly ILogger logger;

    public LoggingCommandHandlerDecorator(ICommandHandler<T> decoratee, ILogger logger)
    {
        this.decoratee = decoratee;
        this.logger = logger;
    }

    public void Handle(T command)
    {
        // serialize command to json and log
        this.logger.Log(serializedcommandData);
        this.decoratee.Handle(command);
    }
}

And this single decorator will log all your commands.

That's my vision of AOP....

Ric .Net
  • 5,540
  • 1
  • 20
  • 39
  • 1
    @ric-net I think your answer not only address Steven comments but also highlights Jeff Atwood's take on logging: [The Problem With Logging](http://blog.codinghorror.com/the-problem-with-logging/). An interesting old but good discussion any architect has to deal with. – Gonzalo Contento Jan 11 '16 at 21:05