zam6ak
zam6ak

Reputation: 7259

Should logging infrastructure be injected when using IoC/DI if logging facade is used?

I am using Autofac as my IoC and from everything I have read on topic of DI teaches to use "constructor injection" to explicitly expose class dependencies... However, I am also using logging facade (Common.Logging) with Log4Net and have created Autofac module to inject it. Now, in every class in which I want to do some logging I have extra constructor parameter (see sample #1)....

I am wondering if logging DI is necessary when using logging facade? I understand that explicitly exposing dependencies via constructor signature is a good architecture. but in case of logging facade I believe following is true:

So, what do others think? Is injecting logging facade an overkill? There are some similar questions on this topic but in more general terms (infrastructure) - I am mainly interested in logging....

// IoC "way"
public class MyController : BaseController
{
    private readonly ILog _logger;

    public MyController(ILog logger)
    {
        _logger = logger;
    }

    public IList<Customers> Get()
    {
        _logger.Debug("I am injected via constructor using some IoC!");
    }
}

// just use the logger "way"
public class MyController : BaseController
{
    private static readonly ILog Logger = LogManager.GetCurrentClassLogger();

    public IList<Customers> Get()
    {
        Logger.Debug("Done! I can use it!");
    }
}

Upvotes: 41

Views: 12311

Answers (6)

snipsnipsnip
snipsnipsnip

Reputation: 2585

Not a generic answer that can be applied in any case, but there is a third option: you can expose it as an event and let others handle.

As a plain event

public class MyController : BaseController
{
    public event Action<string> DebugMessage;

    public IList<Customers> Get()
    {
        DebugMessage?.Invoke("Done! I can use it!");
        // ...
    }
}

This makes logging somewhat optional, and shifts the burden of writing logs to the composition root. If you are developing a minimal library and want to avoid dependency even on System.Diagnostics.Debug, this may be worth considering.

Exposing the event through an interface

You might want to introduce some interface to make it easier to register to DI:

// Collect this interface with your favorite auto-registering DI container
public interface IDebugMessageEmitter
{
    event Action<string> DebugMessage;
}

public class MyController : BaseController, IDebugMessageEmitter
{
    // same as above
}

Upvotes: 0

Steven
Steven

Reputation: 172666

Now, in every class in which I want to do some logging I have extra constructor parameter

If you need logging many classes in your system, there is a good chance your design can be improved. Either you log too much (in too many places) or you're violating the Single Responsibility Principle (or both), since logging is a cross-cutting concern, and you should not clutter your classes with cross-cutting concerns like logging. Both cause maintenance issues and increase the Total Cost of Ownership of your application.

I answered a (totally different) question a half year back, and my answer is applicable to your question. Please read this.

Upvotes: 6

Chris Smith
Chris Smith

Reputation: 353

This may be an older post, but I'd like to chime in.

The idea that IoC of a logging system is overkill is short-sighted.

Logging is a mechanism by which an application developer can communicate with other systems (event logs, flat files, a database, etc.), and each of these things is now an external resource upon which your application is dependent.

How am I supposed to unit test my component's logging if my unit-tested code is now locked to a particular logger? Distributed systems often use loggers which log to centralize sources, not to flat files on the file system.

Injecting a logger, to me, is no different than injecting a database connection API or an external web service. It is a resource which the application requires, and as such, should be injected, so you can test the component's usage of said resourceThe ability to mock said dependence, to test the output of my component independent of the logging recipient, is crucial to strong unit testing.

And given that IoC containers make such injection child's play, it seems to me that not using IoC to inject the logger is no more valid an approach than doing so.

Upvotes: 31

Zodman
Zodman

Reputation: 3904

In many applications, because logging is needed across your entire application's architecture, injecting it everywhere clutters and complicates your code. A static reference to a Logger object is fine, as long as your Logger has some sort of Attach(ILoggerService) mechanism where you attach logging services for the running environment. Your static Logger then stores these in a collection for use when performing the logging commands.

When bootstrapping an application at runtime, attach the logging targets your application requires (file, database, webservice, etc).

When performing automated tests, you can choose to not to use logging by not attach anything (by default). If you want to analyse the logging of your system under test you attach your own ILoggerService instance/subsitute/mock when you set up the test.

A static Logger such as this should have minimal performance issues and no unintended consequences while avoiding code clutter by bypassing the need for dependency injection.

It must be stated that you shouldn't take this static object approach for every object in your architecture; logging is a special case. Generally using static objects in your codebase makes maintenance and testing difficult and should be avoided.

Upvotes: 1

jgauffin
jgauffin

Reputation: 101150

Logging is just infrastructure. Injecting it is overkill. I personally don't even use an abstraction layer. I use the static classes that the library provides directly. My motivation is that it's unlikely that I'll switch logging library in a current project (but might switch for the next project).

However, you are using controllers in your example. Why do you need to log in them? A controller is just an adapter between the view and the model (business logic). There should be no need to log in it.

You typically do only log in classes which contains business logic and at the top layer to be able to log unhandled exceptions. Those are the hard cases to debug and therefore the places where logging is required.

Having to log at other places indicates that you need to refactor to encapsulate your business logic properly.

Upvotes: 35

to StackOverflow
to StackOverflow

Reputation: 124716

I personally think it's overkill.

Logging should ideally have a very low overhead, so a single static logger instance per class seems like a better solution. Keeping the overhead low promotes the use of debug-level logging for instrumentation.

Upvotes: 3

Related Questions