14

On the logging samples in the documentation, there is an example how to inject a logger into a controller:

public class TodoController : Controller
{
    private readonly ITodoRepository _todoRepository;
    private readonly ILogger _logger;

    public TodoController(ITodoRepository todoRepository,
        ILogger<TodoController> logger)
    {
        _todoRepository = todoRepository;
        _logger = logger;
    }
}

Does the DI framework create a new logger each time I inject a logger into something like here? Is there a better way?

poke
  • 369,085
  • 72
  • 557
  • 602
Stephu
  • 3,236
  • 4
  • 21
  • 33

1 Answers1

16

This is easily answered by a look into the source. When you do services.AddLogging(), the default behavior is that ILogger<T> is registered as a singleton:

public static IServiceCollection AddLogging(this IServiceCollection services, Action<ILoggingBuilder> configure)
{
    // …

    services.TryAdd(ServiceDescriptor.Singleton<ILoggerFactory, LoggerFactory>());
    services.TryAdd(ServiceDescriptor.Singleton(typeof(ILogger<>), typeof(Logger<>)));

    // …
}

So no, ILogger<T> instances for a certain type T are kept around for as long as the application is running. So when injecting an ILogger<TodoController> into your controller, the same logger instance will be passed to it every time.

Of course this only applies to the logger, but not your controller itself. By default, controllers are activated outside of DI but effectively live with a scoped lifetime. So on every request, there will be a new controller instance; but that one will then get the logger instance from before.

To answer your last question, is there a better way? No. Apart from the fact that this behavior is already a good one (since there’s no need for new logger instances), the proper way to use logging is indeed to inject ILogger<T> into types T, so you get a properly categorized logger instance. There’s really no need to worry about the very thin logger here when there are so many way more expensive things going on in the background that you will likely never see ;)

Since the ILogger<T> is a singleton, its instance will be reused all throughout the application. Note that this will not have an effect on logging scopes. The ILogger<T> implementation that you use within your application is actually just a thin wrapper that forwards logging calls to the internal loggers (which are also effectively singletons). So the lifetime of ILogger<T> is actually not relevant since they do not keep any state at all.

The logging scopes themselves are persisted using an AsyncLocal which is a mechanism to keep state throughout the asynchronous call flow. That means that logging scopes will just “magically” work and not leak outside of the call flow just because some instances happen to be shared between multiple threads (or asynchronous flows).

poke
  • 369,085
  • 72
  • 557
  • 602
  • "there’s no need for new logger instances": What about logging scopes? If concurrent requests are sharing a logger, it seems undesirable that they share anything added via `ILogger`'s `BeginScope()`... – Kyle McClellan Mar 12 '19 at 17:52
  • @KyleMcClellan That’s not correct. The loggers that you consume are basically just wrappers around the actual (internal) loggers. If you begin a scope, all loggers (including existing instances) will have that scope applied if you log within that scope. – poke Mar 12 '19 at 20:35
  • @poke But how do those instances determine what `BeginScope`-scope they're in? It's unclear how `BeginScope` works when used within a method belonging to a singleton class using a singleton `ILogger` (or multiple instances of a class using an injected `ILoggerFactory`'s `CreateLogger` method). – Dai May 02 '19 at 02:48
  • @Dai The `ILogger` implementations that you consume are just shells that forward to the underlying internal loggers, so the lifetime of those loggers is actually not relevant. They are reused as much as possible. Logging scopes are persisted using async locals, so that they follow your call flow properly (and don’t leak outside of it). – poke May 02 '19 at 13:37
  • @poke From your comments it looks to me like the ILogger obtained from DI is NOT necessarily a singleton but a specific instance (shell) that has scope identity and a reference to the real logger, which is singleton. Your answer is misleading as it states that it's always a singleton. – Guillermo Prandi Sep 09 '20 at 21:01
  • @GuillermoPrandi The `ILogger<>` _is_ registered as a singleton, so the resolved instance is always the same. What I explained in the comments is that these singleton instances don't do that much themselves but just forward calls internally to the underlying loggers. This mechanism does not depend on DI lifetimes. – poke Sep 10 '20 at 17:16
  • @poke Sorry, it's either one way or the other: I'm asking because if I've got this wrong I need to change things in my code: being `ILogger` a singleton, how can't a `BeginScope` started on one thread _not affect_ the `ILogger` used in another thread? Thread A uses `BeginScope("OBJECT A")` and thread B uses `BeginScope("OBJECT B")`... logs from both threads would have both contexts stacked if the ILogger is the same for both. – Guillermo Prandi Sep 11 '20 at 15:44
  • I've just tested and indeed the objects received reference the same object instance. So... I added this question: https://stackoverflow.com/questions/63851259/ – Guillermo Prandi Sep 11 '20 at 16:55
  • 1
    @GuillermoPrandi As I wrote in the comment directly above your first: *“Logging scopes are persisted using async locals, so that they follow your call flow properly (and don’t leak outside of it).”* – So no, it’s not one way or the other. – poke Sep 13 '20 at 10:31