Remove errors.Cause() usage from store-gateway and its clients

[pracucci]

What this PR does

We have few usage of errors.Cause() in the store-gateway and the querier which I believe being superfluous. The usage addressed by this PR is status.FromError(errors.Cause(err)). The actual status.FromError() implementation internally uses errors.As() (which navigates the chain of wrapped errors), so we shouldn't need errors.Cause().

The errors.Cause() in shouldStopQueryFunc() was introduced by #4100 which also introduced the integration test Test_MaxSeriesAndChunksPerQueryLimitHit. The integration test passes on top of this PR. As a counter proof, of we completely remove the status.FromError() check from shouldStopQueryFunc(), then the Test_MaxSeriesAndChunksPerQueryLimitHit integration test fails.

Note: this PR is part of a work I'm doing to completely remove errors.Cause() from Mimir.

Which issue(s) this PR fixes or relates to

N/A

Checklist

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

[narqo]

🔥

[duricanikolic]

I have worked on the error handling improvement on the write path, and I ensured that most of the usages of the status packages there were from gogo instead of grpc.
What is the difference?
On one side, gogo's status package allows you to easily add custom error details, which are crucial for the error handling on the write path. On another side, status.FromError() doesn't use errors.As() internally.
Related to grpc's status package, it is not easy to add custom error details, but status.FromError() uses errors.As() internally.
In order to minimizes differences between different packages used, I then implemented [grpcutil.ErrorToStatus()](https://github.com/grafana/dskit/blob/main/grpcutil/status.go#L12-L36) in dskit that, independently from whether the status is created by gogo or grpc uses errors.As().
I'd recommend using grpcutil.ErrorToStatus() instead of status.FromError().

Nevertheless, in this PR the semantics wouldn't change. But if at a point somebody decides to replace grpc status with gogo status, this would be a breaking change.

[pracucci]

I'd recommend using grpcutil.ErrorToStatus() instead of status.FromError().

That's a very good feedback @duricanikolic (what I was looking for!). I checked grpcutil.ErrorToStatus() implementation (which I didn't remember about it) and switched grpc/status.FromError() to grpcutil.ErrorToStatus().

There are few other cases of grpc/status.FromError() in the codebase which are unrelated from removal of errors.Cause() and I will try to address in a follow up PR, so we can put a faillint rule on grpc/status.FromError().

[duricanikolic]

LGTM
Thank you.

[pracucci]

There are few other cases of grpc/status.FromError() in the codebase which are unrelated from removal of errors.Cause() and I will try to address in a follow up PR, so we can put a faillint rule on grpc/status.FromError().

The follow up PR: #7224

[dimitarvdimitrov]

LGTM, the wrapping logic (using now Unwrap() instead of Cause()) shouldn't change for the errors the store-gateway creates.

IIRC the prometheus libraries also stopped returning errors with Cause() methods (I remember you mentioned it in a PR Marco), but those wouldn't have been gRPC errors anyways.

[pracucci]

IIRC the prometheus libraries also stopped returning errors with Cause() methods (I remember you mentioned it in a PR Marco), but those wouldn't have been gRPC errors anyways.

Yes, already addressed all such cases (in previous PR).