-
Notifications
You must be signed in to change notification settings - Fork 209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
How to use with sinks that depend on ConfigureServices being run first? #84
How to use with sinks that depend on ConfigureServices being run first? #84
Comments
cc @aloneguid |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Actually, as far as the title and original description go, this is still a problem now that TelemetryConfiguration.Active is deprecated (microsoft/ApplicationInsights-dotnet#1152). In short:
Related: serilog-contrib/serilog-sinks-applicationinsights#121 How can this be solved? |
If you're using So at a basic level, you could have something like this:
|
Thanks! But aren't I then losing out on logging startup exceptions, as well as any custom setup performed by serilog-aspnetcore/src/Serilog.AspNetCore/SerilogWebHostBuilderExtensions.cs Lines 139 to 158 in e6e14e6
Case in point: If I call Also, it seems that using |
After checking library sources and experimenting for many hours today, here is a workaround I have arrived at which seems to work. Let me know if I'm shooting myself in the foot. In doing this I have sacrificed logging early startup errors (in
[<EntryPoint>]
let main args =
WebHost
.CreateDefaultBuilder(args)
.CaptureStartupErrors(true)
.UseSerilog(dispose = true)
.UseStartup<Startup>()
.Build()
.Run()
0
type Startup() =
member __.ConfigureServices(services: IServiceCollection) : unit =
services.AddApplicationInsightsTelemetry() |> ignore
let sp = services.BuildServiceProvider()
let telemetryConfig = sp.GetRequiredService<TelemetryConfiguration>()
Log.Logger <-
LoggerConfiguration()
.WriteTo.ApplicationInsights(telemetryConfig, TelemetryConverter.Traces)
.CreateLogger()
services.AddSingleton<ILogger>(Log.Logger) |> ignore |
I can see how that would work - it feels very clunky though; building the service provider to configure something for the next time the service provider is built :) I've got a feeling that calling IServiceCollection.BuildServiceProvider multiple times isn't great (the framework will call it for you once ConfigureServices exits unless you return a pre-built one) I can't remember where I read it - I'll do a bit of digging. Edit: I suspect it'll be something to do with multiple singleton instance construction in addition to it being extra work at start-up time. |
Could you use a lambda ILogger registration instead? I'm unsure of the F# syntax, but in C# it would be: services.AddSingleton<ILogger>(
serviceProvider =>
{
Log.Logger = new LoggerConfiguration().WriteTo.ApplicationInsights(
serviceProvider.GetRequiredService<TelemetryConfiguration>(),
TelemetryConverter.Traces)
.CreateLogger();
return Log.Logger;
}); |
Maybe related to #130 |
@sungam3r Interesting - so I think that might make something like this possible? (untested) WebHost.CreateDefaultBuilder(args)
.UseSerilog((builder, hostingContext, loggerConfiguration) =>
{
var config = loggerConfiguration
.ReadFrom.Configuration(hostingContext.Configuration)
.Enrich.FromLogContext();
builder.ConfigureServices(services => services.AddSingleton(
serviceProvider =>
{
cfg.WriteTo.ApplicationInsights(
serviceProvider.GetRequiredService<TelemetryConfiguration>(),
TelemetryConverter.Traces);
return config;
}));
})
.UseStartup<Startup>(); |
Yes, that's exactly how I wanted to use it. |
Regarding my earlier suggestion of using But In the case of WebHost, I've found that if you're using a startup class, you can request an instance of
EDIT: And now I've just discovered that |
I just tried your suggestion, but then I don't get anything logged at all.
Not sure if by the edit you intended to retract your previous suggestion:
But if you still stand by that: Wouldn't that mean that Serilog would use an unconfigured |
In the case of WebHostBuilder in .NET Core 2.2, I've had good results injecting an instance of
Then in
By this strategy, by the time the web app starts, App Insights and Serilog is fully configured. It does mean that any logging you'd happen to be trying to do earlier in the startup pipeline would be missed, but, I guess things get a bit trickier if you need that (would maybe need to brute-force configuration outside of the host builder). Meanwhile, in generic host (at least in 2.2, haven't tried 3.0 yet), I got good results by registering a hosted service as a "logging initialiser" to run before my main service. Something a bit like this (not an exact copy-paste, so please forgive any gremlins in the code):
|
Hello, I've been facing this problem as well. What are the drawbacks of forcing the InstrumentationKey in a generic ITelemetryInitializer class? Just like you would do to define the Cloud context properties. Thanks, |
I'm using this with Serilog.Sinks.ApplicationInsights, but AFAIK there seems to be a conflict:
.UseSerilog
with theconfigureLogger
parameter,configureLogger
(which among other things calls Serilog.Sinks.ApplicationInsights'WriteTo.ApplicationInsights
) is executed before ASP.NET Core'sConfigureServices
.ConfigureServices
sets upTelemetryConfiguration.Active
WriteTo.ApplicationInsights
needsTelemetryConfiguration.Active
to be set up correctlyIs there a way to solve this?
The text was updated successfully, but these errors were encountered: