Fix metrics duration and http.route tag with exception handling (#52652)

src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs

@@ -150,7 +150,8 @@ public void RequestEnd(HttpContext httpContext, Exception? exception, HostingApp
 
             if (context.MetricsEnabled)
             {
-                var route = httpContext.GetEndpoint()?.Metadata.GetMetadata<IRouteDiagnosticsMetadata>()?.Route;
+                var endpoint = HttpExtensions.GetOriginalEndpoint(httpContext);
+                var route = endpoint?.Metadata.GetMetadata<IRouteDiagnosticsMetadata>()?.Route;
                 var customTags = context.MetricsTagsFeature?.TagsList;
 
                 _metrics.RequestEnd(

src/Hosting/Hosting/src/Microsoft.AspNetCore.Hosting.csproj

@@ -34,6 +34,7 @@
     <Reference Include="Microsoft.Extensions.Options" />
 
     <Compile Include="$(SharedSourceRoot)TypeNameHelper\*.cs" />
+    <Compile Include="$(SharedSourceRoot)HttpExtensions.cs" />
   </ItemGroup>
 
   <ItemGroup>

src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs

@@ -247,12 +247,7 @@ private static void ClearHttpContext(HttpContext context)
 
         // An endpoint may have already been set. Since we're going to re-invoke the middleware pipeline we need to reset
         // the endpoint and route values to ensure things are re-calculated.
-        context.SetEndpoint(endpoint: null);
-        var routeValuesFeature = context.Features.Get<IRouteValuesFeature>();
-        if (routeValuesFeature != null)
-        {
-            routeValuesFeature.RouteValues = null!;
-        }
+        HttpExtensions.ClearEndpoint(context);
     }
 
     private static Task ClearCacheHeaders(object state)

src/Middleware/Diagnostics/src/Microsoft.AspNetCore.Diagnostics.csproj

@@ -16,6 +16,7 @@
     <Compile Include="$(SharedSourceRoot)StackTrace\**\*.cs" />
     <Compile Include="$(SharedSourceRoot)TypeNameHelper\*.cs" />
     <Compile Include="$(SharedSourceRoot)Reroute.cs" />
+    <Compile Include="$(SharedSourceRoot)HttpExtensions.cs" />
   </ItemGroup>
 
   <ItemGroup>

src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesExtensions.cs

@@ -190,11 +190,7 @@ private static Func<StatusCodeContext, Task> CreateHandler(string pathFormat, st
 
             // An endpoint may have already been set. Since we're going to re-invoke the middleware pipeline we need to reset
             // the endpoint and route values to ensure things are re-calculated.
-            context.HttpContext.SetEndpoint(endpoint: null);
-            if (routeValuesFeature != null)
-            {
-                routeValuesFeature.RouteValues = null!;
-            }
+            HttpExtensions.ClearEndpoint(context.HttpContext);
 
             context.HttpContext.Request.Path = newPath;
             context.HttpContext.Request.QueryString = newQueryString;

src/Middleware/Diagnostics/test/UnitTests/ExceptionHandlerMiddlewareTest.cs

@@ -3,6 +3,7 @@
 
 using System.Diagnostics;
 using System.Diagnostics.Metrics;
+using System.Net;
 using System.Net.Http;
 using System.Net.Http.Headers;
 using System.Net.Http.Json;
@@ -11,20 +12,22 @@
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Features;
+using Microsoft.AspNetCore.InternalTesting;
 using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.Routing;
+using Microsoft.AspNetCore.Routing.Patterns;
 using Microsoft.AspNetCore.TestHost;
-using Microsoft.AspNetCore.InternalTesting;
 using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.Diagnostics.Metrics;
 using Microsoft.Extensions.Diagnostics.Metrics.Testing;
 using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Abstractions;
 using Microsoft.Extensions.Options;
 using Moq;
 
 namespace Microsoft.AspNetCore.Diagnostics;
 
-public class ExceptionHandlerMiddlewareTest
+public class ExceptionHandlerMiddlewareTest : LoggedTest
 {
     [Fact]
     public async Task ExceptionIsSetOnProblemDetailsContext()
@@ -291,6 +294,133 @@ public async Task Metrics_ExceptionThrown_DefaultSettings_Handled_Reported()
             m => AssertRequestException(m, "System.InvalidOperationException", "handled", null));
     }
 
+    [Fact]
+    public async Task Metrics_ExceptionThrown_Handled_UseOriginalRoute()
+    {
+        // Arrange
+        var originalEndpointBuilder = new RouteEndpointBuilder(c => Task.CompletedTask, RoutePatternFactory.Parse("/path"), 0);
+        var originalEndpoint = originalEndpointBuilder.Build();
+
+        var meterFactory = new TestMeterFactory();
+        using var requestDurationCollector = new MetricCollector<double>(meterFactory, "Microsoft.AspNetCore.Hosting", "http.server.request.duration");
+        using var requestExceptionCollector = new MetricCollector<long>(meterFactory, DiagnosticsMetrics.MeterName, "aspnetcore.diagnostics.exceptions");
+
+        using var host = new HostBuilder()
+            .ConfigureServices(s =>
+            {
+                s.AddSingleton<IMeterFactory>(meterFactory);
+                s.AddSingleton(LoggerFactory);
+            })
+            .ConfigureWebHost(webHostBuilder =>
+            {
+                webHostBuilder
+                .UseTestServer()
+                .Configure(app =>
+                {
+                    app.UseExceptionHandler(new ExceptionHandlerOptions
+                    {
+                        ExceptionHandler = (c) => Task.CompletedTask
+                    });
+                    app.Run(context =>
+                    {
+                        context.SetEndpoint(originalEndpoint);
+                        throw new Exception("Test exception");
+                    });
+
+                });
+            }).Build();
+
+        await host.StartAsync();
+
+        var server = host.GetTestServer();
+
+        // Act
+        var response = await server.CreateClient().GetAsync("/path");
+        Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
+
+        await requestDurationCollector.WaitForMeasurementsAsync(minCount: 1).DefaultTimeout();
+
+        // Assert
+        Assert.Collection(
+            requestDurationCollector.GetMeasurementSnapshot(),
+            m =>
+            {
+                Assert.True(m.Value > 0);
+                Assert.Equal(500, (int)m.Tags["http.response.status_code"]);
+                Assert.Equal("System.Exception", (string)m.Tags["error.type"]);
+                Assert.Equal("/path", (string)m.Tags["http.route"]);
+            });
+        Assert.Collection(requestExceptionCollector.GetMeasurementSnapshot(),
+            m => AssertRequestException(m, "System.Exception", "handled"));
+    }
+
+    [Fact]
+    public async Task Metrics_ExceptionThrown_Handled_UseNewRoute()
+    {
+        // Arrange
+        var originalEndpointBuilder = new RouteEndpointBuilder(c => Task.CompletedTask, RoutePatternFactory.Parse("/path"), 0);
+        var originalEndpoint = originalEndpointBuilder.Build();
+
+        var newEndpointBuilder = new RouteEndpointBuilder(c => Task.CompletedTask, RoutePatternFactory.Parse("/new"), 0);
+        var newEndpoint = newEndpointBuilder.Build();
+
+        var meterFactory = new TestMeterFactory();
+        using var requestDurationCollector = new MetricCollector<double>(meterFactory, "Microsoft.AspNetCore.Hosting", "http.server.request.duration");
+        using var requestExceptionCollector = new MetricCollector<long>(meterFactory, DiagnosticsMetrics.MeterName, "aspnetcore.diagnostics.exceptions");
+
+        using var host = new HostBuilder()
+            .ConfigureServices(s =>
+            {
+                s.AddSingleton<IMeterFactory>(meterFactory);
+                s.AddSingleton(LoggerFactory);
+            })
+            .ConfigureWebHost(webHostBuilder =>
+            {
+                webHostBuilder
+                .UseTestServer()
+                .Configure(app =>
+                {
+                    app.UseExceptionHandler(new ExceptionHandlerOptions
+                    {
+                        ExceptionHandler = (c) =>
+                        {
+                            c.SetEndpoint(newEndpoint);
+                            return Task.CompletedTask;
+                        }
+                    });
+                    app.Run(context =>
+                    {
+                        context.SetEndpoint(originalEndpoint);
+                        throw new Exception("Test exception");
+                    });
+
+                });
+            }).Build();
+
+        await host.StartAsync();
+
+        var server = host.GetTestServer();
+
+        // Act
+        var response = await server.CreateClient().GetAsync("/path");
+        Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
+
+        await requestDurationCollector.WaitForMeasurementsAsync(minCount: 1).DefaultTimeout();
+
+        // Assert
+        Assert.Collection(
+            requestDurationCollector.GetMeasurementSnapshot(),
+            m =>
+            {
+                Assert.True(m.Value > 0);
+                Assert.Equal(500, (int)m.Tags["http.response.status_code"]);
+                Assert.Equal("System.Exception", (string)m.Tags["error.type"]);
+                Assert.Equal("/new", (string)m.Tags["http.route"]);
+            });
+        Assert.Collection(requestExceptionCollector.GetMeasurementSnapshot(),
+            m => AssertRequestException(m, "System.Exception", "handled"));
+    }
+
     [Fact]
     public async Task Metrics_ExceptionThrown_Unhandled_Reported()
     {

src/Shared/HttpExtensions.cs

@@ -1,6 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
+
 using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Features;
 
 internal static class HttpExtensions
 {
@@ -10,6 +12,9 @@ internal static class HttpExtensions
     internal static bool IsValidHttpMethodForForm(string method) =>
         HttpMethods.IsPost(method) || HttpMethods.IsPut(method) || HttpMethods.IsPatch(method);
 
+    // Key is a string so shared code works across different assemblies (hosting, error handling middleware, etc).
+    internal const string OriginalEndpointKey = "__OriginalEndpoint";
+
     internal static bool IsValidContentTypeForForm(string? contentType)
     {
         if (contentType == null)
@@ -27,4 +32,36 @@ internal static bool IsValidContentTypeForForm(string? contentType)
         return contentType.Equals(UrlEncodedFormContentType, StringComparison.OrdinalIgnoreCase) ||
             contentType.StartsWith(MultipartFormContentType, StringComparison.OrdinalIgnoreCase);
     }
+
+    internal static Endpoint? GetOriginalEndpoint(HttpContext context)
+    {
+        var endpoint = context.GetEndpoint();
+
+        // Some middleware re-execute the middleware pipeline with the HttpContext. Before they do this, they clear state from context, such as the previously matched endpoint.
+        // The original endpoint is stashed with a known key in HttpContext.Items. Use it as a fallback.
+        if (endpoint == null && context.Items.TryGetValue(OriginalEndpointKey, out var e) && e is Endpoint originalEndpoint)
+        {
+            endpoint = originalEndpoint;
+        }
+        return endpoint;
+    }
+
+    internal static void ClearEndpoint(HttpContext context)
+    {
+        var endpoint = context.GetEndpoint();
+        if (endpoint != null)
+        {
+            context.Items[OriginalEndpointKey] = endpoint;
+
+            // An endpoint may have already been set. Since we're going to re-invoke the middleware pipeline we need to reset
+            // the endpoint and route values to ensure things are re-calculated.
+            context.SetEndpoint(endpoint: null);
+        }
+
+        var routeValuesFeature = context.Features.Get<IRouteValuesFeature>();
+        if (routeValuesFeature != null)
+        {
+            routeValuesFeature.RouteValues = null!;
+        }
+    }
 }