From 41a5509ce0d6368ef6ef5820a300cbe4d26a2f05 Mon Sep 17 00:00:00 2001 From: Alisdair MacLeod Date: Thu, 12 Feb 2026 15:19:19 +0000 Subject: [PATCH] fix nil pointer error in roundtripper --- proxy/internal/metrics/metrics.go | 16 +++++-- proxy/internal/metrics/metrics_test.go | 64 ++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 proxy/internal/metrics/metrics_test.go diff --git a/proxy/internal/metrics/metrics.go b/proxy/internal/metrics/metrics.go index 2e77614fd..951ce73dd 100644 --- a/proxy/internal/metrics/metrics.go +++ b/proxy/internal/metrics/metrics.go @@ -106,15 +106,21 @@ func (f roundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) { func (m *Metrics) RoundTripper(next http.RoundTripper) http.RoundTripper { return roundTripperFunc(func(req *http.Request) (*http.Response, error) { - start := time.Now() - res, err := next.RoundTrip(req) - duration := time.Since(start) - labels := prometheus.Labels{ "method": req.Method, "host": req.Host, - "path": req.URL.Path, + // Fill potentially empty labels with default values to avoid cardinality issues. + "path": "/", + "status": "0", + "size": "0", } + if req.URL != nil { + labels["path"] = req.URL.Path + } + + start := time.Now() + res, err := next.RoundTrip(req) + duration := time.Since(start) // Not all labels will be available if there was an error. if res != nil { diff --git a/proxy/internal/metrics/metrics_test.go b/proxy/internal/metrics/metrics_test.go new file mode 100644 index 000000000..72a429653 --- /dev/null +++ b/proxy/internal/metrics/metrics_test.go @@ -0,0 +1,64 @@ +package metrics_test + +import ( + "net/http" + "net/url" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/netbirdio/netbird/proxy/internal/metrics" + "github.com/prometheus/client_golang/prometheus" +) + +type testRoundTripper struct { + response *http.Response + err error +} + +func (t *testRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + return t.response, t.err +} + +func TestMetrics_RoundTripper(t *testing.T) { + testResponse := http.Response{ + StatusCode: http.StatusOK, + Body: http.NoBody, + } + + tests := map[string]struct { + roundTripper http.RoundTripper + request *http.Request + response *http.Response + err error + }{ + "ok": { + roundTripper: &testRoundTripper{response: &testResponse}, + request: &http.Request{Method: "GET", URL: &url.URL{Path: "/foo"}}, + response: &testResponse, + }, + "nil url": { + roundTripper: &testRoundTripper{response: &testResponse}, + request: &http.Request{Method: "GET", URL: nil}, + response: &testResponse, + }, + "nil response": { + roundTripper: &testRoundTripper{response: nil}, + request: &http.Request{Method: "GET", URL: &url.URL{Path: "/foo"}}, + }, + } + + m := metrics.New(prometheus.NewRegistry()) + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + rt := m.RoundTripper(test.roundTripper) + res, err := rt.RoundTrip(test.request) + if diff := cmp.Diff(test.err, err); diff != "" { + t.Errorf("Incorrect error (-want +got):\n%s", diff) + } + if diff := cmp.Diff(test.response, res); diff != "" { + t.Errorf("Incorrect response (-want +got):\n%s", diff) + } + }) + } +}