http probe - fix how the tls phase is calculated #758
Conversation
Signed-off-by: kgersen <[email protected]>
| @@ -411,6 +424,8 @@ func ProbeHTTP(ctx context.Context, target string, module config.Module, registr | |||
| ConnectDone: tt.ConnectDone, | |||
| GotConn: tt.GotConn, | |||
| GotFirstResponseByte: tt.GotFirstResponseByte, | |||
| TLSHandshakeStart: tt.TLSHandshakeStart, | |||
There was a problem hiding this comment.
this got introduced in Go 1.8, which is fine, as we are already requiring at least Go 1.13
| durationGaugeVec.WithLabelValues("tls").Add(trace.gotConn.Sub(trace.dnsDone).Seconds()) | ||
| } else { | ||
| durationGaugeVec.WithLabelValues("connect").Add(trace.gotConn.Sub(trace.dnsDone).Seconds()) | ||
| durationGaugeVec.WithLabelValues("tls").Add(trace.tlsDone.Sub(trace.tlsStart).Seconds()) |
There was a problem hiding this comment.
this is subtly changing the meaning of the tls phase, but I think it's OK since this reflects more accurately the duration of the TLS handshake. The existing method is measuring the handshake plus whatever time it takes to finish setting the connection up plus whatever time it takes to get to the point where the TLS handshake starts after DNS is done.
|
with this fix I ran in a new issue: the global "probe_duration_seconds" metric doesn't match the sum of the phases anymore like before. edit: I just reverted to how 'connect' was calculated before so to not introduce a breaking change. so to clarify this PR: |
Signed-off-by: kgersen <[email protected]>
|
Out of curiosity, is this somehow related to #485 ? |
I have issue with some tls phase with the http probe. It appears it is caused by the way it's calculated (
trace.gotConn - trace.dnsDone) which isn't always reliable.This proposed fix uses
TLSHandshakeStartandTLSHandshakeDonehooks.