Duplicate invocations of Sentry SDK initialization conflict and fail silently #898
Replies: 4 comments
-
Thanks for raising the detailed issue. The logging integration is automatically hooked up also through Agreed that it should reconfigure the SDK at least. |
Beta Was this translation helpful? Give feedback.
-
Well, we are using Serilog, more on that later, and I'm not 100% sure on the .NET logging system best practices with the multiple integrations, so that's part of the reason on why I'm asking. @bruno-garcia do you mean that the It would be nice to understand more explicitly from the docs on what are the recommended settings when multiple integrations are in use so that no duplicate initializations are done but all the necessary instrumentations are enabled. The things I'm wondering are:
I could of course read through the source code of the Sentry .NET SDK implementation to get clarity on these but I'm configuring infrastructure and Sentry SDKs for a bunch of other projects as well and it would be nice to understand the best practices for multiple instrumentations more clearly, as now I feel like I'm guessing for the right configurations. We also have another project using Serilog and I'm wondering on what are the best practices in setting up the combinations of packages. The ones that we commonly use are:
Here's an example of Serilog setup on which I'm also wondering about the correctness: using System;
using Microsoft.AspNetCore.Hosting;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Hosting;
using Serilog;
using Serilog.Events;
namespace Example
{
public class Program
{
public static int Main(string[] args)
{
Log.Logger = new LoggerConfiguration()
.MinimumLevel.Debug()
.MinimumLevel.Override("Microsoft", LogEventLevel.Information)
.Enrich.FromLogContext()
.WriteTo.Console(outputTemplate:
"[{Level}] {Message:lj} {Properties:j}{NewLine}{Exception}")
.WriteTo.Sentry(o =>
{
// Serilog Sentry integration https://docs.sentry.io/platforms/dotnet/guides/serilog/
// Sentry performance tracing configuration
o.TracesSampleRate = 0.02;
})
.CreateLogger();
try
{
Log.Information("Starting web host");
CreateHostBuilder(args).Build().Run();
return 0;
}
catch (Exception ex)
{
Log.Fatal(ex, "Host terminated unexpectedly");
return 1;
}
finally
{
Log.CloseAndFlush();
}
}
public static IHostBuilder CreateHostBuilder(string[] args) =>
Host.CreateDefaultBuilder(args)
.UseSerilog()
.ConfigureAppConfiguration((hostingContext, config) =>
{
config.AddEnvironmentVariables();
})
.ConfigureWebHostDefaults(webBuilder =>
{
webBuilder.UseStartup<Startup>();
// Adding Sentry integration to server error tracking
// https://docs.sentry.io/platforms/dotnet/aspnetcore/
// Startup.cs also invokes app.UseSentryTracing()
webBuilder.UseSentry(o =>
{
// Sentry SDK is already initialized previously in Serilog configuration
o.InitializeSdk = false;
});
});
}
} |
Beta Was this translation helpful? Give feedback.
-
Sounds like we need to improve our docs on the matter. https://docs.sentry.io/platforms/dotnet/guides/serilog/#configuration
Be that ASP.NET Core or WPF or any type of app. For
If you're just using plain ASP.NET Core (which includes NuGetTrends uses Sentry on ASP.NET Core with Serilog: Some relevant parts for reference:
I hope I clarified this above. Generally you can add packages and integrate things as needed (consider ASP.NET Core includes Microsoft.Extensions.Logging as mentioned above so that you don't need to add twice manually.
If you don't provide a
EF Core uses As mentioned above, if install This is the most basic ASP.NET Core app you can have which shows the simplest Sentry integration: https://github.com/getsentry/sentry-dotnet/tree/main/samples/Sentry.Samples.AspNetCore.Basic
For sure, we need to make sure these things are clear. The docs might need to have more clarity then though the points above are mostly in the docs (at least those that I linked to the docs). We'd appreciate some PRs there if you'd like to contribute. You docs repo is here: https://github.com/getsentry/sentry-docs and we have docs on how to contribute to the docs :)
If you're using Serilog as mentioned above is its own thing and needs to be added. Refer to NuGetTrends examples linked above.
By reading your configuration it all looks good. But it also means that since ASP.NET Core related integration will be picked up though. Only the main |
Beta Was this translation helpful? Give feedback.
-
Thanks for the comprehensive answer @bruno-garcia, this makes the correct configuration path a lot easier to understand! I'm happy with the answers and my issue is solved, but there are two separate things I'd like to comment on, on the design level:
1. Configuring multiple target platforms and different SDKs We are actually using the Only traces sample rate config is missing from the current environment variable based configuration. I think it would be nice to have e.g. 2. Configuring the multiple instrumentations for .NET Because I have to initialize .NET Core AND Serilog instrumentations for the .NET SDK there is also fairly high risk for making config errors on the code level unless I supply the exact same configuration keys for all of the initializations, if only the first SDK initialization is meaningful. I'm doing the same instrumentations and writing instructions for a bunch of projects with multiple devs participating so I can't assume everyone will get familiar with the inner workings of the SDK. Of course this is already a recognized issue here, and it's of course possible to write config or initialization code that avoids these errors. However, I don't think that's ideal developer experience for people just looking to correctly configure error and performance tracing with minimal effort. Using environment variable based configuration, one option would be to use Using file based configuration would either require keeping the configuration in the repository or baking it into the target build artifacts or runtime environment, and I don't think that's ideal because development and operations / SRE can have different configuration targets and needs that differ based on runtime environments (local vs. QA vs. production environments for example). Packaging file based configurations to e.g. Docker images or injecting them to runtime environments can be a hurdle, and environment variable based configuration is much more common with e.g. PaaS platforms.
|
Beta Was this translation helpful? Give feedback.
-
Environment
How do you use Sentry?
Sentry SaaS (sentry.io)
Which SDK and version?
.NET 5 and Sentry SDK version 3.0.6
Steps to Reproduce
Use both
AddSentry
inConfigureLogging
UseSentry
inConfigureWebHostDefaults
.Only configure options for the later one (see below).
Expected Result
Later invocation either
Reconfiguration or duplicate SDK configuration should IMO either always produce an error on misconfiguration or correctly reconfigure the options, but never fail silently.
Actual Result
The first invocation sets the SDK options and the options in the second invocation are ignored. Sentry does not e.g. enable debugging or performance tracing mode if options clash in the correct fashion.
EDIT: the logging docs do mention that the SDK should only be initialised once. However, the ordering and semantics of initialisations should perhaps be made clearer in the docs?
https://docs.sentry.io/platforms/dotnet/guides/extensions-logging/#options-and-initialization
The following code DOES NOT work:
The following code DOES work:
The following code also DOES work and produces the same end result:
Configuring options in all invocations DOES work correctly but is error prone as forgetting to add options in one place and remembering the strict invocation ordering produces easy-to-make and hard-to-debug bugs:
I'm assuming that the correct way to initialize the SDK with both error tracing and logging would be something like the following to avoid duplicate initializations? There's not a whole lot of documentation on this.
Beta Was this translation helpful? Give feedback.
All reactions