diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index f6eb83d..7b433c7 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -14,6 +14,7 @@ import ( "github.com/go-chi/chi/v5/middleware" "github.com/go-chi/cors" "github.com/go-chi/httprate" + "github.com/prometheus/client_golang/prometheus" metrics "github.com/slok/go-http-metrics/metrics/prometheus" ghmm "github.com/slok/go-http-metrics/middleware" "github.com/slok/go-http-metrics/middleware/std" @@ -63,6 +64,10 @@ type Middleware struct { log *slog.Logger config *config.Config 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. @@ -212,7 +217,9 @@ func (s *Middleware) CORS() func(http.Handler) http.Handler { // Metrics returns a Prometheus metrics middleware. func (s *Middleware) Metrics() func(http.Handler) http.Handler { 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 { diff --git a/internal/middleware/middleware_internal_test.go b/internal/middleware/middleware_internal_test.go index 91ba3f3..5801513 100644 --- a/internal/middleware/middleware_internal_test.go +++ b/internal/middleware/middleware_internal_test.go @@ -3,12 +3,15 @@ package middleware import ( "bytes" "log/slog" + "maps" "net/http" "net/http/httptest" "net/url" "strings" "testing" + "github.com/go-chi/chi/v5" + "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" "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. 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{}} @@ -220,6 +223,83 @@ 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". 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.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": 3} + 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 / // whose form carries the signing key, leaves no trace of the key in the // request's log line.