Repository: go
Issue: runtime/pprof: NewCPUProfile + cpuProfile.Start to allow profile configuration
Labels: help wanted,Proposal,Proposal-Accepted,compiler/runtime

> I think SetCPUProfileRate is, albeit confusing, still functioning. It needs to be called in the right order with StartCPUProfile. Last time I looked (~a month ago), it probably prints some warnings, but it does change the rate.
>
> The pprof package doesn't seem to have a way to set the profiling rate. Not sure if this is really a useful feature.

_Originally posted by @cherrymui in https://github.com/golang/go/issues/40094#issuecomment-655005554_

The pprof package indeed does not allow us to customize the CPU profiling rate at the moment. This was something critical for my application in https://github.com/google/badwolf/pull/155, since the default profiling rate of 100 was extremely low and did not allow me no see anything in my CPU profile. 

To work around this, I had to make a call to `runtime.SetCPUProfileRate(800)` (with a rate of around 800 hz to see something meaningful and helpful in my CPU profile) before calling `pprof.StartCPUProfile`. Since there is a hard-coded call to `runtime.SetCPUProfileRate(100)` inside the `pprof.StartCPUProfile`, a warning is being printed to my terminal at the moment saying "runtime: cannot set cpu profile rate until previous profile has finished" as this call to `runtime.SetCPUProfileRate(100)` failed (the boolean `cpuprof.on` was already set to true in the previous call to `runtime.SetCPUProfileRate(800)`). This warning is annoying since it is misleading for someone who does not know the internals of `pprof.StartCPUProfile`, but the important here is that the CPU profiling is done with the first profiling rate that I manually set (800, as I wanted and as wanted from the user point of view).

Given this, what I am trying to say is that the public/exported method **`runtime.SetCPUProfileRate`** is still useful, and sometimes even critical for allowing us to use well `pprof` and to see something relevant on our profiles, depending on the application. I am not alone on this as you can see [here (stack overflow)](https://stackoverflow.com/questions/30871691/cant-get-golang-pprof-working) and [here (google groups discussion)](https://groups.google.com/forum/#!topic/Golang-nuts/8MaHpSINa0o). Even one principal software engineer (L8) I have been working with at Google found it useful when reviewing my PR https://github.com/google/badwolf/pull/155 above and copied the code to use it in one of his applications, on which setting a higher profiling rate was critical as well.

That said, I believe we could, from now on, follow three different paths:

1. The ideal would be for the **`pprof.StartCPUProfile`** method **already receive as argument the CPU profile rate** we want, allowing us to customize it, with this argument having its default value set to 100 if it was not specified by the function call. This would be the ideal, if we had support for default arguments in Go. But, we can achieve something similar if we had a similar method with signature `pprof.StartCPUProfileWithSpecifiedRate(w io.Writer, hz int)`, in addition to the `pprof.StartCPUProfile(w io.Writer)` method we already have. This way, the `runtime.SetCPUProfileRate` method would not necessarily need to be an exported method anymore.

2. We could **keep `runtime.SetCPUProfileRate` public/exported**, but we would have to fix the problem with the misleading warning message "runtime: cannot set cpu profile rate until previous profile has finished" if we had already manually set the profiling rate before the call to `pprof.StartCPUProfile`. For this we could:

    2.1. Have an auxiliar boolean method `runtime.isCPUProfileRateAlreadySet`, and call it inside `pprof.StartCPUProfile` before the hard-coded call to `runtime.SetCPUProfileRate(100)` there, trying to set the profiling rate to 100 only if this rate was not already previously set. This way, we would not see any strange warning after a call to `pprof.StartCPUProfile` if we had already previously set the profiling rate manually. I like this solution because it makes minimal changes on your code, it does not change any signature and it resolves exactly the problem we are trying to solve here, while keeping the warning message for other calls to `runtime.SetCPUProfileRate` that were not from `pprof.StartCPUProfile` in the situation above.

    2.2. Make `runtime.SetCPUProfileRate` return an error in the case the profiling rate was already previously set, allowing the caller to handle it and decide what to do from then on (inside `pprof.StartCPUProfile` we could ignore this error for example). This would change the signature of `runtime.SetCPUProfileRate`.

3. We could **move the `SetCPUProfileRate`** public signature **to the `pprof` package** (with some changes as explained above to avoid the misleading warning message), and deprecate it / make it unexported inside the `runtime` package.

These were just some of the alternatives that I thought about for now. I would like to hear more from you on what you think about them and if you have any other suggestions for fixing what I explained above.