admin: add json serialized proto for /clusters #3478
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| syntax = "proto3"; | ||
|
|
||
| package envoy.admin.v2alpha; | ||
|
|
||
| import "envoy/api/v2/core/base.proto"; | ||
| import "envoy/api/v2/core/address.proto"; | ||
| import "envoy/type/percent.proto"; | ||
|
|
||
| // [#protodoc-title: Clusters] | ||
|
|
||
| // Admin endpoint uses this wrapper for `/clusters` to display cluster status information. | ||
| // See :ref:`/clusters <operations_admin_interface_clusters>` for more information. | ||
| message Clusters { | ||
| // Mapping from cluster name to each cluster's status. | ||
| repeated ClusterStatus cluster_statuses = 1; | ||
| } | ||
|
|
||
| // Details an individual cluster's current status. | ||
| message ClusterStatus { | ||
| // Name of the cluster. | ||
| string name = 1; | ||
|
|
||
| // General outlier statistics if installed for this cluster. | ||
| OutlierInfo outlier_info = 2; | ||
|
|
||
| // Denotes whether this cluster was added via API or configured statically. | ||
| bool added_via_api = 3; | ||
|
|
||
| // Mapping from host address to the host's current status. | ||
| repeated HostStatus host_statuses = 4; | ||
| } | ||
|
|
||
| // :ref:`Cluster outlier detection <arch_overview_outlier_detection>` statistics. The omission of | ||
| // any statistic denotes that there was not enough data to compute it. | ||
| message OutlierInfo { | ||
| // The average success rate of the hosts in the Detector for the last aggregation interval. | ||
| // | ||
| // Note: the message will be omitted if there were not enough hosts with enough request volume to | ||
| // proceed with success rate based outlier ejection. | ||
| envoy.type.Percent success_rate_average = 1; | ||
|
Member
There was a problem hiding this comment. Can this be derived from the individual hosts'
Member
Author
There was a problem hiding this comment. Basically, yes (see below for a more detailed answer). Is it worth providing it for usability? They will be close. Technically, they may differ under two scenarios IIUC:
Member
There was a problem hiding this comment. Unless the distinction is important to the use for (1) and (2), thenI would vote to just provide it at the host-level and allow the consumer to compute.
Member
Author
There was a problem hiding this comment. Since the internally computed cluster-wide average is compared to each host's average to determine whether the host gets ejected or not, I would think the exact cluster-wide average might be important for users when debugging why a host got ejected. However, it's something that's easy to add later if someone specifically requests it, so SGTM. |
||
|
|
||
| // The success rate threshold used in the last interval. The threshold is used to eject hosts | ||
| // based on their success rate. | ||
|
Member
There was a problem hiding this comment. Are these percentages/ratios? If so, consider expressing with https://github.com/envoyproxy/envoy/blob/master/api/envoy/type/percent.proto. |
||
| // | ||
| // Note: the message will be omitted if there were not enough hosts with enough request volume to | ||
| // proceed with success rate based outlier ejection. Since this value can technically be negative | ||
| // due to the way that it's computed, negative values will also be represented by the absence of | ||
| // this field because a negative value implies that there was no threshold for that interval. | ||
| envoy.type.Percent success_rate_ejection_threshold = 2; | ||
| } | ||
|
|
||
| // Current state of a particular host. | ||
| message HostStatus { | ||
| // Address of this host. | ||
| envoy.api.v2.core.Address address = 1; | ||
|
|
||
| // Mapping from the name of the statistic to the current value. | ||
| map<string, int64> stats = 2; | ||
|
Member
There was a problem hiding this comment. Is this the first time we're defining in proto the stats data model?
Member
Author
There was a problem hiding this comment. Not exactly - see here. The only more sophisticated definition I could imagine would be differentiating between gauges and counters.
Member
There was a problem hiding this comment. OK, I think it's fine to have a simpler variant without relying on Prometheus protos. Might belong in its own message though, so we can reuse again later. Would suggest the domain to be
Member
Author
There was a problem hiding this comment. SGTM. Where do you think this stat message should go? Under |
||
|
|
||
| // The host's current health status. | ||
| HostHealthStatus health_status = 3; | ||
|
|
||
| // Configured load balancing weight for this host. | ||
| uint64 weight = 4; | ||
|
|
||
| // Configured locality for the host. | ||
| envoy.api.v2.core.Locality locality = 5; | ||
|
Member
There was a problem hiding this comment. Can we reuse https://github.com/envoyproxy/envoy/blob/master/api/envoy/api/v2/endpoint/endpoint.proto#L46 and related structures? Seems like a reasonable overlap.
Member
Author
There was a problem hiding this comment. Hmmmm, yes, there's definitely some overlap, but I worry that this will creep into us providing the entire EDS config in
Member
There was a problem hiding this comment.
Member
There was a problem hiding this comment. Yeah I think it should be provided in
Member
Author
There was a problem hiding this comment. SGTM, I'll remove |
||
|
|
||
| // Denotes whether this is configured as a canary host or not. | ||
| bool canary = 6; | ||
|
|
||
| // Request success rate for this host over the last calculated interval. | ||
| // | ||
| // Note: the message will not be present if host did not have enough request volume to calculate | ||
| // success rate or the cluster did not have enough hosts to run through success rate outlier | ||
| // ejection. | ||
| envoy.type.Percent success_rate = 7; | ||
| } | ||
|
|
||
| // Health status for a host. | ||
| message HostHealthStatus { | ||
| // The host is currently marked as healthy. | ||
| bool healthy = 1; | ||
|
Member
There was a problem hiding this comment. I think we can simplify this to just indicating when a host is unhealthy, and then when there are no bits set saying it's unhealthy, it's considered healthy. I wonder how this relates to https://github.com/envoyproxy/envoy/blob/master/api/envoy/api/v2/core/health_check.proto#L182.. it seems we should have the ability to also indicate draining? Should we have a single health status message?
Member
Author
There was a problem hiding this comment. As for the former, I thought about that. My only hesitation is from a usability perspective. The highest order bit here is whether the host is healthy or not, but that would be the hardest for the user to determine if we were to eliminate the redundant As for how this relates to the EDS health status, that is reduced to a bool here IIUC -
Member
There was a problem hiding this comment. I think we don't have to spend too much time making these "usable", in that the APIs are intended for machine parsing first. So, I think we can drop the healthy status. The EDS health status is really a reflection of other health status possibilities. We might for example, via HDS, discover that a host is draining independent to EDS. So, I think we should be able to convey these possibilities. I would argue that we probably just want a single health status proto and it should be https://github.com/envoyproxy/envoy/blob/master/api/envoy/api/v2/core/health_check.proto#L182, we should make it work if it's not expressive enough for the requiremetns here (e.g. indicating outlier detection). |
||
|
|
||
| // The host is currently failing active health checks. | ||
| bool failed_active_health_check = 2; | ||
|
|
||
| // The host is currently considered an outlier and has been ejected. | ||
| bool failed_outlier_check = 3; | ||
|
|
||
| // The host is currently marked as unhealthy by EDS. | ||
| bool failed_eds_health_check = 4; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,3 +6,4 @@ Admin | |
| :maxdepth: 2 | ||
|
|
||
| ../admin/v2alpha/config_dump.proto | ||
| ../admin/v2alpha/clusters.proto | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
References back to docs on this?