envoyproxy / envoyproxy/envoy

Revamp `upstream_rq_timeout` virtual cluster counter

Open
#23,867 2 comments 0 reactions 0 assignees View on GitHub
area/stats enhancement no stalebot
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 20h
Merged PRs (30d)
437

Description

*Description*:
Envoy Mobile does not increment `upstream_rq_timeout` virtual cluster counter when any of the request it perform times out. For the record, the Envoy Mobile library enables the two following types of timeouts: `per_try_timeout` and `per_try_idle_timeout`.

Instead of incrementing `upstream_rq_timeout` stat, every time Envoy Mobile's request times out `upstream_rx_5xx` and `upstream_rq_504` virtual cluster counters are incremented. From the perspective of mobile clients (Envoy Mobile library) it's not desired to treat timeouts as `504` failures - mobile clients want to see HTTP status codes that come from the server only and do not want Envoy Mobile (or Envoy) to emit its own status codes.

*Proposal*:
- Change the implementation of `upstream_rq_timeout` stat so that it gets incremented when a request timeouts due to either `per_try_timeout` or `per_try_idle_timeout` timeout.
- Stop incrementing `upstream_rx_5xx` and `upstream_rq_504` virtual cluster stats when an Envoy Mobile request timeouts due to either of the aforementioned timeout types.
- Hide the change behind a runtime flag or make it a part of the config - allow customers to decide whether they want to treat certain types of timeouts as 5xx or not.

*Relevant Links*:
Currently available virtual cluster stats are listed at https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/router_filter#virtual-clusters.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.