Where: internal/middleware/middleware.go lines 212-221 (Metrics calls std.Handler("", mdlw, next) with an empty handler ID) and internal/server/routes.go lines 36-39 (installed router-wide, before routing), on v1.0.0-rc1.
What is wrong: with an empty handler ID, go-http-metrics v0.13.0 uses r.URL.Path as the handler label (middleware/middleware.go lines 92-95 in the library), so every distinct path requested creates its own set of series in the Prometheus registry: a duration histogram with eleven buckets, a size histogram and an in-flight gauge. Series are never dropped. The middleware runs when metrics.username and metrics.password are set, which the README and the example Caddyfile recommend.
How it fails: an outsider sends requests with a new path each time: 404 paths, which no rate limit covers, or /v1/image/x/<random>/orig.jpg. A few million paths take the container's memory; nothing frees it short of a restart. As a side effect every /v1/e/<token>/... path ever requested is listed in /metrics as a label value, so the scraper and its storage hold the tokens.
Severity: high
Definition of done:
The handler label is the matched route pattern (chi's RouteContext(ctx).RoutePattern() after routing, or the middleware mounted per route with a fixed ID), never the request path, and unmatched requests share one value.
A test checks that two requests with different paths on the same route produce one handler label value; make check green.
Model: fable-5-1
Where: `internal/middleware/middleware.go` lines 212-221 (`Metrics` calls `std.Handler("", mdlw, next)` with an empty handler ID) and `internal/server/routes.go` lines 36-39 (installed router-wide, before routing), on `v1.0.0-rc1`.
What is wrong: with an empty handler ID, go-http-metrics v0.13.0 uses `r.URL.Path` as the `handler` label (`middleware/middleware.go` lines 92-95 in the library), so every distinct path requested creates its own set of series in the Prometheus registry: a duration histogram with eleven buckets, a size histogram and an in-flight gauge. Series are never dropped. The middleware runs when `metrics.username` and `metrics.password` are set, which the README and the example Caddyfile recommend.
How it fails: an outsider sends requests with a new path each time: 404 paths, which no rate limit covers, or `/v1/image/x/<random>/orig.jpg`. A few million paths take the container's memory; nothing frees it short of a restart. As a side effect every `/v1/e/<token>/...` path ever requested is listed in `/metrics` as a label value, so the scraper and its storage hold the tokens.
Severity: high
Definition of done:
1. The `handler` label is the matched route pattern (chi's `RouteContext(ctx).RoutePattern()` after routing, or the middleware mounted per route with a fixed ID), never the request path, and unmatched requests share one value.
2. A test checks that two requests with different paths on the same route produce one `handler` label value; `make check` green.
Model: fable-5-1
clawbot
self-assigned this 2026-10-13 08:46:11 +02:00
Plan, as the definition of done says: the handler label is the matched chi route pattern, read after routing, and every unmatched request shares one value; no request path ever becomes a label.
Package: internal/middleware (and internal/server/routes.go if the mounting changes). The test is a new one, with its own Prometheus registry where the library allows, so it does not touch the existing /metrics test that #180 asks about.
Model: opus-5-5
Plan, as the definition of done says: the `handler` label is the matched chi route pattern, read after routing, and every unmatched request shares one value; no request path ever becomes a label.
Package: `internal/middleware` (and `internal/server/routes.go` if the mounting changes). The test is a new one, with its own Prometheus registry where the library allows, so it does not touch the existing `/metrics` test that https://git.eeqj.de/sneak/pixa/issues/180 asks about.
Model: opus-5-5
Question: may one value in the existing test TestMetricsRecordsServedRequest (internal/middleware/middleware_internal_test.go) change? CLAUDE.md needs your approval to change an existing test.
The test serves GET /test through the metrics middleware outside any router and expects /metrics to show it with handler="/test", the request path, which is the label this issue removes. With the fix in #260 the same request is labelled unmatched, so the test fails.
Yes (recommended): its expected line changes from handler="/test" to handler="unmatched". It still checks that a served request is recorded; the PR then goes to review.
No: the test can only pass unchanged if the middleware falls back to the path when no router is present, which keeps the label this issue removes.
Model: opus-5-5
Question: may one value in the existing test `TestMetricsRecordsServedRequest` (`internal/middleware/middleware_internal_test.go`) change? `CLAUDE.md` needs your approval to change an existing test.
The test serves `GET /test` through the metrics middleware outside any router and expects `/metrics` to show it with `handler="/test"`, the request path, which is the label this issue removes. With the fix in https://git.eeqj.de/sneak/pixa/pulls/260 the same request is labelled `unmatched`, so the test fails.
- Yes (recommended): its expected line changes from `handler="/test"` to `handler="unmatched"`. It still checks that a served request is recorded; the PR then goes to review.
- No: the test can only pass unchanged if the middleware falls back to the path when no router is present, which keeps the label this issue removes.
Model: opus-5-5
clawbot
removed their assignment 2026-10-15 04:00:05 +02:00
sneak
was assigned by clawbot2026-10-15 04:00:05 +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.
Where:
internal/middleware/middleware.golines 212-221 (Metricscallsstd.Handler("", mdlw, next)with an empty handler ID) andinternal/server/routes.golines 36-39 (installed router-wide, before routing), onv1.0.0-rc1.What is wrong: with an empty handler ID, go-http-metrics v0.13.0 uses
r.URL.Pathas thehandlerlabel (middleware/middleware.golines 92-95 in the library), so every distinct path requested creates its own set of series in the Prometheus registry: a duration histogram with eleven buckets, a size histogram and an in-flight gauge. Series are never dropped. The middleware runs whenmetrics.usernameandmetrics.passwordare set, which the README and the example Caddyfile recommend.How it fails: an outsider sends requests with a new path each time: 404 paths, which no rate limit covers, or
/v1/image/x/<random>/orig.jpg. A few million paths take the container's memory; nothing frees it short of a restart. As a side effect every/v1/e/<token>/...path ever requested is listed in/metricsas a label value, so the scraper and its storage hold the tokens.Severity: high
Definition of done:
handlerlabel is the matched route pattern (chi'sRouteContext(ctx).RoutePattern()after routing, or the middleware mounted per route with a fixed ID), never the request path, and unmatched requests share one value.handlerlabel value;make checkgreen.Model: fable-5-1
Plan, as the definition of done says: the
handlerlabel is the matched chi route pattern, read after routing, and every unmatched request shares one value; no request path ever becomes a label.Package:
internal/middleware(andinternal/server/routes.goif the mounting changes). The test is a new one, with its own Prometheus registry where the library allows, so it does not touch the existing/metricstest that #180 asks about.Model: opus-5-5
Question: may one value in the existing test
TestMetricsRecordsServedRequest(internal/middleware/middleware_internal_test.go) change?CLAUDE.mdneeds your approval to change an existing test.The test serves
GET /testthrough the metrics middleware outside any router and expects/metricsto show it withhandler="/test", the request path, which is the label this issue removes. With the fix in #260 the same request is labelledunmatched, so the test fails.handler="/test"tohandler="unmatched". It still checks that a served request is recorded; the PR then goes to review.Model: opus-5-5
yes