fix(mixin): filter empty-image cgroup series out of the CPU scaling rule - #16320
Conversation
narqo
left a comment
narqo
left a comment
There was a problem hiding this comment.
The changes look good to me. It seems that GitHub actions had issues right around the time when the PR was opened. Could you rebase the changes to re-trigger the CI. Thank you
cluster_namespace_deployment:container_cpu_usage_seconds_total:sum_rate sums container_cpu_usage_seconds_total without the image!="" selector that the sibling memory_usage rule already applies. Both read the same cAdvisor job, so where the scrape retains the sandbox and parent cgroup series next to the real per-container ones, the CPU rule counts the same CPU time twice. The recorded value feeds cpu_required_replicas_count, which divides CPU usage by CPU requests, so the Scaling dashboard recommends roughly double the replicas a deployment actually needs. Measured on one affected cAdvisor scrape: 368.9 cores unfiltered versus 183.5 with image!="", a 2.0x overcount. Fixes grafana#9005
Head branch was pushed to by a user without write access
4a8d170 to
4bf3471
Compare
|
Thanks for the review! Rebased onto latest main. I also rebased #16323, which was in the same state. The rebase alone doesn't seem to be enough though: the PR shows "33 workflows awaiting approval / This workflow requires approval from a maintainer", and on the new head every run comes back as |
2 participants
What this PR does
cluster_namespace_deployment:container_cpu_usage_seconds_total:sum_ratesumscontainer_cpu_usage_seconds_totalwith no selector, while the siblingmemory_usagerulealready reads
container_memory_usage_bytes{image!=""}. Both come from the same cAdvisor job,so the two are inconsistent: where the scrape keeps the sandbox and parent cgroup series next
to the per-container ones, the CPU rule counts the same CPU time twice.
This is not just a cosmetic wrong number, because
cpu_required_replicas_countdivides therecorded CPU usage by CPU requests. A 2x inflated numerator makes the Scaling dashboard
recommend roughly double the replicas a deployment actually needs.
Measured on an affected cAdvisor scrape: 368.9 cores unfiltered versus 183.5 with
image!="".This PR adds
image!=""to the CPU selector so it matchesmemory_usage. Installations thatalready drop those series at scrape time are unaffected. The compiled mixin and the helm
golden records are regenerated accordingly.
Which issue(s) this PR fixes or relates to
Fixes #9005
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]. If changelog entry is not needed, please add thechangelog-not-neededlabel to the PR.about-versioning.mdupdated with experimental features.