25

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.

Brook
  • 5,949
  • 3
  • 31
  • 45
  • 1
    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? – Merlyn Morgan-Graham Jul 21 '11 at 19:18
  • 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`. – Brook Jul 21 '11 at 19:21
  • 1
    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. – Brook Jul 21 '11 at 19:22

7 Answers7

28

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
Ruben Bartelink
  • 59,778
  • 26
  • 187
  • 249
  • 2
    @user1068352 Thanks; damn, now you've reminded me of [the ~fake-aid pattern (third story)](http://thedailywtf.com/Articles/Are-You-Cool,-Man-and-More.aspx) – Ruben Bartelink Dec 12 '12 at 21:16
  • I like your approach for some reason context.Request.Target is null for me..hope I get this approach working – JoshBerke Dec 31 '14 at 15:43
  • @JoshBerke 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 – Ruben Bartelink Jan 01 '15 at 18:57
  • @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. – JoshBerke Jan 05 '15 at 15:45
18

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.

Remo Gloor
  • 32,665
  • 4
  • 68
  • 98
  • That looks perfect, I will definitely check that out! – Brook Jul 21 '11 at 19:54
  • 3
    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. – Brook Jul 21 '11 at 20:02
  • 3
    Why is it two levels of `.ParentRequest`? – Carl G Jan 13 '14 at 23:24
  • 1
    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. – mac10688 Feb 12 '15 at 13:45
9

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.

Brook
  • 5,949
  • 3
  • 31
  • 45
  • 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. – quentin-starin Jul 21 '11 at 20:03
7
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()

BrokenGlass
  • 158,293
  • 28
  • 286
  • 335
4

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();
    }
Leo
  • 398
  • 2
  • 8
  • 1
    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`) – Ruben Bartelink Jan 12 '16 at 18:44
4

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
regisbsb
  • 3,664
  • 2
  • 35
  • 41
  • 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. – Nicholi Sep 04 '15 at 23:05
0

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.

mac10688
  • 2,145
  • 2
  • 24
  • 37
  • 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. – Brook Mar 04 '15 at 04:33
  • 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. – mac10688 Mar 04 '15 at 17:44