Skip to content
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

Add kube_*_annotations metrics #1468

Merged
merged 14 commits into from
Aug 19, 2021
Merged

Conversation

sylr
Copy link
Contributor

@sylr sylr commented Apr 29, 2021

What this PR does / why we need it:

Adds kube_annotations for every existing kube_labels counter parts.

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 #

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 29, 2021
@k8s-ci-robot
Copy link
Contributor

Welcome @sylr!

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. 😃

@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Apr 29, 2021
@sylr sylr marked this pull request as draft April 29, 2021 13:49
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 29, 2021
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
@sylr sylr marked this pull request as ready for review April 30, 2021 07:23
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 30, 2021
@sylr
Copy link
Contributor Author

sylr commented May 18, 2021

Hi @brancz, @mrueg 👋

Could I have a first feedback about this ?

Thank you.

@brancz
Copy link
Member

brancz commented Jun 7, 2021

I'm generally happy with this. cc @lilic as she lead the label allow work

(I tried to approve the CI run, but github is returning 500 errors consistently every time I press it)

@lilic
Copy link
Member

lilic commented Jun 7, 2021

I get the same error yes, but works on other PRs. Can you maybe force push @sylr, thanks!

@lilic lilic mentioned this pull request Jun 7, 2021
@sylr
Copy link
Contributor Author

sylr commented Jun 7, 2021

I merged master into this branch to trigger the CI.

sylr added 2 commits June 7, 2021 15:59
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
@siegenthalerroger
Copy link

I'm just going to leave a link to the relevant issue here so that the PR is correctly linked to the issue with lots of discussion. #941

Furthermore it's probably important to get @sylr 's thoughts on #1495 as it appears to solve the same use-case but more closely following the design of the label metric

@sylr
Copy link
Contributor Author

sylr commented Jun 25, 2021

@lilic can you approve the workflow please ?

Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 6, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 7, 2021
Copy link
Member

@lilic lilic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of comments, thanks for your patience! We can do majority in a follow up as well if you agree to it or in this tone.

docs/cli-arguments.md Show resolved Hide resolved
@@ -172,6 +176,26 @@ func isPrefixedNativeResource(name v1.ResourceName) bool {
return strings.Contains(string(name), v1.ResourceDefaultNamespacePrefix)
}

// createAnnotationKeysValues takes in passed kubernetes annotations and allowed list in kubernetes label format
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// createAnnotationKeysValues takes in passed kubernetes annotations and allowed list in kubernetes label format
// createAnnotationKeysValues takes in passed kubernetes annotations and an allowed list in Kubernetes label format

internal/store/utils.go Outdated Show resolved Hide resolved
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
Co-authored-by: Damien Grisonnet <damien.grisonnet@epita.fr>
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 19, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 19, 2021
@sylr
Copy link
Contributor Author

sylr commented Aug 19, 2021

Guys, could we move forward and merge this ?

The size of it PR make it so basically any other PR merged onto master will create conflicts, I already performed two merge/conflicts resolutions.

Copy link
Member

@dgrisonnet dgrisonnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 19, 2021
@mrueg
Copy link
Member

mrueg commented Aug 19, 2021

/hold cancel
/lgtm
Thanks for your contribution @sylr !

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 19, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dgrisonnet, lilic, mrueg, sylr

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:

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

@k8s-ci-robot k8s-ci-robot merged commit 3193891 into kubernetes:master Aug 19, 2021
mrueg added a commit to mrueg/kube-state-metrics that referenced this pull request Aug 19, 2021
* [FEATURE] Add --use-apiserver-cache flag to set resourceVersion=0 for ListWatch requests kubernetes#1548
* [FEATURE] Introduce metrics for Kubernetes object annotations kubernetes#1468
* [FEATURE] Introduce start time metric for containers in terminated state kubernetes#1519
* [FEATURE] Introduce metrics for cronjob job history limits kubernetes#1535
* [FEATURE] Add system_uuid dimension to kube_node_info metric kubernetes#1535
* [FEATURE] Add available replica metric for statefulsets kubernetes#1532
* [FEATURE] Add ready replica metric for deployments kubernetes#1534
* [CHANGE]  Update go clients for Kubernetes to support 1.22 kubernetes#1545
* [CHANGE]  Use new promlint package and update prometheus cli to 2.28.1 kubernetes#1531
@mrueg mrueg mentioned this pull request Aug 19, 2021
arajkumar added a commit to syncbot-io/kube-state-metrics that referenced this pull request Aug 31, 2021
…eration

