proposal: net/http/pprof: provide a mechanism to render on /debug/pprof profiles than those that invoke pprof.NewProfile

[brancz]

I'm investigating the possibility of discovering available profiles of a process through the /debug/pprof/ index page. If that's a bad idea then we can stop right here 🙂 , but if not, then it would be great if there could be a mechanism to extend the profiles rendered on this page.

I have looked around and found that it appears that if profiles are created using the pprof.NewProfile function, then they are automatically included in profiles returned by pprof.Profiles(), which is what the /debug/pprof index page calls to retrieve the list of available profiles.

The custom profile I was looking at including is fgprof, however, fgprof is unsuitable for making use of pprof.NewProfile(). This led me to opening this issue.

Would it be possible to have another mechanism to add profile endpoints to be rendered into the /debug/pprof/ index page? I could see the possibility of an extensible list of endpoints in net/http/pprof/.

Happy to discuss other strategies if the general idea sounds acceptable, and once agreed I'd also be happy to contribute it.

Related: felixge/fgprof#5 parca-dev/parca#69

[odeke-em]

Thank you for this report @brancz and welcome to the Go project!
Awesome to see diversity of profilers in the Go community. I've converted this issue into a proposal since it is anyways a request
that we allow some hooks for registering hooks into net/http/pprof's /debug/pprof to render profiles.

The proposal committee shall take a look and also others, please feel to chime in here with ideas or suggestions.

[rsc]

It sounds like the request is to be able to add arbitrary HTML (or at least linked text) to the /debug/pprof page?
That's not something we've done before and it seems like it might easy to abuse.

Maybe instead the /debug page (which I think does nothing now) should list the available pages under /debug?
Then when fgprof registers /debug/fgprof, it gets listed in /debug automatically?

[brancz]

I don't think arbitrary HTML is required, just a link with a name, much like what profile.NewProfile(name) does (it wouldn't need to be more flexible than profile.NewProfile).

[rsc]

@brancz What is the exact API, with function signatures and doc comments, that you are proposing?

[brancz]

I'm thinking in the direction of a new function called AddProfile in the net/http/pprof package.

// AddProfile adds the profileHandler, serving a custom profile, to be served under `/debug/pprof/<name>` as well as list the profile with its description on the `/debug/pprof` index page.
func AddProfile(name, description string, profileHandler http.Handler)

[rsc]

It still seems a bit odd that this is for registering non-pprof profiles.

Above I suggested:

Maybe instead the /debug page (which I think does nothing now) should list the available pages under /debug? Then when fgprof registers /debug/fgprof, it gets listed in /debug automatically?

Any reaction to that? It seems clearer not to put it in the /debug/pprof list or URL.

[brancz]

What do you mean by non-pprof profiles? fgprof does produce profiles in pprof format.

Any reaction to that? It seems clearer not to put it in the /debug/pprof list or URL.

