Skip to content

Conversation

@spikecurtis
Copy link
Contributor

@spikecurtis spikecurtis commented Dec 17, 2025

I noticed while looking at scale test metrics that we don't always report a useful path in the API request metrics.

image.png

There are a lot of requests with path /*. I chased this problem to the workspace proxy, where we mount a the proxy router as a child of a "root" router to support some high level endpoints like latency-check.

Because we query the path from the Chi route context in the prometheus middleware before the request is actually handled, we can have a partially resolved pattern match only corresponding to the root router. The fix is to always re-resolve the path, rather than accept a partially resolved path.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@spikecurtis spikecurtis marked this pull request as ready for review December 17, 2025 09:47
return ""
}

if pattern := rctx.RoutePattern(); pattern != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC the idea I had was to avoid double resolution as much as possible, if you are confident that's not going to be the problem - the rest looks good

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be nice to avoid, but since we need to get the path in normalized form before we actually handle the request, we need to resolve it here.

@spikecurtis spikecurtis merged commit c5fc6de into main Dec 17, 2025
37 checks passed
@spikecurtis spikecurtis deleted the spike/api-metrics-path branch December 17, 2025 17:08
@github-actions github-actions bot locked and limited conversation to collaborators Dec 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants