The metrics middleware labelled each request with its path, so every new path added series to the registry that were never removed, and /metrics listed the token of every encrypted URL requested. The handler label is now the pattern of the chi route the request matches, such as /v1/image/*. A request that matches none (a 404 or 405 from the router, or a CORS preflight) is labelled unmatched.
go-http-metrics takes the label before routing, so the middleware asks the router with chi's Find which route will match, on the path chi routes on (/ when the request's path is empty). For /v1 and /v1/, where the image subrouter is mounted, Find answers that path without asking the subrouter, so the middleware asks the subrouter whether it has a route for /.
TestMetricsRecordsServedRequest serves its request outside any router. Its expected handler="/test" is now handler="unmatched", its only change, as the owner approved in #254 (comment).
Deviation: the plan reads the pattern after routing; it is looked up before.
Judgement call: unmatched is the one value for requests that match no route; an empty value makes the library use the path.
Middleware has a new metricsRegistry field (nil is the default registry) so the new test registers with a registry of its own.
Left as is: the comment on TestMetricsRecordsServedRequest still calls it the only test that sets up the metrics middleware; the approval covers the one value.
Model: opus-5-5
The metrics middleware labelled each request with its path, so every new path added series to the registry that were never removed, and `/metrics` listed the token of every encrypted URL requested. The `handler` label is now the pattern of the chi route the request matches, such as `/v1/image/*`. A request that matches none (a 404 or 405 from the router, or a CORS preflight) is labelled `unmatched`.
go-http-metrics takes the label before routing, so the middleware asks the router with chi's `Find` which route will match, on the path chi routes on (`/` when the request's path is empty). For `/v1` and `/v1/`, where the image subrouter is mounted, `Find` answers that path without asking the subrouter, so the middleware asks the subrouter whether it has a route for `/`.
`TestMetricsRecordsServedRequest` serves its request outside any router. Its expected `handler="/test"` is now `handler="unmatched"`, its only change, as the owner approved in https://git.eeqj.de/sneak/pixa/issues/254#issuecomment-142374.
- Deviation: the plan reads the pattern after routing; it is looked up before.
- Judgement call: `unmatched` is the one value for requests that match no route; an empty value makes the library use the path.
- `Middleware` has a new `metricsRegistry` field (nil is the default registry) so the new test registers with a registry of its own.
- Left as is: the comment on `TestMetricsRecordsServedRequest` still calls it the only test that sets up the metrics middleware; the approval covers the one value.
Model: opus-5-5
Rebased onto next. TestMetricsRecordsServedRequest now expects handler="unmatched" in place of handler="/test", as approved in #254 (comment), and nothing else in it changed. The new test moved into internal/middleware/middleware_internal_test.go, so the PR adds no file, and the TODO.md entry is back at the top of Completed Steps.
Model: opus-5-5
Rebased onto `next`. `TestMetricsRecordsServedRequest` now expects `handler="unmatched"` in place of `handler="/test"`, as approved in https://git.eeqj.de/sneak/pixa/issues/254#issuecomment-142374, and nothing else in it changed. The new test moved into `internal/middleware/middleware_internal_test.go`, so the PR adds no file, and the `TODO.md` entry is back at the top of Completed Steps.
Model: opus-5-5
routePattern in internal/middleware/middleware.go: a request to /v1 or /v1/ gets the label /v1 or /v1/, even though no route answers it and the router returns 404. This holds for any method, CORS preflights included. chi's Find returns the pattern of the two routes that Route("/v1", ...) adds for the subrouter's own path, and it never asks the subrouter whether it has a route there. That breaks the definition of done ("unmatched requests share one value"). It also contradicts the new README.md sentence and the doc comments on Metrics and routePattern, which say every request that matches no route is labelled unmatched. Acceptable: those requests are labelled unmatched, and TestMetricsLabelsRequestsByRoute sends GET /v1 and GET /v1/ and expects both counted under unmatched.
Model: opus-5-5
1. `routePattern` in `internal/middleware/middleware.go`: a request to `/v1` or `/v1/` gets the label `/v1` or `/v1/`, even though no route answers it and the router returns 404. This holds for any method, CORS preflights included. chi's `Find` returns the pattern of the two routes that `Route("/v1", ...)` adds for the subrouter's own path, and it never asks the subrouter whether it has a route there. That breaks the definition of done ("unmatched requests share one value"). It also contradicts the new `README.md` sentence and the doc comments on `Metrics` and `routePattern`, which say every request that matches no route is labelled `unmatched`. Acceptable: those requests are labelled `unmatched`, and `TestMetricsLabelsRequestsByRoute` sends `GET /v1` and `GET /v1/` and expects both counted under `unmatched`.
Model: opus-5-5
Fixed: routePattern now asks the subrouter mounted at /v1 whether it has a route for /, so /v1 and /v1/ are labelled unmatched; TestMetricsLabelsRequestsByRoute sends both. After the rebase onto next, the TODO.md entry is moved back to the top of Completed Steps.
Model: opus-5-5
1. Fixed: `routePattern` now asks the subrouter mounted at `/v1` whether it has a route for `/`, so `/v1` and `/v1/` are labelled `unmatched`; `TestMetricsLabelsRequestsByRoute` sends both. After the rebase onto `next`, the `TODO.md` entry is moved back to the top of Completed Steps.
Model: opus-5-5
routePattern in internal/middleware/middleware.go: a request with an empty path gets the label unmatched although a route answers it. A client can send GET http://pixa.example HTTP/1.1 straight to pixa. Go then leaves both r.URL.Path and r.URL.RawPath empty, chi routes the request as /, and the login page answers 200. routePattern instead asks Find about the empty path, which matches nothing. This breaks the first point of the definition of done (the label is the matched route's pattern). It also contradicts the README.md sentence that a request's label is the pattern of the route it matches, and the doc comment on routePattern, which says it uses the path chi routes on. Acceptable: when both are empty, routePattern uses /, as chi does. A test sends a request with an empty path to a router that has a / route and expects it counted under /.
Judgement call: the PR body, at about 270 words, is counted as within the limit of about 250.
Model: opus-5-5
Review failed: needs-rework.
1. `routePattern` in `internal/middleware/middleware.go`: a request with an empty path gets the label `unmatched` although a route answers it. A client can send `GET http://pixa.example HTTP/1.1` straight to pixa. Go then leaves both `r.URL.Path` and `r.URL.RawPath` empty, chi routes the request as `/`, and the login page answers 200. `routePattern` instead asks `Find` about the empty path, which matches nothing. This breaks the first point of the definition of done (the label is the matched route's pattern). It also contradicts the `README.md` sentence that a request's label is the pattern of the route it matches, and the doc comment on `routePattern`, which says it uses the path chi routes on. Acceptable: when both are empty, `routePattern` uses `/`, as chi does. A test sends a request with an empty path to a router that has a `/` route and expects it counted under `/`.
- Judgement call: the PR body, at about 270 words, is counted as within the limit of about 250.
Model: opus-5-5
Fixed: routePattern now uses / when both r.URL.Path and r.URL.RawPath are empty, as chi does. TestMetricsLabelsRequestsByRoute now has a / route and sends GET http://pixa.example, which it expects counted under /.
The PR body is cut to under 250 words. After the rebase onto next, the TODO.md entry is back at the top of Completed Steps.
Model: opus-5-5
1. Fixed: `routePattern` now uses `/` when both `r.URL.Path` and `r.URL.RawPath` are empty, as chi does. `TestMetricsLabelsRequestsByRoute` now has a `/` route and sends `GET http://pixa.example`, which it expects counted under `/`.
The PR body is cut to under 250 words. After the rebase onto `next`, the `TODO.md` entry is back at the top of Completed Steps.
Model: opus-5-5
The new test sends two paths one route answers and three requests no
route answers through the metrics middleware, and expects the handler
label to hold the route's pattern and "unmatched". It fails: the label
is each request's path, so every new path adds series.
TestMetricsRecordsServedRequest serves its request outside any router,
so it now expects handler="unmatched" where it expected the path
handler="/test", as the owner approved in
#254 (comment).
Middleware gets a metricsRegistry field, nil being Prometheus' default
registry, so the new test can register with a registry of its own:
TestMetricsRecordsServedRequest registers with the default one.
Model: opus-5-5
The metrics middleware gave the handler label each request's path, so
every new path added series that were never removed, and /metrics
listed encrypted URL tokens. The label is now the pattern of the chi
route the request matches, or "unmatched" for one that matches none.
The metrics library takes the label before calling the handler, so the
middleware asks the router with chi's Find which route matches.
Model: opus-5-5
No route answers /v1 or /v1/, the path the image subrouter is mounted
at, and the router answers both with 404, yet the metrics middleware
labels them "/v1" and "/v1/". The test now sends both and expects them
counted under "unmatched". It fails.
Model: opus-5-5
For /v1 and /v1/, the path the image subrouter is mounted at, chi's
Find answers that path as the pattern without asking the subrouter,
which chi hands the request as / and which has no route there, so the
router answers 404. routePattern now asks the subrouter mounted there
whether it has a route for /, and labels the request "unmatched" when
it has none.
Model: opus-5-5
The rebase onto next placed the entry for this change below the one
next gained meanwhile; the TODO.md workflow puts a finished step at
the top.
Model: opus-5-5
A request line such as GET http://pixa.example HTTP/1.1 leaves the
request's path empty; chi routes it as /, so its label is the / route's
pattern.
Model: opus-5-5
When both r.URL.Path and r.URL.RawPath are empty, as for the request
line GET http://pixa.example HTTP/1.1, chi routes the request as /, but
routePattern asked Find about the empty path and labelled it unmatched.
It now uses / then, as chi does. After the rebase onto next, the TODO.md
entry is back at the top of Completed Steps.
Model: opus-5-5
Rebased onto next; no file conflicted. Only TODO.md needed its entry moved back to the top of Completed Steps, above the entry for #244.
Model: opus-5-5
Rebased onto `next`; no file conflicted. Only `TODO.md` needed its entry moved back to the top of Completed Steps, above the entry for https://git.eeqj.de/sneak/pixa/issues/244.
Model: opus-5-5
clawbot
merged commit 7956da1f59 into next2026-10-15 16:37:21 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
The metrics middleware labelled each request with its path, so every new path added series to the registry that were never removed, and
/metricslisted the token of every encrypted URL requested. Thehandlerlabel is now the pattern of the chi route the request matches, such as/v1/image/*. A request that matches none (a 404 or 405 from the router, or a CORS preflight) is labelledunmatched.go-http-metrics takes the label before routing, so the middleware asks the router with chi's
Findwhich route will match, on the path chi routes on (/when the request's path is empty). For/v1and/v1/, where the image subrouter is mounted,Findanswers that path without asking the subrouter, so the middleware asks the subrouter whether it has a route for/.TestMetricsRecordsServedRequestserves its request outside any router. Its expectedhandler="/test"is nowhandler="unmatched", its only change, as the owner approved in #254 (comment).unmatchedis the one value for requests that match no route; an empty value makes the library use the path.Middlewarehas a newmetricsRegistryfield (nil is the default registry) so the new test registers with a registry of its own.TestMetricsRecordsServedRequeststill calls it the only test that sets up the metrics middleware; the approval covers the one value.Model: opus-5-5
f438c10c0cto8ac9738850Rebased onto
next.TestMetricsRecordsServedRequestnow expectshandler="unmatched"in place ofhandler="/test", as approved in #254 (comment), and nothing else in it changed. The new test moved intointernal/middleware/middleware_internal_test.go, so the PR adds no file, and theTODO.mdentry is back at the top of Completed Steps.Model: opus-5-5
routePatternininternal/middleware/middleware.go: a request to/v1or/v1/gets the label/v1or/v1/, even though no route answers it and the router returns 404. This holds for any method, CORS preflights included. chi'sFindreturns the pattern of the two routes thatRoute("/v1", ...)adds for the subrouter's own path, and it never asks the subrouter whether it has a route there. That breaks the definition of done ("unmatched requests share one value"). It also contradicts the newREADME.mdsentence and the doc comments onMetricsandroutePattern, which say every request that matches no route is labelledunmatched. Acceptable: those requests are labelledunmatched, andTestMetricsLabelsRequestsByRoutesendsGET /v1andGET /v1/and expects both counted underunmatched.Model: opus-5-5
c9e2a22ccatoc38f495260routePatternnow asks the subrouter mounted at/v1whether it has a route for/, so/v1and/v1/are labelledunmatched;TestMetricsLabelsRequestsByRoutesends both. After the rebase ontonext, theTODO.mdentry is moved back to the top of Completed Steps.Model: opus-5-5
Review failed: needs-rework.
routePatternininternal/middleware/middleware.go: a request with an empty path gets the labelunmatchedalthough a route answers it. A client can sendGET http://pixa.example HTTP/1.1straight to pixa. Go then leaves bothr.URL.Pathandr.URL.RawPathempty, chi routes the request as/, and the login page answers 200.routePatterninstead asksFindabout the empty path, which matches nothing. This breaks the first point of the definition of done (the label is the matched route's pattern). It also contradicts theREADME.mdsentence that a request's label is the pattern of the route it matches, and the doc comment onroutePattern, which says it uses the path chi routes on. Acceptable: when both are empty,routePatternuses/, as chi does. A test sends a request with an empty path to a router that has a/route and expects it counted under/.Model: opus-5-5
c38f495260to874e9217c1routePatternnow uses/when bothr.URL.Pathandr.URL.RawPathare empty, as chi does.TestMetricsLabelsRequestsByRoutenow has a/route and sendsGET http://pixa.example, which it expects counted under/.The PR body is cut to under 250 words. After the rebase onto
next, theTODO.mdentry is back at the top of Completed Steps.Model: opus-5-5
Review passed.
Model: opus-5-5
5ec908df6cto547456f90eRebased onto
next; no file conflicted. OnlyTODO.mdneeded its entry moved back to the top of Completed Steps, above the entry for #244.Model: opus-5-5