From 9d52929356ef8285ba188aea81809902b587f4fd Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 12 Sep 2023 12:59:54 +1200 Subject: [PATCH] Configure Sentry tracing middleware automatically (#2602) * Added SentryTracingStartupFilter so SDK users no longer need to call UseSentryTracing() with ASP.NET Core * Removed UseSentryTracing from samples * Don't register Tracing middleware automatically if OpenTelemetry has been configured * Update CHANGELOG.md --- CHANGELOG.md | 4 ++ Sentry.sln.DotSettings | 2 + .../Program.cs | 3 - .../Program.cs | 3 - .../Sentry.Samples.AspNetCore.Grpc/Startup.cs | 1 - .../Sentry.Samples.AspNetCore.Mvc/Startup.cs | 1 - src/Sentry.AspNetCore/SentryTracingBuilder.cs | 61 +++++++++++++++++++ .../SentryTracingMiddlewareExtensions.cs | 30 +++++++++ .../SentryTracingStartupFilter.cs | 16 +++++ .../SentryWebHostBuilderExtensions.cs | 1 + ...tryHttpMessageHandlerBuilderFilterTests.cs | 1 - .../SentryTracingMiddlewareTests.cs | 13 +--- .../WebIntegrationTests.verify.cs | 2 - 13 files changed, 117 insertions(+), 21 deletions(-) create mode 100644 Sentry.sln.DotSettings create mode 100644 src/Sentry.AspNetCore/SentryTracingBuilder.cs create mode 100644 src/Sentry.AspNetCore/SentryTracingStartupFilter.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index e506ea1f33..2dc2d15e0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Sentry tracing middleware now gets configured automatically ([#2602](https://github.com/getsentry/sentry-dotnet/pull/2602)) + ### Dependencies - Bump CLI from v2.20.6 to v2.20.7 ([#2604](https://github.com/getsentry/sentry-dotnet/pull/2604)) diff --git a/Sentry.sln.DotSettings b/Sentry.sln.DotSettings new file mode 100644 index 0000000000..a0775def1c --- /dev/null +++ b/Sentry.sln.DotSettings @@ -0,0 +1,2 @@ + + True \ No newline at end of file diff --git a/samples/Sentry.Samples.AspNetCore.Basic/Program.cs b/samples/Sentry.Samples.AspNetCore.Basic/Program.cs index 91f83b27fc..d982123b56 100644 --- a/samples/Sentry.Samples.AspNetCore.Basic/Program.cs +++ b/samples/Sentry.Samples.AspNetCore.Basic/Program.cs @@ -31,9 +31,6 @@ public static IWebHost BuildWebHost(string[] args) => { app.UseRouting(); - // Enable Sentry performance monitoring - app.UseSentryTracing(); - // An example ASP.NET Core middleware that throws an // exception when serving a request to path: /throw app.UseEndpoints(endpoints => diff --git a/samples/Sentry.Samples.AspNetCore.Blazor.Server/Program.cs b/samples/Sentry.Samples.AspNetCore.Blazor.Server/Program.cs index 53e574bc2b..f9799ddd50 100644 --- a/samples/Sentry.Samples.AspNetCore.Blazor.Server/Program.cs +++ b/samples/Sentry.Samples.AspNetCore.Blazor.Server/Program.cs @@ -13,9 +13,6 @@ var app = builder.Build(); -// Enable Sentry performance monitoring -app.UseSentryTracing(); - app.UseHttpsRedirection(); app.UseStaticFiles(); diff --git a/samples/Sentry.Samples.AspNetCore.Grpc/Startup.cs b/samples/Sentry.Samples.AspNetCore.Grpc/Startup.cs index 353f597ee5..5bc4580274 100644 --- a/samples/Sentry.Samples.AspNetCore.Grpc/Startup.cs +++ b/samples/Sentry.Samples.AspNetCore.Grpc/Startup.cs @@ -19,7 +19,6 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env) } app.UseRouting(); - app.UseSentryTracing(); app.UseEndpoints(endpoints => { diff --git a/samples/Sentry.Samples.AspNetCore.Mvc/Startup.cs b/samples/Sentry.Samples.AspNetCore.Mvc/Startup.cs index fbe96e9c2a..5c5e944093 100644 --- a/samples/Sentry.Samples.AspNetCore.Mvc/Startup.cs +++ b/samples/Sentry.Samples.AspNetCore.Mvc/Startup.cs @@ -45,7 +45,6 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env) app.UseStaticFiles(); app.UseRouting(); - app.UseSentryTracing(); app.UseSentryTunneling(); diff --git a/src/Sentry.AspNetCore/SentryTracingBuilder.cs b/src/Sentry.AspNetCore/SentryTracingBuilder.cs new file mode 100644 index 0000000000..e34379f4c5 --- /dev/null +++ b/src/Sentry.AspNetCore/SentryTracingBuilder.cs @@ -0,0 +1,61 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace Sentry.AspNetCore; + +/// +/// Delegates/wraps the inner builder, so that we can intercept calls to and add our middleware +/// +internal class SentryTracingBuilder : IApplicationBuilder +{ + private const string EndpointRouteBuilder = "__EndpointRouteBuilder"; + public SentryTracingBuilder(IApplicationBuilder inner) + { + InnerBuilder = inner; + } + + private IApplicationBuilder InnerBuilder { get; } + + /// + public IServiceProvider ApplicationServices + { + get => InnerBuilder.ApplicationServices; + set => InnerBuilder.ApplicationServices = value; + } + + /// + public IDictionary Properties => InnerBuilder.Properties; + + /// + public IFeatureCollection ServerFeatures => InnerBuilder.ServerFeatures; + + /// + public RequestDelegate Build() => InnerBuilder.Build(); + + /// + public IApplicationBuilder New() => new SentryTracingBuilder(InnerBuilder.New()); + + /// + public IApplicationBuilder Use(Func middleware) + { + // UseRouting sets a property on the builder that we use to detect when UseRouting has been added and we + // register SentryTracing immediately afterwards + // https://github.com/dotnet/aspnetcore/blob/8eaf4b51f73ae2b0ed079e4f8253afb53e96b703/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs#L58-L62 + if (Properties.ContainsKey(EndpointRouteBuilder) && this.ShouldRegisterSentryTracing()) + { + var options = InnerBuilder.ApplicationServices.GetService>(); + var instrumenter = options?.Value.Instrumenter ?? Instrumenter.Sentry; + if (instrumenter == Instrumenter.Sentry) + { + return InnerBuilder.Use(middleware).UseSentryTracing(); + } + this.StoreInstrumenter(instrumenter); // Saves us from having to resolve the options to make this check again + } + + return InnerBuilder.Use(middleware); + } +} diff --git a/src/Sentry.AspNetCore/SentryTracingMiddlewareExtensions.cs b/src/Sentry.AspNetCore/SentryTracingMiddlewareExtensions.cs index 142d4f6523..25df6c6f03 100644 --- a/src/Sentry.AspNetCore/SentryTracingMiddlewareExtensions.cs +++ b/src/Sentry.AspNetCore/SentryTracingMiddlewareExtensions.cs @@ -1,4 +1,6 @@ +using Sentry; using Sentry.AspNetCore; +using Sentry.Internal.Extensions; // ReSharper disable once CheckNamespace namespace Microsoft.AspNetCore.Builder; @@ -9,12 +11,40 @@ namespace Microsoft.AspNetCore.Builder; [EditorBrowsable(EditorBrowsableState.Never)] public static class SentryTracingMiddlewareExtensions { + private const string UseSentryTracingKey = "__UseSentryTracing"; + private const string InstrumenterKey = "__SentryInstrumenter"; + + internal static bool IsSentryTracingRegistered(this IApplicationBuilder builder) + => builder.Properties.ContainsKey(UseSentryTracingKey); + internal static void StoreInstrumenter(this IApplicationBuilder builder, Instrumenter instrumenter) + => builder.Properties[InstrumenterKey] = instrumenter; + + internal static bool ShouldRegisterSentryTracing(this IApplicationBuilder builder) + { + if (builder.Properties.ContainsKey(UseSentryTracingKey)) + { + return false; + } + if (builder.Properties.TryGetTypedValue(InstrumenterKey, out Instrumenter instrumenter)) + { + return instrumenter == Instrumenter.Sentry; + } + return true; + } + /// /// Adds Sentry's tracing middleware to the pipeline. /// Make sure to place this middleware after UseRouting(...). /// public static IApplicationBuilder UseSentryTracing(this IApplicationBuilder builder) { + // Don't register twice + if (builder.IsSentryTracingRegistered()) + { + return builder; + } + + builder.Properties[UseSentryTracingKey] = true; builder.UseMiddleware(); return builder; } diff --git a/src/Sentry.AspNetCore/SentryTracingStartupFilter.cs b/src/Sentry.AspNetCore/SentryTracingStartupFilter.cs new file mode 100644 index 0000000000..cbaf67be36 --- /dev/null +++ b/src/Sentry.AspNetCore/SentryTracingStartupFilter.cs @@ -0,0 +1,16 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; + +namespace Sentry.AspNetCore; + +internal class SentryTracingStartupFilter: IStartupFilter +{ + public Action Configure(Action next) + { + return builder => + { + var wrappedBuilder = new SentryTracingBuilder(builder); + next(wrappedBuilder); + }; + } +} diff --git a/src/Sentry.AspNetCore/SentryWebHostBuilderExtensions.cs b/src/Sentry.AspNetCore/SentryWebHostBuilderExtensions.cs index 9b92375b4c..ee83bf9560 100644 --- a/src/Sentry.AspNetCore/SentryWebHostBuilderExtensions.cs +++ b/src/Sentry.AspNetCore/SentryWebHostBuilderExtensions.cs @@ -99,6 +99,7 @@ public static IWebHostBuilder UseSentry( _ = builder.ConfigureServices(c => _ = c.AddTransient() + .AddTransient() .AddTransient() ); diff --git a/test/Sentry.AspNetCore.Tests/SentryHttpMessageHandlerBuilderFilterTests.cs b/test/Sentry.AspNetCore.Tests/SentryHttpMessageHandlerBuilderFilterTests.cs index b326904d3a..628c280df7 100644 --- a/test/Sentry.AspNetCore.Tests/SentryHttpMessageHandlerBuilderFilterTests.cs +++ b/test/Sentry.AspNetCore.Tests/SentryHttpMessageHandlerBuilderFilterTests.cs @@ -57,7 +57,6 @@ public async Task Generated_client_sends_Sentry_trace_header_automatically() .Configure(app => { app.UseRouting(); - app.UseSentryTracing(); app.UseEndpoints(routes => { diff --git a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs index b9ce887f77..f547c5538c 100644 --- a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs +++ b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs @@ -33,7 +33,6 @@ public async Task Transactions_are_grouped_by_route() .Configure(app => { app.UseRouting(); - app.UseSentryTracing(); app.UseEndpoints(routes => { @@ -83,7 +82,6 @@ public async Task Transaction_is_bound_on_the_scope_automatically() .Configure(app => { app.UseRouting(); - app.UseSentryTracing(); app.UseEndpoints(routes => { @@ -127,7 +125,6 @@ public async Task Transaction_is_started_automatically_from_incoming_trace_heade .Configure(app => { app.UseRouting(); - app.UseSentryTracing(); app.UseEndpoints(routes => routes.Map("/person/{id}", _ => Task.CompletedTask)); })); @@ -193,7 +190,6 @@ public async Task TraceID_from_trace_header_propagates_to_outbound_requests(bool .Configure(app => { app.UseRouting(); - app.UseSentryTracing(); app.UseEndpoints(routes => routes.Map("/person/{id}", async _ => { @@ -299,7 +295,6 @@ public async Task Baggage_header_propagates_to_outbound_requests(bool shouldProp .Configure(app => { app.UseRouting(); - app.UseSentryTracing(); app.UseEndpoints(routes => routes.Map("/person/{id}", async _ => { @@ -363,7 +358,6 @@ public async Task Baggage_header_sets_dynamic_sampling_context() .Configure(app => { app.UseRouting(); - app.UseSentryTracing(); app.UseEndpoints(routes => { @@ -417,7 +411,6 @@ public async Task Transaction_is_automatically_populated_with_request_data() .Configure(app => { app.UseRouting(); - app.UseSentryTracing(); app.UseEndpoints(routes => { @@ -475,7 +468,6 @@ public async Task Transaction_sampling_context_contains_HTTP_context_data() .Configure(app => { app.UseRouting(); - app.UseSentryTracing(); app.UseEndpoints(routes => routes.Map("/person/{id}", context => { @@ -522,7 +514,8 @@ public async Task Transaction_binds_exception_thrown() }) .Configure(app => { - app.UseRouting(); + // We have to do this before routing, otherwise it won't wrap our SentryTracingMiddleware, which is what + // binds the ExceptionToSpanMap app.Use(async (_, c) => { try @@ -534,7 +527,7 @@ public async Task Transaction_binds_exception_thrown() // We just want to know if it got into Sentry's Hub } }); - app.UseSentryTracing(); + app.UseRouting(); app.UseEndpoints(routes => routes.Map("/person/{id}", _ => throw exception)); })); diff --git a/test/Sentry.AspNetCore.Tests/WebIntegrationTests.verify.cs b/test/Sentry.AspNetCore.Tests/WebIntegrationTests.verify.cs index 295174c23f..295e5e7e42 100644 --- a/test/Sentry.AspNetCore.Tests/WebIntegrationTests.verify.cs +++ b/test/Sentry.AspNetCore.Tests/WebIntegrationTests.verify.cs @@ -52,7 +52,6 @@ public async Task Versioning() .Configure(app => { app.UseRouting(); - app.UseSentryTracing(); app.UseEndpoints(routeBuilder => routeBuilder.MapControllers()); })); @@ -119,7 +118,6 @@ public async Task PreFlightIgnoresTransaction() .Configure(app => { app.UseRouting(); - app.UseSentryTracing(); app.UseCors(); app.UseEndpoints(_ => _.MapControllers()); }));