Using Ninject to fill Log4Net Dependency

Question

I use Ninject as a DI Container in my application. In order to loosely couple to my logging library, I use an interface like this:

public interface ILogger
    {
        void Debug(string message);
        void Debug(string message, Exception exception);
        void Debug(Exception exception);

        void Info(string message);
        ...you get the idea

And my implementation looks like this

public class Log4NetLogger : ILogger
    {
        private ILog _log;

        public Log4NetLogger(ILog log)
        {
            _log = log;
        }

        public void Debug(string message)
        {
            _log.Debug(message);
        }
        ... etc etc

A sample class with a logging dependency

public partial class HomeController
    {
        private ILogger _logger;

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

When instantiating an instance of Log4Net, you should give it the name of the class for which it will be logging. This is proving to be a challenge with Ninject.

The goal is that when instantiating HomeController, Ninject should instantiate ILog with a "name" of "HomeController"

Here is what I have for config

public class LoggingModule : NinjectModule
    {
        public override void Load()
        {
            Bind<ILog>().ToMethod(x => LogManager.GetLogger(GetParentTypeName(x)))
                .InSingletonScope();

            Bind<ILogger>().To<Log4NetLogger>()
                .InSingletonScope();
        }

        private string GetParentTypeName(IContext context)
        {
            return context.Request.ParentContext.Request.ParentContext.Request.Service.FullName;
        }
    }

However the "Name" that is being passed to ILog is not what I'm expecting. I can't figure out any rhyme or reason either, sometimes it's right, most of the time it's not. The Names that I'm seeing are names of OTHER classes which also have dependencies on the ILogger.

Lorraine

When it isn't right, what does it look like? Can you mock up a class relationship so you can give some fake names and at least explain the relationship between the names you are seeing? Or just copy some of your code? – Ignitron

The names that I'm seeing are names of other classes which have a dependency on ILogger. For example, in my HomeController, It's getting a logger with a name of SomethingRepository. – Lorraine

Acutally, I think you just made me solve it. the ILog/ILogger aren't scoped right. It's newing them for one instance, then reusing it. They should be scoped Transient. – Lorraine

Answer 1

The Ninject.Extension.Logging extension already provides all you are implementing yourself. Including support for log4net, NLog and NLog2.

https://github.com/ninject/ninject.extensions.logging

---

Also you want to use the following as logger type:

context.Request.ParentRequest.ParentRequest.Target.Member.DeclaringType

Otherwise you will get the logger for the service type instead of the implementation type.

Coddle

That looks perfect, I will definitely check that out! – Lorraine

The only downside is that I still have to reference that assembly even if I only want the ILogger interface (eg: to mock a dependency for a unit test). I guess it doesn't hurt anything. Definitely worth it for the sheer convenience of being able to handle all that work with a simple nuget add. – Lorraine

Why is it two levels of .ParentRequest? – Cordon

Am I the only one bothered by taking a dependency on the IoC framework? I like the idea of what the original poster was trying to do. – Carrack

Answer 2

I personally have no interest in abstracting away my logger, so my implementation modules reference log4net.dll directly and my constructors request an ILog as desired.

To achieve this, a one line registration using Ninject v3 looks like this at the end of my static void RegisterServices( IKernel kernel ):

        kernel.Bind<ILog>().ToMethod( context=> 
            LogManager.GetLogger( context.Request.Target.Member.ReflectedType ) );
        kernel.Get<LogCanary>();
    }

    class LogCanary
    {
        public LogCanary(ILog log)
        {
            log.Debug( "Debug Logging Canary message" );
            log.Info( "Logging Canary message" );
        }
    }

For ease of diagnosing logging issues, I stick the following at the start to get a non-DI driven message too:

public static class NinjectWebCommon
{
    public static void Start()
    {
        LogManager.GetLogger( typeof( NinjectWebCommon ) ).Info( "Start" );

Which yields the following on starting of the app:

<datetime> INFO  MeApp.App_Start.NinjectWebCommon           - Start
<datetime> DEBUG MeApp.App_Start.NinjectWebCommon+LogCanary - Debug Logging Canary message
<datetime> INFO  MeApp.App_Start.NinjectWebCommon+LogCanary - Logging Canary message

Ia

@user1068352 Thanks; damn, now you've reminded me of the ~fake-aid pattern (third story) – Ia

I like your approach for some reason context.Request.Target is null for me..hope I get this approach working – Prognostication

@Prognostication Guessing it's because you're doing a test request (vs actually resolving a log dependency indirectly like in my example code). You should be able to work out something sufficiently general by looking at the request tree in a debugger – Ia

@RubenBartelink it was actually a side effect of pulling ninject into some legacy code, that didn't have a root object. We were using a service locator pattern so the target was the one asking for it. Once I got ninject configured to create generic asp.net handlers we were all good. – Prognostication

Answer 3

The Scope of ILog and ILogger needs to be Transient, otherwise it will just reuse the first logger that it creates. Thanks to @Meryln Morgan-Graham for helping me find that.

Lorraine

As Remo mentioned, there are already components for Ninject that do exactly what you want, and judging by your shown implementation of 'GetParentTypeName`, the Ninject extension is likely more robust. – Gleeful

Answer 4

Bind<ILog>().ToMethod(x => LogManager.GetLogger(GetParentTypeName(x)))
            .InSingletonScope();

You are currently binding in Singleton scope, so only one logger is created which will use the name of the first one created. Instead use InTransientScope()

Lhasa

Answer 5

maybe my answer is late but I'm using this format:

private static void RegisterServices(IKernel kernel)
    {
        kernel.Bind<ILog>()
            .ToMethod(c => LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType))
            .InSingletonScope();
    }

Boarder

Hmm. Careful with the InSingletonScope - the point of the parameter of GetLogger is that logging has the correct Type associated with/rendered alongside the log entries. Also using MethodBase.GetCurrentMethod().DeclaringType may not always be reliable - inlining etc can influence what the exact context is (versus actually inspecting the Ninject request context c) – Ia

Answer 6

For all of you that are still looking for the correct answer, the correct implementation is :

public class LoggingModule : NinjectModule
{
    public override void Load()
    {
        Bind<ILog>().ToMethod(x => LogManager.GetLogger(x.Request.Target.Member.DeclaringType));

        Bind<ILogger>().To<Log4NetLogger>()
            .InSingletonScope();
    }
}

Emphasis on:

x.Request.Target.Member.DeclaringType

Plasia

I think this should be remarked as the answer. This shows how to do the binding AND doesn't require you to have a Reference for the ninject facade version of ILogger (from Ninject.Extensions.Logging). Will work with any logger. – Lehet

Answer 7

I do like the idea of wrapping the Log4Net in my own interfaces. I don't want to be dependent on Ninjects implementation, because to me that just means I take a dependency on Ninject throughout my application and I thought that was the exact opposite of what dependency injection is for. Decouple from third party services. So I took the original posters code but I changed the following code to make it work.

    private string GetParentTypeName(IContext context)
    {
        var res = context.Request.ParentRequest.ParentRequest.Service.FullName;
        return res.ToString();
    }

I have to call ParentRequest.ParentRequest so that when I print the layout %logger it will print the class that calls the Log4Net log method instead of the Log4Net class of the method that called the Log method.

Carrack

That's pretty much exactly what I'm using to this day (This question is almost 4 years old now), and for the same reasons you mentioned. – Lorraine

Yeah thanks for the code! I used it verbatim and I am very pleased with it. I just had to fix that method. I also couldn't understand why that method was calling ParentRequest.ParentRequest so I added my own answer for anyone googling upon this. – Carrack