-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
🌱 Revert "Add nilIsZero to all KSM metric configs where needed" #11107
🌱 Revert "Add nilIsZero to all KSM metric configs where needed" #11107
Conversation
This reverts commit 82adc39. Signed-off-by: Tobias Giese <tgiese@nvidia.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
LGTM label has been added. Git tree hash: 64c29fc5c25384501d23775fbed56edf73e34a64
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chrischdi 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 |
I assume this time we tried the config? :) |
For reference, this is the previous PR: #11101 |
Thanks for cleaning it up again :-) |
The config was also tested with the previous PR. There is no error message, it simply does not work (the option has no effect). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
This reverts commit 82adc39.
What this PR does / why we need it:
Unfortunately,
nilIsZero
is not implemented fortype: StateSet
, seeconfig_metrics_types.go#L48-L59
. Sorry folks 😞Also there is no error in the logs using it for this type.
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 #
/area metrics