add kube_*_annotations metrics #770

qw1mb0 · 2019-05-28T11:17:12Z

What this PR does / why we need it:
In this PR, a new metrics kube_*_annotations is added for all Kubernetes objects, in which will be a list of all the annotation of object. This functionality is very necessary for the ability to display data on the basis of annotations in grafana and the construction of the rules, alerts.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #746

k8s-ci-robot · 2019-05-28T11:17:13Z

Welcome @qw1mb0!

It looks like this is your first PR to kubernetes/kube-state-metrics 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kube-state-metrics has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

qw1mb0 · 2019-05-28T11:27:04Z

And I would like to ask for advice, perhaps it is worth removing from the annotation the mention of kubernetes_io_last_applied_configuration? If so, can you tell me how you can do this?
Thank you very much in advance!

brancz · 2019-05-28T12:52:11Z

I think we've recently had a similar case to this. I'm trying to understand why this is not a real field on the deployment? This being an annotation doesn't make me feel like this should be a stable metric.

qw1mb0 · 2019-05-28T12:59:29Z

This metric appears only when manually managing deployment in kubernetes using the kubectl edit deploy app.

Then the metric will look like this:

kube_deployment_annotations{annotation_deployment_kubernetes_io_revision="1",annotation_kubectl_kubernetes_io_last_applied_configuration="{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"annotations":{},"name":"my-nginx","namespace":"default"},"spec":{"replicas":2,"selector":{"matchLabels":{"run":"my-nginx"}},"template":{"metadata":{"labels":{"run":"my-nginx"}},"spec":{"containers":[{"image":"nginx","lifecycle":{"preStop":{"exec":{"command":["/bin/sh","-c","sleep 90; nginx -s quit"]}}},"name":"my-nginx","ports":[{"containerPort":80}]}]}}}} ",deployment="my-nginx",instance="10.244.1.90:8443",job="kube-state-metrics",namespace="default",scrape_endpoint="main"}

brancz · 2019-05-28T13:48:00Z

hmm... maybe someone from sig cli or sig apps could explain why this is not a real field, maybe it's a prospect for an objectmeta field?

qw1mb0 · 2019-05-28T14:03:47Z

This summary is used for the patch: https://kubernetes.io/docs/tasks/manage-kubernetes-objects/declarative-config/#merge-patch-calculation

Now we can leave this annotation. Can I add this label to all other resources?

brancz · 2019-05-28T14:17:17Z

Could you add examples of the annotations you intend to use in the tests? Because I don't think the result that you are currently producing is what you are looking for, as you can't do math with label values.

qw1mb0 · 2019-05-28T15:05:35Z

I added annotations to deployment_test.go in this commit: 44b0763

docs/deployment-metrics.md

qw1mb0 · 2019-05-28T15:40:40Z

Can I make a similar metric for all other resources now?

zuzzas

Some things stood out.

zuzzas · 2019-05-29T09:32:15Z

docs/certificatessigningrequest-metrics.md

@@ -6,3 +6,4 @@
 | kube_certificatesigningrequest_condition | Gauge | `certificatesigningrequest`=&lt;certificatesigningrequest-name&gt; <br> `condition`=&lt;approved\|denied&gt; | STABLE |
 | kube_certificatesigningrequest_labels | Gauge | `certificatesigningrequest`=&lt;certificatesigningrequest-name&gt;| STABLE |
 | kube_certificatesigningrequest_cert_length | Gauge | `certificatesigningrequest`=&lt;certificatesigningrequest-name&gt;| STABLE |
+| kube_certificatesigningrequest_labels | Guage | `annotation_CSR_ANNOTATION`=&lt;CSR_ANNOTATION&gt; <br> `certificatesigningrequest`=&lt;certificatesigningrequest-name&gt; | STABLE |


This seems incorrect.

zuzzas · 2019-05-29T09:35:43Z

internal/collector/certificatesigningrequest.go

