Skip to content

Commit

Permalink
cmd/opm: serve pprof endpoints by default
Browse files Browse the repository at this point in the history
There is no substantial runtime cost to serving pprof endpoints, and
when things hit the fan and we need to investigate performance in situ,
there is no time to restart pods and change flags. Capturing profiles
remains opt-in, since those are costly.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
  • Loading branch information
stevekuznetsov committed Jul 27, 2023
1 parent 36b24a0 commit c6f8062
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
14 changes: 9 additions & 5 deletions cmd/opm/serve/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ type serve struct {
port string
terminationLog string

debug bool
pprofAddr string
debug bool
pprofAddr string
captureProfiles bool

logger *logrus.Entry
}
Expand Down Expand Up @@ -80,7 +81,8 @@ will not be reflected in the served content.
cmd.Flags().BoolVar(&s.debug, "debug", false, "enable debug logging")
cmd.Flags().StringVarP(&s.terminationLog, "termination-log", "t", "/dev/termination-log", "path to a container termination log file")
cmd.Flags().StringVarP(&s.port, "port", "p", "50051", "port number to serve on")
cmd.Flags().StringVar(&s.pprofAddr, "pprof-addr", "", "address of startup profiling endpoint (addr:port format)")
cmd.Flags().StringVar(&s.pprofAddr, "pprof-addr", "localhost:6060", "address of startup profiling endpoint (addr:port format)")
cmd.Flags().BoolVar(&s.captureProfiles, "pprof-capture-profiles", false, "capture pprof CPU profiles")
cmd.Flags().StringVar(&s.cacheDir, "cache-dir", "", "if set, sync and persist server cache directory")
cmd.Flags().BoolVar(&s.cacheOnly, "cache-only", false, "sync the serve cache and exit without serving")
cmd.Flags().BoolVar(&s.cacheEnforceIntegrity, "cache-enforce-integrity", false, "exit with error if cache is not present or has been invalidated. (default: true when --cache-dir is set and --cache-only is false, false otherwise), ")
Expand All @@ -92,8 +94,10 @@ func (s *serve) run(ctx context.Context) error {
if err := p.startEndpoint(); err != nil {
return fmt.Errorf("could not start pprof endpoint: %v", err)
}
if err := p.startCpuProfileCache(); err != nil {
return fmt.Errorf("could not start CPU profile: %v", err)
if s.captureProfiles {
if err := p.startCpuProfileCache(); err != nil {
return fmt.Errorf("could not start CPU profile: %v", err)
}
}

// Immediately set up termination log
Expand Down
2 changes: 1 addition & 1 deletion pkg/cache/tar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func Test_fsToTar(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
w := bytes.Buffer{}
err := fsToTar(&w, tc.fsys())
err := fsToTar(&w, tc.fsys(), nil)
tc.expect(t, w.Bytes(), err)
})
}
Expand Down

0 comments on commit c6f8062

Please sign in to comment.