From 484b2a71ab04b9d4e742bf14adc97abbd17ee950 Mon Sep 17 00:00:00 2001 From: Sam Calder-Mason Date: Wed, 13 May 2026 14:12:00 +1000 Subject: [PATCH 1/4] fix(metrics): use ServeMux route pattern as label, not raw URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Metrics middleware was labeling http_requests_total, http_request_duration_seconds, http_request_size_bytes, and http_response_size_bytes with r.URL.Path. Every unique URL — including vulnerability-scanner probes (/.env, /wp-login.php) and per-network proxy paths (/api/v1//...) — became a permanent metric series in indexdb. Production VictoriaMetrics observed ~48k distinct path values from this app alone. Use r.Pattern (set by net/http.ServeMux after routing) instead, with an "unmatched" sentinel as a defensive fallback. The path label name is preserved for dashboard compatibility but its values are now bounded by the number of registered routes. --- go.mod | 1 + internal/middleware/metrics.go | 43 +++++-- internal/middleware/metrics_test.go | 176 ++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+), 12 deletions(-) create mode 100644 internal/middleware/metrics_test.go diff --git a/go.mod b/go.mod index 7ccd310..a6667c6 100644 --- a/go.mod +++ b/go.mod @@ -20,6 +20,7 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect github.com/kr/text v0.2.0 // indirect + github.com/kylelemons/godebug v1.1.0 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_model v0.6.2 // indirect diff --git a/internal/middleware/metrics.go b/internal/middleware/metrics.go index 2e06fc0..319a2ce 100644 --- a/internal/middleware/metrics.go +++ b/internal/middleware/metrics.go @@ -9,11 +9,16 @@ import ( "github.com/prometheus/client_golang/prometheus/promauto" ) +// unmatchedRoute is the sentinel value used when a request has no matched +// ServeMux pattern. Collapses unmatched URLs (scanner probes, typos) into a +// single series instead of one series per unique URL. +const unmatchedRoute = "unmatched" + var ( httpRequestsTotal = prometheus.NewCounterVec( prometheus.CounterOpts{ Name: "http_requests_total", - Help: "Total number of HTTP requests", + Help: "Total number of HTTP requests. The path label is the matched ServeMux route pattern, not the raw URL.", }, []string{"method", "path", "status"}, ) @@ -21,7 +26,7 @@ var ( httpRequestDuration = prometheus.NewHistogramVec( prometheus.HistogramOpts{ Name: "http_request_duration_seconds", - Help: "HTTP request duration in seconds", + Help: "HTTP request duration in seconds. The path label is the matched ServeMux route pattern, not the raw URL.", Buckets: prometheus.DefBuckets, }, []string{"method", "path"}, @@ -30,7 +35,7 @@ var ( httpRequestSize = prometheus.NewSummaryVec( prometheus.SummaryOpts{ Name: "http_request_size_bytes", - Help: "HTTP request size in bytes", + Help: "HTTP request size in bytes. The path label is the matched ServeMux route pattern, not the raw URL.", }, []string{"method", "path"}, ) @@ -38,7 +43,7 @@ var ( httpResponseSize = prometheus.NewSummaryVec( prometheus.SummaryOpts{ Name: "http_response_size_bytes", - Help: "HTTP response size in bytes", + Help: "HTTP response size in bytes. The path label is the matched ServeMux route pattern, not the raw URL.", }, []string{"method", "path"}, ) @@ -95,6 +100,17 @@ func (mrw *metricsResponseWriter) Write(b []byte) (int, error) { return n, err } +// routePattern returns the ServeMux pattern that matched the request, or +// unmatchedRoute if no pattern matched. Using the raw r.URL.Path here would +// give unbounded label cardinality (one series per unique URL). +func routePattern(r *http.Request) string { + if r.Pattern != "" { + return r.Pattern + } + + return unmatchedRoute +} + // Metrics returns middleware that collects Prometheus metrics. func Metrics() func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { @@ -108,31 +124,34 @@ func Metrics() func(http.Handler) http.Handler { bytesWritten: 0, } - // Record request size - if r.ContentLength > 0 { - httpRequestSize.WithLabelValues(r.Method, r.URL.Path).Observe(float64(r.ContentLength)) - } + // Snapshot request size before invoking the chain. + contentLength := r.ContentLength - // Call next handler + // Call next handler — ServeMux populates r.Pattern during routing. next.ServeHTTP(mrw, r) // Record metrics duration := time.Since(start) + route := routePattern(r) + + if contentLength > 0 { + httpRequestSize.WithLabelValues(r.Method, route).Observe(float64(contentLength)) + } httpRequestsTotal.WithLabelValues( r.Method, - r.URL.Path, + route, strconv.Itoa(mrw.statusCode), ).Inc() httpRequestDuration.WithLabelValues( r.Method, - r.URL.Path, + route, ).Observe(duration.Seconds()) httpResponseSize.WithLabelValues( r.Method, - r.URL.Path, + route, ).Observe(float64(mrw.bytesWritten)) }) } diff --git a/internal/middleware/metrics_test.go b/internal/middleware/metrics_test.go new file mode 100644 index 0000000..0efa787 --- /dev/null +++ b/internal/middleware/metrics_test.go @@ -0,0 +1,176 @@ +package middleware + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func resetMetrics() { + httpRequestsTotal.Reset() + httpRequestDuration.Reset() + httpRequestSize.Reset() + httpResponseSize.Reset() +} + +func TestMetricsMiddleware_UsesRoutePatternNotRawURL(t *testing.T) { + resetMetrics() + + mux := http.NewServeMux() + mux.HandleFunc("GET /api/v1/{network}/bounds", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + handler := Metrics()(mux) + + for _, network := range []string{"mainnet", "sepolia", "holesky"} { + req := httptest.NewRequest(http.MethodGet, "/api/v1/"+network+"/bounds", http.NoBody) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + } + + expectedRoute := "GET /api/v1/{network}/bounds" + count := testutil.ToFloat64(httpRequestsTotal.WithLabelValues(http.MethodGet, expectedRoute, "200")) + assert.Equal(t, float64(3), count, "all three requests should collapse to one route-pattern series") + + for _, network := range []string{"mainnet", "sepolia", "holesky"} { + rawURL := "/api/v1/" + network + "/bounds" + leak := testutil.ToFloat64(httpRequestsTotal.WithLabelValues(http.MethodGet, rawURL, "200")) + assert.Equal(t, float64(0), leak, "raw URL %q must never become a metric label", rawURL) + } +} + +func TestMetricsMiddleware_UnmatchedRouteUsesSentinel(t *testing.T) { + resetMetrics() + + mux := http.NewServeMux() + handler := Metrics()(mux) + + req := httptest.NewRequest(http.MethodGet, "/this/path/does/not/exist", http.NoBody) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + require.Equal(t, http.StatusNotFound, rec.Code) + + count := testutil.ToFloat64(httpRequestsTotal.WithLabelValues(http.MethodGet, unmatchedRoute, "404")) + assert.Equal(t, float64(1), count, "unmatched routes should fall back to the sentinel") + + leak := testutil.ToFloat64(httpRequestsTotal.WithLabelValues(http.MethodGet, "/this/path/does/not/exist", "404")) + assert.Equal(t, float64(0), leak, "raw URL must never become a metric label") +} + +func TestMetricsMiddleware_CatchAllPatternCollapsesGarbage(t *testing.T) { + resetMetrics() + + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + handler := Metrics()(mux) + + garbagePaths := []string{ + "/.env", + "/wp-login.php", + "/$$whyalwaysme@@.php", + "/%2f%2eAwS%2fCrEdEnTiAlS", + } + for _, p := range garbagePaths { + req := httptest.NewRequest(http.MethodGet, p, http.NoBody) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + } + + count := testutil.ToFloat64(httpRequestsTotal.WithLabelValues(http.MethodGet, "/", "200")) + assert.Equal(t, float64(len(garbagePaths)), count, "all garbage URLs should collapse to the catch-all pattern") + + for _, p := range garbagePaths { + leak := testutil.ToFloat64(httpRequestsTotal.WithLabelValues(http.MethodGet, p, "200")) + assert.Equal(t, float64(0), leak, "scanner garbage URL %q must never become a metric label", p) + } +} + +func TestMetricsMiddleware_RequestSizeRecordedAgainstPattern(t *testing.T) { + resetMetrics() + + mux := http.NewServeMux() + mux.HandleFunc("POST /api/v1/upload", func(w http.ResponseWriter, r *http.Request) { + _, _ = io.Copy(io.Discard, r.Body) + w.WriteHeader(http.StatusAccepted) + }) + + handler := Metrics()(mux) + + body := strings.Repeat("x", 1024) + req := httptest.NewRequest(http.MethodPost, "/api/v1/upload", strings.NewReader(body)) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + require.Equal(t, http.StatusAccepted, rec.Code) + + // httpRequestSize is a Summary; the labeled vector should now have one entry. + count := testutil.CollectAndCount(httpRequestSize, "http_request_size_bytes") + assert.Equal(t, 1, count, "request size should be observed exactly once against the route pattern") +} + +func TestMetricsMiddleware_MultipleRoutesGetDistinctLabels(t *testing.T) { + resetMetrics() + + mux := http.NewServeMux() + mux.HandleFunc("GET /health", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + mux.HandleFunc("GET /api/v1/config", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + mux.HandleFunc("GET /api/v1/{network}/bounds", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + handler := Metrics()(mux) + + for _, p := range []string{"/health", "/api/v1/config", "/api/v1/mainnet/bounds", "/api/v1/sepolia/bounds"} { + req := httptest.NewRequest(http.MethodGet, p, http.NoBody) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + } + + assert.Equal(t, float64(1), testutil.ToFloat64( + httpRequestsTotal.WithLabelValues(http.MethodGet, "GET /health", "200"), + )) + assert.Equal(t, float64(1), testutil.ToFloat64( + httpRequestsTotal.WithLabelValues(http.MethodGet, "GET /api/v1/config", "200"), + )) + assert.Equal(t, float64(2), testutil.ToFloat64( + httpRequestsTotal.WithLabelValues(http.MethodGet, "GET /api/v1/{network}/bounds", "200"), + )) +} + +func TestRoutePattern_ReturnsSentinelWhenUnset(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/anything", http.NoBody) + assert.Equal(t, unmatchedRoute, routePattern(req)) +} + +func TestRoutePattern_ReturnsMatchedPattern(t *testing.T) { + mux := http.NewServeMux() + + var observed string + mux.HandleFunc("GET /api/v1/{network}/bounds", func(w http.ResponseWriter, r *http.Request) { + observed = routePattern(r) + w.WriteHeader(http.StatusOK) + }) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/mainnet/bounds", http.NoBody) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, "GET /api/v1/{network}/bounds", observed) +} From 4810fd531e9ae40a1b60ebd1da36d3affc9802fe Mon Sep 17 00:00:00 2001 From: Sam Calder-Mason Date: Wed, 13 May 2026 14:13:47 +1000 Subject: [PATCH 2/4] refactor(metrics): drop narrative comments --- internal/middleware/metrics.go | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/internal/middleware/metrics.go b/internal/middleware/metrics.go index 319a2ce..e91850a 100644 --- a/internal/middleware/metrics.go +++ b/internal/middleware/metrics.go @@ -9,16 +9,13 @@ import ( "github.com/prometheus/client_golang/prometheus/promauto" ) -// unmatchedRoute is the sentinel value used when a request has no matched -// ServeMux pattern. Collapses unmatched URLs (scanner probes, typos) into a -// single series instead of one series per unique URL. const unmatchedRoute = "unmatched" var ( httpRequestsTotal = prometheus.NewCounterVec( prometheus.CounterOpts{ Name: "http_requests_total", - Help: "Total number of HTTP requests. The path label is the matched ServeMux route pattern, not the raw URL.", + Help: "Total number of HTTP requests", }, []string{"method", "path", "status"}, ) @@ -26,7 +23,7 @@ var ( httpRequestDuration = prometheus.NewHistogramVec( prometheus.HistogramOpts{ Name: "http_request_duration_seconds", - Help: "HTTP request duration in seconds. The path label is the matched ServeMux route pattern, not the raw URL.", + Help: "HTTP request duration in seconds", Buckets: prometheus.DefBuckets, }, []string{"method", "path"}, @@ -35,7 +32,7 @@ var ( httpRequestSize = prometheus.NewSummaryVec( prometheus.SummaryOpts{ Name: "http_request_size_bytes", - Help: "HTTP request size in bytes. The path label is the matched ServeMux route pattern, not the raw URL.", + Help: "HTTP request size in bytes", }, []string{"method", "path"}, ) @@ -43,7 +40,7 @@ var ( httpResponseSize = prometheus.NewSummaryVec( prometheus.SummaryOpts{ Name: "http_response_size_bytes", - Help: "HTTP response size in bytes. The path label is the matched ServeMux route pattern, not the raw URL.", + Help: "HTTP response size in bytes", }, []string{"method", "path"}, ) @@ -100,9 +97,6 @@ func (mrw *metricsResponseWriter) Write(b []byte) (int, error) { return n, err } -// routePattern returns the ServeMux pattern that matched the request, or -// unmatchedRoute if no pattern matched. Using the raw r.URL.Path here would -// give unbounded label cardinality (one series per unique URL). func routePattern(r *http.Request) string { if r.Pattern != "" { return r.Pattern @@ -124,13 +118,10 @@ func Metrics() func(http.Handler) http.Handler { bytesWritten: 0, } - // Snapshot request size before invoking the chain. contentLength := r.ContentLength - // Call next handler — ServeMux populates r.Pattern during routing. next.ServeHTTP(mrw, r) - // Record metrics duration := time.Since(start) route := routePattern(r) From 67b76472e2d986eb0703fabdf0586effc849b9a5 Mon Sep 17 00:00:00 2001 From: Sam Calder-Mason Date: Wed, 13 May 2026 14:15:52 +1000 Subject: [PATCH 3/4] test(metrics): satisfy wsl_v5 whitespace lint --- internal/middleware/metrics_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/middleware/metrics_test.go b/internal/middleware/metrics_test.go index 0efa787..eb403ca 100644 --- a/internal/middleware/metrics_test.go +++ b/internal/middleware/metrics_test.go @@ -103,6 +103,7 @@ func TestMetricsMiddleware_RequestSizeRecordedAgainstPattern(t *testing.T) { mux := http.NewServeMux() mux.HandleFunc("POST /api/v1/upload", func(w http.ResponseWriter, r *http.Request) { _, _ = io.Copy(io.Discard, r.Body) + w.WriteHeader(http.StatusAccepted) }) @@ -114,7 +115,6 @@ func TestMetricsMiddleware_RequestSizeRecordedAgainstPattern(t *testing.T) { handler.ServeHTTP(rec, req) require.Equal(t, http.StatusAccepted, rec.Code) - // httpRequestSize is a Summary; the labeled vector should now have one entry. count := testutil.CollectAndCount(httpRequestSize, "http_request_size_bytes") assert.Equal(t, 1, count, "request size should be observed exactly once against the route pattern") } @@ -162,8 +162,10 @@ func TestRoutePattern_ReturnsMatchedPattern(t *testing.T) { mux := http.NewServeMux() var observed string + mux.HandleFunc("GET /api/v1/{network}/bounds", func(w http.ResponseWriter, r *http.Request) { observed = routePattern(r) + w.WriteHeader(http.StatusOK) }) From 57960b09cb4cc8d5c7d2b913f4f8be16bf409e7a Mon Sep 17 00:00:00 2001 From: Sam Calder-Mason Date: Wed, 13 May 2026 14:17:52 +1000 Subject: [PATCH 4/4] ci: pin golangci-lint to v2.11.4 The workflow used 'version: latest' which broke CI when a newer golangci-lint release tightened goconst/wsl rules and started flagging pre-existing strings across the repo. v2.11.4 is the version that ran on the last green master build (#55, 2026-04-24). --- .github/workflows/golangci-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 3e2aeac..d34aaec 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -27,5 +27,5 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@1e7e51e771db61008b38414a730f564565cf7c20 # v9.2.0 with: - version: latest + version: v2.11.4 args: --timeout=10m