Skip to content

Commit afa7fa2

Browse files
committed
Update routing metric to be consistent
1 parent 02d0f03 commit afa7fa2

File tree

6 files changed

+51
-4
lines changed

6 files changed

+51
-4
lines changed

src/Hosting/Hosting/src/Internal/HostingMetrics.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Diagnostics.Metrics;
88

99
using Microsoft.AspNetCore.Http;
10+
using Microsoft.AspNetCore.Shared;
1011

1112
namespace Microsoft.AspNetCore.Hosting;
1213

@@ -69,9 +70,7 @@ public void RequestEnd(string protocol, string scheme, string method, string? ro
6970
tags.Add("http.response.status_code", GetBoxedStatusCode(statusCode));
7071
if (route != null)
7172
{
72-
// An empty route ("") is valid and equivalent to "/" hence it's normalized for metrics
73-
var httpRoute = route == string.Empty ? "/" : route;
74-
tags.Add("http.route", httpRoute);
73+
tags.Add("http.route", RouteDiagnosticsHelpers.ResolveHttpRoute(route));
7574
}
7675

7776
// Add before some built in tags so custom tags are prioritized when dealing with duplicates.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
<Compile Include="$(SharedSourceRoot)Metrics\MetricsExtensions.cs" />
2020
<Compile Include="$(SharedSourceRoot)Metrics\MetricsConstants.cs" />
2121
<Compile Include="$(SharedSourceRoot)Diagnostics\ActivityCreator.cs" />
22+
<Compile Include="$(SharedSourceRoot)Diagnostics\RouteDiagnosticsHelpers.cs" />
2223
</ItemGroup>
2324

2425
<ItemGroup>

src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
<Compile Include="$(SharedSourceRoot)AntiforgeryMetadata.cs" LinkBase="Shared" />
3939
<Compile Include="$(SharedSourceRoot)HttpExtensions.cs" LinkBase="Shared" />
4040
<Compile Include="$(SharedSourceRoot)ContentTypeConstants.cs" LinkBase="Shared" />
41+
<Compile Include="$(SharedSourceRoot)Diagnostics\RouteDiagnosticsHelpers.cs" LinkBase="Shared" />
4142
</ItemGroup>
4243

4344
<ItemGroup>

src/Http/Routing/src/RoutingMetrics.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Diagnostics.Metrics;
5+
using Microsoft.AspNetCore.Shared;
56

67
namespace Microsoft.AspNetCore.Routing;
78

@@ -31,7 +32,7 @@ public RoutingMetrics(IMeterFactory meterFactory)
3132
public void MatchSuccess(string route, bool isFallback)
3233
{
3334
_matchAttemptsCounter.Add(1,
34-
new KeyValuePair<string, object?>("http.route", route),
35+
new KeyValuePair<string, object?>("http.route", RouteDiagnosticsHelpers.ResolveHttpRoute(route)),
3536
new KeyValuePair<string, object?>("aspnetcore.routing.match_status", "success"),
3637
new KeyValuePair<string, object?>("aspnetcore.routing.is_fallback", isFallback ? BoxedTrue : BoxedFalse));
3738
}

src/Http/Routing/test/UnitTests/RoutingMetricsTests.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,34 @@ public async Task Match_Success()
4848
m => AssertSuccess(m, "/{hi}", fallback: false));
4949
}
5050

51+
[Fact]
52+
public async Task Match_EmptyRoute_ResolveForwardSlash()
53+
{
54+
// Arrange
55+
var routeEndpointBuilder = new RouteEndpointBuilder(c => Task.CompletedTask, RoutePatternFactory.Parse(string.Empty), order: 0);
56+
var meterFactory = new TestMeterFactory();
57+
var middleware = CreateMiddleware(
58+
matcherFactory: new TestMatcherFactory(true, c =>
59+
{
60+
c.SetEndpoint(routeEndpointBuilder.Build());
61+
}),
62+
meterFactory: meterFactory);
63+
var httpContext = CreateHttpContext();
64+
var meter = meterFactory.Meters.Single();
65+
66+
using var routingMatchAttemptsCollector = new MetricCollector<long>(meterFactory, RoutingMetrics.MeterName, "aspnetcore.routing.match_attempts");
67+
68+
// Act
69+
await middleware.Invoke(httpContext);
70+
71+
// Assert
72+
Assert.Equal(RoutingMetrics.MeterName, meter.Name);
73+
Assert.Null(meter.Version);
74+
75+
Assert.Collection(routingMatchAttemptsCollector.GetMeasurementSnapshot(),
76+
m => AssertSuccess(m, "/", fallback: false));
77+
}
78+
5179
[Theory]
5280
[InlineData(true)]
5381
[InlineData(false)]
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace Microsoft.AspNetCore.Shared;
5+
6+
internal static class RouteDiagnosticsHelpers
7+
{
8+
public static string ResolveHttpRoute(string route)
9+
{
10+
// A route that matches the root of the website could be an empty string. This is problematic.
11+
// 1. It is potentially confusing, "What does empty string mean?"
12+
// 2. Some telemetry tools have problems with empty string values, e.g. https://github.com/dotnet/aspnetcore/pull/62432
13+
//
14+
// The fix is to resolve empty string route to "/" in metrics.
15+
return string.IsNullOrEmpty(route) ? "/" : route;
16+
}
17+
}

0 commit comments

Comments
 (0)