41

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:

  • I can still "swap out" logging framework at any time
  • The class IMHO is not really dependent on Logger. If no logging is configured NullLogger is used. It is almost "it's thre if you need it" vs. "it won't work unless you supply it" kind of deal...(see sample #2)

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!");
    }
}
Community
  • 1
  • 1
zam6ak
  • 7,229
  • 11
  • 46
  • 84

6 Answers6

35

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.

jgauffin
  • 99,844
  • 45
  • 235
  • 372
  • controller was just an example for the point of my question. Personally, I have a web api global filter that catches all unhandeled exceptions, logs them and does proper Request.CreateErrorResponse(). – zam6ak Sep 26 '12 at 19:03
  • Created a blog post with a simplified explanation: http://blog.gauffin.org/2012/10/when-is-a-logger-a-dependency/ – jgauffin Oct 05 '12 at 08:56
  • 2
    I believe logging should be injected because it creates dependencies on config files and hampers automated testing. – vijayst Oct 01 '14 at 06:12
  • 4
    @Vijay: Sorry, what? No major logging library will affect your application if the configuration cannot be found. It will simply stop logging. So no, you are incorrect. – jgauffin Oct 01 '14 at 11:29
  • 1
    @jgauffin, you are right. forgot abt that :) But, it will be nice to have cleaner code! – vijayst Oct 27 '14 at 15:52
  • Changing a logging framework is not the only adventage and reason for DI. It's useful also for updates of versions, encapsulation of logger-specific code, stubing it in tests, dynamic linking should you need it, using composites for logging if you need it and more. – Danon Jun 11 '23 at 18:49
  • @Danon: I don't see how any of that applies to a logging framework since all that is typically done by configuring the logging framework itself. DI is great for many things, but managing logging isn't one. For me, DI should communicate the required services that the concrete class must have. A logging facade is typically not a mandatory service. – jgauffin Jun 18 '23 at 20:37
  • @jgauffin You can only configure within the framework, what the framework expects to be changed. For anything else, you can't change it, unless you do proper DI. – Danon Jun 26 '23 at 12:48
31

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.

Luke Girvin
  • 13,221
  • 9
  • 64
  • 84
Chris Smith
  • 353
  • 3
  • 3
  • 3
    in *some* cases logging *is* a real dependency. For example, some database systems maintain journals that are used to recover data in case of failures. Without this capability, the database won't be able to provide ACID guarantees anymore and as such it cannot function. – Trasplazio Garzuglio Apr 20 '16 at 15:45
  • 4
    Agreed to that point. However, I would ask if this is really logging then. Logging is not database transactional logs. Database recovery is NOT the responsibility of the software. Database recovery is the responsibility of the database. Logging is a means for a developer to output messages about the state of a business action in the software, and how this business action was successfully or unsuccessfully handled. If we can accept this as a definition of logging, then my previous point stands. – Chris Smith Apr 25 '17 at 10:17
  • 2
    The consideration here is domain vs infrastructure. If the log turns up on your high level design, it is a domain problem. If will need testing because it forms part of an interface. Domain code should be tested and injected – Gusdor Nov 07 '17 at 09:38
  • 1
    Instead of saying it's not "really logging", maybe we need some more nuanced vocabulary. For example, in my experience I've encountered diagnostic logging and telemetry (which tend to not be domain-level, and are not injected), and audit logging (which tends to be domain-level, and injected.) Traditional logging frameworks are geared toward diagnostic logging, and have very limited support for behavior like throwing an exception when the logging operation fails. – StriplingWarrior Dec 19 '19 at 22:03
5

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.

Steven
  • 166,672
  • 24
  • 332
  • 435
  • 1
    +1 for the linked post...However, in order to take your approach when logging, is to implement it using decorators (in addition to DI), which is yet another abstraction that has to be implemented just to do, well :) logging...Is it really worth it? According to discussions I am seeing, the opinion is split mostly based on weather one considers logging to be a "must have" dependency vs. "infrastructure feature you can use"...Interesting discussion to say the least... – zam6ak Sep 26 '12 at 19:01
  • @zam6ak: If it's worth it, that's up to you. However, when you're already practicing such architecture, adding logging as a cross-cutting concern will be a no-brainer. And don't forget, if you don't like such a SOLID design, you can always use interception to decorate your services with a logging interceptor. This 'saves' you from having a generic interface on your services. – Steven Sep 26 '12 at 19:57
  • I like and aim to practice SOLID, but do not always tend to use CQRS (or similar Command pattern) to achieve it. You make a good point about interceptors/filters. As I mention in one of the comments below, I leverage Web API filters to log unhandled exceptions so my controller actions are not littered with try/catch(Exception e) { log(e);} logic... – zam6ak Sep 26 '12 at 20:03
  • 1
    These patterns are just the result of applying the SOLID principles. These patterns is applicable even if you don't practice CQRS. That's how I do it. I use this pattern for all applications I build, but I have never practiced CQRS. This command model enables a whole world of new possibilities. Take a look for instance at [this article about writing highly maintainable WCF services](http://bit.ly/RrJRvD). Although a bit more work, you can do the same with a Web API service. I experimented with this, and planned to write an article about that as well. – Steven Sep 26 '12 at 20:13
  • 1
    -1 the author said 'in every class in which I want to do some logging', and not 'I want to do logging in nearly every class' – controlbox Oct 01 '17 at 19:12
3

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.

Joe
  • 122,218
  • 32
  • 205
  • 338
0

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.

Zodman
  • 3,178
  • 2
  • 32
  • 38
0

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
}
snipsnipsnip
  • 2,268
  • 2
  • 33
  • 34