net/http/httptrace: add hooks for errors and response body closed

[dominikh]

The current version of httptrace allows tracking most states of a request's live cycle, except for an overall error (all dials failed or protocol errors) and the closing of the response body. Both of these things can already be tracked by writing a custom RoundTripper, however if httptrace had hooks for these things, it would allow for the full instrumentation (e.g. metrics and tracing) of requests without needing to split it up between two mechanisms.

/cc @bradfitz

[msterle]

I also submitted a WIP for an error hook at https://go-review.googlesource.com/#/c/25544/.

This is my first contribution to the golang project, so please let me know if I should do anything differently in the future, or if you have any tips.

[gopherbot]

CL https://golang.org/cl/25544 mentions this issue.

[bradfitz]

@dominikh, if you're interested in this, please help do the code review and write the docs for what the hook would look like. See https://golang.org/cl/25544

[dominikh]

@bradfitz Here are the docs for the new hooks the way I imagined them. Let me know if you agree/disagree with me.

type ClientTrace struct {
    // GetConnFailed is called when no connection could be obtained.
    // It will be called at most once per request. The individual Dial
    // errors will be reported by calls to ConnectDone.
    GetConnFailed func()

    // BodyClosed is called after the response body has been closed.
    // If err is nil, no read or close error occured. If err is
    // non-nil, it contains the first error that was returned by Read
    // or Close.
    BodyClosed func(err error)
}

This should allow complete instrumentation of a request, without needing a custom RoundTripper or user-code after Close. If the request couldn't be made at all, either GetConnFailed or WroteRequest with a non-nil Err in WroteRequestInfo will be called. If the request could be made, but an error occured while reading from the body, BodyClosed will be called with a non-nil error. If no errors occured, BodyClosed will be called with a nil error.

[dominikh]

I'm not sure what to do with errors such as "nil Request.URL" or "nil Request.Header" though. I would be tempted to report these via the WroteRequestInfo hook, too. Alternatively, a third hook would need to be added. Ideas?

[dominikh]

I'll withdraw my feature request. It would need to add tracing to the Client itself, while not actually solving any problems. My original intention was to avoid implementing a custom RoundTripper. However, even with these new hooks, a RoundTripper may be necessary to handle redirects correctly. And once one has a custom RoundTripper, the missing hooks can be implemented manually.

[gopherbot]

CL https://golang.org/cl/25510 mentions this issue.

[bradfitz]

Closing per @dominikh's comment. Happy to revisit this in the future after people have more experience with httptrace and its features & shortcomings. It was designed to be extensible, so I'm happy to extend it later.