This PR fixes the regression introduced on `kube_persistentvolumeclaim_labels` functionality after kubernetes#1468. This also implements `kube_persistentvolumeclaim_annotations`.

Signed-off-by: Arunprasad Rajkumar <arajkuma@redhat.com>
mrueg pushed a commit to prometheus-community/helm-charts that referenced this pull request Sep 21, 2021
…#1362)

* Enable configuring kubernetes annotation metrics

With kubernetes/kube-state-metrics#1468 kube-state-metrics v2.2.0 added the possibility to configure annotation metrics, similar to how label metrics are currently configured.
This change adds a configuration option for configuring annotation metrics to this helm chart.

Signed-off-by: Maximilian Bischoff <maximilian.bischoff@inovex.de>

* Bump chart version

Signed-off-by: Maximilian Bischoff <maximilian.bischoff@inovex.de>
fpetkovski pushed a commit to fpetkovski/kube-state-metrics that referenced this pull request Sep 24, 2021
…eration

This PR fixes the regression introduced on `kube_persistentvolumeclaim_labels` functionality after kubernetes#1468. This also implements `kube_persistentvolumeclaim_annotations`.

Signed-off-by: Arunprasad Rajkumar <arajkuma@redhat.com>
fpetkovski pushed a commit to fpetkovski/kube-state-metrics that referenced this pull request Sep 24, 2021
…eration

This PR fixes the regression introduced on `kube_persistentvolumeclaim_labels` functionality after kubernetes#1468. This also implements `kube_persistentvolumeclaim_annotations`.

Signed-off-by: Arunprasad Rajkumar <arajkuma@redhat.com>
fpetkovski pushed a commit to fpetkovski/kube-state-metrics that referenced this pull request Sep 24, 2021
…eration

This PR fixes the regression introduced on `kube_persistentvolumeclaim_labels` functionality after kubernetes#1468. This also implements `kube_persistentvolumeclaim_annotations`.

Signed-off-by: Arunprasad Rajkumar <arajkuma@redhat.com>
QuentinBisson pushed a commit to giantswarm/prometheus-community-helm-charts-upstream that referenced this pull request Oct 5, 2021
…prometheus-community#1362)

* Enable configuring kubernetes annotation metrics

With kubernetes/kube-state-metrics#1468 kube-state-metrics v2.2.0 added the possibility to configure annotation metrics, similar to how label metrics are currently configured.
This change adds a configuration option for configuring annotation metrics to this helm chart.

Signed-off-by: Maximilian Bischoff <maximilian.bischoff@inovex.de>

* Bump chart version

Signed-off-by: Maximilian Bischoff <maximilian.bischoff@inovex.de>
Signed-off-by: QuentinBisson <quentin@giantswarm.io>
jan--f added a commit to jan--f/cluster-monitoring-operator that referenced this pull request Oct 12, 2021
These metrics were added in
kubernetes/kube-state-metrics#1468 but for us
don't add any valuable information.

Signed-off-by: Jan Fajerski <jfajersk@redhat.com>
galina-tochilkin pushed a commit to mtp-devops/3d-party-helm that referenced this pull request Aug 2, 2023
… (#1362)

* Enable configuring kubernetes annotation metrics

With kubernetes/kube-state-metrics#1468 kube-state-metrics v2.2.0 added the possibility to configure annotation metrics, similar to how label metrics are currently configured.
This change adds a configuration option for configuring annotation metrics to this helm chart.

Signed-off-by: Maximilian Bischoff <maximilian.bischoff@inovex.de>

* Bump chart version

Signed-off-by: Maximilian Bischoff <maximilian.bischoff@inovex.de>
dgrisonnet pushed a commit to dgrisonnet/cluster-monitoring-operator that referenced this pull request Sep 5, 2023
These metrics were added in
kubernetes/kube-state-metrics#1468 but for us
don't add any valuable information.

Signed-off-by: Jan Fajerski <jfajersk@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants