From 26a2d4b4c72707d4241705758a067ac8cf31a8a8 Mon Sep 17 00:00:00 2001 From: Ciaran Liedeman <3578740+cliedeman@users.noreply.github.com> Date: Sat, 27 Jan 2024 03:05:18 +0200 Subject: [PATCH] Fixed Missing Endpoint (#5135) Co-authored-by: Alan West <3676547+alanwest@users.noreply.github.com> --- .../Implementation/HttpInListener.cs | 4 +- .../Implementation/HttpInMetricsListener.cs | 5 ++- .../RouteTests/README.net6.0.md | 20 +++++++++ .../RouteTests/README.net7.0.md | 20 +++++++++ .../RouteTests/RoutingTestCases.json | 9 ++++ .../TestApplication/TestApplicationFactory.cs | 42 +++++++++++++++++++ 6 files changed, 98 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 0d5013402d3..9e724c74afb 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -9,6 +9,7 @@ using System.Runtime.CompilerServices; using Microsoft.AspNetCore.Http; #if !NETSTANDARD +using Microsoft.AspNetCore.Diagnostics; using Microsoft.AspNetCore.Routing; #endif using OpenTelemetry.Context.Propagation; @@ -231,7 +232,8 @@ public void OnStopActivity(Activity activity, object payload) var response = context.Response; #if !NETSTANDARD - var routePattern = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText; + var routePattern = (context.Features.Get()?.Endpoint as RouteEndpoint ?? + context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText; if (!string.IsNullOrEmpty(routePattern)) { activity.DisplayName = GetDisplayName(context.Request.Method, routePattern); diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs index ac185fc9b4c..e41cd5dc256 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs @@ -9,6 +9,7 @@ #if NET6_0_OR_GREATER using System.Diagnostics.CodeAnalysis; +using Microsoft.AspNetCore.Diagnostics; using Microsoft.AspNetCore.Routing; #endif using OpenTelemetry.Trace; @@ -86,7 +87,9 @@ public static void OnStopEventWritten(string name, object payload) tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); #if NET6_0_OR_GREATER - var route = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText; + // Check the exception handler feature first in case the endpoint was overwritten + var route = (context.Features.Get()?.Endpoint as RouteEndpoint ?? + context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText; if (!string.IsNullOrEmpty(route)) { tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRoute, route)); diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.net6.0.md b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.net6.0.md index c12dc40dadc..d2c3b5b29f7 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.net6.0.md +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.net6.0.md @@ -22,6 +22,7 @@ | :green_heart: | RazorPages | [Static content](#razorpages-static-content) | | :green_heart: | MinimalApi | [Action without parameter](#minimalapi-action-without-parameter) | | :green_heart: | MinimalApi | [Action with parameter](#minimalapi-action-with-parameter) | +| :green_heart: | ExceptionMiddleware | [Exception Handled by Exception Handler Middleware](#exceptionmiddleware-exception-handled-by-exception-handler-middleware) | ## ConventionalRouting: Root path @@ -590,3 +591,22 @@ } } ``` + +## ExceptionMiddleware: Exception Handled by Exception Handler Middleware + +```json +{ + "IdealHttpRoute": "/Exception", + "ActivityDisplayName": "GET /Exception", + "ActivityHttpRoute": "/Exception", + "MetricHttpRoute": "/Exception", + "RouteInfo": { + "HttpMethod": "GET", + "Path": "/Exception", + "RoutePattern.RawText": null, + "IRouteDiagnosticsMetadata.Route": null, + "HttpContext.GetRouteData()": {}, + "ActionDescriptor": null + } +} +``` diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.net7.0.md b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.net7.0.md index ec252654a2a..35a0e331882 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.net7.0.md +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.net7.0.md @@ -24,6 +24,7 @@ | :green_heart: | MinimalApi | [Action with parameter](#minimalapi-action-with-parameter) | | :green_heart: | MinimalApi | [Action without parameter (MapGroup)](#minimalapi-action-without-parameter-mapgroup) | | :green_heart: | MinimalApi | [Action with parameter (MapGroup)](#minimalapi-action-with-parameter-mapgroup) | +| :green_heart: | ExceptionMiddleware | [Exception Handled by Exception Handler Middleware](#exceptionmiddleware-exception-handled-by-exception-handler-middleware) | ## ConventionalRouting: Root path @@ -632,3 +633,22 @@ } } ``` + +## ExceptionMiddleware: Exception Handled by Exception Handler Middleware + +```json +{ + "IdealHttpRoute": "/Exception", + "ActivityDisplayName": "GET /Exception", + "ActivityHttpRoute": "/Exception", + "MetricHttpRoute": "/Exception", + "RouteInfo": { + "HttpMethod": "GET", + "Path": "/Exception", + "RoutePattern.RawText": null, + "IRouteDiagnosticsMetadata.Route": null, + "HttpContext.GetRouteData()": {}, + "ActionDescriptor": null + } +} +``` diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestCases.json b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestCases.json index 4d871d986a1..2d7dac9727a 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestCases.json +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestCases.json @@ -198,5 +198,14 @@ "expectedStatusCode": 200, "currentHttpRoute": null, "expectedHttpRoute": "/MinimalApiUsingMapGroup/{id}" + }, + { + "name": "Exception Handled by Exception Handler Middleware", + "testApplicationScenario": "ExceptionMiddleware", + "httpMethod": "GET", + "path": "/Exception", + "expectedStatusCode": 500, + "currentHttpRoute": null, + "expectedHttpRoute": "/Exception" } ] diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplication/TestApplicationFactory.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplication/TestApplicationFactory.cs index 87f79f21e96..b030ab7f42b 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplication/TestApplicationFactory.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplication/TestApplicationFactory.cs @@ -5,6 +5,7 @@ using System.Diagnostics; using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Diagnostics; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.FileProviders; @@ -33,6 +34,11 @@ public enum TestApplicationScenario /// An Razor Pages application. /// RazorPages, + + /// + /// Application with Exception Handling Middleware. + /// + ExceptionMiddleware, } internal class TestApplicationFactory @@ -53,6 +59,8 @@ internal class TestApplicationFactory return CreateMinimalApiApplication(); case TestApplicationScenario.RazorPages: return CreateRazorPagesApplication(); + case TestApplicationScenario.ExceptionMiddleware: + return CreateExceptionHandlerApplication(); default: throw new ArgumentException($"Invalid {nameof(TestApplicationScenario)}"); } @@ -154,4 +162,38 @@ private static WebApplication CreateRazorPagesApplication() return app; } + + private static WebApplication CreateExceptionHandlerApplication() + { + var builder = WebApplication.CreateBuilder(); + builder.Logging.ClearProviders(); + + var app = builder.Build(); + + app.UseExceptionHandler(exceptionHandlerApp => + { + exceptionHandlerApp.Run(async context => + { + context.Response.StatusCode = StatusCodes.Status500InternalServerError; + var exceptionHandlerPathFeature = context.Features.Get(); + await context.Response.WriteAsync(exceptionHandlerPathFeature?.Error.Message ?? "An exception was thrown."); + }); + }); + + app.Urls.Clear(); + app.Urls.Add("http://[::1]:0"); + + // TODO: Remove this condition once ASP.NET Core 8.0.2. + // Currently, .NET 8 has a different behavior than .NET 6 and 7. + // This is because ASP.NET Core 8+ has native metric instrumentation. + // When ASP.NET Core 8.0.2 is released then its behavior will align with .NET 6/7. + // See: https://github.com/dotnet/aspnetcore/issues/52648#issuecomment-1853432776 +#if !NET8_0_OR_GREATER + app.MapGet("/Exception", (ctx) => throw new ApplicationException()); +#else + app.MapGet("/Exception", () => Results.Content(content: "Error", contentType: null, contentEncoding: null, statusCode: 500)); +#endif + + return app; + } }