serilog / serilog-extensions-logging

Serilog provider for Microsoft.Extensions.Logging
Apache License 2.0
309 stars 99 forks source link

Create an MEL ILogger from a Serilog Log? #126

Closed PureKrome closed 5 years ago

PureKrome commented 5 years ago

Heya 👋

Title is probably not written right .. but .. I'm playing with the new Core 2.2 Generic Host bits and in a sample console app, trying to do some logging really early on in ConfigureServices. Unfortunately, CS doesn't have access to ILogger (as far as i know) but I've already added serilog .. so serilog knows what I want to log.

I tried asking this in this aspnet/hosting thread/PR and Fowler was kind to try and help provide some thoughts.

So .. is it possible to create (even temporarily) an ILogger from Serilog's static logger ... just for the purpose of that CS scope.

Here's some pseduo code for a CONSOLE APP to try and explain (based on the common Serilog pattern):

    public class Program
    {
        private static IConfiguration Configuration { get; } = new ConfigurationBuilder()
            .SetBasePath(Directory.GetCurrentDirectory())
            .AddJsonFile("appsettings.json", optional: false, reloadOnChange: true)
            .AddJsonFile($"appsettings.{Environment.GetEnvironmentVariable("NETCORE_ENVIRONMENT") ?? "Production"}.json", optional: true)
            .AddEnvironmentVariables()
            .Build();

        public static async Task Main(string[] args)
        {
            Log.Logger = new LoggerConfiguration()
                .ReadFrom.Configuration(Configuration)
                .Enrich.FromLogContext()
                .CreateLogger();

            try
            {
                var builder = new HostBuilder()
                    .ConfigureServices((hostContext, services) =>
                    {
                        services.AddLogging(configure => configure.AddSerilog(dispose: true));

                       var myConfigurationSettings = services.AddConfigurationSettings(Configuration);
                        services.AddHostedService<MyBackgroundTaskService>();

                        services.AddRavenDb(myConfigurationSettings.DatabaseSettings, --here is some ILogger instance-- )
                    });

                await builder.RunConsoleAsync();
            }
            catch (Exception exception)
            {
                Log.Error(exception, "Stopped program because of an exception.");
            }

            Log.Debug("Finished shutting down app.");

            // Ensure to flush and stop internal timers/threads before application-exit (Avoid segmentation fault on Linux)
            Log.CloseAndFlush();
        }
    }
PureKrome commented 5 years ago

Also, having a look at sample in this repo showed an interesting bit of code...

var serviceProvider = services.BuildServiceProvider();
// getting the logger using the class's name is conventional
var logger = serviceProvider.GetRequiredService<ILogger<Program>>();

So i'm not sure if that is an abuse of things, doing those two lines, inside a ConfigureServices scope .. e.g.:

var builder = new HostBuilder()
    .ConfigureServices((hostContext, services) =>
    {
        services.AddLogging(configure => configure.AddSerilog(dispose: true));

        var serviceProvider = services.BuildServiceProvider();
        var logger = serviceProvider.GetRequiredService<ILogger<Program>>();

        var myConfigurationSettings = services.AddConfigurationSettings(Configuration);
        services.AddHostedService<MyBackgroundTaskService>();

        services.AddRavenDb(myConfigurationSettings.DatabaseSettings, logger)
    });
nblumhardt commented 5 years ago

Hi @PureKrome! 👋

I think Log.Logger is the ILogger instance you're looking for (though I haven't had a chance to read through the thread you linked, so sorry if I've missed some details. Let me know if this helps.

Best regards, Nick

poke commented 5 years ago

@nblumhardt I believe he is looking for an M.E.Logging.ILogger, not a Serilog ILogger.

@PureKrome You’re right, building the service provider inside of the ConfigureServices (where you just configure the service collection) is a bad antipattern that you should generally avoid. There are usually better ways to do it.

In your case, you have a Serilog logger ready, so you can just use that directly. If you absolutely need a M.E.Logging.ILogger, then you could create the SerilogLogger directly; unfortunately, you will also need a SerilogProvider then:

var logger = new SerilogLoggerProvider(serilogLogger).CreateLogger(nameof(Program));

This will give you an ILogger; if you just stick to logging methods and avoid logger scopes, then you don’t even need to dispose this since you are reusing the serilog logger anyway. But of course, since you are explicitly using a Serilog logger, this will only log into the Serilog logger. If you have configured other M.E.Logging providers, then those will be ignored for this logging setup (but will be available later when the DI container is built).

If you need an ILogger<T> then unfortunately this gets a bit more complicated since you then need to wrap the logger provider in a logger factory.

