refactor(minipipeline): tweak how we detect HTTP failures (#1412)

Part of ooni/probe#2634
ooni · Nov 30, 2023 · c135b58 · c135b58
1 parent 70d2e64
commit c135b58
Show file tree

Hide file tree

Showing 70 changed files with 123 additions and 144 deletions.
diff --git a/internal/cmd/minipipeline/testdata/analysis.json b/internal/cmd/minipipeline/testdata/analysis.json
@@ -8,6 +8,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": {},
   "HTTPDiffBodyProportionFactor": 1,
@@ -23,6 +24,5 @@
   "HTTPFinalResponsesWithTLS": {
     "4": true
   },
-  "TCPTransactionsWithUnexpectedHTTPFailures": {},
   "TCPTransactionsWithUnexplainedUnexpectedFailures": {}
 }
diff --git a/internal/cmd/minipipeline/testdata/analysis_classic.json b/internal/cmd/minipipeline/testdata/analysis_classic.json
@@ -8,6 +8,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": null,
   "HTTPDiffBodyProportionFactor": 1,
@@ -23,6 +24,5 @@
   "HTTPFinalResponsesWithTLS": {
     "4": true
   },
-  "TCPTransactionsWithUnexpectedHTTPFailures": {},
   "TCPTransactionsWithUnexplainedUnexpectedFailures": {}
 }
