-
Notifications
You must be signed in to change notification settings - Fork 983
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
Add net/http/pprof
profiling endpoint to http port
#913
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Tim Vaillancourt <[email protected]>
e06d910
to
dd7f12f
Compare
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Thanks! We will consider this, but I'm not sure if you are aware there is already a method to get pprof traces using the SSH debug interface. Once you connect you can run Defined here in the code: https://github.com/slackhq/nebula/blob/v1.7.2/ssh.go#L228 |
Signed-off-by: Tim Vaillancourt <[email protected]>
net/http/pprof
profiling endpoint to http port
Thanks @wadey! I didn't notice that when I started this branch, but saw mentions of it in issues later 👍 It seems we don't enable the |
This PR adds the ability to gather
pprof
profiles from Nebula over HTTP in order to understand/improve performance and understand the high CPU usage we see in Slack's production Vitess platformThis change requires making the http server opened by
stats.go
to become more generic than "just stats", so some logic has been shuffled intohttp.go
and a new, generic config variablehttp.listen
has been added to define this portFor backwards compatibility,
http.listen
will fallback to using the value forstats.listen
ifhttp.listen
is undef. I suggest thestats.listen
var (and the fallback) is removed in the future because the http port will not be just stats after this PR