Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Included pprof for profiling the application. #485

Merged
merged 3 commits into from
Nov 15, 2021

Conversation

naveensrinivasan
Copy link
Contributor

Included pprof for profiling the application.

Signed-off-by: naveen 172697+naveensrinivasan@users.noreply.github.com

@dlorenc
Copy link
Member

dlorenc commented Nov 10, 2021

Is this safe to run in production? Would it make more sense to manually register the handlers via a flag?

Included pprof for profiling the application.

Signed-off-by: naveen <172697+naveensrinivasan@users.noreply.github.com>
@naveensrinivasan
Copy link
Contributor Author

naveensrinivasan commented Nov 10, 2021

Is this safe to run in production? Would it make more sense to manually register the handlers via a flag?

Looks like we can still run it in Prod. https://stackoverflow.com/a/64057856/19407

I agree, I have included a flag that will enable pprof. Would this work?

Copy link
Member

@bobcallaway bobcallaway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM minus small nits

cmd/rekor-server/app/root.go Outdated Show resolved Hide resolved
cmd/rekor-server/app/root.go Outdated Show resolved Hide resolved
@@ -88,6 +94,21 @@ func init() {
if GitVersion != "devel" {
return
}
log.Logger.Debugf("pprof enabled %v", enablePprof)
// Enable pprof
if enablePprof {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to check if the user has requested port 6060 for the main rekor endpoint and give a helpful error message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, I would pass on this because this flag has to be turned on and the default rekor endpoint port has to be set to 6060. The odds of that happening are less IMO. Also, we have included the docs in the command line it could help someone debug.

Thoughts?

cmd/rekor-server/app/root.go Outdated Show resolved Hide resolved
naveensrinivasan and others added 2 commits November 10, 2021 18:27
Co-authored-by: Bob Callaway <bobcallaway@users.noreply.github.com>
Co-authored-by: Bob Callaway <bobcallaway@users.noreply.github.com>
@dlorenc dlorenc merged commit e003278 into sigstore:main Nov 15, 2021
@naveensrinivasan naveensrinivasan deleted the naveen/pprof branch November 15, 2021 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants