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

doc: diagnostics document: additional questions and proposed changes #22027

Closed
inkel opened this issue Sep 25, 2017 · 4 comments
Closed

doc: diagnostics document: additional questions and proposed changes #22027

inkel opened this issue Sep 25, 2017 · 4 comments

Comments

@inkel
Copy link

inkel commented Sep 25, 2017

Thank you for putting together the diagnostics document, it is very useful for us people who's starting working with Go.

I'm creating this issue as I've additional doubts and @rakyll pointed me to here to ask for clarifications.


In the profiling section you have answered if it's possible to profile production services: I'm going to copy the answer here and add bold to the additional doubts that I have about it:

Yes. It is safe to profile programs in production, but enabling some profiles (e.g. the CPU profile) adds cost. You should expect to see performance downgrade. The performance penalty can be estimated by measuring the overhead of the profiler before turning it on in production.

You may want to periodically profile your production services. Especially in a system with many replicas of a single process, selecting a random replica periodically is a safe option. Select a production process, profile it for X seconds for every Y seconds and save the results for visualization and analysis; then repeat periodically. Results may be manually and/or automatically reviewed to find problems. Collection of profiles can interfere with each other, so it is recommended to collect only a single profile at a time.

This answer is very clear and concise, yet I've found myself trying to answer an additional question: how do one measure the overhead of the profiler? What do you mean by replicas, multiple copies of the same process running in the same machine?

I am guessing the answer might be obvious and it's escaping me due to my lack of enough experience, but I think that additional details like these could really improve the document.


Another question that I have was when reading about serving the profiler handlers in different path and port. The example used is:

mux := http.NewServeMux()
mux.HandleFunc("/custom_debug_path/profile", pprof.Profile)
http.ListenAndServe(":7777", mux)

However when I went to the net/http/pprof documentation I couldn't find a reference to pprof.Profile but rather I had to go to read the init function of that package to find how to do it. Am I missing something or is it just a bug in the document?


I've also found some package mentions like context.Context and httptrace.ClientTrace that don't provide a link to the documentation, which I think could be more than useful.


Sorry for the length of this issue, and if the answers are simple yet eluding me. Thank you for taking the time to read it! I would like to become a Go contributor someday so I'm more than willing to make changes to this document, provided I find out how to answer these questions 😺

@ianlancetaylor ianlancetaylor changed the title Diagnostics document: additional questions and proposed changes doc: diagnostics document: additional questions and proposed changes Sep 25, 2017
@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Sep 25, 2017
@rsc
Copy link
Contributor

rsc commented Dec 1, 2017

/cc @rakyll

@rsc rsc modified the milestones: Go1.10, Go1.11 Dec 1, 2017
@rakyll rakyll self-assigned this Dec 1, 2017
@gopherbot
Copy link

Change https://golang.org/cl/83916 mentions this issue: doc: make it clear which pprof package is used

@gopherbot
Copy link

Change https://golang.org/cl/83917 mentions this issue: doc: add some links to the diagnostics page

gopherbot pushed a commit that referenced this issue Dec 14, 2017
Updates #22027.

Change-Id: I468348d2b000f146f88ef8b7cf450eea8d1c12a7
Reviewed-on: https://go-review.googlesource.com/83917
Reviewed-by: Andrew Bonventre <andybons@golang.org>
gopherbot pushed a commit that referenced this issue Dec 14, 2017
Updates #22027.

Change-Id: I5a5bae77a744c7a2ecb75172846e6461a98ee8af
Reviewed-on: https://go-review.googlesource.com/83916
Reviewed-by: Andrew Bonventre <andybons@golang.org>
@rakyll
Copy link
Contributor

rakyll commented Dec 14, 2017

I am guessing the answer might be obvious and it's escaping me due to my lack of enough experience, but I think that additional details like these could really improve the document.

The answer is not always very obvious and might require some more than just a few lines. I do not think diagnostics page is the best place to give further advice. I will try to write a Go blog post instead. All of the other concerns are addressed in the CLs.

@rakyll rakyll closed this as completed Dec 14, 2017
@golang golang locked and limited conversation to collaborators Dec 14, 2018
@rsc rsc unassigned rakyll Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants