kube-state-metrics
kube-state-metrics copied to clipboard
Improve --metric-labels-allowlist handling of non-plural resource names
What would you like to be added:
--metric-labels-allowlist to support singular form pod vs pods for resources or warn when user uses singular form of common, predefined resources.
Why is this needed:
Currently, it is trivial to make a mistake when configuring KSM's labels allowlist and provide a singular form of resource name pod, namespace etc. This would not result in any error or warning, but also will not make KSM capture the metrics. This behaviour can confuse the end user.
Describe the solution you'd like Option 1: Support both singular and plural forms. Option 2: Warn when singular form is discovered.
Additional context Happy to code the solution once we agree on the approach to take :)
Thanks, I think this would make sense. I'm just not sure if it is simple to implement without explicit discovery against the API server.
For option 2 - would layman's approach of validating s or es ending of resource name and warning along the lines:
Resource x does not use a required plural form. Ignoring resource x in --metrics-labels-allowlist argument.
work?
Since we have an explicit list of resources from which we export metrics, I think I would prefer a solution where we check against that list and emit a warning if the resource is not present. We can also add a hint to use plural forms, but I wouldn't base it on the ending of a word.
/assign
@dmitri-lerko PLMK if you're not working on this at the moment and I'll pick it back up.
Do we want to diverge from the behavior that --resource follows (fail on singulars), and just warn the user in case of --metric-labels-allowlist? In my PR I've assumed the fact that a singular resource input comes from the user as a mistake, in which case, we fail. But LMK if we still just want to warn the user and I'll make the change.
I would still treat resources in the singular form as invalid, it will just complexify the codebase to have to support both and we would have to propagate that change to all the options that assume that only plural resources are provided.
But I agree that we should definitely add some explicit warnings, I would even go as far as exiting with an error when ksm is misconfigured.