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
getsentry · Sep 12, 2023 · 9d52929 · 9d52929
1 parent b6e63c3
commit 9d52929
Show file tree

Hide file tree

Showing 13 changed files with 117 additions and 21 deletions.
diff --git 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
@@ -0,0 +1,2 @@
+<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=instrumenter/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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;
+
+/// <summary>
+/// Delegates/wraps the inner builder, so that we can intercept calls to <see cref="Use"/> and add our middleware
+/// </summary>
+internal class SentryTracingBuilder : IApplicationBuilder
+{
+    private const string EndpointRouteBuilder = "__EndpointRouteBuilder";
+    public SentryTracingBuilder(IApplicationBuilder inner)
+    {
+        InnerBuilder = inner;
+    }
+
+    private IApplicationBuilder InnerBuilder { get; }
+
+    /// <inheritdoc />
+    public IServiceProvider ApplicationServices
+    {
+        get => InnerBuilder.ApplicationServices;
+        set => InnerBuilder.ApplicationServices = value;
+    }
+
+    /// <inheritdoc />
+    public IDictionary<string, object?> Properties => InnerBuilder.Properties;
+
+    /// <inheritdoc />
+    public IFeatureCollection ServerFeatures => InnerBuilder.ServerFeatures;
+
+    /// <inheritdoc />
+    public RequestDelegate Build() => InnerBuilder.Build();
+
+    /// <inheritdoc />
+    public IApplicationBuilder New() => new SentryTracingBuilder(InnerBuilder.New());
+
+    /// <inheritdoc />
+    public IApplicationBuilder Use(Func<RequestDelegate, RequestDelegate> 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<IOptions<SentryAspNetCoreOptions>>();
+            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
@@ -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;
+    }
+
     /// <summary>
     /// Adds Sentry's tracing middleware to the pipeline.
     /// Make sure to place this middleware after <c>UseRouting(...)</c>.
     /// </summary>
     public static IApplicationBuilder UseSentryTracing(this IApplicationBuilder builder)
     {
+        // Don't register twice
+        if (builder.IsSentryTracingRegistered())
+        {
+            return builder;
+        }
+
+        builder.Properties[UseSentryTracingKey] = true;
         builder.UseMiddleware<SentryTracingMiddleware>();
         return builder;
     }

diff --git a/src/Sentry.AspNetCore/SentryTracingStartupFilter.cs 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<IApplicationBuilder> Configure(Action<IApplicationBuilder> next)
+    {
+        return builder =>
+        {
+            var wrappedBuilder = new SentryTracingBuilder(builder);
+            next(wrappedBuilder);
+        };
+    }
+}
diff --git 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<IStartupFilter, SentryStartupFilter>()
+             .AddTransient<IStartupFilter, SentryTracingStartupFilter>()
              .AddTransient<SentryMiddleware>()
         );
 

diff --git 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
@@ -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
@@ -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());
                 }));