getsentry / sentry-dotnet

Sentry SDK for .NET
https://docs.sentry.io/platforms/dotnet
MIT License
566 stars 203 forks source link

Add extensions for IHostBuilder #190

Closed LordMike closed 1 year ago

LordMike commented 5 years ago

The current implementation for MS's DI is for IWebHostBuilder, but given the new Generic Hosts, I think more and more will use the IHostBuilder builder.

Please add a .UseSentry() for IHostBuilder. :)

bruno-garcia commented 5 years ago

True this is pending. Would be great to get some help on this.

xt0rted commented 4 years ago

I'm migrating a console app over to the new generic host and in order to use appsettings.json for my config with the ILoggingBuilder.AddSentry() method I needed to adjust my setup like so:

 var builder = Host.CreateDefaultBuilder()
     .ConfigureLogging((ctx, loggingBuilder) =>
     {
+        loggingBuilder.Services.Configure<SentryLoggingOptions>(ctx.Configuration.GetSection("Sentry"));
         loggingBuilder.AddSentry();
     })

To make this more reusable I've added a helper function:

public static class SentryHostBuilderExtensions
{
    public static IHostBuilder UseSentry(this IHostBuilder builder) =>
        builder.ConfigureLogging((ctx, logging) =>
        {
            logging.Services.Configure<SentryLoggingOptions>(ctx.Configuration.GetSection("Sentry"));

            logging.AddSentry();
        });
}

And then I use it just like the asp.net core integration:

var builder = Host.CreateDefaultBuilder()
    .UseSentry()
    .ConfigureServices((ctx, services) => { ... });
bruno-garcia commented 4 years ago

This looks straightforward @xt0rted thanks. Isn't there any other hooks we can add via IHostBuilder? With IWebHostBuilder we have the middleware pipeline so that's an obvious place we add code to. But I'm unaware of IHostBuilder.

Is there a way to capture exceptions of BackgroundServices or whatever else is usually setup?

xt0rted commented 4 years ago

@bruno-garcia I'm not seeing any graceful exception handling for these (here's the Host code). In the StopAsync() method it does throw an aggregate exception if anything was thrown inside of the IHostedServices, but that just throws, it doesn't even call logger.LogError(ex) 😞

Here's a question about this and the feature has been moved to the backlog https://github.com/aspnet/Extensions/issues/813.

bruno-garcia commented 4 years ago

I just tried the template for ASP.NET Core 3.0 on preview 9 which is supposed to be the last one (3.0 will be released in a week) and it looks like this:

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

So until we support Generic host, please just add:

weBuilder.UseSentry();

and it should all work fine.

Kampfmoehre commented 4 years ago

Is there a way to configure Sentry release for the .NET core worker template? I generated a new project with .NET core 3.1 with dotnet new worker and this is how CreateHostBuilder looks like:

    public static IHostBuilder CreateHostBuilder(string[] args) =>
      Host.CreateDefaultBuilder(args)
        .ConfigureServices((hostContext, services) =>
        {
          services.AddHostedService<Worker>();
        });

With the solution from @xt0rted I can add Sentry in general but I am unable to configure it. I would like to set the Release property for example.

Edit: Nevermind, I found the solution:

    public static IHostBuilder UseSentry(this IHostBuilder builder) =>
      builder.ConfigureLogging((context, logging) =>
      {
        IConfigurationSection section = context.Configuration.GetSection("Sentry");

        logging.Services.Configure<SentryLoggingOptions>(section);
        logging.AddSentry((c) =>
        {
          var version = Assembly
            .GetEntryAssembly()
            .GetCustomAttribute<AssemblyInformationalVersionAttribute>()
            .InformationalVersion;
          c.Release = $"my-application@{version}";
        });
      });
bruno-garcia commented 4 years ago

@Kampfmoehre do you tag your assemblies? Like AssemblyInformationalVersion or AssemblyVersion? The SDK picks those up automatically.

The docs are here: https://docs.sentry.io/platforms/dotnet/#automatically-discovering-release-version

