From ec3e36fe79341bdc9d819fc687b6d619af4c2360 Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Fri, 23 Aug 2024 10:42:58 -0500 Subject: [PATCH] One additional fix, a portion of PR feedback --- pkg/distributor/distributor_ingest_storage_test.go | 2 +- pkg/distributor/otel.go | 8 ++++---- pkg/distributor/push_test.go | 6 +++--- pkg/storegateway/limiter.go | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/distributor/distributor_ingest_storage_test.go b/pkg/distributor/distributor_ingest_storage_test.go index ae52808919a..285d572b418 100644 --- a/pkg/distributor/distributor_ingest_storage_test.go +++ b/pkg/distributor/distributor_ingest_storage_test.go @@ -98,7 +98,7 @@ func TestDistributor_Push_ShouldSupportIngestStorage(t *testing.T) { // Non-retryable error. 1: testkafka.CreateProduceResponseError(0, kafkaTopic, 1, kerr.InvalidTopicException), }, - expectedErr: fmt.Errorf("%s %d", failedPushingToPartitionMessage, 1), + expectedErr: fmt.Errorf("%s 1", failedPushingToPartitionMessage), expectedSeriesByPartition: map[int32][]string{ // Partition 1 is missing because it failed. 0: {"series_four", "series_one", "series_three"}, diff --git a/pkg/distributor/otel.go b/pkg/distributor/otel.go index bf101833572..725d68c2b59 100644 --- a/pkg/distributor/otel.go +++ b/pkg/distributor/otel.go @@ -90,7 +90,7 @@ func OTLPHandler( protoBodySize, err := util.ParseProtoReader(ctx, reader, int(r.ContentLength), maxRecvMsgSize, buffers, unmarshaler, compression) var tooLargeErr util.MsgSizeTooLargeErr if errors.As(err, &tooLargeErr) { - return exportReq, 0, httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "%s", distributorMaxOTLPRequestSizeErr{ + return exportReq, 0, httpgrpc.Error(http.StatusRequestEntityTooLarge, distributorMaxOTLPRequestSizeErr{ actual: tooLargeErr.Actual, limit: tooLargeErr.Limit, }.Error()) @@ -118,7 +118,7 @@ func OTLPHandler( reader = http.MaxBytesReader(nil, reader, int64(maxRecvMsgSize)) if _, err := buf.ReadFrom(reader); err != nil { if util.IsRequestBodyTooLarge(err) { - return exportReq, 0, httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "%s", distributorMaxOTLPRequestSizeErr{ + return exportReq, 0, httpgrpc.Errorf(http.StatusRequestEntityTooLarge, distributorMaxOTLPRequestSizeErr{ actual: -1, limit: maxRecvMsgSize, }.Error()) @@ -137,7 +137,7 @@ func OTLPHandler( // Check the request size against the message size limit, regardless of whether the request is compressed. // If the request is compressed and its compressed length already exceeds the size limit, there's no need to decompress it. if r.ContentLength > int64(maxRecvMsgSize) { - return httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "%s", distributorMaxOTLPRequestSizeErr{ + return httpgrpc.Errorf(http.StatusRequestEntityTooLarge, distributorMaxOTLPRequestSizeErr{ actual: int(r.ContentLength), limit: maxRecvMsgSize, }.Error()) @@ -229,7 +229,7 @@ func otlpHandler( if err := parser(ctx, r, maxRecvMsgSize, rb, &req, logger); err != nil { // Check for httpgrpc error, default to client error if parsing failed if _, ok := httpgrpc.HTTPResponseFromError(err); !ok { - err = httpgrpc.Errorf(http.StatusBadRequest, "%s", err.Error()) + err = httpgrpc.Error(http.StatusBadRequest, err.Error()) } rb.CleanUp() diff --git a/pkg/distributor/push_test.go b/pkg/distributor/push_test.go index 8df05466ecc..92831c9d41b 100644 --- a/pkg/distributor/push_test.go +++ b/pkg/distributor/push_test.go @@ -605,14 +605,14 @@ func TestHandler_ErrorTranslation(t *testing.T) { }, { name: "a gRPC error with a status during push gets translated into HTTP error without DoNotLogError header", - err: httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "%s", errMsg), + err: httpgrpc.Error(http.StatusRequestEntityTooLarge, errMsg), expectedHTTPStatus: http.StatusRequestEntityTooLarge, expectedErrorMessage: errMsg, expectedLogs: []string{`level=error user=testuser msg="detected an error while ingesting Prometheus remote-write request (the request may have been partially ingested)" httpCode=413 err="rpc error: code = Code(413) desc = this is an error" insight=true`}, }, { name: "a DoNotLogError of a gRPC error with a status during push gets translated into HTTP error without DoNotLogError header", - err: middleware.DoNotLogError{Err: httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "%s", errMsg)}, + err: middleware.DoNotLogError{Err: httpgrpc.Error(http.StatusRequestEntityTooLarge, errMsg)}, expectedHTTPStatus: http.StatusRequestEntityTooLarge, expectedErrorMessage: errMsg, expectedDoNotLogErrorHeader: true, @@ -627,7 +627,7 @@ func TestHandler_ErrorTranslation(t *testing.T) { }, { name: "StatusBadRequest is logged with insight=true", - err: httpgrpc.Errorf(http.StatusBadRequest, "limits reached"), + err: httpgrpc.Error(http.StatusBadRequest, "limits reached"), expectedHTTPStatus: http.StatusBadRequest, expectedErrorMessage: "limits reached", expectedLogs: []string{`level=error user=testuser msg="detected an error while ingesting Prometheus remote-write request (the request may have been partially ingested)" httpCode=400 err="rpc error: code = Code(400) desc = limits reached" insight=true`}, diff --git a/pkg/storegateway/limiter.go b/pkg/storegateway/limiter.go index 34268e6e8cc..51b15262203 100644 --- a/pkg/storegateway/limiter.go +++ b/pkg/storegateway/limiter.go @@ -67,7 +67,7 @@ func (l *Limiter) Reserve(num uint64) error { // We need to protect from the counter being incremented twice due to concurrency // while calling Reserve(). l.failedOnce.Do(l.failedCounter.Inc) - return httpgrpc.Errorf(http.StatusUnprocessableEntity, l.limitErrorMsg) + return httpgrpc.Error(http.StatusUnprocessableEntity, l.limitErrorMsg) } return nil }