diff --git a/pkg/common/http.go b/pkg/common/http.go index f40612a23..330e8a865 100644 --- a/pkg/common/http.go +++ b/pkg/common/http.go @@ -133,8 +133,8 @@ func (t *InstrumentedTransport) RoundTrip(req *http.Request) (*http.Response, er } if resp != nil { - // record latency and increment counter for non-200 status code - recordHTTPResponse(sanitizedURL, resp.StatusCode, duration.Seconds()) + // record latency, response size and increment counter for non-200 status code + recordHTTPResponse(sanitizedURL, resp.StatusCode, duration.Seconds(), resp.ContentLength) } return resp, err @@ -198,7 +198,7 @@ func WithRetryWaitMax(wait time.Duration) ClientOption { func PinnedRetryableHttpClient() *http.Client { httpClient := retryablehttp.NewClient() httpClient.Logger = nil - httpClient.HTTPClient.Transport = NewCustomTransport(&http.Transport{ + httpClient.HTTPClient.Transport = NewInstrumentedTransport(NewCustomTransport(&http.Transport{ TLSClientConfig: &tls.Config{ RootCAs: PinnedCertPool(), }, @@ -212,7 +212,7 @@ func PinnedRetryableHttpClient() *http.Client { IdleConnTimeout: 90 * time.Second, TLSHandshakeTimeout: 10 * time.Second, ExpectContinueTimeout: 1 * time.Second, - }) + })) return httpClient.StandardClient() } @@ -220,7 +220,7 @@ func RetryableHTTPClient(opts ...ClientOption) *http.Client { httpClient := retryablehttp.NewClient() httpClient.RetryMax = 3 httpClient.Logger = nil - httpClient.HTTPClient.Transport = NewCustomTransport(nil) + httpClient.HTTPClient.Transport = NewInstrumentedTransport(NewCustomTransport(nil)) for _, opt := range opts { opt(httpClient) @@ -234,7 +234,7 @@ func RetryableHTTPClientTimeout(timeOutSeconds int64, opts ...ClientOption) *htt httpClient.RetryMax = 3 httpClient.Logger = nil httpClient.HTTPClient.Timeout = time.Duration(timeOutSeconds) * time.Second - httpClient.HTTPClient.Transport = NewCustomTransport(nil) + httpClient.HTTPClient.Transport = NewInstrumentedTransport(NewCustomTransport(nil)) for _, opt := range opts { opt(httpClient) diff --git a/pkg/common/http_metrics.go b/pkg/common/http_metrics.go index 7e0c27287..868112d46 100644 --- a/pkg/common/http_metrics.go +++ b/pkg/common/http_metrics.go @@ -39,6 +39,17 @@ var ( }, []string{"url", "status_code"}, ) + + httpResponseBodySizeBytes = promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Namespace: MetricsNamespace, + Subsystem: "http_client", + Name: "response_body_size_bytes", + Help: "Size of HTTP response bodies in bytes, labeled by URL.", + Buckets: prometheus.ExponentialBuckets(100, 10, 5), // [100B, 1KB, 10KB, 100KB, 1MB] + }, + []string{"url"}, + ) ) // sanitizeURL sanitizes a URL to avoid high cardinality metrics. @@ -92,7 +103,7 @@ func recordHTTPRequest(sanitizedURL string) { } // recordHTTPResponse records metrics for an HTTP response. -func recordHTTPResponse(sanitizedURL string, statusCode int, durationSeconds float64) { +func recordHTTPResponse(sanitizedURL string, statusCode int, durationSeconds float64, contentLength int64) { // Record latency httpRequestDuration.WithLabelValues(sanitizedURL).Observe(durationSeconds) @@ -100,6 +111,11 @@ func recordHTTPResponse(sanitizedURL string, statusCode int, durationSeconds flo if statusCode != 200 { httpNon200ResponsesTotal.WithLabelValues(sanitizedURL, strconv.Itoa(statusCode)).Inc() } + + // Record response body size if known + if contentLength >= 0 { + httpResponseBodySizeBytes.WithLabelValues(sanitizedURL).Observe(float64(contentLength)) + } } // recordNetworkError records metrics for failed HTTP response diff --git a/pkg/common/http_test.go b/pkg/common/http_test.go index 0fb64fd4a..34d602886 100644 --- a/pkg/common/http_test.go +++ b/pkg/common/http_test.go @@ -405,6 +405,73 @@ func TestSaneHttpClientMetrics(t *testing.T) { } } +func TestRetryableHttpClientMetrics(t *testing.T) { + // Create a test server that returns different status codes + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/success": + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("success")) + case "/error": + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte("error")) + case "/notfound": + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte("not found")) + default: + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("default")) + } + })) + defer server.Close() + + // Create a RetryableHttpClient + client := RetryableHTTPClient() + + testCases := []struct { + name string + path string + expectedStatusCode int + }{ + { + name: "successful request", + path: "/success", + expectedStatusCode: 200, + }, + { + name: "not found request", + path: "/notfound", + expectedStatusCode: 404, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var requestURL string + if strings.HasPrefix(tc.path, "http") { + requestURL = tc.path + } else { + requestURL = server.URL + tc.path + } + + // Get initial metric values + sanitizedURL := sanitizeURL(requestURL) + initialRequestsTotal := testutil.ToFloat64(httpRequestsTotal.WithLabelValues(sanitizedURL)) + + // Make the request + resp, err := client.Get(requestURL) + + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, tc.expectedStatusCode, resp.StatusCode) + + // Check that request counter was incremented + requestsTotal := testutil.ToFloat64(httpRequestsTotal.WithLabelValues(sanitizedURL)) + assert.Equal(t, initialRequestsTotal+1, requestsTotal) + }) + } +} + func TestInstrumentedTransport(t *testing.T) { // Create a mock transport that we can control server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {