stats: make the stat name of endpoint configurable#45044
Conversation
Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
/retest |
| // This is useful when there are duplicate addresses in the cluster, for example when multiple | ||
| // endpoints share the same address but have different hostnames or metadata. | ||
| // In this case, the stat name can be used to differentiate between these endpoints in stats and logs. | ||
| string stat_name = 5; |
There was a problem hiding this comment.
The per-endpoint stats export two labels: the ip:port, and the hostname (if present). Which of these is this overriding? Or is it replacing both of those with a 3rd tag? Please document whatever the behavior is clearly.
| for (auto& host : host_set->hosts()) { | ||
| absl::string_view endpoint_observability_name = host->observabilityName(); | ||
| Network::Address::InstanceConstSharedPtr address; | ||
| if (endpoint_observability_name.empty()) { |
There was a problem hiding this comment.
observabilityName() returns address_->asStringView() as a fallback, so I think this condition is always false.
There was a problem hiding this comment.
There is a special case: LogicHost, whose address may be updated dynamically. That will not support the stat_name and also won't fallback to ip:port by default.
My plan is to remove logic host in the future because current default host implementation also could support multiple addresses.
Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
|
cc @ggreenway updated the comment to make it more clear. |
|
/retest |
3 participants
Commit Message: stats: make the stat name of endpoint configurable
Additional Description:
Make the stat name of endpoint configurable to close #44117. Before this PR, only the ip:port will be used as stat name in the per endpoint metrics. But if there are multiple endpoints with same ip:port, then the metrics will be corrupted.
(I initially want to add priority to per endpoint metrics as a tag to resolve this problem, but seems there is requirement to support duplicate endpoints in same priority, so I guess current way should be more flexible).
Risk Level: low.
Testing: unit.
Docs Changes: n/a.
Release Notes: added.
Platform Specific Features: n/a.