Chore: Add pprof to otto server #658
Conversation
StrongMonkey
requested review from
thedadams,
njhale,
iwilltry42 and
g-linville
pkg/server/server.go
Outdated
| @@ -15,6 +16,11 @@ import ( | |||
| var log = logger.Package() | |||
|
|
|||
| func Run(ctx context.Context, c services.Config) error { | |||
| go func() { | |||
There was a problem hiding this comment.
Do we want this to be always on? now that we have a released version, we probably want this off by default so that people aren't accidentally exposing debug info to the world.
There was a problem hiding this comment.
I'm a fan of leaving it on in production, because that's where we're most likely to run into issues that will require profiling info to resolve.
Is there's some other way to ensure we don't accidentally expose the port publicly?
There was a problem hiding this comment.
I don't have an issue with us having it on in production per se, but we shouldn't expose it for everybody who might run a copy of otto themselves.
There was a problem hiding this comment.
it is fine, I can only run it behind an env variable 👍
pkg/server/server.go
Outdated
| @@ -15,6 +16,11 @@ import ( | |||
| var log = logger.Package() | |||
|
|
|||
| func Run(ctx context.Context, c services.Config) error { | |||
| go func() { | |||
| log.Infof("Starting pprof on :6060") | |||
| log.Errorf(http.ListenAndServe("localhost:6060", nil).Error()) | |||
There was a problem hiding this comment.
pico-nit: The chance of one of us configuring otto8 wrong is basically zero, but if we wanted to be really pedantic, we could check c.HTTPListenPort and return an error if it conflicts with the pprof port
Signed-off-by: Daishan Peng <[email protected]>
StrongMonkey
force-pushed
the
add-pprof
branch
from
4870631 to
343c1b9
Compare
|
This is already enabled through /debug/pprof, dropping. |
Going to add pprof to be able to troubleshoot cpu usage in testing env.