In C# should my Common.Logging logger be an instance member or static?

Question

Looking a project that uses Common.Logging for .NET, I noticed that some classes declare the logger instance as a class static member. For instance:

public class HelloJob : IJob
{
    private static ILog _log = LogManager.GetLogger(typeof(HelloJob));

    public HelloJob()
    {
    }

    public virtual void  Execute(IJobExecutionContext context)
    {
        _log.Info(string.Format("Hello World! - {0}", System.DateTime.Now.ToString("r")));
    }
}

And in other classes the logger is declared as an instance member:

public class SimpleExample : IExample
{
    public virtual void Run()
    {
        ILog log = LogManager.GetLogger(typeof (SimpleExample));

        log.Info("------- Initializing ----------------------");

        // etc
    }
}    

Is there a reason to prefer one approach or the other?

In which cases is each approach recommended? Is it related to thread safety?

Would it be a problem if I just declared a "Logger" class with a static "logger" member and the whole project used that (apart from the issue that I would in practice have a global variable)?

Answer 1

Most loggers are thread safe, and creating instances of them has very little overhead, both in terms of time and memory. So the real question needs to be what makes sense from a programming and maintainability standpoint.

On the one hand, since the logger is conceptually tied to your class, and not to the instance of the class, a lot of people prefer to keep it static. That's a perfectly valid argument. For example, if HelloWorldJob extends HelloJob, I think most people would expect the log message written by code in HelloJob to be tied to the HelloJob class, even though you have a more specific subclass instance. It's also nice to be able to access your logger from static methods, which wouldn't be possible if it's not on a static field.

On the other hand, there is no reason that your HelloJob should be responsible for getting its own logger instance. There's a lot to be said for using dependency injection instead (unit testability, additional configurability, and simpler code). So I personally suggest having your logger injected by a DI framework, in which case it would need to be referenced on a per-instance field.

public class HelloJob : IJob
{
    private readonly ILog _log;

    public HelloJob(ILog log)
    {
        _log = log;
    }
    ...
}

Your DI framework can set up the logger based on details it knows at run-time, or you can provide a fake or mocked logger in your unit tests to make sure the expected log messages are produced. Note that even though you are referring to a per-instance field, you're perfectly free to still use a per-class (or even a singleton) instance--those are just details that don't need to be part of this class's concern.

Update

Over time I've shifted my opinion more in the direction of using a static logger. More specifically, I really like using Fody Anotar libraries to generate the static logger (and other helpful code) for me, so all I have to write in my source code is logging statements like:

LogTo.Info("This is a message");