Skip to content

Commit

Permalink
refactor(minipipeline): separate unexplained tcp and tls failures
Browse files Browse the repository at this point in the history
  • Loading branch information
bassosimone committed Nov 30, 2023
1 parent c135b58 commit 1de94a4
Show file tree
Hide file tree
Showing 69 changed files with 259 additions and 152 deletions.
4 changes: 3 additions & 1 deletion internal/cmd/minipipeline/testdata/analysis.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": {},
Expand All @@ -24,5 +26,5 @@
"HTTPFinalResponsesWithTLS": {
"4": true
},
"TCPTransactionsWithUnexplainedUnexpectedFailures": {}
"TCPTransactionsWithUnexplainedUnexpectedFailures": null
}
4 changes: 3 additions & 1 deletion internal/cmd/minipipeline/testdata/analysis_classic.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": null,
Expand All @@ -24,5 +26,5 @@
"HTTPFinalResponsesWithTLS": {
"4": true
},
"TCPTransactionsWithUnexplainedUnexpectedFailures": {}
"TCPTransactionsWithUnexplainedUnexpectedFailures": null
}
89 changes: 27 additions & 62 deletions internal/minipipeline/analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ func AnalyzeWebObservations(container *WebObservationsContainer) *WebAnalysis {
analysis.ComputeDNSExperimentFailure(container)
analysis.ComputeDNSPossiblyNonexistingDomains(container)

analysis.ComputeTCPTransactionsWithUnexplainedUnexpectedFailures(container)

analysis.ComputeHTTPDiffBodyProportionFactor(container)
analysis.ComputeHTTPDiffStatusCodeMatch(container)
analysis.ComputeHTTPDiffUncommonHeadersIntersection(container)
Expand Down Expand Up @@ -59,6 +57,9 @@ type WebAnalysis struct {
// while checking for connectivity, as opposed to fetching a webpage.
TCPConnectUnexpectedFailureDuringConnectivityCheck Set[int64]

// TCPConnectUnexplainedFailure contains failures occurring during redirects.
TCPConnectUnexplainedFailure Set[int64]

// TLSHandshakeUnexpectedFailure contains TLS endpoint transactions with unexpected failures.
TLSHandshakeUnexpectedFailure Set[int64]

Expand All @@ -70,6 +71,9 @@ type WebAnalysis struct {
// while checking for connectivity, as opposed to fetching a webpage.
TLSHandshakeUnexpectedFailureDuringConnectivityCheck Set[int64]

// TLSHandshakeUnexplainedFailure contains failures occurring during redirects.
TLSHandshakeUnexplainedFailure Set[int64]

// HTTPRoundTripUnexpectedFailure contains HTTP endpoint transactions with unexpected failures.
HTTPRoundTripUnexpectedFailure Set[int64]

Expand Down Expand Up @@ -113,11 +117,6 @@ type WebAnalysis struct {
// cases where we're using TLS to fetch the final response, and does not concern
// itself with whether there's control data, because TLS suffices.
HTTPFinalResponsesWithTLS optional.Value[map[int64]bool]

// TCPTransactionsWithUnexplainedUnexpectedFailures contains the TCP transaction IDs for
// which we cannot explain TCP or TLS failures with control information, but for which we
// expect to see a success because the control's HTTP succeeded.
TCPTransactionsWithUnexplainedUnexpectedFailures optional.Value[map[int64]bool]
}

func (wa *WebAnalysis) dnsComputeSuccessMetrics(c *WebObservationsContainer) {
Expand Down Expand Up @@ -277,13 +276,21 @@ func (wa *WebAnalysis) dnsComputeFailureMetrics(c *WebObservationsContainer) {

func (wa *WebAnalysis) tcpComputeMetrics(c *WebObservationsContainer) {
for _, obs := range c.KnownTCPEndpoints {
// dials once we started following redirects should not be considered
if obs.TagDepth.IsNone() || obs.TagDepth.Unwrap() != 0 {
// handle the case where there is no measurement
if obs.TCPConnectFailure.IsNone() {
continue
}

// handle the case where there is no measurement
if obs.TCPConnectFailure.IsNone() {
// dials once we started following redirects should be treated differently
// since we know there's no control information beyond depth==0
if obs.TagDepth.IsNone() || obs.TagDepth.Unwrap() != 0 {
if utilsTCPConnectFailureSeemsMisconfiguredIPv6(obs) {
continue
}
if obs.TCPConnectFailure.Unwrap() != "" {
wa.TCPConnectUnexplainedFailure.Add(obs.EndpointTransactionID.Unwrap())
continue
}
continue
}

Expand Down Expand Up @@ -321,13 +328,18 @@ func (wa *WebAnalysis) tcpComputeMetrics(c *WebObservationsContainer) {

func (wa *WebAnalysis) tlsComputeMetrics(c *WebObservationsContainer) {
for _, obs := range c.KnownTCPEndpoints {
// handshakes once we started following redirects should not be considered
if obs.TagDepth.IsNone() || obs.TagDepth.Unwrap() != 0 {
// handle the case where there is no measurement
if obs.TLSHandshakeFailure.IsNone() {
continue
}

// handle the case where there is no measurement
if obs.TLSHandshakeFailure.IsNone() {
// handshakes once we started following redirects should be treated differently
// since we know there's no control information beyond depth==0
if obs.TagDepth.IsNone() || obs.TagDepth.Unwrap() != 0 {
if obs.TLSHandshakeFailure.Unwrap() != "" {
wa.TLSHandshakeUnexplainedFailure.Add(obs.EndpointTransactionID.Unwrap())
continue
}
continue
}

Expand Down Expand Up @@ -654,53 +666,6 @@ func (wa *WebAnalysis) ComputeHTTPFinalResponsesWithControl(c *WebObservationsCo
wa.HTTPFinalResponsesWithControl = optional.Some(state)
}

// ComputeTCPTransactionsWithUnexplainedUnexpectedFailures computes the TCPTransactionsWithUnexplainedUnexpectedFailures field.
func (wa *WebAnalysis) ComputeTCPTransactionsWithUnexplainedUnexpectedFailures(c *WebObservationsContainer) {
var state map[int64]bool

for _, obs := range c.KnownTCPEndpoints {
// obtain the transaction ID
txid := obs.EndpointTransactionID.UnwrapOr(0)
if txid <= 0 {
continue
}

// to execute the algorithm we must have the reasonable expectation of
// success, which we have iff the control succeeded.
if obs.ControlHTTPFailure.IsNone() || obs.ControlHTTPFailure.Unwrap() != "" {
continue
}

// flip state from None to empty when we have a reasonable
// expectation of success as explained above
if state == nil {
state = make(map[int64]bool)
}

// if we have a TCP connect measurement, the measurement failed, and we don't have
// a corresponding control measurement, we cannot explain this failure using the control
//
// while doing this, deal with misconfigured-IPv6 false positives
if !obs.TCPConnectFailure.IsNone() && obs.TCPConnectFailure.Unwrap() != "" &&
!utilsTCPConnectFailureSeemsMisconfiguredIPv6(obs) &&
obs.ControlTCPConnectFailure.IsNone() {
state[txid] = true
continue
}

// if we have a TLS handshake measurement, the measurement failed, and we don't have
// a corresponding control measurement, we cannot explain this failure using the control
if !obs.TLSHandshakeFailure.IsNone() && obs.TLSHandshakeFailure.Unwrap() != "" &&
obs.ControlTLSHandshakeFailure.IsNone() {
state[txid] = true
continue
}
}

// note that optional.Some constructs None if state is nil
wa.TCPTransactionsWithUnexplainedUnexpectedFailures = optional.Some(state)
}

// ComputeHTTPFinalResponsesWithTLS computes the HTTPFinalResponsesWithTLS field.
func (wa *WebAnalysis) ComputeHTTPFinalResponsesWithTLS(c *WebObservationsContainer) {
var state map[int64]bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": {},
Expand All @@ -30,5 +32,5 @@
"HTTPFinalResponsesWithTLS": {
"4": true
},
"TCPTransactionsWithUnexplainedUnexpectedFailures": {}
"TCPTransactionsWithUnexplainedUnexpectedFailures": null
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": null,
Expand All @@ -21,5 +23,5 @@
"HTTPDiffUncommonHeadersIntersection": null,
"HTTPFinalResponsesWithControl": null,
"HTTPFinalResponsesWithTLS": null,
"TCPTransactionsWithUnexplainedUnexpectedFailures": {}
"TCPTransactionsWithUnexplainedUnexpectedFailures": null
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": "android_dns_cache_no_data",
"DNSPossiblyNonexistingDomains": {},
Expand All @@ -26,5 +28,5 @@
"HTTPFinalResponsesWithTLS": {
"3": true
},
"TCPTransactionsWithUnexplainedUnexpectedFailures": {}
"TCPTransactionsWithUnexplainedUnexpectedFailures": null
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": "android_dns_cache_no_data",
"DNSPossiblyNonexistingDomains": {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": {},
Expand All @@ -28,7 +30,5 @@
"HTTPFinalResponsesWithTLS": {
"3": true
},
"TCPTransactionsWithUnexplainedUnexpectedFailures": {
"4": true
}
"TCPTransactionsWithUnexplainedUnexpectedFailures": null
}
Loading

0 comments on commit 1de94a4

Please sign in to comment.