From 59e22c7e9f0f61d43f392f00b57f2ef42acc613b Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Sun, 25 Oct 2026 16:04:16 -0400 Subject: [PATCH] fix(observability): label HTTP metrics by route pattern, not raw path The Prometheus HTTP middleware is the first middleware on the REST API router, ahead of authentication, and it used r.URL.Path as the path label. Every distinct path any caller sends (/api/v1/, or a server name in /api/v1/servers/{name}/...) therefore created a new counter and histogram series that is never freed, so with metrics enabled an unauthenticated client could grow the daemon's memory and the /metrics output without bound. Use the chi route pattern the request matched, and a single "unmatched" label when no route matched. --- internal/observability/metrics.go | 22 +++++++++++++++++++++- internal/observability/metrics_test.go | 23 +++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/internal/observability/metrics.go b/internal/observability/metrics.go index d7cc32e19..e922e3a45 100644 --- a/internal/observability/metrics.go +++ b/internal/observability/metrics.go @@ -4,6 +4,7 @@ import ( "net/http" "time" + "github.com/go-chi/chi/v5" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/collectors" "github.com/prometheus/client_golang/prometheus/promhttp" @@ -431,11 +432,30 @@ func (mm *MetricsManager) HTTPMiddleware() func(http.Handler) http.Handler { // Record metrics duration := time.Since(start) - mm.RecordHTTPRequest(r.Method, r.URL.Path, http.StatusText(ww.statusCode), duration) + mm.RecordHTTPRequest(r.Method, routeLabel(r), http.StatusText(ww.statusCode), duration) }) } } +// unmatchedRouteLabel is the path label for a request no route matched. +const unmatchedRouteLabel = "unmatched" + +// routeLabel is the path label for r: the chi route pattern it matched +// ("/api/v1/servers/{name}"), never the raw path. The middleware runs before +// authentication, so a raw-path label would let any caller add a counter and +// a histogram series per distinct path it sends. Outside a chi router the raw +// path is kept. +func routeLabel(r *http.Request) string { + rctx := chi.RouteContext(r.Context()) + if rctx == nil { + return r.URL.Path + } + if pattern := rctx.RoutePattern(); pattern != "" { + return pattern + } + return unmatchedRouteLabel +} + // responseWriter wraps http.ResponseWriter to capture status code type responseWriter struct { http.ResponseWriter diff --git a/internal/observability/metrics_test.go b/internal/observability/metrics_test.go index b4e7dbcca..653c02929 100644 --- a/internal/observability/metrics_test.go +++ b/internal/observability/metrics_test.go @@ -1,10 +1,13 @@ package observability import ( + "net/http" + "net/http/httptest" "strings" "testing" "time" + "github.com/go-chi/chi/v5" "github.com/prometheus/client_golang/prometheus/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -55,6 +58,26 @@ func TestToolCallMetric_NoPerUserCardinalityLabels(t *testing.T) { assert.NotContains(t, body, "profile=", "tool-call metric must not carry a profile label") } +// The HTTP middleware runs before authentication, so its path label must be +// the matched route, not the raw request path: otherwise every distinct path +// anyone sends adds a counter and a histogram series that is never freed. +func TestHTTPMiddleware_PathLabelIsRoutePattern(t *testing.T) { + mm := NewMetricsManager(zap.NewNop().Sugar()) + r := chi.NewRouter() + r.Use(mm.HTTPMiddleware()) + r.Get("/api/v1/servers/{name}", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + for _, p := range []string{"/api/v1/servers/a", "/api/v1/servers/b", "/api/v1/nope-1", "/api/v1/nope-2"} { + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, p, nil)) + } + + assert.Equal(t, 2, testutil.CollectAndCount(mm.httpRequests), "one series per route plus one for unmatched paths") + assert.InDelta(t, 2.0, testutil.ToFloat64(mm.httpRequests.WithLabelValues(http.MethodGet, "/api/v1/servers/{name}", "OK")), 1e-9) + assert.InDelta(t, 2.0, testutil.ToFloat64(mm.httpRequests.WithLabelValues(http.MethodGet, "unmatched", "Not Found")), 1e-9) +} + func scrapeMetrics(t *testing.T, mm *MetricsManager) string { t.Helper() families, err := mm.registry.Gather()