Implementation and usage of logger wrapper for log4net

Question

This question is related to Steven’s answer - here. He proposed a very good logger wrapper. I will paste his code below:

public interface ILogger
{
    void Log(LogEntry entry);
}

public static class LoggerExtensions
{
    public static void Log(this ILogger logger, string message)
    {
        logger.Log(new LogEntry(LoggingEventType.Information,
            message, null));
    }

    public static void Log(this ILogger logger, Exception exception)
    {
        logger.Log(new LogEntry(LoggingEventType.Error, 
            exception.Message, exception));
    }

    // More methods here.
}

So, my question is what is the proper way to create implementation that proxies to log4net? Should I just add another Log extension method with type parameter and then create a switch inside? Use different log4net method in case of LoggingEventType ?

And second question, what is the best way to use it later in the code?

Because he wrote:

(…) you can easily create an ILogger implementation (…) and configure your DI container to inject it in classes that have a ILogger in their constructor.

Does that mean that every class that will log sth (so basically every), should have ILogger in its constructor?

Solution

So, my question is what is the proper way to create implementation that proxies to log4net?

you should create something like:

public class Log4netAdapter : ILogger
{
    private readonly log4net.ILog m_Adaptee;

    public Log4netAdapter(log4net.ILog adaptee)
    {
        m_Adaptee = adaptee;
    }

    public void Log(LogEntry entry)
    {
        //Here invoke m_Adaptee
        if(entry.Severity == LoggingEventType.Debug)
            m_Adaptee.Debug(entry.Message, entry.Exception);
        else if(entry.Severity == LoggingEventType.Information)
            m_Adaptee.Info(entry.Message, entry.Exception);
        else if(entry.Severity == LoggingEventType.Warning)
            m_Adaptee.Warn(entry.Message, entry.Exception);
        else if(entry.Severity == LoggingEventType.Error)
            m_Adaptee.Error(entry.Message, entry.Exception);
        else
            m_Adaptee.Fatal(entry.Message, entry.Exception);
    }
}

Does that mean that every class that will log sth (so basically every), should have ILogger in its constructor?

As I understand from Stevens answer: Yes, you should do this.

what is the best way to use it later in the code?

If you are using a DI container, then just use the DI container to map ILogger to Log4netAdapter. You also need to register log4net.ILog, or just give an instance of log4net logger to the DI container to inject it to the Log4netAdapter constructor.

If you don't use a DI container, i.e., you use Pure DI, then you do something like this:

ILog log = log4net.LogManager.GetLogger("MyClass");

ILogger logging_adapter = new Log4netAdapter(log);

var myobject = new MyClass(other_dependencies_here, logging_adapter);

Comments

[Steven]

I have nothing much to add here (+1 for me), except as like I said in the comment that if you are injecting ILogger into almost all components in your system you are either logging too much or you are violating the SOLID principles.

[Tim Laax]

@Steven So this basically means, that in every class when error occurs (or some situation unwanted) I need to throw "new MyException" or rethrow exception that occured with just throw;. This is about errors/exceptions. What about info logging? Like in messaging server I want to log every comming and going message id. I have to add another parameter with logger to that class. Actually every class that needs to log "Info" or "Warning" and not "Error" has to have logger in its constructor (if it's not inherited). Do you agree with that last statement?

[Steven]

@TimLaax you should read that referenced answet again closely. What the answer tells is that with the right set of abstractions, a single decorator can be created that can be wrapped around a large set of classes, that allows you to implement logging for debugging, monitoring and audit trailing in an uniform way and in a few lines of code. This prevents you from having to inject the logger everywhere. In case of errors, exceptions should be thrown. These can be logged in the top most layer of your application.

[Tim Laax]

@Steven Thanks! I think I got the theory. But can you recommend a sample ready to use project with such logger used through DI container so I could see the practice side?

[Steven]

@TimLaax: That's hard to advice. I can point you to my articles (here, here and here) about applying a design that allows you to apply cross-cutting concerns (such as logging) with ease, and you can look at this example project to get a small working example, but I'm not aware of ready-to-use sample projects that show logging specifically.

[Tim Laax]

I will definitely read them. Thank you @Steven and everybody else for your help

[Tsahi Asher]

@Steven, why would the adapter accept an ILog at its constructor, and not instantiate a Log4Net object directly? Is that to allow testing the adapter?

[Steven]

@TsahiAsher calling LogManager.GetLogger is absolutely valid. I see no disadvantage in doing so.

[Krptodr]

@yacoubmassad What do you do if you use DI?

[Krptodr]

Nevermind I found an example written by @Steven and it can be found here: stackoverflow.com/a/25113659/3311255

[Krptodr]

@Steven, can you provide an example of using this example implementation using SimpleInjector as the DI Container? I can't register log4net.ILog in my container. My interface ILogger<T> is implemented by Log4NetAdapter<T> which accepts ILog in it's constructor. I then have constructors on my objects that I wish to log accepting a parameter of ILogger<T>. ILogger<T> is supposed to be injected, but has always failed to do so.

[Steven]

@Krptodr please post a new question here on SO with a full example and details and tag it with simple-injector. I will have a look after my hangover is gone :)