79

I'm trying to figure out what the right patter and usage of log4net is with a dependency injection framework.

Log4Net uses the ILog interface but requires me to call 

LogManager.GetLogger(Reflection.MethodBase.GetCurrentMethod().DeclaringType)

in each class or method where I need to log information. This seems to go against IoC principles and couples me to using Log4Net.

Should I somehow put in another layer of abstraction somewhere?

Also, I need to log custom properties like the current user name like this:

log4net.ThreadContext.Properties["userName"] = ApplicationCache.CurrentUserName;

How can I encapsulate this so that I don't have to remember to do it everytime and still maintain the current method that is logging. should I do something like this or am I totally missing the mark?

public static class Logger
{
    public static void LogException(Type declaringType, string message, Exception ex)
    {
        log4net.ThreadContext.Properties["userName"] = ApplicationCache.CurrentUserName;
        ILog log = LogManager.GetLogger(declaringType);
        log.Error(message, ex);
    }
}
Micah
  • 111,873
  • 86
  • 233
  • 325

4 Answers4

61

I think you're not seeing the forest for the trees here. ILog and LogManager are a lightweight façade almost 1:1 equivalent to Apache commons-logging, and do not actually couple your code to the remainder of log4net.

<rant>
I've also found that almost always when someone creates a MyCompanyLogger wrapper around log4net they miss the point badly and will either lose important and useful capabilities of the framework, throw away useful information, lose the performance gains possible using even the simplified ILog interface, or all of the above. In other words, wrapping log4net to avoid coupling to it is an anti-pattern.
</rant>

If you feel the need to inject it, make your logger instance accessible via a property to enable injection but create a default instance the old-fashioned way.

As for including contextual state in every log message, you need to add a global property whose ToString() resolves to what you're looking for. As an example, for the current heap size:

public class TotalMemoryProperty
{
    public override string ToString()
    {
        return GC.GetTotalMemory(false).ToString();
    }
}

Then to plug it in during startup:

GlobalContext.Properties["TotalMemory"] = new TotalMemoryProperty();
Jeffrey Hantin
  • 35,734
  • 7
  • 75
  • 94
  • 10
    Your rant is way off base here. This guy was asking about how to inject an instance of ILog to his objects that are inside of a container. There are many reasons to do so. – CodeMonkeyKing Nov 16 '11 at 07:53
  • 1
    @CodeMonkeyKing: The rant was directed at his suggested approach of creating an abstraction layer between log4net and his code, and especially at the sample code block at the end of the question. – Jeffrey Hantin Nov 17 '11 at 21:34
  • 4
    @JeffreyHantin It may be an anti-pattern in your opinion, but I've found that there are as many opinions out there as there are engineers writing code. My opinion of course :) – CodeMonkeyKing Nov 18 '11 at 05:17
  • 1
    I realise this is an old post but could you please explain why such a wrapper is "bad"? ILog and LogManager reside in the log4net namespace, and by using these types throughout my code isn't this coupling me to log4net? What am I missing here? – Andrew Stephens Jun 05 '14 at 09:19
  • @AndrewStephens `ILog` and `LogManager` are *already* a wrapper, they just happen to be built as part of log4net's assembly. log4net's true surface area is in the `Configurator`, `Repository`, `Logger`, `Appender` and `Formatter` types, and you should not have any coupling there except in your `App.config`. – Jeffrey Hantin Jun 05 '14 at 21:24
  • As for why it's bad: `ILog` together with the recommended mode of usage is a better design than any house wrapper I've ever seen. It's not that I'm intrinsically opposed to wrapping log4net, it's that in my experience the cure is far worse than the disease. – Jeffrey Hantin Jun 05 '14 at 21:29
  • 1
    Wrapping log4net is fine practice. The problem isn't wrapping the library, it's doing so poorly. – Dan Feb 08 '17 at 21:45
  • @Dan That was rather the point of the portion of the answer inside rant tags :-) API design tends to be a weak skill for a lot of application developers, so directing said developers to wrap things results in inferior wrapper APIs. My recommendation is to use the wrapper library bundled with log4net if possible: to use a different logger, replace the factory and supply a different implementation of `ILog`. Separate the bundled wrapper into its own assembly if you must, but think long and hard before writing a new one. – Jeffrey Hantin Feb 08 '17 at 23:27
  • 1
    But using `ILog` and `LogManager` *does* couple you to the rest of log4net, as it requires you to have a `using log4net` in your code, and have a reference to log4net.dll. `log4net.ILog` and `SomeStd.ILog` may have the same base name and members, but they are, nevertheless, two entirely different classes. – James Curran Jun 28 '17 at 17:14
  • @JamesCurran But that seems to be rarely relevant in practice. It seems unlikely that you'll be swapping in a different logging framework, and if that need does come up, its probably a one and done kind of thing. Its not like you need maximum flexibity there, at least IME. – Andy Aug 28 '18 at 00:04
  • I'm just trying to figure out how to mock it so that the method I'm trying to unit test doesn't do actual logging to our logging provider (Sumo Logic). – Scott Fraley Oct 01 '21 at 18:48
3

Another way of doing it would be to wire up the log4net ILog interface container registration sequence like this: (using Castle in an ASP.NET context below as an example)

container.Register(Component.For<ILog>().LifeStyle
    .PerWebRequest.UsingFactoryMethod(() => LogManager.GetLogger(
    MethodBase.GetCurrentMethod().DeclaringType)));

and just constructor-inject ILog whenever you need it.

santos
  • 434
  • 3
  • 6
  • This works better, I think, as you get the right logger for the implementation class being constructed: `Component.For().UsingFactoryMethod((kernel, model, cc) => LogManager.GetLogger(cc.Handler.ComponentModel.Implementation))` – Mark Mar 26 '15 at 18:38
2

How I did it was to create my own interface, and had a class implement that interface that used Log4Net, and injected that class. In that way you are not tied to Log4Net...you can just create another class for a new logger and inject that class.

CSharpAtl
  • 7,374
  • 8
  • 39
  • 53
2

If you are really concerned about having multiple loggers, you can always declare a global logger and call that for all your logging needs. The downside to this is that you lose the built in ability to identify the class that the log was issued from, but you can also inject your own custom message into the log to identify the class.

It depends on how robust you want the logging to be. You can also simply put the logger at the main class and let all unhandled exceptions bubble up to it for logging.

Dillie-O
  • 29,277
  • 14
  • 101
  • 140