adams85 / filelogger

A lightweight yet feature-rich file logger implementation for the Microsoft.Extensions.Logging framework.
MIT License
148 stars 22 forks source link

Logging from additional containers #15

Closed kostazol closed 2 years ago

kostazol commented 3 years ago

Hello. In my app i need to create additional containers for creating two instances of Rebus.

            var services = new ServiceCollection();
            services.AddLogging(builder =>
            {
                builder.AddConfiguration(_configuration.GetSection("Logging"));
                builder.AddFile(o => o.RootPath = AppContext.BaseDirectory);
            });
            services.AddRebusClient();

            var provider = services.BuildServiceProvider();
            provider.UseRebus();

When I use the same in ConfigureServices((hostContext, services) it works great, but when I use it in ServiceCollection it doesn't write to file. Do you have any ideas? Where did I make mistake? There is my config file: image

kostazol commented 3 years ago

I found solution for Serilog, but I can't solve this problem for your library.

adams85 commented 3 years ago

Hi there!

This is a DI configuration problem, not an issue with the library.

When you build two containers (service providers) from the same ServiceCollection, you'll end up with two instances of singleton services (as both of the containers will create its own instance). This poses a problem for the file logger library because two FileLoggerProvider instances with the same configuration imply two writer queues to the same log files.

When FileAccessMode of a log file is set to a mode other than OpenTemporarily, the queue processor will try to access that file with exclusive write permissions and keep the file open until completion. So if you have two queue processors, one of them won't be able to access the file, thus, it can't write into it. I think this explains the behavior you observed.

Having multiple instances of singleton services may lead to unexpected behavior. Not only in the case of file logger but other services, so I strongly advise against this. If you inevitably need multiple containers, I suggest that you use a DI implementation which supports the concept of nested scopes/registrations (like Autofac or DryIoc) and register your singleton services in the root scope.

For the sake of completeness it's worth mentioning that you can workaround this particular issue using MS DI. It works but feels a bit hacky to me:

var services = new ServiceCollection();

services.AddLogging(builder =>
{
    builder.AddConfiguration(_configuration.GetSection("Logging"));
    builder.AddFile(o => o.RootPath = AppContext.BaseDirectory);
});

// add your other services...

var provider1 = services.BuildServiceProvider();

// replace the ILoggerProvider registration for FileLoggerProvider with the instance acquired from the first container (provider1),
// this way the second container (provider2) will reuse that instance instead of creating another one;
// just one thing to keep in mind: the second container must be disposed before the first one to avoid lifetime issues
var index = FindServiceIndex<ILoggerProvider, FileLoggerProvider>(services);
var fileLoggerProvider = provider1.GetRequiredService<IEnumerable<ILoggerProvider>>().OfType<FileLoggerProvider>().FirstOrDefault();
services[index] = ServiceDescriptor.Singleton<ILoggerProvider>(fileLoggerProvider);

var provider2 = services.BuildServiceProvider();

static int FindServiceIndex<TService, TImplementation>(ServiceCollection services)
    where TService : class
    where TImplementation : class, TService
{
    for (var i = services.Count - 1; i >= 0; i--)
    {
        var service = services[i];

        if (service.ServiceType == typeof(TService) &&
            (service.ImplementationType == typeof(TImplementation) ||
             service.ImplementationInstance?.GetType() == typeof(TImplementation) ||
             service.ImplementationFactory?.Method.ReturnType == typeof(TImplementation)))
        {
            return i;
        }
    }

    return -1;
}