CODEWITHSUNDEEP
c#loggingnlog
Stephen Patten
Stephen Patten

Reputation: 6363

How to wrap NLog in an Adapter Class

Having read Steven's answer here, it got me thinking about switching out logging systems and what a PITA that can be. The simplicity of the interface is what I like the most, and it's raw, no other project to bring in, nothing, except the code you write to produce the events. I have modified the original code so that I can pass in the class name in the context parameter of the LogEntry:

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

public enum LoggingEventType { Debug, Information, Warning, Error, Fatal };

public class LogEntry
{
    public readonly LoggingEventType Severity;
    public readonly string Message;
    public readonly Exception Exception;
    public readonly Type Context;

    public LogEntry(LoggingEventType severity, 
                    string message, 
                    Exception exception = null, 
                    Type context = null)
    {
        if (message == null) throw new ArgumentNullException("message");
        if (message == string.Empty) throw new ArgumentException("empty", "message");

        this.Severity = severity;
        this.Message = message;
        this.Exception = exception;
        this.Context = context;
    }
}

Question #1: Does anything seem wrong about passing in the Type/context param?

This post also shed some light on writing an adapter based on Log4net, and is the basis of my NLog adapter, although I don't use constructor injection of an ILogger.

class NLogAdapter : ILogger
{
    public void Log(LogEntry entry)
    {
        NLog.Logger log;
        if (entry.Context != null)
        {
            log = NLog.LogManager.GetLogger(entry.Context.GetType().Namespace);
        }
        else
        {
            log = NLog.LogManager.GetLogger("DefaultLogger");
        }
        switch (entry.Severity)
        {
            case LoggingEventType.Debug:
                log.Debug(entry.Exception, entry.Message);
                break;
            case LoggingEventType.Information:
                log.Info(entry.Exception, entry.Message);
                break;
            case LoggingEventType.Warning:
                log.Warn(entry.Exception, entry.Message);
                break;
            case LoggingEventType.Error:
                log.Error(entry.Exception, entry.Message);
                break;
            case LoggingEventType.Fatal:
                log.Fatal(entry.Exception, entry.Message);
                break;
            default:
                throw new ArgumentOutOfRangeException(nameof(entry));
        }
    }
}

Question #2: I'm a bit unsure about the use of the log manager for every call, is this the most correct way to get an instance of a NLog Logger? Are there any other recommendations that you might give me?

Note: This Adapter may be a singleton in a DI container, or it could be used/made into a static class.

Thank you, Stephen

Upvotes: 0

Views: 1658

Answers (1)

Steven
Steven

Reputation: 172865

I'm a bit unsure about the use of the log manager for every call, is this the most correct way to get an instance of a NLog Logger? Are there any other recommendations that you might give me?

A more typical design (and performant) design is one where you create a generic implementation, and inject a closed-generic singleton implementation with its generic argument being equal to the class it gets injected into, as can be seen in the following answer:

class NLogAdapter<T> : ILogger
{
    private static readonly NLog.Logger log =
        NLog.LogManager.GetLogger(typeof(T).FullName);
}

This not only prevents you from having to resolve a NLog logger on each call, it prevents you from having to pass along the context to the LogEntry.

Injecting it into consumers, would look like this:

new ProductController(new NLogAdapter<ProductController>())

new HomeController(new NLogAdapter<HomeController>())

If you're using a DI Container, it depends on the container you're using, how this must be configured. With Simple Injector for instance, it's a matter of making a contextual registration as follows:

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

When the logging adapter is a singleton, it means the overhead is reduced to the minimum.

Question #1: Does anything seem wrong about passing in the Type/context param?

When making your logger-implementation contextual, there is no need to pass on contextual information like that during logging.

Big warning: do not make the ILogger abstraction generic (as Microsoft did in their logging library), that would just complicate things for the consumers and their tests. That would be a severe design flaw.

Upvotes: 2

Related Questions