fix: prevent panic when ContentLength is negative in Distributor RequestBodyTooLarge metrics (backport k249)

pkg/distributor/http.go

@@ -63,7 +63,12 @@ func (d *Distributor) pushHandler(w http.ResponseWriter, r *http.Request, pushRe
 			// and thus we don't know the uncompressed size.
 			// In addition we don't add the metric label values for
 			// `retention_hours` and `policy` because we don't know the labels.
-			validation.DiscardedBytes.WithLabelValues(validation.RequestBodyTooLarge, tenantID).Add(float64(r.ContentLength))
+			// Ensure ContentLength is positive to avoid counter panic
+			if r.ContentLength > 0 {
+				// Add empty values for retention_hours and policy labels since we don't have
+				// that information for request body too large errors
+				validation.DiscardedBytes.WithLabelValues(validation.RequestBodyTooLarge, tenantID, "", "").Add(float64(r.ContentLength))
+			}
 			errorWriter(w, err.Error(), http.StatusRequestEntityTooLarge, logger)
 			return
 

pkg/distributor/http_test.go

@@ -112,6 +112,51 @@ func TestRequestParserWrapping(t *testing.T) {
 		require.True(t, called)
 		require.Equal(t, http.StatusNoContent, rec.Code)
 	})
+
+	t.Run("it handles request body too large error with positive content length", func(t *testing.T) {
+		limits := &validation.Limits{}
+		flagext.DefaultValues(limits)
+		limits.RejectOldSamples = false
+		distributors, _ := prepare(t, 1, 3, limits, nil)
+
+		ctx := user.InjectOrgID(context.Background(), "test-user")
+		req, err := http.NewRequestWithContext(ctx, http.MethodPost, "fake-path", nil)
+		require.NoError(t, err)
+
+		// Set a positive content length
+		req.ContentLength = 1000
+
+		parser := newFakeParser()
+		parser.parseErr = push.ErrRequestBodyTooLarge
+
+		rec := httptest.NewRecorder()
+		distributors[0].pushHandler(rec, req, parser.parseRequest, push.HTTPError)
+
+		require.Equal(t, http.StatusRequestEntityTooLarge, rec.Code)
+	})
+
+	t.Run("it handles request body too large error with negative content length", func(t *testing.T) {
+		limits := &validation.Limits{}
+		flagext.DefaultValues(limits)
+		limits.RejectOldSamples = false
+		distributors, _ := prepare(t, 1, 3, limits, nil)
+
+		ctx := user.InjectOrgID(context.Background(), "test-user")
+		req, err := http.NewRequestWithContext(ctx, http.MethodPost, "fake-path", nil)
+		require.NoError(t, err)
+
+		// Set a negative content length to test our guard clause
+		req.ContentLength = -1
+
+		parser := newFakeParser()
+		parser.parseErr = push.ErrRequestBodyTooLarge
+
+		rec := httptest.NewRecorder()
+		distributors[0].pushHandler(rec, req, parser.parseRequest, push.HTTPError)
+
+		require.Equal(t, http.StatusRequestEntityTooLarge, rec.Code)
+		// The test should complete without panicking
+	})
 }
 
 type fakeParser struct {