15

I want to build my log4net logger in my MVC controller abstract base class like so:

protected static readonly ILog Log = LogManager.GetLogger(typeof(AuthorizedController));

In this manner I can define the logger once and be done with it. The only problem is that the logger attribute in the log output will always be AuthorizedController, and if I have FooController inherited from AuthorizedController I'd like the log output to reflect that.

What would be a good KISS, DRY, and efficient way do doing this?

Jeremy Holovacs
  • 22,480
  • 33
  • 117
  • 254

4 Answers4

20

I'm not sure how expensive the call to LogManager.GetLogger() is, but I suspect that there is some clever caching and/or lazy initialization in the log4net system that keeps requested instances available for quick retrieval. After all, there is no reason why calling LogManager.GetLogger() twice with the same type parameter would return a different instance.

That said, perhaps replacing the field with the following property will suffice.

protected ILog Logger
{
    get { return LogManager.GetLogger(GetType()); }
}

GetType() is virtual and overloaded so each concrete type will supply its type when this property is called.

Steve Guidi
  • 19,700
  • 9
  • 74
  • 90
  • 2
    `Retrieves a logger named as the name parameter. If the named logger already exists, then the existing instance will be returned. Otherwise, a new instance is created.` – dotjoe Aug 17 '12 at 15:40
  • 1
    Yeah, I think I was too stuck up on the static instance, when it doesn't really need to be static. – Jeremy Holovacs Aug 17 '12 at 16:04
  • Log4net reads from a Hashtable to resolve the logger. O(1) isn't too bad but I don't like the multiple method calls going each time I need to access the Logger. Maybe the best idea is for each deriving class to call this property in it's constructor and save a copy of the logger itself. – arviman Dec 10 '15 at 08:46
  • constant time "isn't too bad"? is there any functional difference, aside that the code is more cluttered with boilerplate? – Switch386 Aug 28 '19 at 19:30
4

As Steve Guidi suggested - use GetType to create logger for specific instance of type. You can save reference to instance of logger which gets calling LogManager.GetLogger in backing field:

    private ILog _log;
    protected ILog Log => _log ?? (_log = LogManager.GetLogger(GetType()));
Vasyl Senko
  • 1,779
  • 20
  • 33
3

I did something similar by using NInject as an IoC container, but I suppose you can grab the idea to use with your own container. Basically I inject ILog on the requesting types constructor, but instead of binding to an instance, I bind it to a provider.

  kernel.Bind<ILog>().ToProvider<MyProvider>();

public object Create(IContext context)
{
    return LogManager.GetLogger(context.Request.Target.Member.DeclaringType);
}

so each type receive a logger that is the same as doing GetLogger(GetType()) in the constructor.

Felice Pollano
  • 32,832
  • 9
  • 75
  • 115
0

Having a static logger (or static instances in general) is a bad practice. It can easily lead to undesired coupling between unrelated components.

You should consider adding an dependency Injection/Inverse of Control framework to the game. Have the logger injected to the ctor of the class. Most frameworks allows you to define different implementations to in different contexts.

We use Castle Windsor in our products

You can also look at this post to find an example of using logging together with DI

Community
  • 1
  • 1
Zvi
  • 237
  • 3
  • 8
  • 1
    http://logging.apache.org/log4net/release/manual/configuration.html suggests that this is the intended usage. – Jeremy Holovacs Aug 17 '12 at 15:50
  • Fair enough, it's a matter of opinion. Having static instances are a fast solution for accessing the global context. Problem is, that it couples you class with the implementation of an external service. By having the services pushed instead of pulled you create better decoupling. Plus writing tests when using this method works better when writing tests. – Zvi Aug 17 '12 at 16:00
  • http://stackoverflow.com/questions/3452318/dependency-injection-and-named-loggers – Zvi Aug 17 '12 at 16:04