-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Replace static parameters in route pattern for telemetry #64854
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances telemetry for ASP.NET Core routing by replacing static route parameters (like {controller}, {action}, {area}, and {page}) with their actual values in the http.route attribute for OpenTelemetry. This aligns with the OpenTelemetry specification requirement that static path segments with constrained values should use the actual value rather than the parameter placeholder. For example, a conventional route {controller}/{action}/{id?} for the HomeController/Index endpoint will now report as Home/Index/{id?} instead of the template pattern.
- Modified
RoutePattern.DebuggerToString()to replace parameters with required values when available - Enhanced
RoutePatternParameterPart.DebuggerToString()to better format constraints, especially RegexRouteConstraint - Added comprehensive test coverage for the new parameter replacement logic
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Http/Routing/src/Patterns/RoutePattern.cs | Implements the core logic to replace route parameters with their required values in the DebuggerToString method, adding new helper methods GetSegmentDebuggerToString and TryGetRequiredValue |
| src/Http/Routing/src/Patterns/RoutePatternParameterPart.cs | Enhances constraint formatting in DebuggerToString to properly display RegexRouteConstraint patterns |
| src/Http/Routing/test/UnitTests/Patterns/RoutePatternTest.cs | Adds comprehensive test coverage with 18 test methods covering various scenarios including required values, catch-all parameters, complex segments, constraints, and edge cases |
| return string.Join(string.Empty, parts); | ||
| } | ||
|
|
||
| private bool TryGetRequiredValue(string parameterName, [NotNullWhen(true)]out string? value) |
Copilot
AI
Dec 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space between the attribute and the parameter declaration. The code has [NotNullWhen(true)]out string? value but should be [NotNullWhen(true)] out string? value with a space between the closing bracket and out.
| private bool TryGetRequiredValue(string parameterName, [NotNullWhen(true)]out string? value) | |
| private bool TryGetRequiredValue(string parameterName, [NotNullWhen(true)] out string? value) |
OTEL
http.routeattributes have a new rule: static route parameters should be replaced with the static value.In the context of ASP.NET Core that means conventional routes should have
{controller},{action},{area}and{page}parameters replaced when the route is added to telemetry.For example, with conventional route
{controller=Home}/{action=Index}/{id?}, theStoreController.Checkout()endpoint should have a route ofStore/Checkout/{id?}.Fixes #64852