Skip to content

Commit d3ab87a

Browse files
RassKRassK
committed
update
1 parent fc78c0e commit d3ab87a

File tree

4 files changed

+31
-22
lines changed

4 files changed

+31
-22
lines changed

‎src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,12 @@ public static void OnStopEventWritten(string name, object? payload)
9494

9595
#if NET
9696
// Check the exception handler feature first in case the endpoint was overwritten
97-
var route = (context.Features.Get<IExceptionHandlerPathFeature>()?.Endpoint as RouteEndpoint ??
98-
context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
99-
if (!string.IsNullOrEmpty(route))
97+
var endpoint = context.Features.Get<IExceptionHandlerPathFeature>()?.Endpoint as RouteEndpoint
98+
?? context.GetEndpoint() as RouteEndpoint;
99+
100+
if (endpoint != null)
100101
{
101-
tags.Add(new KeyValuePair<string, object?>(SemanticConventions.AttributeHttpRoute, route));
102+
tags.AddRouteAttribute(endpoint, context.Request);
102103
}
103104
#endif
104105
if (context.Items.TryGetValue(ErrorTypeHttpContextItemsKey, out var errorType))

‎src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/RouteAttributeHelper.cs‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,16 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation;
1414

1515
internal static class RouteAttributeHelper
1616
{
17+
public static void AddRouteAttribute(this TagList tags, RouteEndpoint endpoint, HttpRequest request)
18+
{
19+
var routePattern = GetRoutePattern(endpoint.RoutePattern, request.RouteValues);
20+
21+
if (!string.IsNullOrEmpty(routePattern))
22+
{
23+
tags.Add(new KeyValuePair<string, object?>(SemanticConventions.AttributeHttpRoute, routePattern));
24+
}
25+
}
26+
1727
public static void SetRouteAttributeTag(this Activity activity, RouteEndpoint endpoint, HttpRequest request)
1828
{
1929
var routePattern = GetRoutePattern(endpoint.RoutePattern, request.RouteValues);
@@ -29,15 +39,14 @@ private static string GetRoutePattern(RoutePattern routePattern, RouteValueDicti
2939
{
3040
var sb = new StringBuilder();
3141

32-
for (var i = 0; i < routePattern.PathSegments.Count; i++)
42+
foreach (var segment in routePattern.PathSegments)
3343
{
34-
var segment = routePattern.PathSegments[i];
35-
3644
foreach (var part in segment.Parts)
3745
{
3846
if (part is RoutePatternLiteralPart literalPart)
3947
{
4048
sb.Append(literalPart.Content);
49+
sb.Append('/');
4150
}
4251
else if (part is RoutePatternParameterPart parameterPart)
4352
{
@@ -50,30 +59,29 @@ private static string GetRoutePattern(RoutePattern routePattern, RouteValueDicti
5059
if (parameterValue != null)
5160
{
5261
sb.Append(parameterValue);
62+
sb.Append('/');
5363
break;
5464
}
5565

5666
goto default;
5767
default:
58-
if (routeValues.ContainsKey(parameterPart.Name))
68+
if (!parameterPart.IsOptional ||
69+
(parameterPart.IsOptional && routeValues.ContainsKey(parameterPart.Name)))
5970
{
6071
sb.Append('{');
6172
sb.Append(parameterPart.Name);
6273
sb.Append('}');
74+
sb.Append('/');
6375
}
6476

6577
break;
6678
}
6779
}
6880
}
69-
70-
if (i < routePattern.PathSegments.Count - 1)
71-
{
72-
sb.Append('/');
73-
}
7481
}
7582

76-
return sb.ToString();
83+
// Remove the trailing '/'
84+
return sb.ToString(0, sb.Length - 1);
7785
}
7886
}
7987

‎test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestCases.json‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"path": "/",
77
"expectedStatusCode": 200,
88
"currentHttpRoute": "{controller=ConventionalRoute}/{action=Default}/{id?}",
9-
"expectedHttpRoute": "ConventionalRoute/Default/{id?}"
9+
"expectedHttpRoute": "ConventionalRoute/Default"
1010
},
1111
{
1212
"name": "Non-default action with route parameter and query string",
@@ -15,7 +15,7 @@
1515
"path": "/ConventionalRoute/ActionWithStringParameter/2?num=3",
1616
"expectedStatusCode": 200,
1717
"currentHttpRoute": "{controller=ConventionalRoute}/{action=Default}/{id?}",
18-
"expectedHttpRoute": "ConventionalRoute/ActionWithStringParameter/{id?}"
18+
"expectedHttpRoute": "ConventionalRoute/ActionWithStringParameter/{id}"
1919
},
2020
{
2121
"name": "Non-default action with query string",
@@ -24,7 +24,7 @@
2424
"path": "/ConventionalRoute/ActionWithStringParameter?num=3",
2525
"expectedStatusCode": 200,
2626
"currentHttpRoute": "{controller=ConventionalRoute}/{action=Default}/{id?}",
27-
"expectedHttpRoute": "ConventionalRoute/ActionWithStringParameter/{id?}"
27+
"expectedHttpRoute": "ConventionalRoute/ActionWithStringParameter"
2828
},
2929
{
3030
"name": "Not Found (404)",
@@ -42,7 +42,7 @@
4242
"path": "/SomePath/SomeString/2",
4343
"expectedStatusCode": 200,
4444
"currentHttpRoute": null,
45-
"expectedHttpRoute": "SomePath/{id}/{num:int}"
45+
"expectedHttpRoute": "SomePath/{id}/{num}"
4646
},
4747
{
4848
"name": "Path that does not match parameter constraint",
@@ -60,7 +60,7 @@
6060
"path": "/MyArea",
6161
"expectedStatusCode": 200,
6262
"currentHttpRoute": "{area:exists}/{controller=ControllerForMyArea}/{action=Default}/{id?}",
63-
"expectedHttpRoute": "{area:exists}/ControllerForMyArea/Default/{id?}"
63+
"expectedHttpRoute": "MyArea/ControllerForMyArea/Default"
6464
},
6565
{
6666
"name": "Area using `area:exists`, non-default action",
@@ -69,7 +69,7 @@
6969
"path": "/MyArea/ControllerForMyArea/NonDefault",
7070
"expectedStatusCode": 200,
7171
"currentHttpRoute": "{area:exists}/{controller=ControllerForMyArea}/{action=Default}/{id?}",
72-
"expectedHttpRoute": "{area:exists}/ControllerForMyArea/NonDefault/{id?}"
72+
"expectedHttpRoute": "MyArea/ControllerForMyArea/NonDefault"
7373
},
7474
{
7575
"name": "Area w/o `area:exists`, default controller/action",
@@ -78,7 +78,7 @@
7878
"path": "/SomePrefix",
7979
"expectedStatusCode": 200,
8080
"currentHttpRoute": "SomePrefix/{controller=AnotherArea}/{action=Index}/{id?}",
81-
"expectedHttpRoute": "SomePrefix/AnotherArea/Index/{id?}"
81+
"expectedHttpRoute": "SomePrefix/AnotherArea/Index/{id}"
8282
},
8383
{
8484
"name": "Default action",

‎test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public async Task TestHttpRoute_Traces(TestCase testCase)
5252

5353
// TODO: The CurrentHttpRoute property will go away. They only serve to capture status quo.
5454
// If CurrentHttpRoute is null, then that means we already conform to the correct behavior.
55-
var expectedHttpRoute = testCase.CurrentHttpRoute ?? testCase.ExpectedHttpRoute;
55+
var expectedHttpRoute = testCase.ExpectedHttpRoute ?? testCase.CurrentHttpRoute;
5656
Assert.Equal(expectedHttpRoute, activityHttpRoute);
5757

5858
// Activity.DisplayName should be a combination of http.method + http.route attributes, see:

0 commit comments

Comments
 (0)