Pass ILogger or ILoggerFactory to constructors in AspNet Core?

Question

The MS docs article "Introduction to Logging in ASP.NET Core" gives 2 examples of constructor injection

using ILogger

private readonly ILogger _logger;   
public TodoController(ILogger<TodoController> logger)  
{ 
    _logger = logger;   
}

and ILoggerFactory

private readonly ILogger _logger;  
public TodoController( ILoggerFactory loggerFactory)  
{  
    _logger = loggerFactory.CreateLogger<TodoController>();  
}

My question is what should I pass to child classes called from my controller

pass ILoggerFactory to my child classes called from the controller and in each class call LoggerFactoryExtensions.CreateLogger<MyChildClass>() or
pass parent controller's ILogger<MyController> to each child class created from the controller and having non-generic parameter ILogger.

In logs I prefer to see separate category 'MyChildClass' for each class, rather than all classes use the category 'MyController' from the parent controller.
However CreateLogger in each object construction can be an expensive operation (e.g. see https://github.com/aspnet/Logging/issues/524)

Which option will you recommend? Can you suggest any other approach?

Have child classes inject their own logger and then inject the child class into the controller. That way the child loggers will get resolved when the child classes are being resolved and injected into the controller. — Nkosi
I woulds say that both injecting a ILogger<T> and ILoggerFactory are bad approaches. Injecting a ILoggerFactory causes your constructors to do too much, while ILogger<T> just adds more noise to your code. Instead, you should inject a non-generic ILogger instead and let the DI infrastructure automatically inject the right logger for you. The reason however that the MS docs don't show this, is because the built-in container is too limited. It can't inject an ILogger for you that way. — Steven
@Steven: Will more advanced dependency injection libraries be able to pass to non-generic ILogger parameter a logger having category with the name of the current class? Or I will need to insert the category name in each constructor? — Michael Freidgeim
@steven, in order to log asp.net internal things - need to register a logger using Microsoft.Extendions.DependencyInjection - say Serilog - and then need to plug - say - autofac in (easy), and register Serilog again in autofac? — lev haikin

Nkosi Nkosi · Accepted Answer · 2017-09-02T13:15:17

This is more of a design issue.

The controller should not be creating child classes anyway. That is not a concern the controller should be dealing with and goes against SRP (Single Responsibility Principle).

My question is what should I pass to child classes called from my controller

If your preference is to separate the loggers, then there really isn't any other choice here than to have child (dependent) classes have their own loggers.

Have child classes inject their own logger

public class TodoRepository : ITodoRepository {
    private readonly ILogger logger; 

    public TodoRepository(ILogger<TodoRepository> logger) { 
        this.logger = logger;   
    }
  
    //...
}

and then inject the child class into the controller.

public class TodoController : Controller {
    private readonly ILogger logger;
    private readonly ITodoRepository todoRepository;

    public TodoController(ILogger<TodoController> logger, ITodoRepository todoRepository) {
        this.logger = logger;   
        this.todoRepository = todoRepository;
    }

    //...
}

That way, the child loggers will get resolved when the child classes are being resolved and injected into the controller.

Pass ILogger or ILoggerFactory to constructors in AspNet Core?

1 Answers