Code is:

https://github.com/getsentry/sentry-dotnet/blob/c77736679b500f7cc7ad760d0274f2f703d7b34c/src/Sentry/Reflection/AssemblyExtensions.cs#L22-L31

https://github.com/getsentry/sentry-dotnet/blob/eceab66624387f3bf3db1982b47dde769320cd35/src/Sentry/Internal/ApplicationVersionLocator.cs#L6-L22

https://github.com/getsentry/sentry-dotnet/blob/b770762dc3ecd93a10d6a9038799e1377b7cfd5d/src/Sentry/Internal/ReleaseLocator.cs#L11-L13

https://github.com/getsentry/sentry-dotnet/blob/c77736679b500f7cc7ad760d0274f2f703d7b34c/src/Sentry/Internal/MainSentryEventProcessor.cs#L18

Kampfmoehre commented 4 years ago

I don't want to go too much off topic here, but here is a little background: We use Semantic Release (NodeJS) with a self written plugin (that just uses the Semantic Release CLI) that creates releases. During Semantic Release the Version property in Directory.build.props is set to the version Semantic Release offers. During deploy we use the Sentry CLI to notify Sentry about the deployment. When I set all those up I stumbled upon a few problems and ended up, using a project-key in combination with the version number to distinguish more between versions. Maybe this is not necessary but at least the examples in the docs use that too. Would that name be picked up from the Assembly name?

bruno-garcia commented 4 years ago

The snippet you added to the comment you edited is not needed because the SDK (code I pasted) does exactly the same.

It looks for SENTRY_RELEASE env var, it not set it read values from AssemblyInformationalVersionAttribute which can have a string (useful for adding a git commit hash which btw is automatically patched by sourcelink ) and if that was also not defined, uses the standard AssemblyInfo which is the format 0.0.0.0.

You can control these via Directory.Build.props so sounds like it's what you are doing.

Kampfmoehre commented 4 years ago

Thank you @bruno-garcia for the info. So would I need to set AssemblyInformationalVersionAttribute to my-application@1.0.0 for the release to be app@version, or is the my-application part taken from somewhere else?

bruno-garcia commented 4 years ago

Correct. It'll take what's in either of those assemblies and set as Release. In the order I mentioned.

ltechera commented 3 years ago

I just tried the template for ASP.NET Core 3.0 on preview 9 which is supposed to be the last one (3.0 will be released in a week) and it looks like this:

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

So until we support Generic host, please just add:

weBuilder.UseSentry();

and it should all work fine.

@bruno-garcia Didn't work for me.. Using this I get an error saying I need to have a Startup class in my console app project. @xt0rted 's post did the trick!

bruno-garcia commented 3 years ago

@ltechera Thanks for sharing that. This is something we could improve, adding to our backlog

rh78 commented 2 years ago

Supporting this! It is missing.

Bowenf3 commented 2 years ago

Bump - running into issues with this.

a-legotin commented 2 years ago

any updates here?

mattjohnsonpint commented 2 years ago

Yes. I picked this up again a couple of weeks ago, starting with reviving #1015. There's a lot of work still to do, and I'm not sure exactly when it will be completed, but I will update this issue when it's ready.

In the meantime, you can use the current release as follows:

When using ASP.NET Core with the Web Host:

WebHost.CreateDefaultBuilder()
    .UseSentry(...);

When using ASP.NET Core with the Generic Host:

Host.CreateDefaultBuilder()
    .ConfigureWebHostDefaults(webBuilder =>
    {
        webBuilder.UseSentry(...);
    });

When using the Generic Host without ASP.NET Core:

