-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Support controller-runtime integration for pprof
#16594
Comments
Tomorrow I'll try to draft a PR with this feature, you can assign the issue to me if you want :) |
This will be more complicated than I expected: The built-in feature was added in [email protected] but there are too much replaces to k8s 0.26 and there are several incompatibilities. Apart from that, only applicationset-controller uses config, err := clientConfig.ClientConfig()
....
kubeClient := kubernetes.NewForConfigOrDie(config)
appClient := appclientset.NewForConfigOrDie(config) I need to think about how to solve it in a single way for all the |
@JorTurFer I think those replaces are caused by having |
The problem is that the replaces aren't the only problem I faced with. As only the applicationset-controller is directly build on top of Based on that, my proposal is to create a helper shared for all ArgoCD components that starts up a server with the same endpoints as WDYT? |
@JorTurFer it makes sense to me, especially given that something like this would be useful for |
I think that we can include something like controller-runtime did: https://github.com/kubernetes-sigs/controller-runtime/pull/1943/files Basically with a few lines we could create an optional server that we enable based on a given config. I think that it's something easy and "clean" |
@JorTurFer makes sense to me 👍 |
nice! I'll draft a PR when I have some time :) |
Late is always better than never, so I've just drafted the PR: #17068 🤣 Let's move the discussion there :) |
Noting here that per #17068 (comment), EDIT: well, it was only enabled for the Application Controller and the Server (not all the current components may have existed at the time, the Repo Server may have been the only one missing then). |
Summary
We are facing with some performance issues that we want to troubleshoot. For going deeper, we want to profile the app for detecting the bottlenecks, but I've noticed that controller-runtime built-in integration isn't configured.
Controller-runtime can enable (or not) the profiling just configuring the address (or not). This is really easy to integrate/configure and allows profiling ArgoCD components just changing a value on cm and restarting a pod, which is so much better than adding the profiling code in the old (and hard) way.
Motivation
I have to profile applicationset-controller and it requires changes in code for enabling the profiler. It also means that I have to checkout the exact version I've deployed to base my changes on it, which is more effort. Integrating this built-in feature I'd not need to change a line of code and I can profile the components easily when I need it
Proposal
I'd just configure
PprofBindAddress
on controllers. IDK if reading the value from cm or maybe from argsThe text was updated successfully, but these errors were encountered: