kubernetes / kube-state-metrics

Add-on agent to generate and expose cluster-level metrics.
https://kubernetes.io/docs/concepts/cluster-administration/kube-state-metrics/
Apache License 2.0
5.45k stars 2.02k forks source link

Include `reason` label to the `kube_deployment_status_condition` metric #1991

Open augustfengd opened 1 year ago

augustfengd commented 1 year ago

What would you like to be added:

I would like the label reason to be added to the kube_deployment_status_condition metric.

Why is this needed:

The kube_deployment_status_condition has only two properties relating to the DeploymentCondition object: condition and status, which is not enough to distinguish between a Progressing Deployment and a Complete Deployment. The reason label will add sufficient information to distinguish the two states.

Describe the solution you'd like

Simply extract and emit the reason label for the deployments.

Additional context

.

CatherineF-dev commented 1 year ago

However, it's a stable metric stated in https://github.com/kubernetes/kube-state-metrics/blob/main/docs/deployment-metrics.md

Any ideas for this case? Should we introduce a new metric? cc @dgrisonnet

dgrisonnet commented 1 year ago

Adding a new label is not a breaking change, so as long as its values are bounded, I would be fine with adding it.

k/k stability framework forbids that, but for kube-state-metrics I would allow it since new fields can be added to existing APIs without having to create a new version.

CatherineF-dev commented 1 year ago

Got it. Created https://github.com/kubernetes/kube-state-metrics/issues/1995 and https://github.com/kubernetes/kube-state-metrics/pull/1996 to discuss definitions of stable metrics in kube-state-metrics repo.

logicalhan commented 1 year ago

Adding a new label is not a breaking change, so as long as its values are bounded, I would be fine with adding it.

It depends on your ingestion. Since people ingest metrics into stores other than prometheus, I would argue that the lowest common denominator dictates that it is a breaking change.

logicalhan commented 1 year ago

/triage accepted

CatherineF-dev commented 1 year ago

TLDR: we can add new labels for KSM stable metric.

cc @augustfengd Feel free to add a new label if you're available.

a-hilaly commented 1 year ago

Anyone working on this? If none i'd like to give it a shot :) /cc @dgrisonnet @CatherineF-dev

Gopinath-Selvam commented 1 year ago

Anyone working on this? If not, then I'd like to contribute for this issue!

opeco17 commented 1 year ago

Since there is no update for a long time, I created PR (https://github.com/kubernetes/kube-state-metrics/pull/2146).

k8s-triage-robot commented 3 months ago

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

dgrisonnet commented 1 month ago

@opeco17 would you be interested in reviving your PR?

Otherwise would @a-hilaly or @Gopinath-Selvam be interested in taking over Opeco's PR and address the pending comments?

/triage accepted