Host.CreateDefaultBuilder()
    .ConfigureServices(services =>
    {
        services.AddHostedService<YourHostedService>();
    }
    .ConfigureLogging(loggingBuilder =>
    {
        loggingBuilder.AddSentry(...);
    });

The first two require Sentry.AspNetCore, but you only need Sentry.Extensions.Logging for the last one.

ryanelian commented 1 year ago

I just hit this issue when trying to configure Sentry and Serilog inside an extension method for IHostBuilder which is used in separate ASP.NET Core Web API project and Generic Host (Worker service) project.

It would be nice if I can UseSentry from the IHostBuilder to configure both ASP.NET Core and Worker service with one method...

domagojmedo commented 1 year ago

Bumping for support

mattjohnsonpint commented 1 year ago

We haven't forgotten about this, there's just been a few other things prioritized higher. For now, use the workaround above. Thanks.

PMExtra commented 1 year ago

@mattjohnsonpint Hurry up please, it's been almost 1 year. In my project I have some other integrations that have to use IHostBuilder instead of IWebHostBuilder. So I can't integrate my service with SentryMiddleware. It means I have to manually capture exceptions and manually send to Sentry. It's so depressing.

PMExtra commented 1 year ago

A real equivalent of IWebHostBuilder.UseSentry():

var builder = WebAppliccation.CreateBuilder(args);
builder.Host
    .ConfigureLogging((context, logging) =>
    {
        var section = context.Configuration.GetSection("Sentry");
        logging.Services
            .Configure<SentryAspNetCoreOptions>(section)
            .AddSingleton<IConfigureOptions<SentryAspNetCoreOptions>, SentryAspNetCoreOptionsSetup>()
            .AddSingleton<ILoggerProvider, SentryAspNetCoreLoggerProvider>();
        logging.AddFilter<SentryAspNetCoreLoggerProvider>(
            "Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware",
            LogLevel.None
        );
        logging.Services.AddSentry().AddSentryOptions(options =>
        {
            var hostingEnvironment = context.HostingEnvironment;
            var settingLocator = options.GetType()
                .GetProperty("SettingLocator", BindingFlags.Instance | BindingFlags.NonPublic)!;
            var getEnvironment = settingLocator.PropertyType
                .GetMethod("GetEnvironment", new [] { typeof(bool) })!;
            var environment = getEnvironment
                .Invoke(settingLocator.GetValue(options), new object[] { false });
            if (environment is not null) return;
            if (options.AdjustStandardEnvironmentNameCasing)
            {
                options.Environment =
                    hostingEnvironment.IsProduction() ? "production" :
                    hostingEnvironment.IsStaging() ? "staging" :
                    hostingEnvironment.IsDevelopment() ? "development" :
                    hostingEnvironment.EnvironmentName;
            }
            else
            {
                options.Environment = hostingEnvironment.EnvironmentName;
            }
        });
    })
    .ConfigureServices(services =>
    {
        services.AddTransient<IStartupFilter, SentryStartupFilter>();
        var sentryMiddleware =
            typeof(SentryStartupFilter).Assembly.GetType("Sentry.AspNetCore.SentryMiddleware")!;
        services.AddTransient(sentryMiddleware);
    });
mattjohnsonpint commented 1 year ago

Hi again. Sorry, but this has been de-prioritized because the implementation is quite challenging, and there are already valid ways to register Sentry.

As an example of the problem, consider that the SentryMiddleware is only valid within ASP.NET Core. It would not be useable with a generic host that wasn't hosting ASP.NET Core, because there's no request/response pipeline. So a UseSentry call at the IHostBuilder level would have to try to figure out if the host was a web host or not. It would need to work correctly in both cases, which makes it not necessarily related to ASP.NET Core at all, so likely it would be implemented in Sentry.Extensions.Logging (or a new package entirely). (See https://github.com/getsentry/sentry-dotnet/pull/1015#issuecomment-906853952). But - if we don't implement it in Sentry.AspNetCore, then we have no clean way to register the middleware with the web host. We also lose ability to reference SentryAspNetCoreOptions.

Addressing your comment:

In my project I have some other integrations that have to use IHostBuilder instead of IWebHostBuilder.

If your project is referencing ASP.NET Core, then you can use the ConfigureWebHost extension method to get an IWebHostBuilder from an IHostBuilder.

If your project is not referencing ASP.NET Core, then you can't take advantage of Sentry's middleware anyway. You can initialize via .ConfigureLogging(loggingBuilder => loggingBuilder.AddSentry()) instead.

So I can't integrate my service with SentryMiddleware. It means I have to manually capture exceptions and manually send to Sentry.

I'm not sure what you mean by that. SentryMiddleware isn't something one integrates other things into. You can register your own middleware independently of SentryMiddleware. Can you please provide an example that highlights your use case? Thanks.

Also, the code you gave just appears to be inlining the internals of the existing UseSentry method, and you've already got IWebHostBuilder, so I'm not sure what you're illustrating there?

PMExtra commented 1 year ago

@mattjohnsonpint

I'm using IHostBuilder rather than IWebHostBuilder with ASP.NET Core.

I have some other integrations such as UseAutofac() that have to depends on IHostBuilder rather than IWebHostBuilder.

When I called .ConfigureWebHostDefaults it throws an exception.

mattjohnsonpint commented 1 year ago

Can you give an example of the exception?

Can you try ConfigureWebHost (without "Defaults")?

Thanks.

mattjohnsonpint commented 1 year ago

Also, I'm not sure why it matters that you've got other integrations. You certainly can use both Host and WebHost properties of a WebApplicationBuilder. For example:

var builder = WebApplication.CreateBuilder(args);
builder.Host.UseAutofac();
builder.WebHost.UseSentry();
var app = builder.Build();

Also, I haven't used Autofac in ages, but the docs tell me its actually builder.Host.UseServiceProviderFactory(new AutofacServiceProviderFactory()); - Unless you're using something like ABP Framework.

PMExtra commented 1 year ago

@mattjohnsonpint Thanks! It's solved. I'm confused why there be both builder.Host and builder.WebHost, and even they can work together at the same project. I was wrong to think there is only builder.Host I can use. And a more confusing thing is that there are actually IHostBuilder.ConfigureWebHost() and IHostBuilder.ConfigureWebHostDefaults() methods. And they will through exceptions.

mattjohnsonpint commented 1 year ago

Glad that worked for you. 🙂

Yeah, it's even more confusing if you take into account all the naming changes that have happened over the years between versions. It certainly is challenging for us to support them all, but we do our best to keep up!

mattjohnsonpint commented 1 year ago

Update

After much deliberation, trial, and error, we've decided not to do this.

The main reason is that the IHostBuilder API is an abstraction used on any type of host. A UseSentry API there would either be limited to a subset of features common to all hosts, or would have to make assumptions about what the real host was.

In other words, host.UseSentry() shouldn't assume that ASP.NET Core is being hosted. It would have to try to figure out if ASP.NET Core was present or not. Making that happen would require placing all the logic within Sentry.AspNetCore (as was done in #1015), and that would not be good for applications that are not using ASP.NET Core.

Additionally, it is too difficult to provide different types of options objects to the configuration delegates. We need SentryAspNetCoreOptions exposed for ASP.NET Core, and we use SentryLoggingOptions when registering with ILoggingBuilder.

Guidance for ASP.NET Core

Use the Sentry.AspNetCore nuget package, and register using an IWebHostBuilder, such as the one exposed by WebApplicationBuilder.WebHost. For example:

var builder = WebApplication.CreateBuilder(args);
builder.WebHost.UseSentry(...);

Related - we will also complete #2275 to make this clearer.

Guidance for Generic Hosts / Worker Services

Use the Sentry.Extensions.Logging package and register with the ILoggingBuilder instance, such as:

var hostBuilder = Host.CreateDefaultBuilder(args);
hostBuilder.ConfigureLogging(loggingBuilder =>
{
    loggingBuilder.AddSentry(...);
});

Related - we will also complete #2274 to make this clearer.

Thanks.

Kampfmoehre commented 2 months ago

Since this cost me some time to find, I want to add, when using Host.CreateApplicationBuilder(args); (.NET 7+) for non Web projects Sentry can be added like this

HostApplicationBuilder builder = Host.CreateApplicationBuilder(args);

builder.Logging.AddSentry(o =>
{
  o.Dsn = "...";
});