5 Commits
Author SHA1 Message Date
sneak c38f495260 Keep the metrics entry at the top of Completed Steps
check / check (push) Waiting to run
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
2026-10-15 10:39:12 +00:00
sneak bf4ae21974 Label requests for a subrouter's own path unmatched in metrics
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
2026-10-15 10:38:40 +00:00
sneak 358f01a2a4 Test that metrics label requests for /v1 and /v1/ unmatched
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
2026-10-15 10:38:40 +00:00
sneak 6ca43e26d7 Label request metrics by route, not by path (closes #254)
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
2026-10-15 10:38:40 +00:00
sneak 75ce7a9ea8 Test that metrics label requests by route, not path
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
2026-10-15 10:38:40 +00:00
4 changed files with 151 additions and 5 deletions
+4 -1
View File
@@ -639,7 +639,10 @@ See `configs/config.example.yml` for all options with defaults.
- **Metrics**: Prometheus, at `/metrics`: generic HTTP request metrics - **Metrics**: Prometheus, at `/metrics`: generic HTTP request metrics
(duration, response size, requests in flight) and the Go runtime and process (duration, response size, requests in flight) and the Go runtime and process
metrics; requests are measured and `/metrics` is served only when metrics; requests are measured and `/metrics` is served only when
`metrics.username` and `metrics.password` are set `metrics.username` and `metrics.password` are set. A request's `handler` label
is the pattern of the route it matches, such as `/v1/image/*`, never its path.
Requests that match no route share the label `unmatched`: those the router
answers 404 or 405, and CORS preflights
- **Logging**: stdlib slog - **Logging**: stdlib slog
## Entrypoints ## Entrypoints
+9
View File
@@ -30,6 +30,15 @@ P2: security: per-origin rate limiting
# Completed Steps # Completed Steps
- 2026-10-15 the request metrics are labelled by route, not by path (closes
#254): their `handler` label was each request's path, so every new path, a 404
or an encrypted URL, added series to the Prometheus registry that were never
removed, and `/metrics` listed every encrypted URL's token. The label is now
the pattern of the chi route the request matches, such as `/v1/image/*`, or
`unmatched` for a request that matches none: one the router answers 404 or
405, or a CORS preflight. The metrics library takes the label before the
request is routed, so the metrics middleware asks the router which route
matches.
- 2026-10-15 the host of a `/v1/image/` URL is lower-cased, and one with a port - 2026-10-15 the host of a `/v1/image/` URL is lower-cased, and one with a port
needs a signature (closes #246): with `cdn.example.com` allowlisted, needs a signature (closes #246): with `cdn.example.com` allowlisted,
`CDN.Example.com` and `cdn.example.com:8443` passed the allowlist too, and `CDN.Example.com` and `cdn.example.com:8443` passed the allowlist too, and
+54 -3
View File
@@ -8,12 +8,15 @@ import (
"net/http" "net/http"
"net/netip" "net/netip"
"regexp" "regexp"
"strings"
"time" "time"
basicauth "github.com/99designs/basicauth-go" basicauth "github.com/99designs/basicauth-go"
"github.com/go-chi/chi/v5"
"github.com/go-chi/chi/v5/middleware" "github.com/go-chi/chi/v5/middleware"
"github.com/go-chi/cors" "github.com/go-chi/cors"
"github.com/go-chi/httprate" "github.com/go-chi/httprate"
"github.com/prometheus/client_golang/prometheus"
metrics "github.com/slok/go-http-metrics/metrics/prometheus" metrics "github.com/slok/go-http-metrics/metrics/prometheus"
ghmm "github.com/slok/go-http-metrics/middleware" ghmm "github.com/slok/go-http-metrics/middleware"
"github.com/slok/go-http-metrics/middleware/std" "github.com/slok/go-http-metrics/middleware/std"
@@ -63,6 +66,10 @@ type Middleware struct {
log *slog.Logger log *slog.Logger
config *config.Config config *config.Config
clientIP *clientip.Resolver clientIP *clientip.Resolver
// metricsRegistry is the registry Metrics registers its metrics with.
// nil is Prometheus' default registry, the one /metrics serves.
metricsRegistry prometheus.Registerer
} }
// New creates a new Middleware instance. // New creates a new Middleware instance.
@@ -209,17 +216,61 @@ func (s *Middleware) CORS() func(http.Handler) http.Handler {
}) })
} }
// Metrics returns a Prometheus metrics middleware. // Metrics returns a Prometheus metrics middleware. Its handler label is the
// pattern of the chi route a request matches, such as /v1/image/*, or
// "unmatched" for a request that matches none, so the routes fix how many
// values the label has. It is never the path, with which any client could
// add series to the registry without limit.
func (s *Middleware) Metrics() func(http.Handler) http.Handler { func (s *Middleware) Metrics() func(http.Handler) http.Handler {
mdlw := ghmm.New(ghmm.Config{ mdlw := ghmm.New(ghmm.Config{
Recorder: metrics.NewRecorder(metrics.Config{}), Recorder: metrics.NewRecorder(metrics.Config{
Registry: s.metricsRegistry,
}),
}) })
return func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler {
return std.Handler("", mdlw, next) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
std.Handler(routePattern(r), mdlw, next).ServeHTTP(w, r)
})
} }
} }
// routePattern returns the pattern of the chi route r matches, or
// "unmatched" when it matches none or is not served by a chi router. The
// metrics library takes the label before the request is routed, so the
// router is asked which route matches, with the path it routes on:
// r.URL.RawPath when set, else r.URL.Path.
func routePattern(r *http.Request) string {
pattern := ""
rctx := chi.RouteContext(r.Context())
if rctx != nil {
path := r.URL.RawPath
if path == "" {
path = r.URL.Path
}
pattern = rctx.Routes.Find(chi.NewRouteContext(), r.Method, path)
// For the path a subrouter is mounted at, such as /v1 or /v1/, Find
// answers that path without asking the subrouter, which chi hands
// the request as /.
for _, route := range rctx.Routes.Routes() {
if route.SubRoutes != nil &&
route.Pattern == strings.TrimSuffix(pattern, "/")+"/*" &&
!route.SubRoutes.Match(chi.NewRouteContext(), r.Method, "/") {
pattern = ""
}
}
}
if pattern == "" {
return "unmatched"
}
return pattern
}
// MetricsAuth returns a basic auth middleware for the metrics endpoint. // MetricsAuth returns a basic auth middleware for the metrics endpoint.
func (s *Middleware) MetricsAuth() func(http.Handler) http.Handler { func (s *Middleware) MetricsAuth() func(http.Handler) http.Handler {
return basicauth.New( return basicauth.New(
@@ -3,12 +3,15 @@ package middleware
import ( import (
"bytes" "bytes"
"log/slog" "log/slog"
"maps"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"net/url" "net/url"
"strings" "strings"
"testing" "testing"
"github.com/go-chi/chi/v5"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promhttp" "github.com/prometheus/client_golang/prometheus/promhttp"
"sneak.berlin/go/pixa/internal/config" "sneak.berlin/go/pixa/internal/config"
@@ -198,7 +201,7 @@ func TestMetricsRecordsServedRequest(t *testing.T) {
// The line /metrics shows once one GET /test has been served. // The line /metrics shows once one GET /test has been served.
const want = `http_request_duration_seconds_count{` + const want = `http_request_duration_seconds_count{` +
`code="200",handler="/test",method="GET",service=""} 1` `code="200",handler="unmatched",method="GET",service=""} 1`
mw := &Middleware{log: slog.Default(), config: &config.Config{}} mw := &Middleware{log: slog.Default(), config: &config.Config{}}
@@ -220,6 +223,86 @@ func TestMetricsRecordsServedRequest(t *testing.T) {
} }
} }
// TestMetricsLabelsRequestsByRoute checks that the metrics middleware labels
// each request with the pattern of the route that answers it, never with its
// path: two paths one route answers share that route's pattern, and requests
// no route answers, a 404 or a 405, share "unmatched", among them requests for
// the path the subrouter is mounted at, /v1 and /v1/. With a label per path,
// any client could add series to the registry without limit. The middleware
// registers with a registry of its own here, as another test in this package
// registers with Prometheus' default registry, which can be done only once.
func TestMetricsLabelsRequestsByRoute(t *testing.T) {
t.Parallel()
registry := prometheus.NewRegistry()
mw := &Middleware{
log: slog.Default(),
config: &config.Config{},
metricsRegistry: registry,
}
// The image route is in a subrouter, as in pixa's router.
router := chi.NewRouter()
router.Use(mw.Metrics())
router.Route("/v1", func(r chi.Router) {
r.Get("/image/*", func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
})
})
for _, req := range []struct{ method, path string }{
{http.MethodGet, "/v1/image/a.example.com/one.jpg/1x1.png"},
{http.MethodGet, "/v1/image/b.example.com/two.jpg/1x1.png"},
{http.MethodGet, "/no-such-route"},
{http.MethodGet, "/v1/no-such-route"},
{http.MethodGet, "/v1"},
{http.MethodGet, "/v1/"},
{http.MethodPost, "/v1/image/a.example.com/one.jpg/1x1.png"},
} {
router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequestWithContext(
t.Context(), req.method, req.path, nil))
}
got := requestCountsByHandler(t, registry)
t.Logf("requests counted by handler label: %v", got)
want := map[string]uint64{"/v1/image/*": 2, "unmatched": 5}
if !maps.Equal(got, want) {
t.Errorf("requests counted by handler label = %v, want %v", got, want)
}
}
// requestCountsByHandler returns how many requests the request duration
// histogram in registry counted under each value of its handler label.
func requestCountsByHandler(
t *testing.T, registry *prometheus.Registry,
) map[string]uint64 {
t.Helper()
families, err := registry.Gather()
if err != nil {
t.Fatalf("registry.Gather() error = %v", err)
}
counts := map[string]uint64{}
for _, family := range families {
if family.GetName() != "http_request_duration_seconds" {
continue
}
for _, metric := range family.GetMetric() {
for _, label := range metric.GetLabel() {
if label.GetName() == "handler" {
counts[label.GetValue()] += metric.GetHistogram().GetSampleCount()
}
}
}
}
return counts
}
// TestLoggingLeavesOutSubmittedSigningKey checks that a login, a POST / // TestLoggingLeavesOutSubmittedSigningKey checks that a login, a POST /
// whose form carries the signing key, leaves no trace of the key in the // whose form carries the signing key, leaves no trace of the key in the
// request's log line. // request's log line.