fix: trim trailing slash from the SLI operation name#63
Merged
Conversation
A Minimal API route group's root endpoint (e.g. a "/orders" group mapped with the "/" pattern) resolves to the raw route template "/orders/", so the Operation tag was emitted as "POST /orders/" — an unwanted trailing slash that also split it into a separate metric series from "/orders". GetOperation now trims a trailing slash from the resolved route template (MVC and Minimal API alike), while preserving the literal root path "/".
There was a problem hiding this comment.
Pull request overview
This PR updates the ASP.NET Core middleware’s Operation tag normalization to trim a trailing / from resolved route templates (while preserving the literal root /) so route-group root endpoints like /orders/ don’t split metrics into a separate time series. This keeps Minimal APIs and MVC aligned and reduces unintended metric cardinality.
Changes:
- Trim trailing slashes from resolved route templates in
GetOperation(excluding the literal/). - Add regression tests for route-group roots, explicitly-authored trailing-slash routes, and the preserved root
/. - Update package and root documentation to describe the normalization behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Trellis.ServiceLevelIndicators.Asp/tests/ServiceLevelIndicatorMinimalApiTests.cs | Adds regression tests validating trailing-slash normalization (including route groups) and preserving /. |
| Trellis.ServiceLevelIndicators.Asp/src/ServiceLevelIndicatorMiddleware.cs | Normalizes resolved route templates by trimming trailing / while keeping / intact. |
| Trellis.ServiceLevelIndicators.Asp/src/README.md | Documents trailing-slash trimming behavior for resolved templates. |
| README.md | Updates public-facing description of Operation resolution to include trailing-slash trimming and root-path preservation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The issue
The
OperationSLI tag is the HTTP method plus the endpoint's route template. A Minimal API route group's root endpoint — e.g.app.MapGroup("/orders").MapPost("/")— resolves to the raw route template/orders/(the group prefix combined with the/pattern adds a trailing slash). So the emitted operation wasPOST /orders/: an unwanted trailing slash that also splits the metric into a separate series from/orders.The fix
GetOperationtrims a trailing slash from the resolved route template before composing the operation name, so/orders/and/ordersshare one series. The literal root path/is preserved. The normalization applies to any resolved template (MVCAttributeRouteInfo.Templateand Minimal APIRouteEndpoint.RoutePattern.RawTextalike), keeping the two hosting models aligned.Regression tests cover a route-group root, an explicitly authored trailing-slash route, and the preserved literal root
/.