@@ -32,6 +32,9 @@ var (
 	descCSRLabelsHelp          = "Kubernetes labels converted to Prometheus labels."
 	descCSRLabelsDefaultLabels = []string{"certificatesigningrequest"}

+	descCSRAnnotationsName          = "kube_certificatesigningrequest_annotations"


What's the reasoning behind adding a variable that is only used once? Useless indirection creates confusion.

That applies to all such changes.

you're right

qw1mb0 · 2019-05-29T16:12:19Z

@brancz I added the metric kube _ * _ annotations for all resources, everything works correctly on the test cluster and the tests were successful. What do you think about this PR?

brancz

Additionally to skipping the pod annotations, could we mark all of these as experimental? I would like to be able to remove them if there is a cardinality problem with any of them.

brancz · 2019-06-05T08:26:43Z

docs/pod-metrics.md

@@ -40,3 +40,4 @@
 | kube_pod_spec_volumes_persistentvolumeclaims_info | Gauge | `pod`=&lt;pod-name&gt; <br> `namespace`=&lt;pod-namespace&gt; <br> `volume`=&lt;volume-name&gt;  <br> `persistentvolumeclaim`=&lt;persistentvolumeclaim-claimname&gt; | STABLE |
 | kube_pod_spec_volumes_persistentvolumeclaims_readonly | Gauge | `pod`=&lt;pod-name&gt; <br> `namespace`=&lt;pod-namespace&gt;  <br> `volume`=&lt;volume-name&gt;  <br> `persistentvolumeclaim`=&lt;persistentvolumeclaim-claimname&gt; | STABLE |
 | kube_pod_status_scheduled_time | Gauge | `pod`=&lt;pod-name&gt; <br> `namespace`=&lt;pod-namespace&gt; | STABLE |
+| kube_pod_annotations | Gauge | `annotation_POD_ANNOTATION`=&lt;POD_ANNOTATION&gt; <br> `pod`=&lt;pod-name&gt; <br> `namespace`=&lt;pod-namespace&gt; | STABLE |


this was deemed to be too high cardinality previously, it needs more discussion I'd say, let's not add this here

Good. I deleted the metric for the pods

qw1mb0 · 2019-06-20T15:06:56Z

/retest

k8s-ci-robot · 2019-06-20T15:07:12Z

@qw1mb0: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

qw1mb0 · 2019-06-20T15:08:20Z

/assign @tariq1890

tariq1890 · 2019-06-20T17:17:50Z

Can you rebase this PR?

qw1mb0 · 2019-06-24T10:37:53Z

Can you rebase this PR?

done

docs/certificatessigningrequest-metrics.md

tariq1890

LGTM to me. I'll defer to @brancz for the final decision. Thanks for your contribution :)!

/lgtm
/approve
/hold

k8s-ci-robot · 2019-06-25T20:25:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qw1mb0, tariq1890

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~~ [tariq1890]

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

brancz · 2019-06-26T07:27:32Z

lgtm :)

/hold cancel

diemus · 2019-07-12T01:30:01Z

when will this become a stable version?

tariq1890 · 2019-07-12T03:06:44Z

@diemus Thanks for checking in! We will wait for a period of atleast 10 7 days from the date of publishing the release candidate. If there are no issues/bugs reported within that window, we will publish a stable release.

Until then, please feel free to test this feature with the 1.7.0-rc1 build and let us know if you face issues :).

brancz · 2019-07-12T06:06:38Z

Small correction: 7 days is the normal wait time to promote from release candidate to final release.

tariq1890 · 2019-07-12T06:35:22Z

Thanks @brancz ! I'll amend my previous comment.

lilic · 2019-07-12T11:49:11Z

@tariq1890 @brancz should we add this info about the release process in the docs somewhere or do we have it already?

brancz · 2019-07-12T11:51:38Z

it is not as far as I can tell, please feel free to add it to the RELEASE.md

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 28, 2019

k8s-ci-robot requested review from mxinden and tariq1890 May 28, 2019 11:17

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 28, 2019

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 28, 2019

brancz reviewed May 28, 2019

View reviewed changes

docs/deployment-metrics.md Outdated Show resolved Hide resolved

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 29, 2019

zuzzas suggested changes May 29, 2019

View reviewed changes

qw1mb0 changed the title ~~add kube_deployment_annotations metric~~ add kube_*_annotations metrics May 29, 2019

brancz reviewed Jun 5, 2019

View reviewed changes

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 8, 2019

