diff --git a/pkg/common/http.go b/pkg/common/http.go index 330e8a865..b723a50f2 100644 --- a/pkg/common/http.go +++ b/pkg/common/http.go @@ -113,11 +113,8 @@ type InstrumentedTransport struct { } func (t *InstrumentedTransport) RoundTrip(req *http.Request) (*http.Response, error) { - - sanitizedURL := sanitizeURL(req.URL.String()) - - // increment counter for the URL - recordHTTPRequest(sanitizedURL) + // increment counter for the request + recordHTTPRequest() // Record start time for latency measurement start := time.Now() @@ -128,13 +125,13 @@ func (t *InstrumentedTransport) RoundTrip(req *http.Request) (*http.Response, er duration := time.Since(start) if err != nil { - recordNetworkError(sanitizedURL) + recordNetworkError() return nil, err } if resp != nil { // record latency, response size and increment counter for non-200 status code - recordHTTPResponse(sanitizedURL, resp.StatusCode, duration.Seconds(), resp.ContentLength) + recordHTTPResponse(resp.StatusCode, duration.Seconds(), resp.ContentLength) } return resp, err diff --git a/pkg/common/http_metrics.go b/pkg/common/http_metrics.go index 868112d46..3fd5b73f5 100644 --- a/pkg/common/http_metrics.go +++ b/pkg/common/http_metrics.go @@ -1,7 +1,6 @@ package common import ( - "net/url" "strconv" "github.com/prometheus/client_golang/prometheus" @@ -9,25 +8,23 @@ import ( ) var ( - httpRequestsTotal = promauto.NewCounterVec( + httpRequestsTotal = promauto.NewCounter( prometheus.CounterOpts{ Namespace: MetricsNamespace, Subsystem: "http_client", Name: "requests_total", - Help: "Total number of HTTP requests made, labeled by URL.", + Help: "Total number of HTTP requests made.", }, - []string{"url"}, ) - httpRequestDuration = promauto.NewHistogramVec( + httpRequestDuration = promauto.NewHistogram( prometheus.HistogramOpts{ Namespace: MetricsNamespace, Subsystem: "http_client", Name: "request_duration_seconds", - Help: "HTTP request latency in seconds, labeled by URL.", + Help: "HTTP request latency in seconds.", Buckets: prometheus.DefBuckets, }, - []string{"url"}, ) httpNon200ResponsesTotal = promauto.NewCounterVec( @@ -35,90 +32,44 @@ var ( Namespace: MetricsNamespace, Subsystem: "http_client", Name: "non_200_responses_total", - Help: "Total number of non-200 HTTP responses, labeled by URL and status code.", + Help: "Total number of non-200 HTTP responses, labeled by status code.", }, - []string{"url", "status_code"}, + []string{"status_code"}, ) - httpResponseBodySizeBytes = promauto.NewHistogramVec( + httpResponseBodySizeBytes = promauto.NewHistogram( prometheus.HistogramOpts{ Namespace: MetricsNamespace, Subsystem: "http_client", Name: "response_body_size_bytes", - Help: "Size of HTTP response bodies in bytes, labeled by URL.", + Help: "Size of HTTP response bodies in bytes.", Buckets: prometheus.ExponentialBuckets(100, 10, 5), // [100B, 1KB, 10KB, 100KB, 1MB] }, - []string{"url"}, ) ) -// sanitizeURL sanitizes a URL to avoid high cardinality metrics. -// It keeps only the host and path, removing query parameters, fragments, and user info. -func sanitizeURL(rawURL string) string { - if rawURL == "" { - return "unknown" - } - - parsedURL, err := url.Parse(rawURL) - if err != nil { - return "invalid_url" - } - - // Build sanitized URL with just scheme, host, and path - sanitized := &url.URL{ - Scheme: parsedURL.Scheme, - Host: parsedURL.Host, - Path: parsedURL.Path, - } - - // If host is empty, try to extract from the raw URL - if sanitized.Host == "" { - // For relative URLs or malformed URLs, just use a placeholder - return "relative_or_invalid" - } - - // Normalize path - if sanitized.Path == "" { - sanitized.Path = "/" - } - - // Limit path length to avoid extremely long paths creating high cardinality - if len(sanitized.Path) > 100 { - sanitized.Path = sanitized.Path[:100] + "..." - } - - result := sanitized.String() - - // Final fallback to avoid empty strings - if result == "" { - return "unknown" - } - - return result -} - // recordHTTPRequest records metrics for an HTTP request. -func recordHTTPRequest(sanitizedURL string) { - httpRequestsTotal.WithLabelValues(sanitizedURL).Inc() +func recordHTTPRequest() { + httpRequestsTotal.Inc() } // recordHTTPResponse records metrics for an HTTP response. -func recordHTTPResponse(sanitizedURL string, statusCode int, durationSeconds float64, contentLength int64) { +func recordHTTPResponse(statusCode int, durationSeconds float64, contentLength int64) { // Record latency - httpRequestDuration.WithLabelValues(sanitizedURL).Observe(durationSeconds) + httpRequestDuration.Observe(durationSeconds) // Record non-200 responses if statusCode != 200 { - httpNon200ResponsesTotal.WithLabelValues(sanitizedURL, strconv.Itoa(statusCode)).Inc() + httpNon200ResponsesTotal.WithLabelValues(strconv.Itoa(statusCode)).Inc() } // Record response body size if known if contentLength >= 0 { - httpResponseBodySizeBytes.WithLabelValues(sanitizedURL).Observe(float64(contentLength)) + httpResponseBodySizeBytes.Observe(float64(contentLength)) } } // recordNetworkError records metrics for failed HTTP response -func recordNetworkError(sanitizedURL string) { - httpNon200ResponsesTotal.WithLabelValues(sanitizedURL, "network_error").Inc() +func recordNetworkError() { + httpNon200ResponsesTotal.WithLabelValues("network_error").Inc() } diff --git a/pkg/common/http_test.go b/pkg/common/http_test.go index 55e37e48d..90fb54e40 100644 --- a/pkg/common/http_test.go +++ b/pkg/common/http_test.go @@ -273,62 +273,6 @@ func TestRetryableHTTPClientTimeout(t *testing.T) { } } -func TestSanitizeURL(t *testing.T) { - testCases := []struct { - name string - input string - expected string - }{ - { - name: "valid https URL", - input: "https://api.example.com/v1/users", - expected: "https://api.example.com/v1/users", - }, - { - name: "URL with query parameters", - input: "https://api.example.com/search?q=secret&limit=10", - expected: "https://api.example.com/search", - }, - { - name: "URL with fragment", - input: "https://example.com/page#section", - expected: "https://example.com/page", - }, - { - name: "URL with user info", - input: "https://user:pass@api.example.com/path", - expected: "https://api.example.com/path", - }, - { - name: "empty URL", - input: "", - expected: "unknown", - }, - { - name: "invalid URL", - input: "not-a-url", - expected: "relative_or_invalid", - }, - { - name: "very long path", - input: "https://example.com/" + strings.Repeat("a", 150), - expected: "https://example.com/" + strings.Repeat("a", 99) + "...", // 99 + 1 ("/") = 100 chars - }, - { - name: "root path", - input: "https://example.com", - expected: "https://example.com/", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - result := sanitizeURL(tc.input) - assert.Equal(t, tc.expected, result) - }) - } -} - func TestSaneHttpClientMetrics(t *testing.T) { // Create a test server that returns different status codes server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -388,8 +332,7 @@ func TestSaneHttpClientMetrics(t *testing.T) { } // Get initial metric values - sanitizedURL := sanitizeURL(requestURL) - initialRequestsTotal := testutil.ToFloat64(httpRequestsTotal.WithLabelValues(sanitizedURL)) + initialRequestsTotal := testutil.ToFloat64(httpRequestsTotal) // Make the request resp, err := client.Get(requestURL) @@ -399,7 +342,7 @@ func TestSaneHttpClientMetrics(t *testing.T) { assert.Equal(t, tc.expectedStatusCode, resp.StatusCode) // Check that request counter was incremented - requestsTotal := testutil.ToFloat64(httpRequestsTotal.WithLabelValues(sanitizedURL)) + requestsTotal := testutil.ToFloat64(httpRequestsTotal) assert.Equal(t, initialRequestsTotal+1, requestsTotal) }) } @@ -455,8 +398,7 @@ func TestRetryableHttpClientMetrics(t *testing.T) { } // Get initial metric values - sanitizedURL := sanitizeURL(requestURL) - initialRequestsTotal := testutil.ToFloat64(httpRequestsTotal.WithLabelValues(sanitizedURL)) + initialRequestsTotal := testutil.ToFloat64(httpRequestsTotal) // Make the request resp, err := client.Get(requestURL) @@ -466,7 +408,7 @@ func TestRetryableHttpClientMetrics(t *testing.T) { assert.Equal(t, tc.expectedStatusCode, resp.StatusCode) // Check that request counter was incremented - requestsTotal := testutil.ToFloat64(httpRequestsTotal.WithLabelValues(sanitizedURL)) + requestsTotal := testutil.ToFloat64(httpRequestsTotal) assert.Equal(t, initialRequestsTotal+1, requestsTotal) }) } @@ -488,8 +430,7 @@ func TestInstrumentedTransport(t *testing.T) { } // Get initial metric value - sanitizedURL := sanitizeURL(server.URL) - initialCount := testutil.ToFloat64(httpRequestsTotal.WithLabelValues(sanitizedURL)) + initialCount := testutil.ToFloat64(httpRequestsTotal) // Make a request resp, err := client.Get(server.URL) @@ -500,7 +441,7 @@ func TestInstrumentedTransport(t *testing.T) { assert.Equal(t, http.StatusOK, resp.StatusCode) // Verify metrics were recorded - finalCount := testutil.ToFloat64(httpRequestsTotal.WithLabelValues(sanitizedURL)) + finalCount := testutil.ToFloat64(httpRequestsTotal) assert.Equal(t, initialCount+1, finalCount) // Note: Testing histogram metrics is complex due to the way Prometheus handles them