-
Notifications
You must be signed in to change notification settings - Fork 1.6k
GA Metrics Cardinality Enforcement #4422
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
Conversation
rexagod
commented
Jan 20, 2024
- One-line PR description: Mark the feature as stable, since we've been on beta for a while now.
- Issue link: Metric cardinality enforcement #2305
keps/sig-instrumentation/2305-metrics-cardinality-enforcement/kep.yaml
Outdated
Show resolved
Hide resolved
keps/sig-instrumentation/2305-metrics-cardinality-enforcement/README.md
Outdated
Show resolved
Hide resolved
730e33f to
a7a372f
Compare
Mark the feature as stable, since we've been on beta for a while now. Signed-off-by: Pranshu Srivastava <[email protected]>
|
/assign |
|
/assign @dashpole |
| alpha: "v1.21" | ||
| beta: "v1.28" | ||
| stable: "v1.30" | ||
| disable-supported: true |
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.
Do we still support disabling it?
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.
I don't think we have historically flipped that after reaching GA. For the next year at least, people will be using older versions so it's still relevant for those. Not very consistent though. FYI, looks like 94 of "stable" KEPs still say "true", 25 say "false" (not sure if they were always false though):
[hi on] jbelamaric@jbelamaric:~/proj/gh/kubernetes/enhancements$ grep ^disable-supported $(grep -l '^stage: stable' $(find . -name kep.yaml)) | cut -d : -f 3 | sort | uniq -c
25 false
94 true
[hi on] jbelamaric@jbelamaric:~/proj/gh/kubernetes/enhancements$ 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.
Generally available means you can't disable the feature gate, according to https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/#feature-stages
|
PRR looks good, we need SIG approval though. Do we have any tooling/plan to identify potential run-away metrics cardinality so we can preemptively deal with this, rather than making the admin do it ad hoc? |
dgrisonnet
left a comment
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
|
@johnbelamaric we thought about that in the past not only in Kubernetes but also with the Prometheus community but we haven't been able to find any solution that would allow us to do it preemptively without too much overhead. Here is a thread I started at some point, but we didn't pursue it prometheus/client_golang#970. Since this cardinality enforcement method has been in place and metrics are more actively reviewed and developers are generally more aware of cardinality issues, cardinality explosions have been extremely rare and so we haven't discuss adding a preemptive mechanism since. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dgrisonnet, johnbelamaric, 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:
Approvers can indicate their approval by writing |