Add messageSizeLargerErrFmt error to errors catalog

[zenador]

What this PR does

Add messageSizeLargerErrFmt error to errors catalog.

Which issue(s) this PR fixes or relates to

Fixes #2432

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[zenador]

A few questions:

1. the util package previously didn't import anything from within mimir, but now it has to as it needs globalerrors at least. it also needs to import the config flag name from distributors, but that would cause an import cycle, so i put that in globalerrors to minimise the imports. is this okay?
2. the limit uses int most of the time, but there's a part of the code that uses int64 in mimir/pkg/util/push/otel.go. this is the same error right? for simplicity i kept it as int and casted int64 to int when displaying the error from that code, so the numbers might not make sense in that case. do we expect the number to be that big? should we be casting all the ints to int64 instead?
3. this config isn't per-tenant right? there seems to be other new errors related to configs that are not per-tenant like DistributorMaxInflightPushRequestsBytes but they use MessageWithLimitConfig which calls the limit 'per-tenant'. should we add a boolean argument to that to decide whether we add the 'per-tenant' string or not?

[56quarters]

A few questions:

1. the util package previously didn't import anything from within mimir, but now it has to as it needs globalerrors at least. it also needs to import the config flag name from distributors, but that would cause an import cycle, so i put that in globalerrors to minimise the imports. is this okay?

I think a bit of copy/paste or removing the flag name from the error message is a better choice that moving a distributor specific flag outside the distributor package.

2. the limit uses int most of the time, but there's a part of the code that uses int64 in `mimir/pkg/util/push/otel.go`. this is the same error right? for simplicity i kept it as int and casted int64 to int when displaying the error from that code, so the numbers might not make sense in that case. do we expect the number to be that big? should we be casting all the ints to int64 instead?

I've noticed this too 😭. Most of the things we use int or even int64 for, shouldn't ever be negative. I think your approach is fine for now.

3. this config isn't per-tenant right? there seems to be other new errors related to configs that are not per-tenant like DistributorMaxInflightPushRequestsBytes but they use MessageWithLimitConfig which calls the limit 'per-tenant'. should we add a boolean argument to that to decide whether we add the 'per-tenant' string or not?

I think having two different methods for the different types of limits would be a better choice than a bool that switches between two behaviors but that's personal preference.

[zenador]

Thanks! Switched to copy/paste and 2 different methods

[pracucci]

This looks like a generic function used to generate the error, but then it contains "distributor.max-recv-msg-size" which is very specific. This error is returned from the exported function ParseProtoReader() which is not just called by the remote write.

I would suggest a different approach:

1. From this package, return a globally defined static error, like var ErrMessageTooLarge = errors.New("the request has been rejected because its message size exceed the limit"). The message is intentionally generic and static
2. In pkg/util/push/ check the error returned by ParseProtoReader and if it's a ErrMessageTooLarge then remap it into a more specific error (mentioning the distributor flag in this case, which is correct)

[zenador]

1. I replaced it with a struct because the existing error messages are already dynamic with the actual and limit numbers included in the msg, is that okay?

[pracucci]

Good job! The new logic looks good to me. Just a few final comments, and then we should be good go!

[pracucci]

LGTM, thanks!