Add pprof to all VPA components #7153
Conversation
This change was inspured by how the cluster-autoscaler impliments pprof. Fixes kubernetes#6946
k8s-ci-robot
added
kind/feature
|
Hi @adrianmoisey. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
the
size/XXL
|
@kwiesmueller any chance you can look at this? |
|
/assign @voelzmo @raywainman |
vertical-pod-autoscaler/go.mod
Outdated
| @@ -13,6 +13,7 @@ require ( | |||
| golang.org/x/time v0.4.0 | |||
| k8s.io/api v0.28.3 | |||
| k8s.io/apimachinery v0.28.3 | |||
| k8s.io/apiserver v0.0.0-00010101000000-000000000000 | |||
There was a problem hiding this comment.
It's weird that it is using a pseudo-version number... Do you know why it is importing this?
What happens if you try to pull in a specific version? Maybe we can use the same as cluster autoscaler?
autoscaler/cluster-autoscaler/go.mod
Line 47 in 386f0f7
(I'm not an expert in this so sorry if this is totally off - this just looks weird to me)
There was a problem hiding this comment.
I think 679d6bd fixes it
I used 0.28.3, since that's what the VPA currently uses
| metrics_admission.Register() | ||
|
|
||
| go func() { | ||
| pathRecorderMux := mux.NewPathRecorderMux("vertical-pod-autoscaler") |
There was a problem hiding this comment.
reading the documentation and it looks like this name is used for logging, should we make this unique across each binary?
eg. here we would make it vertical-pod-autoscaler-admission-controller
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: adrianmoisey The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
/assign voelzmo |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Add proof to all 3 VPA components.
Which issue(s) this PR fixes:
Fixes #6946
Special notes for your reviewer:
The change was inspired by the cluster-autoscaler implementation of pprof. I figured it made sense to move a lot of the logic out of the metrics package, as that code handled metrics, health check and pprof.
EDIT: I don't know if I handled the dependencies and vendoring correctly, so I left them in their own commit in case that needs to be changed.
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: