-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[receiver/k8sclusterreceiver]: add k8s.pod.ready metric #32933
base: main
Are you sure you want to change the base?
Conversation
Hey, please create an issue, attach the issue to description and changelog entry. |
Done, see #32941 |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
not stale |
@@ -287,6 +287,13 @@ metrics: | |||
gauge: | |||
value_type: int | |||
|
|||
k8s.pod.ready: | |||
enabled: true |
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.
I think maybe this should be disabled by default?
I'm a bit worried It would add quite a bit of new metrics 🤔
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.
I'd argue that pod ready state is pretty fundamental to k8s, and it should be part of the default set. If k8s.container.ready
is, then this should be too. I'd think for most workloads pod ready state is more important at a high level than container, as it affects whether a Service directs traffic to it. It's also part of the default set in KSM.
That said, I'm happy to flip it if that's what it takes to get this over the line.
Description:
Introduce a new metric,
k8s.pod.ready
, to go alongsidek8s.container.ready
. #32941This is helpful to represent, as
k8s.container.ready
alone doesn't tell the full picture of whether a pod's ready for service, due to pod ReadinessGates.With this, we don't need to rely on
kube-state-metrics
for the info.This does the simple thing and adds an analog for the pod ready condition, but I could also see a more generalized version, i.e.
k8s.pod.status.condition
, where we could capture all conditions (API docs), e.g. 0=False, 1=True, 2=Unknown, along withreason
in attributes. Maybemessage
too, if cardinality potential isn't a concern here.Testing: A couple unit tests were added to assert ready pod condition is correctly recorded for both true and false states.
Documentation: Only auto-generated docs.