- Notifications
You must be signed in to change notification settings - Fork1.1k
fix: report correct request paths from workspace proxy metrics#21302
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
spikecurtis commentedDec 17, 2025
| return"" | ||
| } | ||
| ifpattern:=rctx.RoutePattern();pattern!="" { |
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.
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
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.
It would be nice to avoid, but since we need to get the path in normalized formbefore we actually handle the request, we need to resolve it here.
c5fc6de intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
I noticed while looking at scale test metrics that we don't always report a useful path in the API request metrics.
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 likelatency-check.Because we query the path from the Chi route context in the prometheus middlewarebefore 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.