diff --git a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs index 129542fec15a..d8d27f96c80b 100644 --- a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs +++ b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs @@ -6,6 +6,7 @@ using System.Diagnostics.CodeAnalysis; using System.Diagnostics.Metrics; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Shared; namespace Microsoft.AspNetCore.Hosting; @@ -68,7 +69,7 @@ public void RequestEnd(string protocol, string scheme, string method, string? ro tags.Add("http.response.status_code", GetBoxedStatusCode(statusCode)); if (route != null) { - tags.Add("http.route", route); + 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/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs b/src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs index ff6735d1b5e3..33d7fae6c4b2 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_RouteTagIsRootWhenEmpty() + { + // 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.Equal("/", m.Tags["http.route"]); + }); + } + [Fact] public void Metrics_DisableHttpMetricsWithMetadata_NoMetrics() { 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; + } +}