From b30da54ba2aa978ad7d4a166630f671e5bef69f6 Mon Sep 17 00:00:00 2001 From: Elanis Date: Sat, 21 Jun 2025 12:59:39 +0200 Subject: [PATCH 1/7] HostingMetrics: empty "http.route" tags shouldn't be present --- .../Hosting/src/Internal/HostingMetrics.cs | 2 +- .../HostingApplicationDiagnosticsTests.cs | 65 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs index 129542fec15a..03361465b539 100644 --- a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs +++ b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs @@ -66,7 +66,7 @@ public void RequestEnd(string protocol, string scheme, string method, string? ro // Add information gathered during request. tags.Add("http.response.status_code", GetBoxedStatusCode(statusCode)); - if (route != null) + if (!string.IsNullOrEmpty(route)) { tags.Add("http.route", route); } diff --git a/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs b/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs index ff6735d1b5e3..8c0bb6313250 100644 --- a/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs +++ b/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs @@ -352,6 +352,71 @@ public void Metrics_Route_RouteTagReported() }); } + private sealed class EmptyRouteDiagnosticsMetadata : IRouteDiagnosticsMetadata + { + public string Route { get; } = ""; + } + + [Fact] + public void Metrics_Route_RouteTagMissingWhenEmpty() + { + // Arrange + var hostingEventSource = new HostingEventSource(Guid.NewGuid().ToString()); + + var testMeterFactory = new TestMeterFactory(); + using var activeRequestsCollector = new MetricCollector(testMeterFactory, HostingMetrics.MeterName, "http.server.active_requests"); + using var requestDurationCollector = new MetricCollector(testMeterFactory, HostingMetrics.MeterName, "http.server.request.duration"); + + // Act + var hostingApplication = CreateApplication(out var features, eventSource: hostingEventSource, meterFactory: testMeterFactory, configure: c => + { + c.Request.Protocol = "1.1"; + c.Request.Scheme = "http"; + c.Request.Method = "POST"; + c.Request.Host = new HostString("localhost"); + c.Request.Path = ""; + c.Request.ContentType = "text/plain"; + c.Request.ContentLength = 1024; + }); + var context = hostingApplication.CreateContext(features); + + Assert.Collection(activeRequestsCollector.GetMeasurementSnapshot(), + m => + { + Assert.Equal(1, m.Value); + Assert.Equal("http", m.Tags["url.scheme"]); + Assert.Equal("POST", m.Tags["http.request.method"]); + }); + + context.HttpContext.SetEndpoint(new Endpoint( + c => Task.CompletedTask, + new EndpointMetadataCollection(new EmptyRouteDiagnosticsMetadata()), + "Test empty endpoint")); + + hostingApplication.DisposeContext(context, null); + + // Assert + Assert.Collection(activeRequestsCollector.GetMeasurementSnapshot(), + m => + { + Assert.Equal(1, m.Value); + Assert.Equal("http", m.Tags["url.scheme"]); + Assert.Equal("POST", m.Tags["http.request.method"]); + }, + m => + { + Assert.Equal(-1, m.Value); + Assert.Equal("http", m.Tags["url.scheme"]); + Assert.Equal("POST", m.Tags["http.request.method"]); + }); + Assert.Collection(requestDurationCollector.GetMeasurementSnapshot(), + m => + { + Assert.True(m.Value > 0); + Assert.False(m.Tags.ContainsKey("http.route")); + }); + } + [Fact] public void Metrics_DisableHttpMetricsWithMetadata_NoMetrics() { From 024ddfe1c76693c28fad4e50d5cef5181ab0e36f Mon Sep 17 00:00:00 2001 From: Elanis Date: Tue, 29 Jul 2025 21:13:35 +0200 Subject: [PATCH 2/7] HostingMetrics: empty "http.route" tags should be set to "/" These changes have been decided in #62431 --- src/Hosting/Hosting/src/Internal/HostingMetrics.cs | 7 +++++-- .../Hosting/test/HostingApplicationDiagnosticsTests.cs | 6 ++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs index 03361465b539..0f0b8c78462f 100644 --- a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs +++ b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs @@ -5,6 +5,7 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Diagnostics.Metrics; + using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.Hosting; @@ -66,10 +67,12 @@ public void RequestEnd(string protocol, string scheme, string method, string? ro // Add information gathered during request. tags.Add("http.response.status_code", GetBoxedStatusCode(statusCode)); - if (!string.IsNullOrEmpty(route)) + var httpRoute = route; + if (string.IsNullOrEmpty(httpRoute)) { - tags.Add("http.route", route); + httpRoute = "/"; } + tags.Add("http.route", httpRoute); // Add before some built in tags so custom tags are prioritized when dealing with duplicates. if (customTags != null) diff --git a/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs b/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs index 8c0bb6313250..1d84af39d6c1 100644 --- a/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs +++ b/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs @@ -5,6 +5,7 @@ using System.Diagnostics.Metrics; using System.Diagnostics.Tracing; using System.Reflection; + using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; @@ -15,6 +16,7 @@ using Microsoft.Extensions.Diagnostics.Metrics.Testing; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; + using Moq; namespace Microsoft.AspNetCore.Hosting.Tests; @@ -358,7 +360,7 @@ private sealed class EmptyRouteDiagnosticsMetadata : IRouteDiagnosticsMetadata } [Fact] - public void Metrics_Route_RouteTagMissingWhenEmpty() + public void Metrics_Route_RouteTagIsRootWhenEmpty() { // Arrange var hostingEventSource = new HostingEventSource(Guid.NewGuid().ToString()); @@ -413,7 +415,7 @@ public void Metrics_Route_RouteTagMissingWhenEmpty() m => { Assert.True(m.Value > 0); - Assert.False(m.Tags.ContainsKey("http.route")); + Assert.Equal("/", m.Tags["http.route"]); }); } From 9dcf6bb0db02fd0571cc711c538749d82c89d4c3 Mon Sep 17 00:00:00 2001 From: Elanis Date: Tue, 29 Jul 2025 21:24:55 +0200 Subject: [PATCH 3/7] Do not change behavior if route is null, only if it's empty --- src/Hosting/Hosting/src/Internal/HostingMetrics.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs index 0f0b8c78462f..ff4fb7ad5b8a 100644 --- a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs +++ b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs @@ -67,12 +67,11 @@ public void RequestEnd(string protocol, string scheme, string method, string? ro // Add information gathered during request. tags.Add("http.response.status_code", GetBoxedStatusCode(statusCode)); - var httpRoute = route; - if (string.IsNullOrEmpty(httpRoute)) + if (route != null) { - httpRoute = "/"; + var httpRoute = (route == string.Empty) ? "/" : route; + tags.Add("http.route", httpRoute); } - tags.Add("http.route", httpRoute); // Add before some built in tags so custom tags are prioritized when dealing with duplicates. if (customTags != null) From 298a1548a26f09c3f579870541e397ab62627929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ros?= Date: Tue, 29 Jul 2025 12:28:26 -0700 Subject: [PATCH 4/7] Add comment --- src/Hosting/Hosting/src/Internal/HostingMetrics.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs index ff4fb7ad5b8a..95bb3ab8bbb9 100644 --- a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs +++ b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs @@ -69,7 +69,8 @@ public void RequestEnd(string protocol, string scheme, string method, string? ro tags.Add("http.response.status_code", GetBoxedStatusCode(statusCode)); if (route != null) { - var httpRoute = (route == string.Empty) ? "/" : route; + // An empty route ("") is valid and equivalent to "/" hence it's normalized for metrics + var httpRoute = route == string.Empty ? "/" : route; tags.Add("http.route", httpRoute); } From d81f940ee3ba9c9934b1f353875c2df1d006f92e Mon Sep 17 00:00:00 2001 From: Elanis Date: Tue, 29 Jul 2025 21:29:23 +0200 Subject: [PATCH 5/7] Rollback visual studio autoformatting on tests --- src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs b/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs index 1d84af39d6c1..33d7fae6c4b2 100644 --- a/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs +++ b/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs @@ -5,7 +5,6 @@ using System.Diagnostics.Metrics; using System.Diagnostics.Tracing; using System.Reflection; - using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; @@ -16,7 +15,6 @@ using Microsoft.Extensions.Diagnostics.Metrics.Testing; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; - using Moq; namespace Microsoft.AspNetCore.Hosting.Tests; From 6a488e0393766fc261e74291165aa55cabff28be Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 19 Aug 2025 21:25:30 +0800 Subject: [PATCH 6/7] Update routing metric to be consistent --- .../Hosting/src/Internal/HostingMetrics.cs | 5 ++-- .../src/Microsoft.AspNetCore.Hosting.csproj | 1 + .../src/Microsoft.AspNetCore.Routing.csproj | 1 + src/Http/Routing/src/RoutingMetrics.cs | 3 +- .../test/UnitTests/RoutingMetricsTests.cs | 28 +++++++++++++++++++ .../Diagnostics/RouteDiagnosticsHelpers.cs | 17 +++++++++++ 6 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 src/Shared/Diagnostics/RouteDiagnosticsHelpers.cs diff --git a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs index 95bb3ab8bbb9..dce206ee90da 100644 --- a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs +++ b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs @@ -7,6 +7,7 @@ using System.Diagnostics.Metrics; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Shared; namespace Microsoft.AspNetCore.Hosting; @@ -69,9 +70,7 @@ public void RequestEnd(string protocol, string scheme, string method, string? ro tags.Add("http.response.status_code", GetBoxedStatusCode(statusCode)); if (route != null) { - // An empty route ("") is valid and equivalent to "/" hence it's normalized for metrics - var httpRoute = route == string.Empty ? "/" : route; - tags.Add("http.route", httpRoute); + tags.Add("http.route", RouteDiagnosticsHelpers.ResolveHttpRoute(route)); } // Add before some built in tags so custom tags are prioritized when dealing with duplicates. diff --git a/src/Hosting/Hosting/src/Microsoft.AspNetCore.Hosting.csproj b/src/Hosting/Hosting/src/Microsoft.AspNetCore.Hosting.csproj index e91f7124f54d..fb5a2e295272 100644 --- a/src/Hosting/Hosting/src/Microsoft.AspNetCore.Hosting.csproj +++ b/src/Hosting/Hosting/src/Microsoft.AspNetCore.Hosting.csproj @@ -19,6 +19,7 @@ + diff --git a/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj b/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj index 61036f37c6d3..fc7303fe15dd 100644 --- a/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj +++ b/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj @@ -38,6 +38,7 @@ + diff --git a/src/Http/Routing/src/RoutingMetrics.cs b/src/Http/Routing/src/RoutingMetrics.cs index 7b1388b235bf..9024f41c850e 100644 --- a/src/Http/Routing/src/RoutingMetrics.cs +++ b/src/Http/Routing/src/RoutingMetrics.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics.Metrics; +using Microsoft.AspNetCore.Shared; namespace Microsoft.AspNetCore.Routing; @@ -31,7 +32,7 @@ public RoutingMetrics(IMeterFactory meterFactory) public void MatchSuccess(string route, bool isFallback) { _matchAttemptsCounter.Add(1, - new KeyValuePair("http.route", route), + new KeyValuePair("http.route", RouteDiagnosticsHelpers.ResolveHttpRoute(route)), new KeyValuePair("aspnetcore.routing.match_status", "success"), new KeyValuePair("aspnetcore.routing.is_fallback", isFallback ? BoxedTrue : BoxedFalse)); } diff --git a/src/Http/Routing/test/UnitTests/RoutingMetricsTests.cs b/src/Http/Routing/test/UnitTests/RoutingMetricsTests.cs index fb3ac0b30fc2..778f114404d4 100644 --- a/src/Http/Routing/test/UnitTests/RoutingMetricsTests.cs +++ b/src/Http/Routing/test/UnitTests/RoutingMetricsTests.cs @@ -48,6 +48,34 @@ public async Task Match_Success() m => AssertSuccess(m, "/{hi}", fallback: false)); } + [Fact] + public async Task Match_EmptyRoute_ResolveForwardSlash() + { + // Arrange + var routeEndpointBuilder = new RouteEndpointBuilder(c => Task.CompletedTask, RoutePatternFactory.Parse(string.Empty), order: 0); + var meterFactory = new TestMeterFactory(); + var middleware = CreateMiddleware( + matcherFactory: new TestMatcherFactory(true, c => + { + c.SetEndpoint(routeEndpointBuilder.Build()); + }), + meterFactory: meterFactory); + var httpContext = CreateHttpContext(); + var meter = meterFactory.Meters.Single(); + + using var routingMatchAttemptsCollector = new MetricCollector(meterFactory, RoutingMetrics.MeterName, "aspnetcore.routing.match_attempts"); + + // Act + await middleware.Invoke(httpContext); + + // Assert + Assert.Equal(RoutingMetrics.MeterName, meter.Name); + Assert.Null(meter.Version); + + Assert.Collection(routingMatchAttemptsCollector.GetMeasurementSnapshot(), + m => AssertSuccess(m, "/", fallback: false)); + } + [Theory] [InlineData(true)] [InlineData(false)] diff --git a/src/Shared/Diagnostics/RouteDiagnosticsHelpers.cs b/src/Shared/Diagnostics/RouteDiagnosticsHelpers.cs new file mode 100644 index 000000000000..3084f3caf57d --- /dev/null +++ b/src/Shared/Diagnostics/RouteDiagnosticsHelpers.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Shared; + +internal static class RouteDiagnosticsHelpers +{ + public static string ResolveHttpRoute(string route) + { + // A route that matches the root of the website could be an empty string. This is problematic. + // 1. It is potentially confusing, "What does empty string mean?" + // 2. Some telemetry tools have problems with empty string values, e.g. https://github.com/dotnet/aspnetcore/pull/62432 + // + // The fix is to resolve empty string route to "/" in metrics. + return string.IsNullOrEmpty(route) ? "/" : route; + } +} From d39f10ad644ba9bfd99daf2a198da3dd85d50730 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 19 Aug 2025 21:39:05 +0800 Subject: [PATCH 7/7] Clean up --- src/Hosting/Hosting/src/Internal/HostingMetrics.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs index dce206ee90da..d8d27f96c80b 100644 --- a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs +++ b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs @@ -5,7 +5,6 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Diagnostics.Metrics; - using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Shared;