Skip to content

Commit

Permalink
operator debug: fix pprof interval handling (#20206)
Browse files Browse the repository at this point in the history
The `nomad operator debug` command saves a CPU profile for each interval, and
names these files based on the interval.

The same functions takes a goroutine profile, heap profile, etc. but is missing
the logic to interpolate the file name with the interval. This results in the
operator debug command making potentially many expensive profile requests, and
then overwriting the data. Update the command to save every profile it scrapes,
and number them similarly to the existing CPU profile.

Additionally, the command flags for `-pprof-interval` and `-pprof-duration` were
validated backwards, which meant that we always coerced the `-pprof-interval` to
be the same as the `-pprof-duration`, which always resulted in a single profile
being taken at the start of the bundle. Correct the check as well as change the
defaults to be more sensible.

Fixes: #20151
  • Loading branch information
tgross committed Mar 25, 2024
1 parent bdf3ff3 commit 02d98b9
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 25 deletions.
3 changes: 3 additions & 0 deletions .changelog/20206.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
cli: Fixed a bug where `operator debug` did not respect the `-pprof-interval` flag and would take only one profile
```
28 changes: 14 additions & 14 deletions command/operator_debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ Debug Options:
-pprof-interval=<pprof-interval>
The interval between pprof collections. Set interval equal to
duration to capture a single snapshot. Defaults to 250ms or
-pprof-duration, whichever is less.
duration to capture a single snapshot. Defaults to 30s or
-pprof-duration, whichever is more.
-server-id=<server1>,<server2>
Comma separated list of Nomad server names to monitor for logs, API
Expand Down Expand Up @@ -372,7 +372,7 @@ func (c *OperatorDebugCommand) Run(args []string) int {
flags.BoolVar(&allowStale, "stale", false, "")
flags.StringVar(&output, "output", "", "")
flags.StringVar(&pprofDuration, "pprof-duration", "1s", "")
flags.StringVar(&pprofInterval, "pprof-interval", "250ms", "")
flags.StringVar(&pprofInterval, "pprof-interval", "30s", "")
flags.BoolVar(&c.verbose, "verbose", false, "")

c.consul = &external{tls: &api.TLSConfig{}}
Expand Down Expand Up @@ -439,7 +439,7 @@ func (c *OperatorDebugCommand) Run(args []string) int {
c.Ui.Error(fmt.Sprintf("Error parsing pprof-interval: %s: %s", pprofInterval, err.Error()))
return 1
}
if pi.Seconds() > pd.Seconds() {
if pi.Seconds() < pd.Seconds() {
pi = pd
}
c.pprofInterval = pi
Expand Down Expand Up @@ -1036,27 +1036,27 @@ func (c *OperatorDebugCommand) collectPprof(path, id string, client *api.Client,

// goroutine debug type 1 = legacy text format for human readable output
opts.Debug = 1
c.savePprofProfile(path, "goroutine", opts, client)
c.savePprofProfile(path, "goroutine", opts, client, interval)

// goroutine debug type 2 = goroutine stacks in panic format
opts.Debug = 2
c.savePprofProfile(path, "goroutine", opts, client)
c.savePprofProfile(path, "goroutine", opts, client, interval)

// Reset to pprof binary format
opts.Debug = 0

c.savePprofProfile(path, "goroutine", opts, client) // Stack traces of all current goroutines
c.savePprofProfile(path, "trace", opts, client) // A trace of execution of the current program
c.savePprofProfile(path, "heap", opts, client) // A sampling of memory allocations of live objects. You can specify the gc GET parameter to run GC before taking the heap sample.
c.savePprofProfile(path, "allocs", opts, client) // A sampling of all past memory allocations
c.savePprofProfile(path, "threadcreate", opts, client) // Stack traces that led to the creation of new OS threads
c.savePprofProfile(path, "goroutine", opts, client, interval) // Stack traces of all current goroutines
c.savePprofProfile(path, "trace", opts, client, interval) // A trace of execution of the current program
c.savePprofProfile(path, "heap", opts, client, interval) // A sampling of memory allocations of live objects. You can specify the gc GET parameter to run GC before taking the heap sample.
c.savePprofProfile(path, "allocs", opts, client, interval) // A sampling of all past memory allocations
c.savePprofProfile(path, "threadcreate", opts, client, interval) // Stack traces that led to the creation of new OS threads
}

// savePprofProfile retrieves a pprof profile and writes to disk
func (c *OperatorDebugCommand) savePprofProfile(path string, profile string, opts api.PprofOptions, client *api.Client) {
fileName := fmt.Sprintf("%s.prof", profile)
func (c *OperatorDebugCommand) savePprofProfile(path string, profile string, opts api.PprofOptions, client *api.Client, interval int) {
fileName := fmt.Sprintf("%s_%04d.prof", profile, interval)
if opts.Debug > 0 {
fileName = fmt.Sprintf("%s-debug%d.txt", profile, opts.Debug)
fileName = fmt.Sprintf("%s-debug%d_%04d.txt", profile, opts.Debug, interval)
}

bs, err := retrievePprofProfile(profile, opts, client, c.queryOpts())
Expand Down
14 changes: 7 additions & 7 deletions command/operator_debug_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -452,14 +452,14 @@ func TestDebug_CapturedFiles(t *testing.T) {
}

pprofFiles := []string{
"allocs.prof",
"goroutine-debug1.txt",
"goroutine-debug2.txt",
"goroutine.prof",
"heap.prof",
"allocs_0000.prof",
"goroutine-debug1_0000.txt",
"goroutine-debug2_0000.txt",
"goroutine_0000.prof",
"heap_0000.prof",
"profile_0000.prof",
"threadcreate.prof",
"trace.prof",
"threadcreate_0000.prof",
"trace_0000.prof",
}

clientFiles := []string{
Expand Down
7 changes: 3 additions & 4 deletions website/content/docs/commands/operator/debug.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,9 @@ true.
- `-pprof-duration=<duration>`: Duration for pprof
collection. Defaults to 1s or `-duration`, whichever is less.

- `-pprof-interval=<pprof-interval>`: The interval between pprof
collections. Set interval equal to duration to capture a single
snapshot. Defaults to 250ms or `-pprof-duration`, whichever is
less.
- `-pprof-interval=30s`: The interval between pprof collections. Set interval
equal to pprof duration to capture a single snapshot. Defaults to 30s or
`-pprof-duration`, whichever is more.

- `-server-id=<server1>,<server2>`: Comma separated list of Nomad server names to
monitor for logs, API outputs, and pprof profiles. Accepts server names, "leader", or
Expand Down

0 comments on commit 02d98b9

Please sign in to comment.