From 70aafeb9ee015c4959a6d49fe9c575954be6cf76 Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Wed, 21 Aug 2024 15:43:32 -0500 Subject: [PATCH 01/16] Upgrade to 1.23.0-bookworm --- mimir-build-image/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mimir-build-image/Dockerfile b/mimir-build-image/Dockerfile index ecfb3536c77..4ca2a434e3f 100644 --- a/mimir-build-image/Dockerfile +++ b/mimir-build-image/Dockerfile @@ -5,7 +5,7 @@ FROM registry.k8s.io/kustomize/kustomize:v5.4.3 as kustomize FROM alpine/helm:3.14.4 as helm -FROM golang:1.22.5-bookworm +FROM golang:1.23.0-bookworm ARG goproxyValue ENV GOPROXY=${goproxyValue} ENV SKOPEO_DEPS="libgpgme-dev libassuan-dev libbtrfs-dev libdevmapper-dev pkg-config" From dd4eb4e0e5beadd33c6f2e2a9918e183f6a21ba9 Mon Sep 17 00:00:00 2001 From: alexweav Date: Wed, 21 Aug 2024 21:20:08 +0000 Subject: [PATCH 02/16] Update build image version to pr9070-2850516d73 --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 9e9d7273d56..1b686223e02 100644 --- a/Makefile +++ b/Makefile @@ -275,7 +275,7 @@ mimir-build-image/$(UPTODATE): mimir-build-image/* # All the boiler plate for building golang follows: SUDO := $(shell docker info >/dev/null 2>&1 || echo "sudo -E") BUILD_IN_CONTAINER ?= true -LATEST_BUILD_IMAGE_TAG ?= pr9082-d0a22a8a96 +LATEST_BUILD_IMAGE_TAG ?= pr9070-2850516d73 # TTY is parameterized to allow Google Cloud Builder to run builds, # as it currently disallows TTY devices. This value needs to be overridden From a9bf892e3ff34b37b62ceb01527769506f64541d Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Wed, 21 Aug 2024 17:52:42 -0500 Subject: [PATCH 03/16] empty commit to kick drone From 4e2d41cfd89b19b017af8ed216bea3c319dc988f Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Thu, 22 Aug 2024 15:29:19 -0500 Subject: [PATCH 04/16] Upgrade golangci-lint --- mimir-build-image/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mimir-build-image/Dockerfile b/mimir-build-image/Dockerfile index 4ca2a434e3f..433944976bf 100644 --- a/mimir-build-image/Dockerfile +++ b/mimir-build-image/Dockerfile @@ -39,7 +39,7 @@ RUN GOARCH=$(go env GOARCH) && \ curl -fSL -o "/usr/bin/tk" "https://github.com/grafana/tanka/releases/download/v${TANKA_VERSION}/tk-linux-${GOARCH}" && \ chmod a+x /usr/bin/tk -RUN curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b /usr/bin v1.59.1 +RUN curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b /usr/bin v1.60.2 ENV SKOPEO_VERSION=v1.15.1 RUN git clone --depth 1 --branch ${SKOPEO_VERSION} https://github.com/containers/skopeo /go/src/github.com/containers/skopeo && \ From d1556f924dbaba0e5a9244c9b333db82d2d86748 Mon Sep 17 00:00:00 2001 From: alexweav Date: Thu, 22 Aug 2024 21:06:19 +0000 Subject: [PATCH 05/16] Update build image version to pr9070-3b4c4993ed --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 1b686223e02..ab5e813f765 100644 --- a/Makefile +++ b/Makefile @@ -275,7 +275,7 @@ mimir-build-image/$(UPTODATE): mimir-build-image/* # All the boiler plate for building golang follows: SUDO := $(shell docker info >/dev/null 2>&1 || echo "sudo -E") BUILD_IN_CONTAINER ?= true -LATEST_BUILD_IMAGE_TAG ?= pr9070-2850516d73 +LATEST_BUILD_IMAGE_TAG ?= pr9070-3b4c4993ed # TTY is parameterized to allow Google Cloud Builder to run builds, # as it currently disallows TTY devices. This value needs to be overridden From 73ddc113bfa4055a31c9f9d83aa1cad0fc459799 Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Thu, 22 Aug 2024 16:54:22 -0500 Subject: [PATCH 06/16] empty commit, drone disconnected again From cc76be08f00056d0284dbfc2164890a0a8d7b925 Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Fri, 23 Aug 2024 10:11:07 -0500 Subject: [PATCH 07/16] Address various linter complaints following upgrade --- pkg/distributor/distributor_ingest_storage_test.go | 2 +- pkg/distributor/otel.go | 12 ++++++------ pkg/frontend/querymiddleware/codec_test.go | 2 +- pkg/scheduler/scheduler.go | 2 +- pkg/util/objtools/bucket.go | 4 ++-- tools/tsdb-index-toc/main.go | 6 +++--- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/distributor/distributor_ingest_storage_test.go b/pkg/distributor/distributor_ingest_storage_test.go index 8fa01dc6a7c..ae52808919a 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(fmt.Sprintf("%s %d", failedPushingToPartitionMessage, 1)), + expectedErr: fmt.Errorf("%s %d", failedPushingToPartitionMessage, 1), 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 536cbe0c9fd..62742df6249 100644 --- a/pkg/distributor/otel.go +++ b/pkg/distributor/otel.go @@ -90,10 +90,10 @@ 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, distributorMaxOTLPRequestSizeErr{ + return exportReq, 0, httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "%w", distributorMaxOTLPRequestSizeErr{ actual: tooLargeErr.Actual, limit: tooLargeErr.Limit, - }.Error()) + }) } return exportReq, protoBodySize, err } @@ -118,10 +118,10 @@ 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, distributorMaxOTLPRequestSizeErr{ + return exportReq, 0, httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "%w", distributorMaxOTLPRequestSizeErr{ actual: -1, limit: maxRecvMsgSize, - }.Error()) + }) } return exportReq, 0, errors.Wrap(err, "read write request") @@ -137,10 +137,10 @@ 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, distributorMaxOTLPRequestSizeErr{ + return httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "%w", distributorMaxOTLPRequestSizeErr{ actual: int(r.ContentLength), limit: maxRecvMsgSize, - }.Error()) + }) } spanLogger, ctx := spanlogger.NewWithLogger(ctx, logger, "Distributor.OTLPHandler.decodeAndConvert") diff --git a/pkg/frontend/querymiddleware/codec_test.go b/pkg/frontend/querymiddleware/codec_test.go index d8dea2184fc..2665016107e 100644 --- a/pkg/frontend/querymiddleware/codec_test.go +++ b/pkg/frontend/querymiddleware/codec_test.go @@ -703,7 +703,7 @@ func TestPrometheusCodec_EncodeMetricsQueryRequest_AcceptHeader(t *testing.T) { case formatProtobuf: require.Equal(t, "application/vnd.mimir.queryresponse+protobuf,application/json", encodedRequest.Header.Get("Accept")) default: - t.Fatalf(fmt.Sprintf("unknown query result payload format: %v", queryResultPayloadFormat)) + t.Fatalf("unknown query result payload format: %v", queryResultPayloadFormat) } }) } diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index fc08abb82b7..68ab65632b2 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -405,7 +405,7 @@ func (s *Scheduler) cancelRequestAndRemoveFromPending(key queue.RequestKey, reas req := s.schedulerInflightRequests[key] if req != nil { - req.CancelFunc(cancellation.NewErrorf(reason)) + req.CancelFunc(cancellation.NewErrorf("%s", reason)) } delete(s.schedulerInflightRequests, key) diff --git a/pkg/util/objtools/bucket.go b/pkg/util/objtools/bucket.go index 03181a43c03..134bdb4ebef 100644 --- a/pkg/util/objtools/bucket.go +++ b/pkg/util/objtools/bucket.go @@ -138,7 +138,7 @@ func (c *BucketConfig) Validate() error { func (c *BucketConfig) validate(descriptor string) error { descriptorFlagPrefix := ifNotEmptySuffix(descriptor, ".") if c.backend == "" { - return fmt.Errorf("--" + descriptorFlagPrefix + "backend is missing") + return fmt.Errorf("--%sbackend is missing", descriptorFlagPrefix) } switch c.backend { case bucket.Azure: @@ -148,7 +148,7 @@ func (c *BucketConfig) validate(descriptor string) error { case bucket.S3: return c.s3.Validate(bucket.S3 + "." + descriptorFlagPrefix) default: - return fmt.Errorf("unknown backend provided in --" + descriptorFlagPrefix + "backend") + return fmt.Errorf("unknown backend provided in --%sbackend", descriptorFlagPrefix) } } diff --git a/tools/tsdb-index-toc/main.go b/tools/tsdb-index-toc/main.go index 322f5c44baf..d5c24b5e785 100644 --- a/tools/tsdb-index-toc/main.go +++ b/tools/tsdb-index-toc/main.go @@ -33,18 +33,18 @@ func main() { finfo, err := os.Stat(filepath) if err != nil { - log.Fatalf(err.Error()) + log.Fatalf("%w", err) } indexSize := finfo.Size() f, err := fileutil.OpenMmapFile(filepath) if err != nil { - log.Fatalf(err.Error()) + log.Fatalf("%w", err) } toc, err := index.NewTOCFromByteSlice(realByteSlice(f.Bytes())) if err != nil { - log.Fatalf(err.Error()) + log.Fatalf("%w", err) } // See https://github.com/prometheus/prometheus/blob/main/tsdb/docs/format/index.md on the index format. From 16a34259025c7ff406574f3c432e4bf681da8465 Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Fri, 23 Aug 2024 10:23:34 -0500 Subject: [PATCH 08/16] More lint errors --- pkg/distributor/otel.go | 12 ++++++------ pkg/distributor/push.go | 2 +- pkg/distributor/push_test.go | 12 ++++++------ tools/tsdb-index-toc/main.go | 6 +++--- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/pkg/distributor/otel.go b/pkg/distributor/otel.go index 62742df6249..7f8d859f66f 100644 --- a/pkg/distributor/otel.go +++ b/pkg/distributor/otel.go @@ -90,10 +90,10 @@ 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, "%w", distributorMaxOTLPRequestSizeErr{ + return exportReq, 0, httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "%s", distributorMaxOTLPRequestSizeErr{ actual: tooLargeErr.Actual, limit: tooLargeErr.Limit, - }) + }.Error()) } return exportReq, protoBodySize, err } @@ -118,10 +118,10 @@ 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, "%w", distributorMaxOTLPRequestSizeErr{ + return exportReq, 0, httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "%s", distributorMaxOTLPRequestSizeErr{ actual: -1, limit: maxRecvMsgSize, - }) + }.Error()) } return exportReq, 0, errors.Wrap(err, "read write request") @@ -137,10 +137,10 @@ 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, "%w", distributorMaxOTLPRequestSizeErr{ + return httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "%s", distributorMaxOTLPRequestSizeErr{ actual: int(r.ContentLength), limit: maxRecvMsgSize, - }) + }.Error()) } spanLogger, ctx := spanlogger.NewWithLogger(ctx, logger, "Distributor.OTLPHandler.decodeAndConvert") diff --git a/pkg/distributor/push.go b/pkg/distributor/push.go index b42f50c4975..7d48966bf22 100644 --- a/pkg/distributor/push.go +++ b/pkg/distributor/push.go @@ -164,7 +164,7 @@ func handler( 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, err.Error()) + err = httpgrpc.Errorf(http.StatusBadRequest, "%s", err.Error()) } rb.CleanUp() diff --git a/pkg/distributor/push_test.go b/pkg/distributor/push_test.go index 9dff6da8729..8df05466ecc 100644 --- a/pkg/distributor/push_test.go +++ b/pkg/distributor/push_test.go @@ -534,14 +534,14 @@ func TestHandler_ErrorTranslation(t *testing.T) { }{ { name: "a generic error during request parsing gets an HTTP 400", - err: fmt.Errorf(errMsg), + err: fmt.Errorf("%s", errMsg), expectedHTTPStatus: http.StatusBadRequest, 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=400 err="rpc error: code = Code(400) desc = this is an error" insight=true`}, }, { name: "a gRPC error with a status during request parsing gets translated into HTTP error without DoNotLogError header", - err: httpgrpc.Errorf(http.StatusRequestEntityTooLarge, errMsg), + err: httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "%s", 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`}, @@ -590,14 +590,14 @@ func TestHandler_ErrorTranslation(t *testing.T) { }, { name: "a generic error during push gets a HTTP 500 without DoNotLogError header", - err: fmt.Errorf(errMsg), + err: fmt.Errorf("%s", errMsg), expectedHTTPStatus: http.StatusInternalServerError, 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=500 err="this is an error"`}, }, { name: "a DoNotLogError of a generic error during push gets a HTTP 500 with DoNotLogError header", - err: middleware.DoNotLogError{Err: fmt.Errorf(errMsg)}, + err: middleware.DoNotLogError{Err: fmt.Errorf("%s", errMsg)}, expectedHTTPStatus: http.StatusInternalServerError, expectedErrorMessage: errMsg, expectedDoNotLogErrorHeader: true, @@ -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, errMsg), + err: httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "%s", 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, errMsg)}, + err: middleware.DoNotLogError{Err: httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "%s", errMsg)}, expectedHTTPStatus: http.StatusRequestEntityTooLarge, expectedErrorMessage: errMsg, expectedDoNotLogErrorHeader: true, diff --git a/tools/tsdb-index-toc/main.go b/tools/tsdb-index-toc/main.go index d5c24b5e785..4a9c1edbf16 100644 --- a/tools/tsdb-index-toc/main.go +++ b/tools/tsdb-index-toc/main.go @@ -33,18 +33,18 @@ func main() { finfo, err := os.Stat(filepath) if err != nil { - log.Fatalf("%w", err) + log.Fatalf("%s", err.Error()) } indexSize := finfo.Size() f, err := fileutil.OpenMmapFile(filepath) if err != nil { - log.Fatalf("%w", err) + log.Fatalf("%s", err.Error()) } toc, err := index.NewTOCFromByteSlice(realByteSlice(f.Bytes())) if err != nil { - log.Fatalf("%w", err) + log.Fatalf("%s", err.Error()) } // See https://github.com/prometheus/prometheus/blob/main/tsdb/docs/format/index.md on the index format. From a52826a4721dbb4b2969f8095ac10ab7ba19b2e2 Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Fri, 23 Aug 2024 10:34:04 -0500 Subject: [PATCH 09/16] reports lint errors in batches apparently, more --- pkg/distributor/otel.go | 2 +- .../shard_active_native_histogram_metrics_test.go | 2 +- pkg/frontend/querymiddleware/shard_active_series_test.go | 2 +- pkg/frontend/transport/handler.go | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/distributor/otel.go b/pkg/distributor/otel.go index 7f8d859f66f..bf101833572 100644 --- a/pkg/distributor/otel.go +++ b/pkg/distributor/otel.go @@ -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, err.Error()) + err = httpgrpc.Errorf(http.StatusBadRequest, "%s", err.Error()) } rb.CleanUp() diff --git a/pkg/frontend/querymiddleware/shard_active_native_histogram_metrics_test.go b/pkg/frontend/querymiddleware/shard_active_native_histogram_metrics_test.go index a4e37346263..12f6e1ddf97 100644 --- a/pkg/frontend/querymiddleware/shard_active_native_histogram_metrics_test.go +++ b/pkg/frontend/querymiddleware/shard_active_native_histogram_metrics_test.go @@ -721,7 +721,7 @@ func TestShardActiveNativeHistogramMetricsMiddlewareMergeResponseContextCancella }() cancelCause := "request canceled while streaming response" - cancel(fmt.Errorf(cancelCause)) + cancel(fmt.Errorf("%s", cancelCause)) g.Wait() diff --git a/pkg/frontend/querymiddleware/shard_active_series_test.go b/pkg/frontend/querymiddleware/shard_active_series_test.go index f94f6aa813c..cb226e1d2d3 100644 --- a/pkg/frontend/querymiddleware/shard_active_series_test.go +++ b/pkg/frontend/querymiddleware/shard_active_series_test.go @@ -486,7 +486,7 @@ func runTestShardActiveSeriesMiddlewareMergeResponseContextCancellation(t *testi require.NoError(t, err) cancelCause := "request canceled while streaming response" - cancel(fmt.Errorf(cancelCause)) + cancel(fmt.Errorf("%s", cancelCause)) _, err = io.Copy(&buf, resp.Body) require.NoError(t, err) diff --git a/pkg/frontend/transport/handler.go b/pkg/frontend/transport/handler.go index c30efc1fa32..4c86ae32691 100644 --- a/pkg/frontend/transport/handler.go +++ b/pkg/frontend/transport/handler.go @@ -46,8 +46,8 @@ const ( ) var ( - errCanceled = httpgrpc.Errorf(StatusClientClosedRequest, context.Canceled.Error()) - errDeadlineExceeded = httpgrpc.Errorf(http.StatusGatewayTimeout, context.DeadlineExceeded.Error()) + errCanceled = httpgrpc.Errorf(StatusClientClosedRequest, "%s", context.Canceled.Error()) + errDeadlineExceeded = httpgrpc.Errorf(http.StatusGatewayTimeout, "%s", context.DeadlineExceeded.Error()) errRequestEntityTooLarge = httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "http: request body too large") ) From ec3e36fe79341bdc9d819fc687b6d619af4c2360 Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Fri, 23 Aug 2024 10:42:58 -0500 Subject: [PATCH 10/16] 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 } From 316d710d99c15eacfea493ee170d73a9e1241a0f Mon Sep 17 00:00:00 2001 From: Alexander Weaver Date: Fri, 23 Aug 2024 10:44:44 -0500 Subject: [PATCH 11/16] Apply suggestions from code review Co-authored-by: Arve Knudsen --- pkg/distributor/push.go | 2 +- pkg/distributor/push_test.go | 8 ++++---- pkg/scheduler/scheduler.go | 2 +- tools/tsdb-index-toc/main.go | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/distributor/push.go b/pkg/distributor/push.go index 7d48966bf22..4816954c0af 100644 --- a/pkg/distributor/push.go +++ b/pkg/distributor/push.go @@ -164,7 +164,7 @@ func handler( 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 92831c9d41b..2cba6f9db61 100644 --- a/pkg/distributor/push_test.go +++ b/pkg/distributor/push_test.go @@ -534,14 +534,14 @@ func TestHandler_ErrorTranslation(t *testing.T) { }{ { name: "a generic error during request parsing gets an HTTP 400", - err: fmt.Errorf("%s", errMsg), + err: errors.New(errMsg), expectedHTTPStatus: http.StatusBadRequest, 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=400 err="rpc error: code = Code(400) desc = this is an error" insight=true`}, }, { name: "a gRPC error with a status during request parsing 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`}, @@ -590,14 +590,14 @@ func TestHandler_ErrorTranslation(t *testing.T) { }, { name: "a generic error during push gets a HTTP 500 without DoNotLogError header", - err: fmt.Errorf("%s", errMsg), + err: errors.New(errMsg), expectedHTTPStatus: http.StatusInternalServerError, 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=500 err="this is an error"`}, }, { name: "a DoNotLogError of a generic error during push gets a HTTP 500 with DoNotLogError header", - err: middleware.DoNotLogError{Err: fmt.Errorf("%s", errMsg)}, + err: middleware.DoNotLogError{Err: errors.New(errMsg)}, expectedHTTPStatus: http.StatusInternalServerError, expectedErrorMessage: errMsg, expectedDoNotLogErrorHeader: true, diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index 68ab65632b2..5fd6f2d05c1 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -405,7 +405,7 @@ func (s *Scheduler) cancelRequestAndRemoveFromPending(key queue.RequestKey, reas req := s.schedulerInflightRequests[key] if req != nil { - req.CancelFunc(cancellation.NewErrorf("%s", reason)) + req.CancelFunc(cancellation.NewError(reason)) } delete(s.schedulerInflightRequests, key) diff --git a/tools/tsdb-index-toc/main.go b/tools/tsdb-index-toc/main.go index 4a9c1edbf16..a226d68fec8 100644 --- a/tools/tsdb-index-toc/main.go +++ b/tools/tsdb-index-toc/main.go @@ -33,18 +33,18 @@ func main() { finfo, err := os.Stat(filepath) if err != nil { - log.Fatalf("%s", err.Error()) + log.Fatal(err.Error()) } indexSize := finfo.Size() f, err := fileutil.OpenMmapFile(filepath) if err != nil { - log.Fatalf("%s", err.Error()) + log.Fatal(err.Error()) } toc, err := index.NewTOCFromByteSlice(realByteSlice(f.Bytes())) if err != nil { - log.Fatalf("%s", err.Error()) + log.Fatal(err.Error()) } // See https://github.com/prometheus/prometheus/blob/main/tsdb/docs/format/index.md on the index format. From 7438520c239c8df3a8a07d2ed49482055fb5b378 Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Fri, 23 Aug 2024 10:47:15 -0500 Subject: [PATCH 12/16] Manually apply a few more following logic from comments --- .../shard_active_native_histogram_metrics_test.go | 2 +- pkg/frontend/querymiddleware/shard_active_series_test.go | 2 +- pkg/frontend/transport/handler.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/frontend/querymiddleware/shard_active_native_histogram_metrics_test.go b/pkg/frontend/querymiddleware/shard_active_native_histogram_metrics_test.go index 12f6e1ddf97..7d9fe37f021 100644 --- a/pkg/frontend/querymiddleware/shard_active_native_histogram_metrics_test.go +++ b/pkg/frontend/querymiddleware/shard_active_native_histogram_metrics_test.go @@ -721,7 +721,7 @@ func TestShardActiveNativeHistogramMetricsMiddlewareMergeResponseContextCancella }() cancelCause := "request canceled while streaming response" - cancel(fmt.Errorf("%s", cancelCause)) + cancel(errors.New(cancelCause)) g.Wait() diff --git a/pkg/frontend/querymiddleware/shard_active_series_test.go b/pkg/frontend/querymiddleware/shard_active_series_test.go index cb226e1d2d3..33525d696aa 100644 --- a/pkg/frontend/querymiddleware/shard_active_series_test.go +++ b/pkg/frontend/querymiddleware/shard_active_series_test.go @@ -486,7 +486,7 @@ func runTestShardActiveSeriesMiddlewareMergeResponseContextCancellation(t *testi require.NoError(t, err) cancelCause := "request canceled while streaming response" - cancel(fmt.Errorf("%s", cancelCause)) + cancel(errors.New(cancelCause)) _, err = io.Copy(&buf, resp.Body) require.NoError(t, err) diff --git a/pkg/frontend/transport/handler.go b/pkg/frontend/transport/handler.go index 4c86ae32691..1e438bb50a2 100644 --- a/pkg/frontend/transport/handler.go +++ b/pkg/frontend/transport/handler.go @@ -46,8 +46,8 @@ const ( ) var ( - errCanceled = httpgrpc.Errorf(StatusClientClosedRequest, "%s", context.Canceled.Error()) - errDeadlineExceeded = httpgrpc.Errorf(http.StatusGatewayTimeout, "%s", context.DeadlineExceeded.Error()) + errCanceled = httpgrpc.Error(StatusClientClosedRequest, context.Canceled.Error()) + errDeadlineExceeded = httpgrpc.Error(http.StatusGatewayTimeout, context.DeadlineExceeded.Error()) errRequestEntityTooLarge = httpgrpc.Errorf(http.StatusRequestEntityTooLarge, "http: request body too large") ) From 35c113b98882af1e1318250a0c47e5672da9a1d5 Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Fri, 23 Aug 2024 10:50:53 -0500 Subject: [PATCH 13/16] Reverse cancellation.NewError one --- pkg/scheduler/scheduler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index 5fd6f2d05c1..68ab65632b2 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -405,7 +405,7 @@ func (s *Scheduler) cancelRequestAndRemoveFromPending(key queue.RequestKey, reas req := s.schedulerInflightRequests[key] if req != nil { - req.CancelFunc(cancellation.NewError(reason)) + req.CancelFunc(cancellation.NewErrorf("%s", reason)) } delete(s.schedulerInflightRequests, key) From 936611f0ac901095b5ef7ca486a023981520740e Mon Sep 17 00:00:00 2001 From: Alexander Weaver Date: Fri, 23 Aug 2024 10:59:38 -0500 Subject: [PATCH 14/16] Update pkg/scheduler/scheduler.go Co-authored-by: Arve Knudsen --- pkg/scheduler/scheduler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index 68ab65632b2..c9773098fa2 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -405,7 +405,7 @@ func (s *Scheduler) cancelRequestAndRemoveFromPending(key queue.RequestKey, reas req := s.schedulerInflightRequests[key] if req != nil { - req.CancelFunc(cancellation.NewErrorf("%s", reason)) + req.CancelFunc(cancellation.NewError(errors.New(reason))) } delete(s.schedulerInflightRequests, key) From 38d46d928e3d7ffb06b5656b762ede914dd56a89 Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Fri, 23 Aug 2024 11:09:51 -0500 Subject: [PATCH 15/16] last round, hopefully --- pkg/distributor/otel.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/distributor/otel.go b/pkg/distributor/otel.go index 725d68c2b59..44d8e0957a7 100644 --- a/pkg/distributor/otel.go +++ b/pkg/distributor/otel.go @@ -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, distributorMaxOTLPRequestSizeErr{ + return exportReq, 0, httpgrpc.Error(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, distributorMaxOTLPRequestSizeErr{ + return httpgrpc.Error(http.StatusRequestEntityTooLarge, distributorMaxOTLPRequestSizeErr{ actual: int(r.ContentLength), limit: maxRecvMsgSize, }.Error()) From 4bed73efc3b0bc547a8f378bc2e751e9fd5805e4 Mon Sep 17 00:00:00 2001 From: alexweav Date: Fri, 23 Aug 2024 16:47:56 +0000 Subject: [PATCH 16/16] Update build image version to pr9070-5347ec889b Signed-off-by: Arve Knudsen --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ab5e813f765..03b6b8a467e 100644 --- a/Makefile +++ b/Makefile @@ -275,7 +275,7 @@ mimir-build-image/$(UPTODATE): mimir-build-image/* # All the boiler plate for building golang follows: SUDO := $(shell docker info >/dev/null 2>&1 || echo "sudo -E") BUILD_IN_CONTAINER ?= true -LATEST_BUILD_IMAGE_TAG ?= pr9070-3b4c4993ed +LATEST_BUILD_IMAGE_TAG ?= pr9070-5347ec889b # TTY is parameterized to allow Google Cloud Builder to run builds, # as it currently disallows TTY devices. This value needs to be overridden