Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
* [BUGFIX] Ingester, Block-builder: silently ignore duplicate sample if it's due to zero sample from created timestamp. Created timestamp equal to the timestamp of the first sample of series is a common case if created timestamp comes from OTLP where start time equal to timestamp of the first sample simply means unknown start time. #12726
* [BUGFIX] Distributor: Fix error when native histograms bucket limit is set then no NHCB passes validation. #12741
* [BUGFIX] Ingester: Fix continous reload of active series counters when cost-attribution labels are above the max cardinality. #12822
* [BUGFIX] Distributor: Report correct size in `err-mimir-distributor-max-write-message-size` error. #12799
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* [BUGFIX] Distributor: Report correct size in `err-mimir-distributor-max-write-message-size` error. #12799
* [BUGFIX] Distributor: Report the correct size in the `err-mimir-distributor-max-write-message-size` error. #12799


### Mixin

Expand Down
5 changes: 3 additions & 2 deletions pkg/distributor/otel.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,9 @@ func newOTLPParser(
var tooLargeErr util.MsgSizeTooLargeErr
if errors.As(err, &tooLargeErr) {
return exportReq, 0, httpgrpc.Error(http.StatusRequestEntityTooLarge, distributorMaxOTLPRequestSizeErr{
actual: tooLargeErr.Actual,
limit: tooLargeErr.Limit,
compressed: tooLargeErr.Compressed,
actual: tooLargeErr.Actual,
limit: tooLargeErr.Limit,
}.Error())
}
return exportReq, protoBodySize, err
Expand Down
18 changes: 9 additions & 9 deletions pkg/distributor/otel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1164,9 +1164,9 @@ func TestHandlerOTLPPush(t *testing.T) {
},
responseCode: http.StatusRequestEntityTooLarge,
responseContentType: pbContentType,
responseContentLength: 292,
errMessage: "the incoming OTLP request has been rejected because its message size of 89 bytes is larger",
expectedLogs: []string{`level=warn user=test msg="detected an error while ingesting OTLP metrics request (the request may have been partially ingested)" httpCode=413 err="rpc error: code = Code(413) desc = the incoming OTLP request has been rejected because its message size of 89 bytes is larger than the allowed limit of 30 bytes (err-mimir-distributor-max-otlp-request-size). To adjust the related limit, configure -distributor.max-otlp-request-size, or contact your service administrator." insight=true`},
responseContentLength: 307,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that this isn't new but this seems very fragile. Where does this number come from? Can we verify it some other way?

errMessage: "the incoming OTLP request has been rejected because its message size of 89 bytes (uncompressed) is larger",
expectedLogs: []string{`level=warn user=test msg="detected an error while ingesting OTLP metrics request (the request may have been partially ingested)" httpCode=413 err="rpc error: code = Code(413) desc = the incoming OTLP request has been rejected because its message size of 89 bytes (uncompressed) is larger than the allowed limit of 30 bytes (err-mimir-distributor-max-otlp-request-size). To adjust the related limit, configure -distributor.max-otlp-request-size, or contact your service administrator." insight=true`},
},
{
name: "Write samples. Unsupported compression",
Expand Down Expand Up @@ -1213,9 +1213,9 @@ func TestHandlerOTLPPush(t *testing.T) {
},
responseCode: http.StatusRequestEntityTooLarge,
responseContentType: pbContentType,
responseContentLength: 293,
errMessage: "the incoming OTLP request has been rejected because its message size of 104 bytes is larger",
expectedLogs: []string{`level=warn user=test msg="detected an error while ingesting OTLP metrics request (the request may have been partially ingested)" httpCode=413 err="rpc error: code = Code(413) desc = the incoming OTLP request has been rejected because its message size of 104 bytes is larger than the allowed limit of 30 bytes (err-mimir-distributor-max-otlp-request-size). To adjust the related limit, configure -distributor.max-otlp-request-size, or contact your service administrator." insight=true`},
responseContentLength: 308,
errMessage: "the incoming OTLP request has been rejected because its message size of 104 bytes (uncompressed) is larger",
expectedLogs: []string{`level=warn user=test msg="detected an error while ingesting OTLP metrics request (the request may have been partially ingested)" httpCode=413 err="rpc error: code = Code(413) desc = the incoming OTLP request has been rejected because its message size of 104 bytes (uncompressed) is larger than the allowed limit of 30 bytes (err-mimir-distributor-max-otlp-request-size). To adjust the related limit, configure -distributor.max-otlp-request-size, or contact your service administrator." insight=true`},
},
{
name: "Write samples. With lz4 compression, request too big",
Expand All @@ -1229,9 +1229,9 @@ func TestHandlerOTLPPush(t *testing.T) {
},
responseCode: http.StatusRequestEntityTooLarge,
responseContentType: pbContentType,
responseContentLength: 293,
errMessage: "the incoming OTLP request has been rejected because its message size of 106 bytes is larger",
expectedLogs: []string{`level=warn user=test msg="detected an error while ingesting OTLP metrics request (the request may have been partially ingested)" httpCode=413 err="rpc error: code = Code(413) desc = the incoming OTLP request has been rejected because its message size of 106 bytes is larger than the allowed limit of 30 bytes (err-mimir-distributor-max-otlp-request-size). To adjust the related limit, configure -distributor.max-otlp-request-size, or contact your service administrator." insight=true`},
responseContentLength: 308,
errMessage: "the incoming OTLP request has been rejected because its message size of 106 bytes (uncompressed) is larger",
expectedLogs: []string{`level=warn user=test msg="detected an error while ingesting OTLP metrics request (the request may have been partially ingested)" httpCode=413 err="rpc error: code = Code(413) desc = the incoming OTLP request has been rejected because its message size of 106 bytes (uncompressed) is larger than the allowed limit of 30 bytes (err-mimir-distributor-max-otlp-request-size). To adjust the related limit, configure -distributor.max-otlp-request-size, or contact your service administrator." insight=true`},
},
{
name: "Rate limited request",
Expand Down
24 changes: 14 additions & 10 deletions pkg/distributor/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ func Handler(
) http.Handler {
return handler(maxRecvMsgSize, newRequestBuffers, sourceIPs, allowSkipLabelNameValidation, allowSkipLabelCountValidation, limits, retryCfg, push, logger, func(ctx context.Context, r *http.Request, maxRecvMsgSize int, buffers *util.RequestBuffers, req *mimirpb.PreallocWriteRequest, _ log.Logger) error {
protoBodySize, err := util.ParseProtoReader(ctx, r.Body, int(r.ContentLength), maxRecvMsgSize, buffers, req, util.RawSnappy)
if errors.Is(err, util.MsgSizeTooLargeErr{}) {
err = distributorMaxWriteMessageSizeErr{actual: int(r.ContentLength), limit: maxRecvMsgSize}
if e := (util.MsgSizeTooLargeErr{}); errors.As(err, &e) {
err = distributorMaxWriteMessageSizeErr{compressed: e.Compressed, actual: e.Actual, limit: e.Limit}
}
if err != nil {
return err
Expand All @@ -115,25 +115,29 @@ func Handler(
}

type distributorMaxWriteMessageSizeErr struct {
actual, limit int
compressed, actual, limit int
}

func (e distributorMaxWriteMessageSizeErr) Error() string {
msgSizeDesc := fmt.Sprintf(" of %d bytes", e.actual)
if e.actual < 0 {
msgSizeDesc = ""
msgSizeDesc := ""
if e.actual > 0 {
msgSizeDesc = fmt.Sprintf(" of %d bytes (uncompressed)", e.actual)
} else if e.compressed > 0 {
msgSizeDesc = fmt.Sprintf(" of %d bytes (compressed)", e.compressed)
}
return globalerror.DistributorMaxWriteMessageSize.MessageWithPerInstanceLimitConfig(fmt.Sprintf("the incoming push request has been rejected because its message size%s is larger than the allowed limit of %d bytes", msgSizeDesc, e.limit), "distributor.max-recv-msg-size")
}

type distributorMaxOTLPRequestSizeErr struct {
actual, limit int
compressed, actual, limit int
}

func (e distributorMaxOTLPRequestSizeErr) Error() string {
msgSizeDesc := fmt.Sprintf(" of %d bytes", e.actual)
if e.actual < 0 {
msgSizeDesc = ""
msgSizeDesc := ""
if e.actual > 0 {
msgSizeDesc = fmt.Sprintf(" of %d bytes (uncompressed)", e.actual)
} else if e.compressed > 0 {
msgSizeDesc = fmt.Sprintf(" of %d bytes (compressed)", e.compressed)
}
return globalerror.DistributorMaxOTLPRequestSize.MessageWithPerInstanceLimitConfig(fmt.Sprintf("the incoming OTLP request has been rejected because its message size%s is larger than the allowed limit of %d bytes", msgSizeDesc, e.limit), maxOTLPRequestSizeFlag)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to not have either the compressed or actual size?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because we only decompress requests until reaching the limit, so we don't exactly know by how much a compressed request is over the limit after decompression.

I added a specific constructor for this case at 8250a2f.

}
Expand Down
2 changes: 1 addition & 1 deletion pkg/distributor/push_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ func (n bufCloser) BytesBuffer() *bytes.Buffer { return n.Buffer }

func TestNewDistributorMaxWriteMessageSizeErr(t *testing.T) {
err := distributorMaxWriteMessageSizeErr{actual: 100, limit: 50}
msg := `the incoming push request has been rejected because its message size of 100 bytes is larger than the allowed limit of 50 bytes (err-mimir-distributor-max-write-message-size). To adjust the related limit, configure -distributor.max-recv-msg-size, or contact your service administrator.`
msg := `the incoming push request has been rejected because its message size of 100 bytes (uncompressed) is larger than the allowed limit of 50 bytes (err-mimir-distributor-max-write-message-size). To adjust the related limit, configure -distributor.max-recv-msg-size, or contact your service administrator.`

assert.Equal(t, msg, err.Error())
}
Expand Down
29 changes: 25 additions & 4 deletions pkg/util/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,29 @@ func ParseProtoReader(ctx context.Context, reader io.Reader, expectedSize, maxSi
}

type MsgSizeTooLargeErr struct {
Actual, Limit int
Compressed, Actual, Limit int
}

func NewMsgCompressedSizeTooLargeErr(size, limit int) MsgSizeTooLargeErr {
return MsgSizeTooLargeErr{Compressed: size, Limit: limit}
}

func NewMsgUncompressedSizeTooLargeErr(size, limit int) MsgSizeTooLargeErr {
return MsgSizeTooLargeErr{Actual: size, Limit: limit}
}

func NewMsgUnknownSizeTooLargeErr(limit int) MsgSizeTooLargeErr {
return MsgSizeTooLargeErr{Limit: limit}
}

func (e MsgSizeTooLargeErr) Error() string {
return fmt.Sprintf("the request has been rejected because its size of %d bytes exceeds the limit of %d bytes", e.Actual, e.Limit)
msgSizeDesc := ""
if e.Actual > 0 {
msgSizeDesc = fmt.Sprintf(" of %d bytes (uncompressed)", e.Actual)
} else if e.Compressed > 0 {
msgSizeDesc = fmt.Sprintf(" of %d bytes (compressed)", e.Compressed)
}
return fmt.Sprintf("the request has been rejected because its size%s exceeds the limit of %d bytes", msgSizeDesc, e.Limit)
}

// Needed for errors.Is to work properly.
Expand All @@ -195,7 +213,10 @@ func (e MsgSizeTooLargeErr) Is(err error) bool {

func decompressRequest(buffers *RequestBuffers, reader io.Reader, expectedSize, maxSize int, compression CompressionType, sp trace.Span) ([]byte, error) {
if expectedSize > maxSize {
return nil, MsgSizeTooLargeErr{Actual: expectedSize, Limit: maxSize}
if compression == NoCompression {
return nil, NewMsgUncompressedSizeTooLargeErr(expectedSize, maxSize)
}
return nil, NewMsgCompressedSizeTooLargeErr(expectedSize, maxSize)
}

switch compression {
Expand Down Expand Up @@ -265,7 +286,7 @@ func decompressRequest(buffers *RequestBuffers, reader io.Reader, expectedSize,
}

if buf.Len() > maxSize {
return nil, MsgSizeTooLargeErr{Actual: -1, Limit: maxSize}
return nil, NewMsgUnknownSizeTooLargeErr(maxSize)
}
return buf.Bytes(), nil
}
Expand Down
14 changes: 12 additions & 2 deletions pkg/util/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,18 @@ func TestIsRequestBodyTooLargeRegression(t *testing.T) {
}

func TestNewMsgSizeTooLargeErr(t *testing.T) {
err := MsgSizeTooLargeErr{Actual: 100, Limit: 50}
msg := `the request has been rejected because its size of 100 bytes exceeds the limit of 50 bytes`
err := NewMsgUncompressedSizeTooLargeErr(100, 50)
msg := `the request has been rejected because its size of 100 bytes (uncompressed) exceeds the limit of 50 bytes`

assert.Equal(t, msg, err.Error())

err = NewMsgCompressedSizeTooLargeErr(100, 50)
msg = `the request has been rejected because its size of 100 bytes (compressed) exceeds the limit of 50 bytes`

assert.Equal(t, msg, err.Error())

err = NewMsgUnknownSizeTooLargeErr(50)
msg = `the request has been rejected because its size exceeds the limit of 50 bytes`

assert.Equal(t, msg, err.Error())
}
Expand Down
Loading