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.

Sign up for GitHub

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

Jump to bottom

refactor(minipipeline): separate unexplained tcp and tls failures #1413

Merged
merged 4 commits into from
Nov 30, 2023
Merged

refactor(minipipeline): separate unexplained tcp and tls failures #1413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1de94a4
refactor(minipipeline): separate unexplained tcp and tls failures
bassosimone Nov 30, 2023
cc1b0fb
x
bassosimone Nov 30, 2023
d47cbe4
x
bassosimone Nov 30, 2023
3b28f4e
x
bassosimone Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions internal/cmd/minipipeline/testdata/analysis.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TCPConnectUnexplainedFailureDuringWebFetch": [],
"TCPConnectUnexplainedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"TLSHandshakeUnexplainedFailureDuringWebFetch": [],
"TLSHandshakeUnexplainedFailureDuringConnectivityCheck": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": {},
Expand All @@ -23,6 +29,5 @@
},
"HTTPFinalResponsesWithTLS": {
"4": true
},
"TCPTransactionsWithUnexplainedUnexpectedFailures": {}
}
}
9 changes: 7 additions & 2 deletions internal/cmd/minipipeline/testdata/analysis_classic.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TCPConnectUnexplainedFailureDuringWebFetch": [],
"TCPConnectUnexplainedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"TLSHandshakeUnexplainedFailureDuringWebFetch": [],
"TLSHandshakeUnexplainedFailureDuringConnectivityCheck": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": null,
Expand All @@ -23,6 +29,5 @@
},
"HTTPFinalResponsesWithTLS": {
"4": true
},
"TCPTransactionsWithUnexplainedUnexpectedFailures": {}
}
}
117 changes: 55 additions & 62 deletions internal/minipipeline/analysis.go
View file
Open in desktop
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,17 @@ 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]

// TCPConnectUnexplainedFailureDuringWebFetch contains failures occurring during redirects
// while performing a web fetch, as opposed to checking for connectivity.
TCPConnectUnexplainedFailureDuringWebFetch Set[int64]

// TCPConnectUnexplainedFailureDuringConnectivityCheck contains failures occurring during redirects
// while checking for connectivity, as opposed to fetching a webpage.
TCPConnectUnexplainedFailureDuringConnectivityCheck Set[int64]

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

Expand All @@ -70,6 +79,17 @@ 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]

// TLSHandshakeUnexplainedFailureDuringWebFetch contains failures occurring during redirects
// while performing a web fetch, as opposed to checking for connectivity.
TLSHandshakeUnexplainedFailureDuringWebFetch Set[int64]

// TLSHandshakeUnexplainedFailureDuringConnectivityCheck contains failures occurring during redirects
// while checking for connectivity, as opposed to fetching a webpage.
TLSHandshakeUnexplainedFailureDuringConnectivityCheck Set[int64]

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

Expand Down Expand Up @@ -113,11 +133,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 +292,27 @@ 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() != "" {
switch {
case !obs.TagFetchBody.IsNone() && obs.TagFetchBody.Unwrap():
wa.TCPConnectUnexplainedFailureDuringWebFetch.Add(obs.EndpointTransactionID.Unwrap())
case !obs.TagFetchBody.IsNone() && !obs.TagFetchBody.Unwrap():
wa.TCPConnectUnexplainedFailureDuringConnectivityCheck.Add(obs.EndpointTransactionID.Unwrap())
}
wa.TCPConnectUnexplainedFailure.Add(obs.EndpointTransactionID.Unwrap())
continue
}
continue
}

Expand Down Expand Up @@ -321,13 +350,24 @@ 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() != "" {
switch {
case !obs.TagFetchBody.IsNone() && obs.TagFetchBody.Unwrap():
wa.TLSHandshakeUnexplainedFailureDuringWebFetch.Add(obs.EndpointTransactionID.Unwrap())
case !obs.TagFetchBody.IsNone() && !obs.TagFetchBody.Unwrap():
wa.TLSHandshakeUnexplainedFailureDuringConnectivityCheck.Add(obs.EndpointTransactionID.Unwrap())
}
wa.TLSHandshakeUnexplainedFailure.Add(obs.EndpointTransactionID.Unwrap())
continue
}
continue
}

Expand Down Expand Up @@ -654,53 +694,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
19 changes: 0 additions & 19 deletions internal/minipipeline/analysis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,25 +314,6 @@ func TestWebAnalysisComputeHTTPFinalResponses(t *testing.T) {
})
}

func TestWebAnalysisComputeTCPTransactionsWithUnexplainedUnexpectedFailures(t *testing.T) {
t.Run("when we don't have a transaction ID", func(t *testing.T) {
container := &WebObservationsContainer{
KnownTCPEndpoints: map[int64]*WebObservation{
1: {
EndpointTransactionID: optional.None[int64](),
},
},
}

wa := &WebAnalysis{}
wa.ComputeTCPTransactionsWithUnexplainedUnexpectedFailures(container)

if v := wa.TCPTransactionsWithUnexplainedUnexpectedFailures.UnwrapOr(nil); len(v) > 0 {
t.Fatal("should be empty")
}
})
}

