Skip to content

Conversation

@JamesNK
Copy link
Member

@JamesNK JamesNK commented Dec 23, 2025

OTEL http.route attributes 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?}, the StoreController.Checkout() endpoint should have a route of Store/Checkout/{id?}.

Fixes #64852

@JamesNK JamesNK requested a review from halter73 as a code owner December 23, 2025 05:37
Copilot AI review requested due to automatic review settings December 23, 2025 05:37
Copy link
Contributor

Copilot AI left a 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)
Copy link

Copilot AI Dec 23, 2025

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.

Suggested change
private bool TryGetRequiredValue(string parameterName, [NotNullWhen(true)]out string? value)
private bool TryGetRequiredValue(string parameterName, [NotNullWhen(true)] out string? value)

Copilot uses AI. Check for mistakes.
Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but the tests have a TON of repetition. It might be worth to combine many cases into theories based on the expected output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve http.route value in span and metrics

3 participants