Maybe I misunderstood the suggestion. For me, if the result is an endpoint (I don't really care about the path) that lists all available profile endpoints and having this list be extensible with further profiles like fgprof, then that would be sufficient.

[rsc]

Sorry, I didn't realize fgprof used pprof format.

Suppose we did this somehow. It's not enough to hijack the HTTP view. We'd need to make runtime/pprof.Profiles include it in the list. That would mean some way to provide a different implementation that constructed a *Profile and could answer the WriteTo method directly. Maybe that would be enough?

type Custom interface {
    WriteTo(w io.Writer, debug int) error
}

func NewCustom(name string, impl Custom) *Profile

The Add/Count/Remove methods on this Profile would panic I guess.

This raises the question of why fgprof isn't using NewProfile and p.Add directly, but I assume there's a good reason.

[rsc]

I spent a little bit looking at fgprof. I don't see how this would help fgprof, which has additional parameters on the URL handler. /debug/pprof/fgprof would not accept (or pass through) the format= argument in this example from the home page:

curl -s 'localhost:6060/debug/fgprof?seconds=3&format=folded' > fgprof.folded

Also, fgprof seems to be a special case in that I am aware of no other profile that would need this functionality.
I still lean toward letting it use /debug/fgprof and leave things alone.

I looked at whether /debug/ can show an index of things registered under /debug/, but http.ServeMux doesn't make that available, and I'm reluctant to change that at this point.

It sounds like maybe there's not much to do here.

[brancz]

It does behave by default like the CPU profile though, producing pprof format instead of the folder format.

That said, if this is too niche, I could understand until there may be more cases like fgprof.

[felixge]

@rsc fgprof author here. The format argument for fgprof isn't critical, I think most people will prefer the default pprof output.

Also, do you think it's worth proposing something similar to fgprof to be included in the go project itself? I think that could simplify the integration with http/pprof, as it could be done without expanding the public API surface. If there is no obvious reason against it, I'd be happy to write a proposal and submit some code for it.

[rsc]

@felixge, I'm not really sure how viable fgprof is beyond small examples (and in particular single-goroutine programs).

I see in the code that fgprof samples all goroutines 99 times per second. That won't scale to large programs with 1000s of goroutines. (Not to mention that the current implementation stops the program entirely to get the stack traces.)

Instead we'd probably have to pick a random goroutine and profile that one. But we still don't have a way to stop a goroutine running on a different thread. I suppose we could pretend the OS has already chosen among the running threads fairly and say that if we randomly pick any running goroutine then the we record the one on the current thread. That might be OK. It's still even racy to grab the stack of a stopped goroutine without stopping the world, although maybe we could fix that (the fix would be pretty subtle).

But then once that is done, any multi-goroutine program is going to end up with a profile weighted by number of goroutines. Kick off 10 goroutines calling Sleep, and Sleep looks 10X more expensive than the rest of the program. It's just not clear to me that this is a useful kind of profile in general. I do see the benefit in small programs like in your example. It works out to a nice trace for those.

For us on the Go team, I think improved tracing would be a better place to spend our effort than working out how to build fgprof into the core Go runtime.

[rsc]

Based on the discussion above, this seems like a likely decline.

[felixge]

@rsc thanks for your detailed reply. Below are a few more thoughts on a potential proposal for supporting wallclock profiling similar to fgprof in Go itself. Please let me know if you really think it's a dead-end or if I should open a new issue for this.

@felixge, I'm not really sure how viable fgprof is beyond small examples (and in particular single-goroutine programs).

I see in the code that fgprof samples all goroutines 99 times per second. That won't scale to large programs with 1000s of goroutines. (Not to mention that the current implementation stops the program entirely to get the stack traces.)

Scalability is a concern, I agree.

Instead we'd probably have to pick a random goroutine and profile that one. But we still don't have a way to stop a goroutine running on a different thread. I suppose we could pretend the OS has already chosen among the running threads fairly and say that if we randomly pick any running goroutine then the we record the one on the current thread. That might be OK. It's still even racy to grab the stack of a stopped goroutine without stopping the world, although maybe we could fix that (the fix would be pretty subtle).

Yeah, I'd be happy to investigate and evaluate different approaches. I understand why the Go core should be hesitant to integrate fgprof "as is".

But then once that is done, any multi-goroutine program is going to end up with a profile weighted by number of goroutines. Kick off 10 goroutines calling Sleep, and Sleep looks 10X more expensive than the rest of the program. It's just not clear to me that this is a useful kind of profile in general. I do see the benefit in small programs like in your example. It works out to a nice trace for those.

I agree that the overall profile/flamegraph produced by fgprof will be unintelligible for non-trivial programs. That being said, once you filter the graph down to a stack prefix that corresponds to e.g. your http handler, the results become pretty useful in my experience.

For us on the Go team, I think improved tracing would be a better place to spend our effort than working out how to build fgprof into the core Go runtime.

I agree that fgprof is problematic as-is. But I think wallclock profiling is a big gap in Go's otherwise excellent profiling functionality. And the number of stars on the fgprof repo shows a decent amount of community interest.

So I'd be okay to go back to the drawing board for the implementation based on your concerns. If there are already tracing based ideas floating around, please let me know, otherwise I'd be happy to create a new issue/proposal for "wallclock profiling". What do you think?

[rsc]

If there are already tracing based ideas floating around, please let me know, otherwise I'd be happy to create a new issue/proposal for "wallclock profiling". What do you think?

I don't know of any tracing ideas that are concrete enough to be proposals.

I would suggest not proposing "wallclock profiling", for the reasons I gave in my earlier reply. One you get past small examples, it stops being useful. Probably the right thing to do is figure out more of a trace like the current trace profiles but perhaps less low level.

[felixge]

I would suggest not proposing "wallclock profiling", for the reasons I gave in my earlier reply. One you get past small examples, it stops being useful. Probably the right thing to do is figure out more of a trace like the current trace profiles but perhaps less low level.

Ok, got it. I'll take a look and let you know if I can come up with some ideas based on this direction.

[rsc]

No change in consensus, so declined.