Skip to content

Commit

Permalink
One additional fix, a portion of PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
alexweav committed Aug 23, 2024
1 parent a52826a commit ec3e36f
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 9 deletions.
2 changes: 1 addition & 1 deletion pkg/distributor/distributor_ingest_storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down
8 changes: 4 additions & 4 deletions pkg/distributor/otel.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand All @@ -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())
Expand Down Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions pkg/distributor/push_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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`},
Expand Down
2 changes: 1 addition & 1 deletion pkg/storegateway/limiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit ec3e36f

Please sign in to comment.