Not obvious DSN settings Serilog and ASP .NET Core

olsh commented 2 years ago

If you use both Sentry.AspNetCore and Sentry.Serilog and set DSN with the Serilog integration, Sentry doesn't work. So, to fix it, you should configure Sentry with UseSentry() from ASP .NET Core integration package.

It's kind of not obvious, probably examples and the methods (UseSentry) comments should be updated.

Environment

How do you use Sentry? Sentry SaaS (sentry.io)

Which SDK and version? Sentry.AspNetCore 3.9.3 Sentry.Serilog 3.9.3

Steps to Reproduce

Create default ASP.NET Core 3.1 Project

Use the following code in Program.cs

    public static IHostBuilder CreateHostBuilder(string[] args)
    {
        return Host.CreateDefaultBuilder(args)
            .ConfigureWebHostDefaults(webBuilder =>
            {
                webBuilder.UseStartup<Startup>()
                    .UseSerilog((_, c) =>
                        c.Enrich
                            .FromLogContext()
                            .WriteTo.Sentry("PUT_YOUR_DSN_HERE"))
                    .UseSentry();
            });
    }

    public static void Main(string[] args)
    {
        CreateHostBuilder(args)
            .Build()
            .Run();
    }

Try to throw an exception somewhere

Expected Result

Sentry received the error

Actual Result

Sentry doesn't receive the error

bruno-garcia commented 2 years ago

Thanks for raising @olsh. I agree it's confusing. In fact, it's also not ideal right now for a few reasons.

Technically the 'right' way would be to add the DSN to both of them. That is, if there's a crash while bootstrapping the ASP.NET Core host after the logging infra started, only if Serilog had the DSN that crash would be captured. That results in the SDK having to be initialized twice (through the Serilog integration then again after that through ASP.NET Core integration).

Things were built in a way that at least work even after reinitialization but it does mean that any callback (i.e: BeforeSend or eventProcessor) would need to be set twice which is far from sub-optimal. I had this challenge also on NuGet Trends and identified the need to improve this. There I left all config in appsettings.json/env var and I defined the DSN twice.

Other than documenting this behavior, and suggesting best practices I'm not sure the path forward. Since these are two different packages, disconnected with different configuration options, etc.

Some somewhat related issues are: #1001 #257 #1137

mattjohnsonpint commented 2 years ago

So, if I understand correctly, this isn't really a bug but rather a pitfall that needs to be either better documented or to have a new design.

getsentry / sentry-dotnet