Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
4 changes: 3 additions & 1 deletion src/Hosting/Hosting/src/Internal/HostingMetrics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
using System.Diagnostics;
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 +70,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