OutlierDetection documentation wrong for error types

[howardjohn]

https://istio.io/docs/reference/config/networking/v1alpha3/destination-rule/#OutlierDetection

Under consecutiveErrors: 502, 503, and 504s only, 500 codes will not trigger the outlier detection

But in the docs above:

For HTTP services, hosts that continually return 5xx errors for API calls are ejected from the pool for a pre-defined period of time.

So sounds like the docs are incorrect here. It will only work for 502/503/504 from looking at the code and #617

[codependent]

@howardjohn Thanks for clearing it up.

Just one question why isn't 500 status code included in the circuit breaking? In my view a service that returns continuously Internal Server Errors should also be also ejected from the pool. Otherwise we are keeping a service that is failing for whatever reason.

[howardjohn]

Some context from the change #617 that introduced it:

We try not to expose the plethora of sometimes confusing Envoy options to end users, in the routing api.

Within a mesh, gateway errors will be more common (502/503/504) while most sensible external services will return a 503 to shed load.

Secondly we just made the outlier detection generic to both tcp and http. The consecutive gateway error applies only to http and will make no sense in tcp context.

I also feel that 500 error code is not something indicative of overload. The whole idea behind outliers is to remove overloaded servers from the lb pool.

We don’t have very many users relying on this behavior I think. We kept it intentionally generic so that we can switch to a more specific error code in future (which happens to be now).

[codependent]

I understand the motivation now however I think there's another valid scenario that is not supported.

In our microservice infrastructure we were relying on Hystrix as an application circuit breaker. When downstream services are crashing (500 errors) we don't want to keep sending traffic to them, and Hystrix removed them temporaly from the pool.

Right now with Istio, this failing services are being invoked despite being actually down. That is to say we are protected from latency/overloading but not from failure. We can always translate 500 errors to 502/503/504 at the microservices but this seems like a work around.

Does this make sense?

[codependent]

Envoy explicitly distinguishes between consecutive gateway failures (502,503,504) errors and consecutive errors (5xx), supporting both of them.

https://www.envoyproxy.io/docs/envoy/v1.5.0/intro/arch_overview/outlier

Istio’s consecutiveErrors field is therefore misleading, shouldn’t it be aligned with Envoys terminology? Also, why not allow both kinds of configurations?

[venosov]

I think the 500 status code should be included in the circuit breaking, resilience4j and lagom give this support too.

[howardjohn]

@rshriram any thoughts ^?

[frankbu]

So there are really 2 issues here.

1. The documentation is currently wrong. The consecutiveErrors field is documented correctly, but the overview/example is wrong and needs to be fixed.

2. Should Istio support both kinds of outlier detection that Envoy supports (5xx and Gateway failures). If so, would it be sufficeint to add a boolean (e.g., allFailures to choose weather consecutiveErrors maps to all 5xx errors or just the Gateway ones?

@rshriram ^^^^

[rshriram]

wow.. this has come a full circle. So options here are to change the semantics of consecutiveErrors once again [but then people like @arnecls will want to set this up only for 502/3,etc.]. Or as @frankbu says, we could add a boolean that says "treat500ErrorsAsFailure` such that people can decide to use application error (HTTP 500) as a sign of failure.