-
Notifications
You must be signed in to change notification settings - Fork 621
Enable net/http/pprof handlers in pprof web UI. #943
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
base: main
Are you sure you want to change the base?
Conversation
This is so that pprof itself can be profiled when needed. Tested: started the web UI, collected /debug/pprof/heap and /debug/pprof/profile profiles.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #943 +/- ##
==========================================
- Coverage 66.86% 66.86% -0.01%
==========================================
Files 44 44
Lines 9824 9868 +44
==========================================
+ Hits 6569 6598 +29
- Misses 2794 2831 +37
+ Partials 461 439 -22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@@ -210,6 +210,9 @@ func defaultWebServer(args *plugin.HTTPServerArgs) error { | |||
mux := http.NewServeMux() | |||
mux.Handle("/ui/", http.StripPrefix("/ui", handler)) | |||
mux.Handle("/", redirectWithQuery("/ui", http.StatusTemporaryRedirect)) | |||
// Enable self-profiling handlers if net/http/pprof is imported by the main | |||
// program. |
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.
Are there going to be users who are unhappy about having this access point opened?
@@ -210,6 +210,9 @@ func defaultWebServer(args *plugin.HTTPServerArgs) error { | |||
mux := http.NewServeMux() | |||
mux.Handle("/ui/", http.StripPrefix("/ui", handler)) | |||
mux.Handle("/", redirectWithQuery("/ui", http.StatusTemporaryRedirect)) | |||
// Enable self-profiling handlers if net/http/pprof is imported by the main | |||
// program. | |||
mux.HandleFunc("/debug/pprof/", http.DefaultServeMux.ServeHTTP) |
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.
Should we apply the same auth checks (localhost etc.) that we apply to other handlers?
@@ -24,6 +24,8 @@ import ( | |||
|
|||
"github.com/chzyer/readline" | |||
"github.com/google/pprof/driver" | |||
|
|||
_ "net/http/pprof" // Enable profiling pprof itself in the web UI mode. |
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.
In the past I have wanted to profile startup performance itself. This approach does not allow profiling startup time. An alternative would be flags (separate ones will be needed for cpu and memory profiling).
Not sure how we exit cleanly though to allow the profile to be saved if we are using flags.
This is so that pprof itself can be profiled when needed.
Tested: started the web UI, collected /debug/pprof/heap and /debug/pprof/profile profiles.