From c30a3f5f9899349353dd611e79de1ce95bc51388 Mon Sep 17 00:00:00 2001 From: Salva Corts Date: Fri, 7 Jun 2024 16:40:17 +0200 Subject: [PATCH] fix(otel): Map 500 errors to 503 (#13173) (cherry picked from commit b31e04e3f1b7424cc52b518dc974a382a25bf045) --- pkg/distributor/http.go | 21 ++++++++++++++++++++- pkg/distributor/http_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/pkg/distributor/http.go b/pkg/distributor/http.go index 00c3ba53a2806..ec0660b91bc01 100644 --- a/pkg/distributor/http.go +++ b/pkg/distributor/http.go @@ -23,7 +23,26 @@ func (d *Distributor) PushHandler(w http.ResponseWriter, r *http.Request) { } func (d *Distributor) OTLPPushHandler(w http.ResponseWriter, r *http.Request) { - d.pushHandler(w, r, push.ParseOTLPRequest) + interceptor := newOtelErrorHeaderInterceptor(w) + d.pushHandler(interceptor, r, push.ParseOTLPRequest) +} + +// otelErrorHeaderInterceptor maps 500 errors to 503. +// According to the OTLP specification, 500 errors are never retried on the client side, but 503 are. +type otelErrorHeaderInterceptor struct { + http.ResponseWriter +} + +func newOtelErrorHeaderInterceptor(w http.ResponseWriter) *otelErrorHeaderInterceptor { + return &otelErrorHeaderInterceptor{ResponseWriter: w} +} + +func (i *otelErrorHeaderInterceptor) WriteHeader(statusCode int) { + if statusCode == http.StatusInternalServerError { + statusCode = http.StatusServiceUnavailable + } + + i.ResponseWriter.WriteHeader(statusCode) } func (d *Distributor) pushHandler(w http.ResponseWriter, r *http.Request, pushRequestParser push.RequestParser) { diff --git a/pkg/distributor/http_test.go b/pkg/distributor/http_test.go index 0ecf70fa9a498..b6281b81bf3d7 100644 --- a/pkg/distributor/http_test.go +++ b/pkg/distributor/http_test.go @@ -82,6 +82,38 @@ func TestRequestParserWrapping(t *testing.T) { require.True(t, called) } +func Test_OtelErrorHeaderInterceptor(t *testing.T) { + for _, tc := range []struct { + name string + inputCode int + expectedCode int + }{ + { + name: "500", + inputCode: http.StatusInternalServerError, + expectedCode: http.StatusServiceUnavailable, + }, + { + name: "400", + inputCode: http.StatusBadRequest, + expectedCode: http.StatusBadRequest, + }, + { + name: "204", + inputCode: http.StatusNoContent, + expectedCode: http.StatusNoContent, + }, + } { + t.Run(tc.name, func(t *testing.T) { + r := httptest.NewRecorder() + i := newOtelErrorHeaderInterceptor(r) + + http.Error(i, "error", tc.inputCode) + require.Equal(t, tc.expectedCode, r.Code) + }) + } +} + func stubParser(_ string, _ *http.Request, _ push.TenantsRetention, _ push.Limits, _ push.UsageTracker) (*logproto.PushRequest, *push.Stats, error) { return &logproto.PushRequest{}, &push.Stats{}, nil }