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
net/http/httptrace: add hooks for errors and response body closed #16400
Comments
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. |
CL https://golang.org/cl/25544 mentions this issue. |
@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 |
@bradfitz Here are the docs for the new hooks the way I imagined them. Let me know if you agree/disagree with me.
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. |
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? |
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. |
CL https://golang.org/cl/25510 mentions this issue. |
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. |
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
The text was updated successfully, but these errors were encountered: