Docs: Reading configuration twice

[wdolek]

There's following snippet of code in README.md:

public class Program
{
    public static int Main(string[] args)
    {
        Log.Logger = new LoggerConfiguration()
            .MinimumLevel.Debug()
            .MinimumLevel.Override("Microsoft", LogEventLevel.Information)
            .Enrich.FromLogContext()
            .WriteTo.Console()
            .CreateLogger();

        try
        {
            // ...
        }
        finally
        {
            Log.CloseAndFlush();
        }
    }
}

No configuration (config file) is used - you configure logger in place programmatically - this way, you can't put your configuration to appsettings.json (unless there's something magical which takes care later).

There's link to sample project which contains following code:

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

        public static int Main(string[] args)
        {
            Log.Logger = new LoggerConfiguration()
                .ReadFrom.Configuration(Configuration)
                .Enrich.FromLogContext()
                .WriteTo.Debug()
                .WriteTo.Console(
                    outputTemplate: "[{Timestamp:HH:mm:ss} {Level:u3}] {Message:lj} {Properties:j}{NewLine}{Exception}")
                .CreateLogger();

            try
            {
                // ...
            }
            finally
            {
                Log.CloseAndFlush();
            }
        }

        public static IHostBuilder CreateHostBuilder(string[] args) =>
            Host.CreateDefaultBuilder(args)
                .ConfigureWebHostDefaults(webBuilder =>
                {
                    webBuilder.UseStartup<Startup>();
                })
                .UseSerilog();
    }

Notice that user is encouraged to use use ConfigurationBuilder and read appsettings.json at this very place. However, if you check what ConfigureWebHostDefaults does later on - yeah, it reads appsettings.json as well.

Sure, this has tiny impact on startup time, however I'm bit worried about duplication here. It may also happen you read configuration different way than is read second time - debugging is going to be fun. Last but not least, there's additional complexity which may not be clear to some.

Is this really the right way how to use Serilog?

---

Instead of all that above, example should start just with:

    public class Program
    {
        public static void Main(string[] args) =>
            CreateHostBuilder(args).Build().Run();

        public static IHostBuilder CreateHostBuilder(string[] args) =>
            Host.CreateDefaultBuilder(args)
                .ConfigureWebHostDefaults(
                    webBuilder =>
                    {
                        webBuilder.UseStartup<Startup>();
                    })
                // Use Serilog - use configuration read by web host
                .UseSerilog(
                    (hostingContext, loggerConfiguration) =>
                        loggerConfiguration.ReadFrom.Configuration(hostingContext.Configuration));

    }

[phlashdev]

You have taken quite some thought about this, and mentioned already some important things. The sample you are referring is called EarlyInitializationSample and it's use is just that: using the configuration system for initialization of serilog as early as possible. This has some benefits and some downsides, e.g. that it's impossible to log errors while building the host. I tend to find it brittle and prone to error, but some folks might require it so it just depends on what your specific needs are.

I encourage you read through this excellent blog post from @nblumhardt himself, in which he details a lot of his thoughts and considerations about your questions.

At least take a look at the section about Environment-specific sink configuration, the following excerpt is coming from there.

I mentioned earlier on that Serilog can read settings from appSettings.json. I want to spend a few moments convincing you that it’s not the best place for environment-specific configuration like this.

First, the appSettings.json configuration system isn’t available until some time into the execution of CreateHostBuilder(), and this would cause us to delay setting up the logger, potentially missing out on those all-important startup errors.

It’s possible to set up the configuration system early, and this is shown in one of the Serilog.AspNetCore examples, but this still leaves us suceptible to problems with reading that configuration at runtime: missing configuration files, broken JSON syntax, assembly loading issues, and so-on.

You’re probably detecting a theme by now. Logging setup should be absolutely bullet-proof, as early as possible, and therefore dependency-free.

I hope this answers your question!

[wdolek]

@phlashdev thanks for your response! True, I didn't really think about logging startup before things get initialized. Thank you for linking blog post - haven't found that before.

I'm still bit puzzled though - if I got it right, it is preferable to configure logger in place programmatically rather than using appsettings.json (yep, no one will change those values in runtime). However, when I was thinking about reusability of configuration, I was still hitting snippets of how to configure logger via JSON.

Would it be worth saying it clearly (~"best practices"):

• don't use appsettings.json; remove all related examples across all Serilog projects (demonstrate that functionality only on Serilog.Settings.Configuration)
• go with LoggerConfiguration (benefit of importing classes, it was pain to figure out all namespaces/class names when using JSON config)
• ... and if really having to use config file, name it differently - separate log configuration from the rest

I was thinking of submitting PR updating documentation, putting UseSerilog((hostingContext, loggerConfiguration) => loggerConfiguration.ReadFrom.Configuration(hostingContext.Configuration)); at the beginning, now I see it's not the best idea.

Feel free to close this ticket.

[phlashdev]

Glad I could clarify some bits for you!

I'm not sure if we really should get rid of all related samples outside Serilog.Settings.Configuration like you sad, but I think we should go over the documentation and see if we can clarify this better.

[nblumhardt]

👋 https://nblumhardt.com/2020/10/bootstrap-logger/ may be relevant, too.

[phlashdev]

@nblumhardt thanks for sharing the link, I already tested it and I think this approach is truly the best of both worlds. It simplifies the configuration in my application drastically! We should try to get the hosting-extensions package to a production state, and get the docs aligned than it would be a really good solution for this issue.

@wdolek does this help you further?

[wdolek]

@phlashdev :) I think I learned something here, closing this issue. Thanks for valuable discussion 🙇

[nblumhardt]

@phlashdev agreed! Unfortunately I stalled a little on dropping .NET Framework support in serilog/serilog-extensions-hosting#35 - I'm on the fence about moving forward on it just yet. Thoughts pro/con would be welcome over there, if you can help move it along :-)