Add Metrics for EndpointSlices#1910
Conversation
9027651 to
d122c47
Compare
Implements https://pkg.go.dev/k8s.io/api/discovery/v1#EndpointSlice This resourcetype is disabled by default as they are very verbose and have a high cardinality. Metrics from endpointslices can be used to identify if specific pods are part of an endpoint and thus discoverable through a service. Signed-off-by: Manuel Rüger <manuel@rueg.eu>
d122c47 to
b4b8c23
Compare
|
/lgtm |
|
/hold |
| for _, port := range e.Ports { | ||
| m = append(m, &metric.Metric{ | ||
| LabelValues: []string{*port.Name, string(*port.Protocol), strconv.FormatInt(int64(*port.Port), 10)}, | ||
| LabelKeys: []string{"port_name", "port_protocol", "port_number"}, |
There was a problem hiding this comment.
| LabelKeys: []string{"port_name", "port_protocol", "port_number"}, | |
| LabelKeys: []string{"name", "protocol", "number"}, |
Prefixing the labels by port feels a bit redundant
There was a problem hiding this comment.
I noticed afterward that we inherit that choice from the endpoint metrics: https://github.com/kubernetes/kube-state-metrics/blob/master/docs/endpoint-metrics.md.
There is not much we can do about it anymore I guess.
There was a problem hiding this comment.
Yep, initially I would have preferred to keep it shorter as well, then decided to stay consistent with endpoints.
| }), | ||
| ), | ||
| *generator.NewFamilyGeneratorWithStability( | ||
| "kube_endpointslice_endpoints", |
There was a problem hiding this comment.
I think we are plumbing too maybe information inside of this metric. What would you think about moving the conditions, targetref and addresses into their own metrics? There are some precedence to that kind of split with statuses metrics and kube_endpoint_address.
There was a problem hiding this comment.
I decided to combine everything in it that is in https://pkg.go.dev/k8s.io/api/discovery/v1#Endpoint (which is used in
https://pkg.go.dev/k8s.io/api/discovery/v1#EndpointSlice). This would make it easier for users to query it instead of combining two or more metrics. Do you see any downsides?
|
/lgtm cancel |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fpetkovski, logicalhan, mrueg The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
Implements https://pkg.go.dev/k8s.io/api/discovery/v1#EndpointSlice
Metrics from endpointslices can be used to identify if specific pods are part of an endpoint and thus discoverable through a service.
How does this change affect the cardinality of KSM: (increases, decreases or does not change cardinality)
This resource type is disabled by default as they are very verbose and have a high cardinality.