Fixes New metric for deployment revision #746

[bvoss]

Add metric for deployment revision based on the annotation 'deployment.kubernetes.io/revision'

[k8s-ci-robot]

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
• If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
• Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: login-issues@jira.linuxfoundation.org

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bvoss
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: mxinden

If they are not already assigned, you can assign the PR to them by writing /assign @mxinden in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[tariq1890]

I am not sure how I feel about exposing metrics from annotations. Thoughts @brancz @mxinden ?

[mxinden]

Instead of exposing specific annotations, I would advocate towards exposing all annotations like we do with Kubernetes labels.

This has been raised in #574 before. The cardinality increase introduced through this metric could have a high impact on big clusters, thus I think making the metric optional and disabled by default is the way forward.

@tariq1890 @bvoss @brancz what do you think?

[bvoss]

@mxinden @tariq1890 @brancz
Actual we have the problem with a large cluster, so all annotations isn't an option for us.

I understood, that the annotation and its value is essential for kubectl rollout --revision and so more likely part of the API and stable.
Honestly I don't know why it's not a property like in StatefulSetStatus.

So I thought it would be ok - in this special case - to expose this annotation value.

Nevertheless the general annotation approach is also fine for me, if it's possible to configure a white-list for annotations to be contained in the metric.

[brancz]

Do you have some docs that show that this annotation is indeed considered stable? I'm only asking because I wonder why this is not a real field.

[k8s-ci-robot]

@bvoss: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[tariq1890]

I think this can be closed since the kube annotations PR was merged.

[tariq1890]

Thanks for the feature request and taking a stab at this @bvoss :).

[brancz]

I think there was a distinction here. The value of the annotation was actually parsed and exposed as the metric's value.