net/http/pprof: respect timeouts set using http.ResponseController

[komuw]

Example program with issue: https://go.dev/play/p/DoZD1wMDPhw

Most time I find myself wanting to run my http requests with tight deadlines using http.Server.WriteTimeout.
While at the same time, I would like requests to the pprof endpoints to operate under more lenient timeouts.
For example I would set http.Server.WriteTimeout to 3 seconds and at the same time want to be able to take a pprof sample that would take a long time curl -vkL "http://localhost:9192/debug/pprof/profile?seconds=30"

I thought I would use http.NewResponseController to do that, see https://go.dev/play/p/DoZD1wMDPhw
However, that fails with profile duration exceeds server's WriteTimeout.
This is because the pprof handlers only consider the server timeout;

go/src/net/http/pprof/pprof.go

Lines 117 to 120 in a9d4b2d

	func durationExceedsWriteTimeout(r *http.Request, seconds float64) bool {
	srv, ok := r.Context().Value(http.ServerContextKey).(*http.Server)
	return ok && srv.WriteTimeout != 0 && seconds >= srv.WriteTimeout.Seconds()
	}

My proposal is that they should also consider any timeout that has been set by http.ResponseController.
I don't know how such a functionality might be implemented since durationExceedsWriteTimeout takes a http.Request and its context does not contain information on whether http.ResponseController has been used.

The current workaround is to use two servers, one for application handler and one for pprof handlers each with its own http.Server.WriteTimeout. But I like having one server.

[komuw]

Another option would be to re-implement https://github.com/golang/go/blob/master/src/net/http/pprof/pprof.go on my own without durationExceedsWriteTimeout. However that file imports internal/profile

[ianlancetaylor]

CC @neild @bradfitz

[komuw]

Another option would be to re-implement pprof.go ...
However that file imports internal/profile

Since serveDeltaProfile is the only one that uses the internal package. A way around it would be to export it.

// ServeDeltaProfile reports the difference between two measurements collected between the given seconds
// It can be used by people who want to register their own pprof handlers.
func ServeDeltaProfile(name string, w http.ResponseWriter, r *http.Request, p *pprof.Profile, secStr string)

[komuw]

I found a workaround, set http.ServerContextKey to a fake server which has a long timeout; https://go.dev/play/p/KaybTdDd1ll
This solves my issue and this proposal can be closed; unless someone thinks it still has merit and wants to change net/http/pprof itself.

[AlexanderYastrebov]

I guess it could also be an exported function:

// SetWriteTimeout configures write timeout for  HTTP handlers
func SetWriteTimeout(timeout time.Duration)

[lukestoward]

This solves my issue and this proposal can be closed; unless someone thinks it still has merit and wants to change net/http/pprof itself.

I have just stumbled on to this issue myself. I believe there is still merit in making the net/http/pprof more aware of timeout controls.

[gopherbot]

Change https://go.dev/cl/544756 mentions this issue: net/http/pprof: configure WriteDeadline

[AlexanderYastrebov]

I think we can adjust write timeout based on the requested profiling duration, see https://go-review.googlesource.com/c/go/+/544756

[neild]

Is there any good reason for net/http/pprof not to disable the write timeout entirely on its handlers?

[rsc]

Looked at the CL. I don't believe this needs a proposal but thanks for checking with us.

[AlexanderYastrebov]

I think the fix will benefit PGO.
@mknyszek What's the processes to merge "Ready to submit" CL?