grpc success/failure stat should count more than 0 as success

[richunger]

https://github.com/lyft/envoy/blob/3e62653863125c07fc78fcc4bd967a10f026114b/source/common/grpc/common.cc#L34

We use this success stat to calculate our services' success rate. In the REST world, I've generally seen success rate calculated as 2xx+3xx+4xx/total_count. What envoy does now for grpc seems like the equivalent of just having 2xx in the numerator. For example, FailedPrecondition should not count as a failure in calculating SR, right?

[mattklein123]

I think @fengli79 and @lizan have some thoughts on future work on the gRPC stats. Perhaps they have some thoughts here.

[fengli79]

It depends on how you define the "success".
gRPC always use 200 as response code, and its status is encoded in grpc-status header, a "0" value means the rpc is success and others are failure.
In that sense, the stats is correct.

However, if you want to count FailedPrecondition as success when calculating SR, you have to do that by yourself. It's not feasible, as today, we only record either success or failure.

I'm going to create a PR to add additional stats for each gRPC status code, thus you can use them calculate the SR which meets your business logic.

[mattklein123]

@richunger yeah it's a little unclear to me how FailedPrecondition is not an app error here so it does seem to me like the stats are correct for gross success/fail. In any case, as @fengli79 says we will soon have per code stats like we do for HTTP so can do custom stuff as needed.

[richunger]

Yeah, per-code stats seem like the best path overall.
IMHO FailedPrecondition is a client error (the moral equivalent of an HTTP 400), and not a failure of the service being measured.

[mattklein123]

@richunger I agree with you that from app perspective, it's an error. From sever perspective, it's not. Let's get the per code stats in place then we can figure out how to change our internal alarms to be better.

[mattklein123]

@richunger calling this fixed via #1125. Will get deployed next week. We can discuss alarm/graph changes internally.