k8s-ci-robot assigned tariq1890 Jun 20, 2019

tariq1890 reviewed Jun 25, 2019

View reviewed changes

docs/certificatessigningrequest-metrics.md Outdated Show resolved Hide resolved

add kube_*_annotations metrics for all objects

5ae00bb

tariq1890 approved these changes Jun 25, 2019

View reviewed changes

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jun 25, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 25, 2019

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 26, 2019

k8s-ci-robot merged commit f24d831 into kubernetes:master Jun 26, 2019

lilic mentioned this pull request Jul 15, 2019

RELEASE.md: Mention period of time for rc #823

Merged

tariq1890 mentioned this pull request Jul 17, 2019

Feature request: include pod annotations #826

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add kube_*_annotations metrics #770

add kube_*_annotations metrics #770

qw1mb0 commented May 28, 2019 •

edited

Loading

k8s-ci-robot commented May 28, 2019

qw1mb0 commented May 28, 2019

brancz commented May 28, 2019

qw1mb0 commented May 28, 2019

brancz commented May 28, 2019

qw1mb0 commented May 28, 2019

brancz commented May 28, 2019

qw1mb0 commented May 28, 2019 •

edited

Loading

qw1mb0 commented May 28, 2019

zuzzas left a comment

zuzzas May 29, 2019

zuzzas May 29, 2019

zuzzas May 29, 2019

qw1mb0 May 29, 2019

qw1mb0 commented May 29, 2019

brancz left a comment

brancz Jun 5, 2019

qw1mb0 Jun 13, 2019

qw1mb0 commented Jun 20, 2019

k8s-ci-robot commented Jun 20, 2019

qw1mb0 commented Jun 20, 2019

tariq1890 commented Jun 20, 2019

qw1mb0 commented Jun 24, 2019

tariq1890 left a comment •

edited

Loading

k8s-ci-robot commented Jun 25, 2019

brancz commented Jun 26, 2019

diemus commented Jul 12, 2019

tariq1890 commented Jul 12, 2019 •

edited

Loading

brancz commented Jul 12, 2019

tariq1890 commented Jul 12, 2019

lilic commented Jul 12, 2019

brancz commented Jul 12, 2019

add kube_*_annotations metrics #770

add kube_*_annotations metrics #770

Conversation

qw1mb0 commented May 28, 2019 • edited Loading

k8s-ci-robot commented May 28, 2019

qw1mb0 commented May 28, 2019

brancz commented May 28, 2019

qw1mb0 commented May 28, 2019

brancz commented May 28, 2019

qw1mb0 commented May 28, 2019

brancz commented May 28, 2019

qw1mb0 commented May 28, 2019 • edited Loading

qw1mb0 commented May 28, 2019

zuzzas left a comment

Choose a reason for hiding this comment

zuzzas May 29, 2019

Choose a reason for hiding this comment

zuzzas May 29, 2019

Choose a reason for hiding this comment

zuzzas May 29, 2019

Choose a reason for hiding this comment

qw1mb0 May 29, 2019

Choose a reason for hiding this comment

qw1mb0 commented May 29, 2019

brancz left a comment

Choose a reason for hiding this comment

brancz Jun 5, 2019

Choose a reason for hiding this comment

qw1mb0 Jun 13, 2019

Choose a reason for hiding this comment

qw1mb0 commented Jun 20, 2019

k8s-ci-robot commented Jun 20, 2019

qw1mb0 commented Jun 20, 2019

tariq1890 commented Jun 20, 2019

qw1mb0 commented Jun 24, 2019

tariq1890 left a comment • edited Loading

Choose a reason for hiding this comment

k8s-ci-robot commented Jun 25, 2019

brancz commented Jun 26, 2019

diemus commented Jul 12, 2019

tariq1890 commented Jul 12, 2019 • edited Loading

brancz commented Jul 12, 2019

tariq1890 commented Jul 12, 2019

lilic commented Jul 12, 2019

brancz commented Jul 12, 2019

qw1mb0 commented May 28, 2019 •

edited

Loading

qw1mb0 commented May 28, 2019 •

edited

Loading

tariq1890 left a comment •

edited

Loading

tariq1890 commented Jul 12, 2019 •

edited

Loading