diff --git a/internal/minipipeline/analysis.go b/internal/minipipeline/analysis.go
@@ -16,11 +16,11 @@ func AnalyzeWebObservations(container *WebObservationsContainer) *WebAnalysis {
 
 	analysis.tcpComputeMetrics(container)
 	analysis.tlsComputeMetrics(container)
+	analysis.httpComputeFailureMetrics(container)
 
 	analysis.ComputeDNSExperimentFailure(container)
 	analysis.ComputeDNSPossiblyNonexistingDomains(container)
 
-	analysis.ComputeTCPTransactionsWithUnexpectedHTTPFailures(container)
 	analysis.ComputeTCPTransactionsWithUnexplainedUnexpectedFailures(container)
 
 	analysis.ComputeHTTPDiffBodyProportionFactor(container)
@@ -70,6 +70,11 @@ type WebAnalysis struct {
 	// while checking for connectivity, as opposed to fetching a webpage.
 	TLSHandshakeUnexpectedFailureDuringConnectivityCheck Set[int64]
 
+	// HTTPRoundTripUnexpectedFailure contains HTTP endpoint transactions with unexpected failures.
+	HTTPRoundTripUnexpectedFailure Set[int64]
+
+	// TODO(bassosimone): there are probably redundant metrics from this point on
+
 	// DNSExperimentFailure is the first failure experienced by a getaddrinfo-like resolver.
 	DNSExperimentFailure optional.Value[string]
 
@@ -109,10 +114,6 @@ type WebAnalysis struct {
 	// itself with whether there's control data, because TLS suffices.
 	HTTPFinalResponsesWithTLS optional.Value[map[int64]bool]
 
-	// TCPSTransactionsWithUnexpectedHTTPFailures contains the TCP transaction IDs that
-	// contain HTTP failures while the control measurement succeeded.
-	TCPTransactionsWithUnexpectedHTTPFailures 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.
@@ -359,6 +360,39 @@ func (wa *WebAnalysis) tlsComputeMetrics(c *WebObservationsContainer) {
 	}
 }
 
+func (wa *WebAnalysis) httpComputeFailureMetrics(c *WebObservationsContainer) {
+	for _, obs := range c.KnownTCPEndpoints {
+		// Implementation note: here we don't limit the search to depth==0 because the
+		// control we have for HTTP is relative to the final response.
+
+		// handle the case where there is no measurement
+		if obs.HTTPFailure.IsNone() {
+			continue
+		}
+
+		// handle the case where there is no control information
+		if obs.ControlHTTPFailure.IsNone() {
+			continue
+		}
+
+		// handle the case where both the probe and the control fail
+		if obs.HTTPFailure.Unwrap() != "" && obs.ControlHTTPFailure.Unwrap() != "" {
+			continue
+		}
+
+		// handle the case where only the control fails
+		if obs.ControlHTTPFailure.Unwrap() != "" {
+			continue
+		}
+
+		// handle the case where only the probe fails
+		if obs.HTTPFailure.Unwrap() != "" {
+			wa.HTTPRoundTripUnexpectedFailure.Add(obs.EndpointTransactionID.Unwrap())
+			continue
+		}
+	}
+}
+
 // ComputeDNSExperimentFailure computes the DNSExperimentFailure field.
 func (wa *WebAnalysis) ComputeDNSExperimentFailure(c *WebObservationsContainer) {
 
@@ -463,40 +497,6 @@ func (wa *WebAnalysis) ComputeDNSPossiblyNonexistingDomains(c *WebObservationsCo
 	wa.DNSPossiblyNonexistingDomains = optional.Some(state)
 }
 
-// ComputeTCPTransactionsWithUnexpectedHTTPFailures computes the TCPTransactionsWithUnexpectedHTTPFailures field.
-func (wa *WebAnalysis) ComputeTCPTransactionsWithUnexpectedHTTPFailures(c *WebObservationsContainer) {
-	var state map[int64]bool
-
-	for _, obs := range c.KnownTCPEndpoints {
-		// we cannot do anything unless we have both records
-		if obs.HTTPFailure.IsNone() || obs.ControlHTTPFailure.IsNone() {
-			continue
-		}
-
-		// flip state from None to empty once we have seen the first
-		// suitable set of measurement/control pairs
-		if state == nil {
-			state = make(map[int64]bool)
-		}
-
-		// skip cases with no failures
-		if obs.HTTPFailure.Unwrap() == "" {
-			continue
-		}
-
-		// skip cases where also the control failed
-		if obs.ControlHTTPFailure.Unwrap() != "" {
-			continue
-		}
-
-		// update state
-		state[obs.EndpointTransactionID.Unwrap()] = true
-	}
-
-	// note that optional.Some constructs None if state is nil
-	wa.TCPTransactionsWithUnexpectedHTTPFailures = optional.Some(state)
-}
-
 // ComputeHTTPDiffBodyProportionFactor computes the HTTPDiffBodyProportionFactor field.
 func (wa *WebAnalysis) ComputeHTTPDiffBodyProportionFactor(c *WebObservationsContainer) {
 	for _, obs := range c.KnownTCPEndpoints {

diff --git a/internal/minipipeline/analysis_test.go b/internal/minipipeline/analysis_test.go
@@ -73,27 +73,6 @@ func TestWebAnalysisComputeDNSExperimentFailure(t *testing.T) {
 	})
 }
 
-func TestWebAnalysisComputeTCPTransactionsWithUnexpectedHTTPFailures(t *testing.T) {
-	t.Run("when both measurement and control fail", func(t *testing.T) {
-		container := &WebObservationsContainer{
-			KnownTCPEndpoints: map[int64]*WebObservation{
-				1: {
-					HTTPFailure:        optional.Some("connection_reset"),
-					ControlHTTPFailure: optional.Some("connection_reset"),
-				},
-			},
-		}
-
-		wa := &WebAnalysis{}
-		wa.ComputeTCPTransactionsWithUnexpectedHTTPFailures(container)
-
-		result := wa.TCPTransactionsWithUnexpectedHTTPFailures.Unwrap()
-		if len(result) != 0 {
-			t.Fatal("should not have added any entry")
-		}
-	})
-}
-
 func TestWebAnalysisComputeHTTPDiffBodyProportionFactor(t *testing.T) {
 	t.Run("when there is no probe response body length", func(t *testing.T) {
 		container := &WebObservationsContainer{

diff --git a/...inipipeline/testdata/webconnectivity/generated/badSSLWithExpiredCertificate/analysis.json b/...inipipeline/testdata/webconnectivity/generated/badSSLWithExpiredCertificate/analysis.json
@@ -8,6 +8,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": {},
   "HTTPDiffBodyProportionFactor": null,
@@ -16,6 +17,5 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/...ine/testdata/webconnectivity/generated/badSSLWithExpiredCertificate/analysis_classic.json b/...ine/testdata/webconnectivity/generated/badSSLWithExpiredCertificate/analysis_classic.json
@@ -8,6 +8,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": null,
   "HTTPDiffBodyProportionFactor": null,
@@ -16,6 +17,5 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/...tdata/webconnectivity/generated/badSSLWithUnknownAuthorityWithConsistentDNS/analysis.json b/...tdata/webconnectivity/generated/badSSLWithUnknownAuthorityWithConsistentDNS/analysis.json
@@ -8,6 +8,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": {},
   "HTTPDiffBodyProportionFactor": null,
@@ -16,6 +17,5 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/...bconnectivity/generated/badSSLWithUnknownAuthorityWithConsistentDNS/analysis_classic.json b/...bconnectivity/generated/badSSLWithUnknownAuthorityWithConsistentDNS/analysis_classic.json
@@ -8,6 +8,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": null,
   "HTTPDiffBodyProportionFactor": null,
@@ -16,6 +17,5 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/...ata/webconnectivity/generated/badSSLWithUnknownAuthorityWithInconsistentDNS/analysis.json b/...ata/webconnectivity/generated/badSSLWithUnknownAuthorityWithInconsistentDNS/analysis.json
@@ -14,6 +14,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": {},
   "HTTPDiffBodyProportionFactor": 1,
@@ -29,6 +30,5 @@
   "HTTPFinalResponsesWithTLS": {
     "4": true
   },
-  "TCPTransactionsWithUnexpectedHTTPFailures": {},
   "TCPTransactionsWithUnexplainedUnexpectedFailures": {}
 }
diff --git a/...onnectivity/generated/badSSLWithUnknownAuthorityWithInconsistentDNS/analysis_classic.json b/...onnectivity/generated/badSSLWithUnknownAuthorityWithInconsistentDNS/analysis_classic.json
@@ -12,6 +12,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": null,
   "HTTPDiffBodyProportionFactor": null,
@@ -20,6 +21,5 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": {}
 }
diff --git a/...l/minipipeline/testdata/webconnectivity/generated/badSSLWithWrongServerName/analysis.json b/...l/minipipeline/testdata/webconnectivity/generated/badSSLWithWrongServerName/analysis.json
@@ -8,6 +8,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": {},
   "HTTPDiffBodyProportionFactor": null,
@@ -16,6 +17,5 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/...peline/testdata/webconnectivity/generated/badSSLWithWrongServerName/analysis_classic.json b/...peline/testdata/webconnectivity/generated/badSSLWithWrongServerName/analysis_classic.json
@@ -8,6 +8,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": null,
   "HTTPDiffBodyProportionFactor": null,
@@ -16,6 +17,5 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/...testdata/webconnectivity/generated/controlFailureWithSuccessfulHTTPSWebsite/analysis.json b/...testdata/webconnectivity/generated/controlFailureWithSuccessfulHTTPSWebsite/analysis.json
@@ -8,6 +8,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": null,
   "HTTPDiffBodyProportionFactor": null,
@@ -18,6 +19,5 @@
   "HTTPFinalResponsesWithTLS": {
     "3": true
   },
-  "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/.../webconnectivity/generated/controlFailureWithSuccessfulHTTPSWebsite/analysis_classic.json b/.../webconnectivity/generated/controlFailureWithSuccessfulHTTPSWebsite/analysis_classic.json
@@ -8,6 +8,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": null,
   "HTTPDiffBodyProportionFactor": null,
@@ -18,6 +19,5 @@
   "HTTPFinalResponsesWithTLS": {
     "3": true
   },
-  "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/.../testdata/webconnectivity/generated/controlFailureWithSuccessfulHTTPWebsite/analysis.json b/.../testdata/webconnectivity/generated/controlFailureWithSuccessfulHTTPWebsite/analysis.json
@@ -8,6 +8,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": null,
   "HTTPDiffBodyProportionFactor": null,
@@ -16,6 +17,5 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/...a/webconnectivity/generated/controlFailureWithSuccessfulHTTPWebsite/analysis_classic.json b/...a/webconnectivity/generated/controlFailureWithSuccessfulHTTPWebsite/analysis_classic.json
@@ -8,6 +8,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": null,
   "DNSPossiblyNonexistingDomains": null,
   "HTTPDiffBodyProportionFactor": null,
@@ -16,6 +17,5 @@
   "HTTPDiffUncommonHeadersIntersection": null,
   "HTTPFinalResponsesWithControl": null,
   "HTTPFinalResponsesWithTLS": null,
-  "TCPTransactionsWithUnexpectedHTTPFailures": null,
   "TCPTransactionsWithUnexplainedUnexpectedFailures": null
 }
diff --git a/...ipeline/testdata/webconnectivity/generated/dnsBlockingAndroidDNSCacheNoData/analysis.json b/...ipeline/testdata/webconnectivity/generated/dnsBlockingAndroidDNSCacheNoData/analysis.json
@@ -10,6 +10,7 @@
   "TLSHandshakeUnexpectedFailure": [],
   "TLSHandshakeUnexpectedFailureDuringWebFetch": [],
   "TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
+  "HTTPRoundTripUnexpectedFailure": [],
   "DNSExperimentFailure": "android_dns_cache_no_data",
   "DNSPossiblyNonexistingDomains": {},
   "HTTPDiffBodyProportionFactor": 1,
@@ -25,6 +26,5 @@
   "HTTPFinalResponsesWithTLS": {
     "3": true
   },
-  "TCPTransactionsWithUnexpectedHTTPFailures": {},
   "TCPTransactionsWithUnexplainedUnexpectedFailures": {}
 }