Btw. in my opinion you should not set up the global Serilog Log.Logger but pass the instance directly to AddSerilog so that you don’t rely on the static instance.

PureKrome commented 5 years ago

then you could create the SerilogLogger directly; unfortunately, you will also need a SerilogProvider then

But .. why would I have to create another one if we already have Logger.Log? wouldn't that have one already?

Btw. in my opinion you should not set up the global Serilog Log.Logger but pass the instance directly to AddSerilog so that you don’t rely on the static instance.

Interesting. This was a similar qustion/opinion I had recently with a answer promptly provided :) In your opinion, how would you do it differently?

poke commented 5 years ago

why would I have to create another one if we already have Logger.Log?

You can use Log.Logger, the Serilog.ILogger, directly, but that’s a different ILogger than the one you have when using M.E.Logging.

In your opinion, how would you do it differently?

I would just not assign the static Log.Logger. That way, you don’t find yourself accidentally using the static logger (directly or indirectly). So the code could look like this:

.ConfigureServices((hostContext, services) =>
{
    var serilogLogger = new LoggerConfiguration()
        // …
        .CreateLogger();

    services.AddLogging(configure => configure.AddSerilog(serilogLogger, dispose: true));
});
connorads commented 5 years ago

@PureKrome you may find Andrew Lock's library Serilog.Extensions.Hosting useful. Here's his blog post about it: Adding Serilog to the ASP.NET Core Generic Host.

nblumhardt commented 5 years ago

Assuming this ended up resolved via Serilog.Extensions.Hosting; if not, please let us know :-)

mwittmann commented 4 years ago

This may not be entirely germane, but just now I was trying to work out how to create a Microsoft.Extensions.Logging.ILogger<T> logger instance using Serilog. Unless I'm missing something, it seems that Serilog.Extensions.Logging does not provide this?

As you know, for better or worse, .Net Core's standard pattern is to provide typed loggers via DI, as illustrated below.

namespace MyJobProcessor 
{
    public class ProcessFiles
    {
        private readonly Microsoft.Extensions.Logging.ILogger<ProcessFiles> _log;

        public ProcessFiles(ILogger<ProcessFiles> logger)
        {
            _log = logger;
        }

        public void Test1(string[] args)
        {
            _log.LogInformation("Starting Test1");
            ...do stuff...
            _log.LogInformation("Finished Test1");
        }
    }
}

For testing and debugging, I wanted to be able to instantiate such classes and call methods like Test1() including logging outside of the .Net Core webhost framework , so I wanted to provide an MEL ILogger <ProcessFiles> instance to the constructor. I ended up with the following. (Sorry for the fully qualified names... I wanted to completely disambiguate Serilog and MEL interfaces.)

using Microsoft.Extensions.Logging;
using Serilog;
using Serilog.Extensions.Logging;

public class SerilogTypedLogger<T> : Microsoft.Extensions.Logging.ILogger<T>
{
    Microsoft.Extensions.Logging.ILogger _logger;

    public SerilogTypedLogger(Serilog.ILogger logger) 
    {
        using (var logfactory = new SerilogLoggerFactory(logger))
            _logger = logfactory.CreateLogger(typeof(T).FullName);
    }

    IDisposable Microsoft.Extensions.Logging.ILogger.BeginScope<TState>(TState state) =>
        _logger.BeginScope<TState>(state);

    bool Microsoft.Extensions.Logging.ILogger.IsEnabled(LogLevel logLevel) => 
        _logger.IsEnabled(logLevel);

    void Microsoft.Extensions.Logging.ILogger.Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter) => 
        _logger.Log<TState>(logLevel, eventId, state, exception, formatter);
}

So this then let me do:

...create Configuration object...

Serilog.Log.Logger = new Serilog.LoggerConfiguration()
    .ReadFrom.Configuration(Configuration)
    .CreateLogger();

var typedLogger = new SerilogTypedLogger<JobProcessor.ProcessFiles>(Serilog.Log.Logger);
new ProcessFiles(typedLogger).Test1(args);

Is this available elsewhere? (Did I reinvent the wheel?) If not, perhaps it's worth adding to Serilog.Extensions.Logging?

jamotaylor commented 3 years ago
var logger = new SerilogLoggerProvider(serilogLogger).CreateLogger(nameof(Program));

@poke this helped me, thanks. I need a Microsoft.Extensions.Logging.ILogger to inject into a 3rd party library (Raven.Migrations).

Just a note for others: the full name of the class is Serilog.Extensions.Logging.SerilogLoggerProvider