Metrics by error type when distributer fail to call ingester

[danielblando]

Is your feature request related to a problem? Please describe.
When using cortex, we sometimes have calls between distributor and ingester that might fails. We do have metrics for call made to ingester as well as metrics for failed responses. These failed responses collect any type of error. Some of these errors as 4xx does not show an issue on the system while errors like 5xx do show that something unexpected is happening with ingester. This could lead us to have a bad ingester without actually knowing it, because ingesterAppendFailures metric does not mean we have an issue. This become a bigger issue when a second ingester is being restarted or goes down. We thought that we were being redundant running with 3 ingester, but actually we were already running with limit capacity 2 out of 3.
It would be of good value to have a differentiation between which error we are receiving from ingester.

Describe the solution you'd like
The solution proposed would be adding a new label to the existent metric which would be the error family code (eg. 5xx or 4xx). This would allow us to filter the metrics.

Describe alternatives you've considered
We could also introduce a new metric for 5xx errors instead of adding a new label for an existent metric. 5xx is mostly what we are looking for to have a better visibility of the health of the system. We could add another metric called ingesterAppendFatalFailures where we would only add errors with responses different from 4xx.
This is still a valuable option, but it just create another metric for an information that we are already generating.

[bboreham]

Interested to know whether you considered cortex_ingester_client_request_duration_seconds_count which has a status_code.

[danielblando]

Hey @bboreham, thanks for the input.

To be honest I didn't know that this metric existed. Investigating it I found two issues that block for now using it.

1- The metric itself do not have a destination label. We know that the distributor is receiving 5xx but we are not sure from where it comes from. I guess we could try to get this info from which ingester is not sending metrics or for which ingester the distributor did not receive 2xx recently. This just don't seems a nice way to use the metric.

2- The status_code in the metric is actually misleading. Looking at some metrics example I could only notice 200 and 500. Looking at the code that seems to be the only status we return. This will not differentiate us from 5xx and 4xx.

I can think of two way forward here.
1 - We can improve the statusCode to add 400 (honestly prefer xx than 00 here) and add the ingester desntination as a new label and ignore my current PR to change "cortex_distributor_ingester_append_failures_total"

2- We can still use my PR to add the "status" into "cortex_distributor_ingester_append_failures_total" and optionally just improve the statusCode error to add 400.

Any suggestion?

[bboreham]

Referring to your first "1": since it's a histogram it would blow up cardinality quite a lot to put all the destinations in as a new label.
I'm ok to have ingester_client_request_duration_seconds covering response times and distributor_ingester_append_failures_total (without buckets) providing the destination and response type detail.

So agreed with your second "2".

Referring to your first "2": the conversion from error/no-error to "500" and "200" is inserted here:

cortex/pkg/util/middleware/grpc.go

Line 19 in 32a86c5

metric.WithLabelValues(method, instrument.ErrorCode(err)).Observe(time.Since(start).Seconds())

if you want to try another PR making it richer.

It would look a bit like this:

cortex/vendor/github.com/weaveworks/common/middleware/grpc_instrumentation.go

Lines 17 to 23 in 32a86c5

	if errResp, ok := httpgrpc.HTTPResponseFromError(err); ok {
	respStatus = strconv.Itoa(int(errResp.Code))
	} else if grpcUtils.IsCanceled(err) {
	respStatus = "cancel"
	} else {
	respStatus = "error"
	}

(which has yet another mapping of status codes ...)