Skip to content

[release/10.0-rc1] Empty "http.route" tags should be set to "/" #63336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Aug 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Hosting/Hosting/src/Internal/HostingMetrics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Diagnostics.CodeAnalysis;
using System.Diagnostics.Metrics;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Shared;

namespace Microsoft.AspNetCore.Hosting;

Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<Compile Include="$(SharedSourceRoot)Metrics\MetricsExtensions.cs" />
<Compile Include="$(SharedSourceRoot)Metrics\MetricsConstants.cs" />
<Compile Include="$(SharedSourceRoot)Diagnostics\ActivityCreator.cs" />
<Compile Include="$(SharedSourceRoot)Diagnostics\RouteDiagnosticsHelpers.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
65 changes: 65 additions & 0 deletions src/Hosting/Hosting/test/HostingApplicationDiagnosticsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<long>(testMeterFactory, HostingMetrics.MeterName, "http.server.active_requests");
using var requestDurationCollector = new MetricCollector<double>(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()
{
Expand Down
1 change: 1 addition & 0 deletions src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
<Compile Include="$(SharedSourceRoot)AntiforgeryMetadata.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)HttpExtensions.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)ContentTypeConstants.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)Diagnostics\RouteDiagnosticsHelpers.cs" LinkBase="Shared" />
</ItemGroup>

<ItemGroup>
Expand Down
3 changes: 2 additions & 1 deletion src/Http/Routing/src/RoutingMetrics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -31,7 +32,7 @@ public RoutingMetrics(IMeterFactory meterFactory)
public void MatchSuccess(string route, bool isFallback)
{
_matchAttemptsCounter.Add(1,
new KeyValuePair<string, object?>("http.route", route),
new KeyValuePair<string, object?>("http.route", RouteDiagnosticsHelpers.ResolveHttpRoute(route)),
new KeyValuePair<string, object?>("aspnetcore.routing.match_status", "success"),
new KeyValuePair<string, object?>("aspnetcore.routing.is_fallback", isFallback ? BoxedTrue : BoxedFalse));
}
Expand Down
28 changes: 28 additions & 0 deletions src/Http/Routing/test/UnitTests/RoutingMetricsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<long>(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)]
Expand Down
17 changes: 17 additions & 0 deletions src/Shared/Diagnostics/RouteDiagnosticsHelpers.cs
Original file line number Diff line number Diff line change
@@ -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;
}
}
Loading