Rename cli flag from label-metrics-allow-list to metric-labels-allowlist #1424
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/XS
lilic
reviewed
Mar 26, 2021
docs/cli-arguments.md
Outdated
| --log_backtrace_at traceLocation when logging hits line file:N, emit a stack trace (default :0) | ||
| --log_dir string If non-empty, write log files in this directory | ||
| --log_file string If non-empty, use this log file | ||
| --log_file_max_size uint Defines the maximum size a log file can grow to. Unit is megabytes. If the value is 0, the maximum file size is unlimited. (default 1800) | ||
| --logtostderr log to standard error instead of files (default true) | ||
| --metric-allowlist string Comma-separated list of metrics to be exposed. This list comprises of exact metric names and/or regex patterns. The allowlist and denylist are mutually exclusive. | ||
| --metric-denylist string Comma-separated list of metrics not to be enabled. This list comprises of exact metric names and/or regex patterns. The allowlist and denylist are mutually exclusive. | ||
| --metric-labels-passthrough string 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],...)' |
There was a problem hiding this comment.
Either is fine by me, whatever is less confusing for our users. @brancz @tariq1890 thoughts for or against this change? We should get it into rc.1 if we want it.
There was a problem hiding this comment.
hmm I think I preferred allowlist, but whatever we do, the exact spelling should be consistent with other flags .. I don't feel strongly whether allow-list or allowlist, but it should be consistent.
There was a problem hiding this comment.
Sounds good to me as well, @mrueg would you be okay with making the change to the allowlist instead?
In either case I would say lets get this in before the rc.1 is out.
There was a problem hiding this comment.
Thanks for the feedback. I changed it to metric-labels-allowlist and regenerated the docs.
This might be more clear on what the flag is doing. There were two ways to spell allow-list / allowlist which it removes as well Fixes: kubernetes#1421
k8s-ci-robot
added
size/M
mrueg
changed the title
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lilic, mrueg 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 |
k8s-ci-robot
added
the
approved
mrueg
added a commit
to mrueg/kube-state-metrics
that referenced
this pull request
Mar 26, 2021
* [CHANGE] Rename --labels-metric-allow-list to --metric-labels-allowlist kubernetes#1424 * [CHANGE] Remove deprecated Kubernetes APIs kubernetes#1423 * [CHANGE] Remove vendor folder kubernetes#1419
Merged
mrueg
added a commit
to mrueg/kube-state-metrics
that referenced
this pull request
Mar 26, 2021
* [CHANGE] Rename --labels-metric-allow-list to --metric-labels-allowlist kubernetes#1424 * [CHANGE] Remove deprecated Kubernetes APIs kubernetes#1423 * [CHANGE] go.mod: Update Dependencies kubernetes#1419 * [CHANGE] Remove vendor folder kubernetes#1419 * [CHANGE] `k8s.gcr.io/kube-state-metrics/kube-state-metrics` becomes the authoritative registry Location on quay.io will not be updated anymore. Previously pushed images will be kept around to avoid breaking existing deployments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
This might be more clear on what the flag is doing. There were two ways
to spell allow-list / allowlist which it removes as well
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: #1421