Add profiler server for pprof #591
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| package api | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "net" | ||
| "net/http" | ||
| _ "net/http/pprof" | ||
| "strconv" | ||
|
|
||
| "github.com/rs/zerolog" | ||
| ) | ||
|
|
||
| type ProfileServer struct { | ||
| logger zerolog.Logger | ||
| server *http.Server | ||
| endpoint string | ||
| } | ||
|
|
||
| func NewProfileServer( | ||
| logger zerolog.Logger, | ||
| host string, | ||
| port int, | ||
| ) *ProfileServer { | ||
| endpoint := net.JoinHostPort(host, strconv.Itoa(port)) | ||
| return &ProfileServer{ | ||
| logger: logger, | ||
| server: &http.Server{Addr: endpoint}, | ||
| endpoint: endpoint, | ||
| } | ||
| } | ||
|
|
||
| func (h *ProfileServer) ListenAddr() string { | ||
| return h.endpoint | ||
| } | ||
|
|
||
| func (s *ProfileServer) Start() { | ||
| go func() { | ||
| err := s.server.ListenAndServe() | ||
| if err != nil { | ||
| if errors.Is(err, http.ErrServerClosed) { | ||
| s.logger.Warn().Msg("Profiler server shutdown") | ||
| return | ||
| } | ||
| s.logger.Err(err).Msg("failed to start Profiler server") | ||
| panic(err) | ||
| } | ||
| }() | ||
| } | ||
|
Comment on lines
+37
to
+49
There was a problem hiding this comment. Consider error handling improvement in Start method. The Start method correctly launches the server in a goroutine for non-blocking operation. However, panicking in case of a server start failure might not be the best approach in a production environment. Consider logging the error and returning it to the caller instead of panicking. This allows the caller to decide how to handle the error. Here's a suggested modification: func (s *ProfileServer) Start() error {
- go func() {
+ errCh := make(chan error, 1)
+ go func() {
err := s.server.ListenAndServe()
if err != nil && !errors.Is(err, http.ErrServerClosed) {
s.logger.Err(err).Msg("failed to start Profiler server")
- panic(err)
+ errCh <- err
}
+ close(errCh)
}()
+
+ // Wait a short time to catch immediate failures
+ select {
+ case err := <-errCh:
+ return err
+ case <-time.After(100 * time.Millisecond):
+ return nil
+ }
}This change allows the method to return any immediate errors while still running the server in a goroutine.
|
||
|
|
||
| func (s *ProfileServer) Stop() error { | ||
| ctx, cancel := context.WithTimeout(context.Background(), shutdownTimeout) | ||
| defer cancel() | ||
|
|
||
| return s.server.Shutdown(ctx) | ||
| } | ||
|
Comment on lines
+51
to
+56
There was a problem hiding this comment. Define shutdownTimeout constant. The Stop method correctly uses a context with timeout for graceful shutdown. However, the Please define the const shutdownTimeout = 5 * time.SecondAdjust the duration as appropriate for your use case. |
||
|
|
||
| func (s *ProfileServer) Close() error { | ||
| return s.server.Close() | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,6 +97,12 @@ type Config struct { | |
| IndexOnly bool | ||
| // Cache size in units of items in cache, one unit in cache takes approximately 64 bytes | ||
| CacheSize uint | ||
| // ProfilerEnabled sets whether the profiler server is enabled | ||
| ProfilerEnabled bool | ||
| // ProfilerHost is the host for the profiler server will listen to (e.g. localhost, 0.0.0.0) | ||
| ProfilerHost string | ||
| // ProfilerPort is the port for the profiler server | ||
| ProfilerPort int | ||
| } | ||
|
|
||
| func FromFlags() (*Config, error) { | ||
|
|
@@ -159,6 +165,9 @@ func FromFlags() (*Config, error) { | |
| flag.StringVar(&walletKey, "wallet-api-key", "", "ECDSA private key used for wallet APIs. WARNING: This should only be used locally or for testing, never in production.") | ||
| flag.IntVar(&cfg.MetricsPort, "metrics-port", 9091, "Port for the metrics server") | ||
| flag.BoolVar(&cfg.IndexOnly, "index-only", false, "Run the gateway in index-only mode which only allows querying the state and indexing, but disallows sending transactions.") | ||
| flag.BoolVar(&cfg.ProfilerEnabled, "profiler-enabled", false, "Run the profiler server to capture pprof data.") | ||
| flag.StringVar(&cfg.ProfilerHost, "profiler-host", "localhost", "Host for the Profiler server") | ||
| flag.IntVar(&cfg.ProfilerPort, "profiler-port", 6060, "Port for the Profiler server") | ||
| flag.Parse() | ||
|
|
||
| if coinbase == "" { | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Address potential API key exposure
The static analysis tool has flagged potential API key exposure in these lines. This reinforces the need to move sensitive information out of the Makefile.
To address this:
.envfile (if used) is added to.gitignore.🧰 Tools
Gitleaks