Skip to content
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 TLS handshake timing information #16965

Closed
sajal opened this issue Sep 2, 2016 · 6 comments
Closed

net/http/httptrace: Add TLS handshake timing information #16965

sajal opened this issue Sep 2, 2016 · 6 comments

Comments

@sajal
Copy link

@sajal sajal commented Sep 2, 2016

httptrace currently does not offer a way to measure the TLS handshake of a request when using https. This might be trivial to implement in net/http/transport.dialConn by adding a few more callbacks in httptrace. I suggest TLSHandshakeStart and TLSHandshakeDone .

A hacky solution I am considering for now is to use ConnectDone as a proxy for TLSHandshakeStart and WroteRequest as a proxy for TLSHandshakeDone, but its better to have the real measurements.

PS: Thanks a lot for httptrace. I am now removing lots of ugly hacks from my project and letting the standard library do its thing.

@quentinmit quentinmit added this to the Go1.8Maybe milestone Sep 6, 2016
@quentinmit
Copy link
Contributor

@quentinmit quentinmit commented Sep 6, 2016

/cc @bradfitz

@moorereason
Copy link

@moorereason moorereason commented Oct 1, 2016

The delta between ConnectDone and GotConn is the TLS negotiation time. My understanding is that ConnectDone means "TCP dial completed" and GotConn means the http package received a connection on which it can begin sending HTTP requests. The TLS handshake happens between those two events.

If nothing else, the godocs could be updated to make this more apparent.

@freeformz
Copy link
Contributor

@freeformz freeformz commented Oct 4, 2016

If there is interest in a CL for this I will submit one. I was thinking of adding the following methods to httptrace.ClientTrace:

    // TLSHandshakeStart is called when tls.Handshake() is started.
    // When connecting to a https destination through a https proxy
    // TLSHandshakeStart could be called twice
    TLSHandshakeStart func()

    // TLSHandshakeDone is called after tls.Handshake is done
    // When connecting to a https destination through a https proxy
    // TLSHandshakeDone could be called twice.
    TLSHandshakeDone func(tls.ConnectionState)
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Oct 4, 2016

The "could be called twice" part is a little confusing. It should probably distinguish which conn it is.

How do we deal with the proxy conn stuff elsewhere in httptrace?

@freeformz
Copy link
Contributor

@freeformz freeformz commented Oct 5, 2016

@bradfitz Looks like I misread the code last night.

How just this instead...

    // TLSHandshakeStart is called when tls.Handshake() is started.
    TLSHandshakeStart func()

    // TLSHandshakeDone is called after tls.Handshake is done
    TLSHandshakeDone func(tls.ConnectionState)

Side Note: When we connect to a https site through a http proxy, the TLSHandshakeStart/Done fire after the CONNECT bridges the connection through the proxy. Not sure if we should bother calling that out or not.

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 5, 2016

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

@quentinmit quentinmit added the NeedsFix label Oct 10, 2016
@gopherbot gopherbot closed this in abdd73c Oct 19, 2016
@golang golang locked and limited conversation to collaborators Oct 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.