func TestWebAnalysisComputeHTTPFinalResponsesWithTLS(t *testing.T) {
t.Run("when there is no endpoint transaction ID", func(t *testing.T) {
container := &WebObservationsContainer{
Expand Down
9 changes: 7 additions & 2 deletions ...inipipeline/testdata/webconnectivity/generated/badSSLWithExpiredCertificate/analysis.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TCPConnectUnexplainedFailureDuringWebFetch": [],
"TCPConnectUnexplainedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"TLSHandshakeUnexplainedFailureDuringWebFetch": [],
"TLSHandshakeUnexplainedFailureDuringConnectivityCheck": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": {},
Expand All @@ -16,6 +22,5 @@
"HTTPDiffTitleDifferentLongWords": null,
"HTTPDiffUncommonHeadersIntersection": null,
"HTTPFinalResponsesWithControl": null,
"HTTPFinalResponsesWithTLS": null,
"TCPTransactionsWithUnexplainedUnexpectedFailures": null
"HTTPFinalResponsesWithTLS": null
}
9 changes: 7 additions & 2 deletions ...ine/testdata/webconnectivity/generated/badSSLWithExpiredCertificate/analysis_classic.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TCPConnectUnexplainedFailureDuringWebFetch": [],
"TCPConnectUnexplainedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"TLSHandshakeUnexplainedFailureDuringWebFetch": [],
"TLSHandshakeUnexplainedFailureDuringConnectivityCheck": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": null,
Expand All @@ -16,6 +22,5 @@
"HTTPDiffTitleDifferentLongWords": null,
"HTTPDiffUncommonHeadersIntersection": null,
"HTTPFinalResponsesWithControl": null,
"HTTPFinalResponsesWithTLS": null,
"TCPTransactionsWithUnexplainedUnexpectedFailures": null
"HTTPFinalResponsesWithTLS": null
}
9 changes: 7 additions & 2 deletions ...tdata/webconnectivity/generated/badSSLWithUnknownAuthorityWithConsistentDNS/analysis.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TCPConnectUnexplainedFailureDuringWebFetch": [],
"TCPConnectUnexplainedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"TLSHandshakeUnexplainedFailureDuringWebFetch": [],
"TLSHandshakeUnexplainedFailureDuringConnectivityCheck": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": {},
Expand All @@ -16,6 +22,5 @@
"HTTPDiffTitleDifferentLongWords": null,
"HTTPDiffUncommonHeadersIntersection": null,
"HTTPFinalResponsesWithControl": null,
"HTTPFinalResponsesWithTLS": null,
"TCPTransactionsWithUnexplainedUnexpectedFailures": null
"HTTPFinalResponsesWithTLS": null
}
9 changes: 7 additions & 2 deletions ...bconnectivity/generated/badSSLWithUnknownAuthorityWithConsistentDNS/analysis_classic.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TCPConnectUnexplainedFailureDuringWebFetch": [],
"TCPConnectUnexplainedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"TLSHandshakeUnexplainedFailureDuringWebFetch": [],
"TLSHandshakeUnexplainedFailureDuringConnectivityCheck": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": null,
Expand All @@ -16,6 +22,5 @@
"HTTPDiffTitleDifferentLongWords": null,
"HTTPDiffUncommonHeadersIntersection": null,
"HTTPFinalResponsesWithControl": null,
"HTTPFinalResponsesWithTLS": null,
"TCPTransactionsWithUnexplainedUnexpectedFailures": null
"HTTPFinalResponsesWithTLS": null
}
9 changes: 7 additions & 2 deletions ...ata/webconnectivity/generated/badSSLWithUnknownAuthorityWithInconsistentDNS/analysis.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,15 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TCPConnectUnexplainedFailureDuringWebFetch": [],
"TCPConnectUnexplainedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"TLSHandshakeUnexplainedFailureDuringWebFetch": [],
"TLSHandshakeUnexplainedFailureDuringConnectivityCheck": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": {},
Expand All @@ -29,6 +35,5 @@
},
"HTTPFinalResponsesWithTLS": {
"4": true
},
"TCPTransactionsWithUnexplainedUnexpectedFailures": {}
}
}
9 changes: 7 additions & 2 deletions ...onnectivity/generated/badSSLWithUnknownAuthorityWithInconsistentDNS/analysis_classic.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,15 @@
"TCPConnectUnexpectedFailure": [],
"TCPConnectUnexpectedFailureDuringWebFetch": [],
"TCPConnectUnexpectedFailureDuringConnectivityCheck": [],
"TCPConnectUnexplainedFailure": [],
"TCPConnectUnexplainedFailureDuringWebFetch": [],
"TCPConnectUnexplainedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexpectedFailure": [],
"TLSHandshakeUnexpectedFailureDuringWebFetch": [],
"TLSHandshakeUnexpectedFailureDuringConnectivityCheck": [],
"TLSHandshakeUnexplainedFailure": [],
"TLSHandshakeUnexplainedFailureDuringWebFetch": [],
"TLSHandshakeUnexplainedFailureDuringConnectivityCheck": [],
"HTTPRoundTripUnexpectedFailure": [],
"DNSExperimentFailure": null,
"DNSPossiblyNonexistingDomains": null,
Expand All @@ -20,6 +26,5 @@
"HTTPDiffTitleDifferentLongWords": null,
"HTTPDiffUncommonHeadersIntersection": null,
"HTTPFinalResponsesWithControl": null,
"HTTPFinalResponsesWithTLS": null,
"TCPTransactionsWithUnexplainedUnexpectedFailures": {}
"HTTPFinalResponsesWithTLS": null
}
Loading