VAULT-12732: Add Heap Profiling Option to Vault Server Command Line #27033
Conversation
github-actions
bot
added
the
hashicorp-contributed-pr
|
CI Results: |
|
Build Results: |
Co-authored-by: Sarah Chavis <[email protected]>
command/server.go
Outdated
| c.logger.Info(fmt.Sprintf("Wrote pprof files to: %s", pprofPath)) | ||
| case <-heapProfCh: | ||
| path := c.flagHeapProfile | ||
| if _, err := os.Stat(path); err != nil { |
There was a problem hiding this comment.
Isn't this going to error if the path to save the files in doesn't exist, which means we never make it to line 1832 below, where you're creating the new directory?
Off the cuff, it feels like we need 3 different branches of logic here:
- err is nil, meaning the path exists, so that's where we put the files
- err is not nil but the error is ErrNotExist, which is fine, and not actually an error case for us, in which case we need to create the directory
- err is not nil and is also not ErrNotExist, meaning there's some legitimate error that will prevent this process from continuing, in which case we need to exit early.
Am I misunderstanding this?
There was a problem hiding this comment.
Added a check for ErrNotExist, I was previously lumping case 2 and 3 together.
| @@ -1809,6 +1821,49 @@ func (c *ServerCommand) Run(args []string) int { | |||
| pFile.Close() | |||
| } | |||
|
|
|||
| c.logger.Info(fmt.Sprintf("Wrote pprof files to: %s", pprofPath)) | |||
There was a problem hiding this comment.
It feels like this log message would be better moved to inside the case statement, after everything has completed, no?
There was a problem hiding this comment.
This is actually an already existing log from the case above (git doing some interesting diffing here). I will add something to the message however, so that the two log messages aren't identical.
| continue | ||
| } | ||
|
|
||
| dumps := []string{"goroutine", "heap", "allocs", "threadcreate", "profile"} |
There was a problem hiding this comment.
The command line flag references "heap profile" but we're actually writing out many profiles here. Maybe it would be a good idea to rename the CLI flag to better reflect what we're actually doing?
Background
Currently, we utilize
sys/pprofand/orvault debugto generate pprof dumps for performance and debugging purposes. However, these tools are not effective if the issue we want to investigate occurs during the startup phase, or if Vault exits too quickly to issue such requests.Objective
To address this limitation, this PR introduces a new command-line option to Vault that allows specifying a file path to generate a heap profile dump at startup. This enhancement is intended to facilitate debugging in scenarios where Vault starts and exits rapidly, making it challenging to use the existing profiling tools.
Implementation
New Command-Line Option:
-pprof-dump-dir=/path/to/dirUsage: This is a one-off option and does not require HCL configuration. It is intended for temporary use during debugging sessions and not for repeated or regular use.
Example
To start the Vault server and generate a heap profile dump, use the following command: