This may be somewhat related to Pass ILogger or ILoggerFactory to constructors in AspNet Core?, however this is specifically about Library Design, not about how the actual application that uses those libraries implement its logging.
I am writing a .net Standard 2.0 Library that will be installed via Nuget, and to allow people using that Library to get some debug info, I'm depending on Microsoft.Extensions.Logging.Abstractions to allow a standardized Logger to be injected.
However, I'm seeing multiple interfaces, and sample code on the web sometimes uses ILoggerFactory and creates a logger in the ctor of the class. There's also ILoggerProvider which looks like a read-only version of the Factory, but implementations may or may not implement both interfaces, so I'd have to pick. (Factory seems more common than Provider).
Some code I've seen uses the non-generic ILogger interface and might even share one instance of the same logger, and some take an ILogger<T> in their ctor and expect the DI container to support open generic types or explicit registration of each and every ILogger<T> variation my library uses.
Right now, I do think that ILogger<T> is the right approach, and maybe a ctor that doesn't take that argument and just passes a Null Logger instead. That way, if no logging is needed, none is used. However, some DI containers pick the largest ctor and thus would fail anyway.
I'm curious of what I'm supposed to be doing here to create the least amount of headache for users, while still allowing proper logging support if desired.
Definition
We have 3 interfaces: ILogger, ILoggerProvider and ILoggerFactory. Let's look at the source code to find out their responsibilities:
ILogger: is responsible to write a log message of a given Log Level.
ILoggerProvider: is responsible to create an instance of ILogger (you are not supposed to use ILoggerProvider directly to create a logger)
ILoggerFactory: you can register one or more ILoggerProviders with the factory, which in turn uses all of them to create an instance of ILogger. ILoggerFactory holds a collection of ILoggerProviders.
In the example below, we are registering 2 providers (console and file) with the factory. When we create a logger, the factory uses both of these providers to create an instance of logger:
ILoggerFactory factory = new LoggerFactory().AddConsole(); // add console provider
factory.AddProvider(new LoggerFileProvider("c:\\log.txt")); // add file provider
Logger logger = factory.CreateLogger(); // <-- creates a console logger and a file logger
So the logger itself, is maintaining a collection of ILoggers, and it writes the log message to all of them. Looking at Logger source code we can confirm that Logger has an array of ILoggers (i.e. LoggerInformation[]), and at the same time it is implementing ILogger interface.
Dependency Injection
MS documentation provides 2 methods for injecting a logger:
1. Injecting the factory:
public TodoController(ITodoRepository todoRepository, ILoggerFactory logger)
{
_todoRepository = todoRepository;
_logger = logger.CreateLogger("TodoApi.Controllers.TodoController");
}
creates a Logger with Category = TodoApi.Controllers.TodoController.
2. Injecting a generic ILogger<T>:
public TodoController(ITodoRepository todoRepository, ILogger<TodoController> logger)
{
_todoRepository = todoRepository;
_logger = logger;
}
creates a logger with Category = fully qualified type name of TodoController
In my opinion, what makes the documentation confusing is that it does not mention anything about injecting a non-generic, ILogger. In the same example above, we are injecting a non-generic ITodoRepository and yet, it does not explain why we are not doing the same for ILogger.
According to Mark Seemann:
An Injection Constructor should do no more than receiving the
dependencies.
Injecting a factory into the Controller is not a good approach, because it is not Controller's responsibility to initialize the Logger (violation of SRP). At the same time injecting a generic ILogger<T> adds unnecessary noise. See Simple Injector's blog for more details: What’s wrong with the ASP.NET Core DI abstraction?
What should be injected (at least according to the article above) is a non-generic ILogger, but then, that's not something that Microsoft's Built-in DI Container can do, and you need to use a 3rd party DI Library.
This is another article by Nikola Malovic, in which he explains his 5 laws of IoC.
Nikola’s 4th law of IoC
Every constructor of a class being resolved should not have any
implementation other than accepting a set of its own dependencies.
Those are all valid except for the ILoggerProvider. ILogger and ILogger<T> are what you're supposed to use for Logging. To get an ILogger, you use an ILoggerFactory. The ILogger<T> is a shortcut to get a logger for a particular category (shortcut for the type as the category).
When you use the ILogger to perform logging, each of the registered ILoggerProviders gets a chance to handle that log message. It's not really valid for consuming code to call into the ILoggerProvider directly.
The ILogger<T> was the actual one that is made for DI. The ILogger came in order to help implement the factory pattern much more easily, instead of you writing on your own all the DI and Factory logic, that was one of the smartest decisions in asp.net core.
You can choose between:
ILogger<T> if you have a need to use factory and DI patterns in your code or you could use the ILogger, to implement simple logging with no DI needed.
given that, The ILoggerProvider is just a bridge to handle each of the registered log's messages. There is no need to use it, as it does not effect anything that you should intervene in code, It listens to the registered ILoggerProvider and handles the messages. That's about it.
Sticking to the question, I believe ILogger<T> is the right option, considering downside of other options:
- Injecting
ILoggerFactory force your user to give away the control of the mutable global logger factory to your class library. Moreover, by accepting ILoggerFactory your class now can write to log with any arbitrary category name with CreateLogger method. While ILoggerFactory is usually available as a singleton in DI container, I as a user would doubt why any library would need to use it.
- While the method
ILoggerProvider.CreateLogger looks like it, it is not intended for injection. It is used with ILoggerFactory.AddProvider so the factory can create aggregated ILogger that writes to multiple ILogger created from each registered providers. This is clear when you inspect the implementation of LoggerFactory.CreateLogger
- Accepting
ILogger also looks like the way to go, but it is impossible with .NET Core DI. This actually sounds like the reason why they needed to provide ILogger<T> at the first place.
So after all, we have no better choice than ILogger<T>, if we were to choose from those classes.
Another approach would be to inject something else that wraps non-generic ILogger, which in this case should be non-generic one. The idea is that by wrapping it with your own class, you take full control of how user could configure it.
For library design good approach would be:
1.Do not force consumers to inject logger to your classes. Simply create another ctor passing NullLoggerFactory.
class MyClass
{
private readonly ILoggerFactory _loggerFactory;
public MyClass():this(NullLoggerFactory.Instance)
{
}
public MyClass(ILoggerFactory loggerFactory)
{
this._loggerFactory = loggerFactory ?? NullLoggerFactory.Instance;
}
}
2. Limit number of categories which you use when you create loggers to allow consumers configure logs filtering easily.
this._loggerFactory.CreateLogger(Consts.CategoryName)
The default approach is meant to be ILogger<T>. This means that in the log the logs from the specific class will be clearly visible because they will include the full class name as the context. For example if the full name of your class is MyLibrary.MyClass you will get this in the log entries created by this class. For example:
MyLibrary.MyClass:Information: My information log
You should use the ILoggerFactory if you want to specify your own context. For example that all the logs from your library have the same log context instead every class. For example:
loggerFactory.CreateLogger("MyLibrary");
And then the log will look like this:
MyLibrary:Information: My information log
If you do that in all classes then the context will be just MyLibrary for all classes. I imagine you would want to do that for a library if you don't want to expose the inner class structure in the logs.
Regarding the optional logging. I think you should always require the ILogger or ILoggerFactory in the constructor and leave it to the consumer of the library to turn it off or provide a Logger that does nothing in the dependency injection if they don't want logging. It is very easy to turn of the logging for a specific context in the configuration. For example:
{
"Logging": {
"LogLevel": {
"Default": "Warning",
"MyLibrary": "None"
}
}
}
