Distributor returns 500 HTTP status code on 400 from ingester

[Logiraptor]

Describe the bug

Between weekly-r206 and weekly-r207 a regression was introduced that causes the distributor to return a 500 status code on receiving a 400 from the ingester. This is problematic because it means remote write clients will retry the request indefinitely despite it being invalid.

To Reproduce

Steps to reproduce the behavior:

1. Start Mimir (weekly-r207)
2. Write a too-old sample
3. Observe that the ingester rejects with 400
4. Observe that the distributor rejects with 500

Expected behavior

The distributor should reject with the same status code as ingester.

Environment

• Infrastructure: [e.g., Kubernetes, bare-metal, laptop]
• Deployment tool: [e.g., helm, jsonnet]

Additional Context

[Logiraptor]

I think I know what the issue is.

#3055 wraps errors from the ingesters with errors.Wrap.
This changes the type of the error from status.ErrorProto created here:

mimir/vendor/github.com/weaveworks/common/httpgrpc/httpgrpc.go

Lines 29 to 33 in d857bf9

	return status.ErrorProto(&spb.Status{
	Code: resp.Code,
	Message: string(resp.Body),
	Details: []*types.Any{a},
	})

In turn, this makes the httpgrpc error handling break here:

mimir/vendor/github.com/gogo/status/status.go

Lines 144 to 146 in d857bf9

	if se, ok := err.(interface{ GRPCStatus() *status.Status }); ok {
	return FromGRPCStatus(se.GRPCStatus()), true
	}

The error no longer satisfies that interface.

[pr00se]

Yes, any error that does not pass status.FromError() will be turned into a 500:

mimir/pkg/util/push/push.go

Lines 131 to 133 in d857bf9

	if !ok {
	http.Error(w, err.Error(), http.StatusInternalServerError)
	return