.NET Core 2.0 minimum level handling/roadmap

[nblumhardt]

The current provider works with .NET Core 2.0, but the APIs have shifted since the 1.x version and I'm not confident we've got a decent story here now.

.NET Core 2.0 has its own internal "minimum level" that will be set to Information regardless of what we do in Serilog, unless we play in the JSON configuration system introduced with that version:

datalust/seq-extensions-logging#22

The net effect will be that apps using Serilog today will have to either separately specify ILoggingBuilder.SetMinimumLevel() in addition to the Serilog minimum level, or add a "Serilog.Extensions.Logging.SerilogLoggerProvider" element to the "Logging" configuration section in appsettings.json with level configuration specific to Serilog, even if this duplicates what's already been set through LoggerConfiguration.

(We can improve this experience by adding [ProviderAlias("Serilog")] to SerilogLoggerProvider, but, it just wins us a shorter key under "Logging", no other improvement in level handling.)

The conundrum we're facing is: the Serilog level and level overrides are set via LoggerConfiguration. When we call loggingBuilder.AddSerilog(), we don't communicate to .NET Core that we're handling our own levels. So, the framework will apply an Information filter, regardless.

I'm not sure what ways forward are open to us; it doesn't look like .NET Core 2.0 allows any level configuration to take place via Serilog (MinimumLevel, MinimumLevel.Override()) without the user duplicating all of that configuration now through Microsoft.Extensions.Logging's APIs.

(About all I can come up with is taking the nuclear option and completely replacing the whole logging subsystem with a Serilog one, but that will paint us into a corner of another kind.)

Needing some guidance here - any help appreciated @pakrym @davidfowl & co.

[pakrym]

Couple notes:

1. Logging filter configuration comes from LoggerFilterOptions object in DI it is bound to JSON configuration by default but can be changed by usual means of services.Configure<LoggerFilterOptions>()

2. You can always call loggingBuilder.AddFilter<SerilogProvider>(null, LogLevel.Trace) in AddSerilog and we will forward all messages to Serilog if not specifically overridden by user.

[skomis-mm]

loggingBuilder.AddFilter< SerilogLoggerProvider >(null, LogLevel.Trace)

👍
Seems like this is what we are missing in Serilog.Extensions.Logging

Btw, plain upgrade to .net core 2.0 preserving ILoggingFactory.AddSerilog in the code doesn't introduce this kind of behavior change.

[kdaveid]

@skomis-mm: But on WebHostBuilder().ConfigureLogging(...) with asp.net core 2.0 you don't have direct access to ILoggerFactory anymore. This is the recommended way of adding services. You would have to build a Service Provider first and get the ILoggerFactory first out.

[nblumhardt]

Thanks for all the weigh-in, folks! I think the extra AddFilter call is a good workaround to get us out of the current hole 👍.

I've done some more digging, and have had good results with:

public static void Main(string[] args)
{
    Log.Logger = new LoggerConfiguration()
        .MinimumLevel.Debug()
        .WriteTo.Console(theme: AnsiConsoleTheme.Code)
        .CreateLogger();

    var host = new WebHostBuilder()
        .UseKestrel()
        .UseContentRoot(Directory.GetCurrentDirectory())
        .UseIISIntegration()
        .UseStartup<Startup>()
        .ConfigureServices(collection => collection.AddSingleton<ILoggerFactory>(new SerilogLoggerFactory()))
        .Build();

    host.Run();
}

SerilogLoggerFactory is really simple, I'll paste it in below. With this in place, "Logging" can be completely removed from appsettings.json (replaced with "Serilog" if you want to do JSON config there).

Why I like this:

• There's only one set of level controls/filters - everything just works, efficiently, via the one Serilog configuration
• There's no crazy console-via-MEL-but-other-things-via-Serilog confusion
• You only get the loggers you actually ask for, no defaults to remove etc.
• It's back to using MEL purely as a facade, as it was originally intended (no extra layers of filtering &c.)
• You actually get logging configured before hitting any other components that may fail, so just adding try/catch around the web host builder enables full exception handling

Factory:

public class SerilogLoggerFactory : ILoggerFactory
{
    readonly SerilogLoggerProvider _provider;

    public SerilogLoggerFactory(Serilog.ILogger logger = null, bool dispose = false)
    {
        _provider = new SerilogLoggerProvider(logger, dispose);
    }

    public void Dispose()
    {
        _provider.Dispose();
    }

    public ILogger CreateLogger(string categoryName)
    {
        return _provider.CreateLogger(categoryName);
    }

    public void AddProvider(ILoggerProvider provider)
    {
        SelfLog.WriteLine("Ignoring added logger provider {0}", provider);
    }
}

I'm going to PR the quick-fix into 2.0, and create another PR with the factory and updated docs for a 3.0.

Thoughts/feedback welcome 👍

[nblumhardt]

Updated Program.cs, with some extension method goodness and full EH:

public static int Main(string[] args)
{
    Log.Logger = new LoggerConfiguration()
        .MinimumLevel.Debug()
        .WriteTo.Console(theme: AnsiConsoleTheme.Code)
        .CreateLogger();

    try
    {
        var host = new WebHostBuilder()
            .UseKestrel()
            .UseContentRoot(Directory.GetCurrentDirectory())
            .UseIISIntegration()
            .UseStartup<Startup>()
            .UseSerilog()
            .Build();

        host.Run();
        return 0;
    }
    catch (Exception ex)
    {
        Log.Fatal(ex, "Unhandled exception");
        return 1;
    }
    finally
    {
        Log.CloseAndFlush();
    }
}

[analogrelay]

I think you'd want .MinimumLevel.Verbose() no? Trace (which maps to Verbose, I believe) is lower than Debug.

[analogrelay]

Would it be a terrible idea to have an overload .UseSerilog(this IWebHostBuilder builder, Action<LoggerConfiguration> configuration, bool dispose = false)? (and maybe even making that the only one? If you want to build the logger yourself there's always .AddSerilog ...). I ask that way because I'm not sure how I feel about it myself 😝.

That way, the configuration we give you in the lambda can be preconfigured with some sensible M.E.L defaults. It feels like a way to encourage people into the happy path of making sure the basic settings aren't going to get in their way, but still gives them all the necessary opt-outs.