Support filtering label allowlist by "*" #1823

rexagod · 2022-09-01T22:27:35Z

What this PR does / why we need it: Support filtering label allowlist by "*", which will expand to the enabled resources, while inferring their values based on its value(s).

How does this change affect the cardinality of KSM: No change.

Which issue(s) this PR fixes: Fixes #1815.

k8s-ci-robot · 2022-09-01T22:27:43Z

Welcome @rexagod!

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

rexagod · 2022-09-09T10:35:37Z

cc @dgrisonnet @mrueg

dgrisonnet · 2022-10-21T15:49:31Z

.gitignore

@@ -37,6 +37,7 @@ _testmain.go

 *.iml
 .idea/
+.run/


since this is not generated by ksm, it shouldn't be added to the gitignore

dgrisonnet

I think this will be a pretty useful feature, thank you for taking it up @rexagod.

There are a couple of things that would need to be done though before merging this effort:

Update the description of the command to explain the new behavior:

kube-state-metrics/pkg/options/options.go

Line 110 in a3a509a

    
           o.flags.Var(&o.LabelsAllowList, "metric-labels-allowlist", "Comma-separated list of additional Kubernetes label keys that will be used in the resource' labels metric. By default the metric contains only name and namespace labels. To include additional labels provide a list of resource names in their plural form and Kubernetes label keys you would like to allow for them (Example: '=namespaces=[k8s-label-1,k8s-label-n,...],pods=[app],...)'. A single '*' can be provided per resource instead to allow any labels, but that has severe performance implications (Example: '=pods=[*]').")

Add a new test for the all wildcard in

kube-state-metrics/pkg/options/types_test.go

Line 197 in a3a509a

func TestLabelsAllowListSet(t *testing.T) {
Test that the all wildcard really returns label metrics for all the resources. This could be an e2e test like the one we have to test the --resources option in or a unit test in for the builder:

kube-state-metrics/tests/e2e/main_test.go

Line 247 in a3a509a

func TestDefaultCollectorMetricsAvailable(t *testing.T) {

mrueg · 2022-10-21T17:27:17Z

internal/store/builder.go

@@ -202,7 +202,16 @@ func (b *Builder) WithAllowAnnotations(annotations map[string][]string) {
 // WithAllowLabels configures which labels can be returned for metrics
 func (b *Builder) WithAllowLabels(labels map[string][]string) {
 	if len(labels) > 0 {
-		b.allowLabelsList = labels
+		// "all" takes precedence over other specifications
+		if allowedLabels, ok := labels["all"]; ok {


Can we use * instead of all?

rexagod · 2022-11-06T22:54:03Z

Ready for review.

internal/store/builder.go

Support filtering label allowlist by "*", which will expand to the enabled resources, while infering their values based on its value(s). Signed-off-by: Pranshu Srivastava <[email protected]>

mrueg

/lgtm
/hold
Thanks!

dgrisonnet · 2022-11-08T11:41:20Z

Awesome, great stuff @rexagod!

/lgtm
/approve

/unhold

k8s-ci-robot · 2022-11-08T11:41:27Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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~~ [dgrisonnet,mrueg]

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 added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 1, 2022

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 1, 2022

k8s-ci-robot requested review from dgrisonnet and mrueg September 1, 2022 22:28

dgrisonnet reviewed Oct 21, 2022

View reviewed changes

mrueg reviewed Oct 21, 2022

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 21, 2022

rexagod force-pushed the 1815 branch from 3e66989 to 576b7e7 Compare November 6, 2022 19:58

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 6, 2022

rexagod force-pushed the 1815 branch 3 times, most recently from 730713f to 1919a94 Compare November 6, 2022 20:54

rexagod changed the title ~~Support filtering label allowlist by "all"~~ Support filtering label allowlist by "*all*" Nov 6, 2022

rexagod changed the title ~~Support filtering label allowlist by "*all*"~~ Support filtering label allowlist by "*" Nov 6, 2022

mrueg reviewed Nov 7, 2022

View reviewed changes

internal/store/builder.go Outdated Show resolved Hide resolved

rexagod force-pushed the 1815 branch from 1919a94 to 62b1fee Compare November 7, 2022 11:53

mrueg reviewed Nov 7, 2022

View reviewed changes

internal/store/builder.go Outdated Show resolved Hide resolved

Support filtering label allowlist by "*"

d982bbc

Support filtering label allowlist by "*", which will expand to the enabled resources, while infering their values based on its value(s). Signed-off-by: Pranshu Srivastava <[email protected]>

rexagod force-pushed the 1815 branch from 62b1fee to d982bbc Compare November 7, 2022 18:30

mrueg approved these changes Nov 8, 2022

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 8, 2022

k8s-ci-robot assigned mrueg Nov 8, 2022

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 8, 2022

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

k8s-ci-robot assigned dgrisonnet Nov 8, 2022

k8s-ci-robot merged commit d5bd2c8 into kubernetes:master Nov 8, 2022

xonvanetta mentioned this pull request Oct 30, 2023

feat: Support filtering annotations allowlist by "*" #2234

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support filtering label allowlist by "*" #1823

Support filtering label allowlist by "*" #1823

rexagod commented Sep 1, 2022 •

edited

Loading

k8s-ci-robot commented Sep 1, 2022

rexagod commented Sep 9, 2022

dgrisonnet Oct 21, 2022 •

edited

Loading

dgrisonnet left a comment •

edited

Loading

mrueg Oct 21, 2022

rexagod commented Nov 6, 2022

mrueg left a comment

dgrisonnet commented Nov 8, 2022

k8s-ci-robot commented Nov 8, 2022

@@ @@ -37,6 +37,7 @@ _testmain.go @@
               *.iml
               .idea/
+              .run/

Support filtering label allowlist by "*" #1823

Support filtering label allowlist by "*" #1823

Conversation

rexagod commented Sep 1, 2022 • edited Loading

k8s-ci-robot commented Sep 1, 2022

rexagod commented Sep 9, 2022

dgrisonnet Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

dgrisonnet left a comment • edited Loading

Choose a reason for hiding this comment

mrueg Oct 21, 2022

Choose a reason for hiding this comment

rexagod commented Nov 6, 2022

mrueg left a comment

Choose a reason for hiding this comment

dgrisonnet commented Nov 8, 2022

k8s-ci-robot commented Nov 8, 2022

rexagod commented Sep 1, 2022 •

edited

Loading

dgrisonnet Oct 21, 2022 •

edited

Loading

dgrisonnet left a comment •

edited

Loading