search

serilog / serilog-aspnetcore

Serilog integration for ASP.NET Core
Apache License 2.0
1.31k stars 205 forks source link

Update `README.md` example to show `builder.Services.AddSerilog()` in a minimal API project #368

Closed nblumhardt closed 6 months ago

nblumhardt commented 6 months ago

See https://github.com/serilog/serilog/issues/1855#issuecomment-2024546740

crozone commented 6 months ago

Should this be builder.Services.AddSerilog() or builder.Logging.Services.AddSerilog()?

nblumhardt commented 6 months ago

Digging into this some more, I think it should actually be:

builder.Host.UseSerilog()

The reason being, currently, that the callback accepted by this version exposes the appsettings.json IConfiguration, while builder.Services.AddSerilog() doesn't.

I think builder.Services and builder.Logging.Services will end up equivalent, but let me know if it turns out that I'm mistaken :-)

nblumhardt commented 6 months ago

Just noting that the minimal API template uses WebApplication.CreateBuilder(), which is different from the IHostApplicationBuilder being discussed in https://github.com/serilog/serilog-extensions-hosting/issues/76

nblumhardt commented 6 months ago

Seems like we should encourage builder.Services.AddSerilog(lc => ..) everywhere, since it works with both host styles, and handles configuration a bit more elegantly with AddSerilog(lc => lc.ReadFrom.Configuration(builder.Configuration)) instead of needing the second callback arg.