elastic / apm-agent-dotnet

https://www.elastic.co/guide/en/apm/agent/dotnet/current/index.html
Apache License 2.0
580 stars 207 forks source link

Allow Bootstrapping with WebApplicationBuilder #1738

Open Puchaczov opened 2 years ago

Puchaczov commented 2 years ago

Is your feature request related to a problem? Please describe. .NET Core team has done some significant changes in the lastest 6 release of dotnet. Right now, there is brand new way of bootstrapping application so we doesn't use startup.cs file anymore and instead of IApplicationBuilder, they have introduced WebApplicationBuilder to configure the framework'

Describe the solution you'd like Extend ApmMiddlewareExtension.UseElasticApm that would make use of newer WebApplicationBuilder

gregkalapos commented 2 years ago

Thanks for reporting, we'll definitely look at this.

z1c0 commented 2 years ago

Hi @Puchaczov,

As I'm currently looking into this, may I ask you to elaborate on your use case, please?

Bootstrapping a fresh .NET 6 application results in the following code for Program.cs:

var builder = WebApplication.CreateBuilder(args);

// Add services to the container.
builder.Services.AddRazorPages();

var app = builder.Build();

// Configure the HTTP request pipeline.
if (!app.Environment.IsDevelopment())
{
    app.UseExceptionHandler("/Error");
    // The default HSTS value is 30 days. You may want to change this for production scenarios, see https://aka.ms/aspnetcore-hsts.
    app.UseHsts();
}

app.UseHttpsRedirection();
app.UseStaticFiles();

app.UseRouting();

app.UseAuthorization();

app.MapRazorPages();

app.Run();

Using the Elastic agent is still possible by adding this line after var app = builder.Build();

var app = builder.Build();
app.UseElasticApm();

or

var app = builder.Build();
app.UseAllElasticApm();

Please let me know if this already covers your use case.

FYI @gregkalapos

z1c0 commented 2 years ago

Hi @Puchaczov,

I'm closing this issue based on my above comment. Feel free to reopen it though, should I have missed something.

gregkalapos commented 1 year ago

I was working on something totally different, but then I ran into this.

The suggestion above works, but that ends up in this method which is an extension method for IApplicationBuilder, and it ends up in the following line:

https://github.com/elastic/apm-agent-dotnet/blob/2959306e7e0fcb4651fcc19b04b5e94ebd425752/src/Elastic.Apm.AspNetCore/ApmMiddlewareExtension.cs#L45

That was our original code to capture incoming HTTP calls and that registers an ASP.NET Core middleware. This all works and that code is still maintained.

However... we also offer a UseAllElasticApm extension method for IHostBuilder, which is here which calls into https://github.com/elastic/apm-agent-dotnet/blob/2959306e7e0fcb4651fcc19b04b5e94ebd425752/src/Elastic.Apm.Extensions.Hosting/HostBuilderExtensions.cs#L44

That does not rely on an ASP.NET Core middleware, but it registers a diagnostic listener to start/stop transactions for incoming HTTP calls. The AspNetCoreDiagnosticSubscriber (and listener) was born because of the zero code change agent injection... we can easily register these listeners during startup, but it was way more difficult to register a middleware.

But there are other advantages as well:

From what I understand Microsoft moved away from IHostBuilder in .NET 6. It's not there in the default setup discussed above (you can still use it, but it's not in the default project template anymore) and here comes the problem. In .NET 5 users were able to call into our IHostBuilder based UseAllElasticApm extension method and have the features mentioned above, but that does not exist anymore for .NET6 in the default template.

There is now a WebApplicationBuilder, so we could implement something like this:

public static WebApplicationBuilder UseAllElasticApm(this WebApplicationBuilder builder)

and then on the WebApplicationBuilder we can still register everything which we registered on the IHostBuilder.

I reopen this since direct support for WebApplicationBuilder sounds important to me.

I'd suggest we do the following: