fix(qatool): reduce generated test cases churn (#1505)

We add dns.nextdns.io to the QA suite, such that we don't get a wrong
NXDOMAIN when trying to use it as the resolver.

We optionally sort the test keys to ensure there's less churn in diffs
produced when regenerating minipipeline test data.

We fix ./script/updateminipipeline.bash to use ./script/go.bash for
building as opposed to using go.

Also, include endpoint information in `tls_handshake_{start,stop}` and
`http_transaction_{start,stop}` events such that they are sorted
correctly when we sort events to reduce churn. This is necessary because
we're using the endpoint information to sort the network events.

Part of ooni/probe#2677; the churn is still
too high, and I need more changes.

internal/cmd/qatool/main.go

@@ -135,6 +135,9 @@ func main() {
 	// build the regexp
 	selector := regexp.MustCompile(*runFlag)
 
+	// make sure we produce more predictable observations in output
+	webconnectivitylte.SortObservations.Add(1)
+
 	// select which test cases to run
 	for _, tc := range webconnectivityqa.AllTestCases() {
 		name := "webconnectivitylte/" + tc.Name

internal/experiment/webconnectivitylte/cleartextflow.go

@@ -253,8 +253,16 @@ func (t *CleartextFlow) httpTransaction(ctx context.Context, network, address, a
 
 	// Implementation note: we want to emit http_transaction_start when we actually start doing
 	// HTTP things such that it's possible to correctly classify network events
-	t.TestKeys.AppendNetworkEvents(measurexlite.NewAnnotationArchivalNetworkEvent(
-		trace.Index(), started, "http_transaction_start", trace.Tags()...,
+	t.TestKeys.AppendNetworkEvents(measurexlite.NewArchivalNetworkEvent(
+		trace.Index(),
+		started,
+		"http_transaction_start",
+		network,
+		address,
+		0,
+		nil,
+		started,
+		trace.Tags()...,
 	))
 
 	resp, err := txp.RoundTrip(req)
@@ -272,8 +280,16 @@ func (t *CleartextFlow) httpTransaction(ctx context.Context, network, address, a
 	}
 
 	finished := trace.TimeSince(trace.ZeroTime())
-	t.TestKeys.AppendNetworkEvents(measurexlite.NewAnnotationArchivalNetworkEvent(
-		trace.Index(), finished, "http_transaction_done", trace.Tags()...,
+	t.TestKeys.AppendNetworkEvents(measurexlite.NewArchivalNetworkEvent(
+		trace.Index(),
+		finished,
+		"http_transaction_done",
+		network,
+		address,
+		0,
+		nil,
+		finished,
+		trace.Tags()...,
 	))
 
 	ev := measurexlite.NewArchivalHTTPRequestResult(

internal/experiment/webconnectivitylte/measurer.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/ooni/probe-cli/v3/internal/experiment/webconnectivity"
 	"github.com/ooni/probe-cli/v3/internal/inputparser"
+	"github.com/ooni/probe-cli/v3/internal/minipipeline"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"golang.org/x/net/publicsuffix"
 )
@@ -144,6 +145,23 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
 		}
 	}
 
+	// sort test keys to make output more predictable and avoid churn when generating
+	// minipipeline test cases; see https://github.com/ooni/probe/issues/2677.
+	//
+	// Note that tk.Do53 and tk.DoH are initialized by NewTestKeys so we know they're not nil.
+	//
+	// Note that we MUST NOT sort tk.Requests because its order matters for historical
+	// reasons and we don't wnat to break existing data consumers.
+	if SortObservations.Load() > 0 {
+		tk.Queries = minipipeline.SortDNSLookupResults(tk.Queries)
+		tk.Do53.Queries = minipipeline.SortDNSLookupResults(tk.Do53.Queries)
+		tk.DoH.Queries = minipipeline.SortDNSLookupResults(tk.DoH.Queries)
+		tk.DNSDuplicateResponses = minipipeline.SortDNSLookupResults(tk.DNSDuplicateResponses)
+		tk.NetworkEvents = minipipeline.SortNetworkEvents(tk.NetworkEvents)
+		tk.TCPConnect = minipipeline.SortTCPConnectResults(tk.TCPConnect)
+		tk.TLSHandshakes = minipipeline.SortTLSHandshakeResults(tk.TLSHandshakes)
+	}
+
 	// return whether there was a fundamental failure, which would prevent
 	// the measurement from being submitted to the OONI collector.
 	return tk.fundamentalFailure
@@ -159,3 +177,7 @@ func registerExtensions(m *model.Measurement) {
 	model.ArchivalExtTLSHandshake.AddTo(m)
 	model.ArchivalExtTunnel.AddTo(m)
 }
+
+// SortObservations allows to configure sorting observations when that's needed to
+// reduce churn in the generated JSON files (mainly for minipipeline).
+var SortObservations = &atomic.Int64{}

internal/experiment/webconnectivitylte/qa_test.go

@@ -7,6 +7,7 @@ import (
 )
 
 func TestQA(t *testing.T) {
+	SortObservations.Add(1) // make sure we have predictable observations
 	for _, tc := range webconnectivityqa.AllTestCases() {
 		t.Run(tc.Name, func(t *testing.T) {
 			if (tc.Flags & webconnectivityqa.TestCaseFlagNoLTE) != 0 {

internal/experiment/webconnectivitylte/secureflow.go

@@ -308,8 +308,16 @@ func (t *SecureFlow) httpTransaction(ctx context.Context, network, address, alpn
 
 	// Implementation note: we want to emit http_transaction_start when we actually start doing
 	// HTTP things such that it's possible to correctly classify network events
-	t.TestKeys.AppendNetworkEvents(measurexlite.NewAnnotationArchivalNetworkEvent(
-		trace.Index(), started, "http_transaction_start", trace.Tags()...,
+	t.TestKeys.AppendNetworkEvents(measurexlite.NewArchivalNetworkEvent(
+		trace.Index(),
+		started,
+		"http_transaction_start",
+		network,
+		address,
+		0,
+		nil,
+		started,
+		trace.Tags()...,
 	))
 
 	resp, err := txp.RoundTrip(req)
@@ -327,8 +335,16 @@ func (t *SecureFlow) httpTransaction(ctx context.Context, network, address, alpn
 	}
 
 	finished := trace.TimeSince(trace.ZeroTime())
-	t.TestKeys.AppendNetworkEvents(measurexlite.NewAnnotationArchivalNetworkEvent(
-		trace.Index(), finished, "http_transaction_done", trace.Tags()...,
+	t.TestKeys.AppendNetworkEvents(measurexlite.NewArchivalNetworkEvent(
+		trace.Index(),
+		finished,
+		"http_transaction_done",
+		network,
+		address,
+		0,
+		nil,
+		finished,
+		trace.Tags()...,
 	))
 
 	ev := measurexlite.NewArchivalHTTPRequestResult(

internal/experiment/webconnectivitylte/testkeys.go

@@ -291,13 +291,6 @@ func (tk *TestKeys) WithDNSWhoami(fun func(*DNSWhoamiInfo)) {
 	tk.mu.Unlock()
 }
 
-// SetClientResolver sets the ClientResolver field.
-func (tk *TestKeys) SetClientResolver(value string) {
-	tk.mu.Lock()
-	tk.ClientResolver = value
-	tk.mu.Unlock()
-}
-
 // AppendConnPriorityLogEntry appends an entry to ConnPriorityLog.
 func (tk *TestKeys) AppendConnPriorityLogEntry(entry *ConnPriorityLogEntry) {
 	tk.mu.Lock()

internal/measurexlite/tls.go

@@ -43,8 +43,17 @@ func (thx *tlsHandshakerTrace) Handshake(
 func (tx *Trace) OnTLSHandshakeStart(now time.Time, remoteAddr string, config *tls.Config) {
 	t := now.Sub(tx.ZeroTime())
 	select {
-	case tx.networkEvent <- NewAnnotationArchivalNetworkEvent(
-		tx.Index(), t, "tls_handshake_start", tx.tags...):
+	case tx.networkEvent <- NewArchivalNetworkEvent(
+		tx.Index(),
+		t,
+		"tls_handshake_start",
+		"tcp",
+		remoteAddr,
+		0,
+		nil,
+		t,
+		tx.tags...,
+	):
 	default: // buffer is full
 	}
 }
@@ -70,8 +79,17 @@ func (tx *Trace) OnTLSHandshakeDone(started time.Time, remoteAddr string, config
 	}
 
 	select {
-	case tx.networkEvent <- NewAnnotationArchivalNetworkEvent(
-		tx.Index(), t, "tls_handshake_done", tx.tags...):
+	case tx.networkEvent <- NewArchivalNetworkEvent(
+		tx.Index(),
+		t,
+		"tls_handshake_done",
+		"tcp",
+		remoteAddr,
+		0,
+		nil,
+		t,
+		tx.tags...,
+	):
 	default: // buffer is full
 	}
 }

internal/measurexlite/tls_test.go

@@ -143,11 +143,11 @@ func TestNewTLSHandshakerStdlib(t *testing.T) {
 
 			t.Run("tls_handshake_start", func(t *testing.T) {
 				expect := &model.ArchivalNetworkEvent{
-					Address:   "",
+					Address:   "1.1.1.1:443",
 					Failure:   nil,
 					NumBytes:  0,
 					Operation: "tls_handshake_start",
-					Proto:     "",
+					Proto:     "tcp",
 					T:         0,
 					Tags:      []string{"antani"},
 				}
@@ -159,11 +159,11 @@ func TestNewTLSHandshakerStdlib(t *testing.T) {
 
 			t.Run("tls_handshake_done", func(t *testing.T) {
 				expect := &model.ArchivalNetworkEvent{
-					Address:   "",
+					Address:   "1.1.1.1:443",
 					Failure:   nil,
 					NumBytes:  0,
 					Operation: "tls_handshake_done",
-					Proto:     "",
+					Proto:     "tcp",
 					T0:        time.Second.Seconds(),
 					T:         time.Second.Seconds(),
 					Tags:      []string{"antani"},
@@ -308,11 +308,11 @@ func TestNewTLSHandshakerStdlib(t *testing.T) {
 
 			t.Run("tls_handshake_start", func(t *testing.T) {
 				expect := &model.ArchivalNetworkEvent{
-					Address:   "",
+					Address:   server.Endpoint(),
 					Failure:   nil,
 					NumBytes:  0,
 					Operation: "tls_handshake_start",
-					Proto:     "",
+					Proto:     "tcp",
 					T:         0,
 					Tags:      []string{},
 				}
@@ -324,11 +324,11 @@ func TestNewTLSHandshakerStdlib(t *testing.T) {
 
 			t.Run("tls_handshake_done", func(t *testing.T) {
 				expect := &model.ArchivalNetworkEvent{
-					Address:   "",
+					Address:   server.Endpoint(),
 					Failure:   nil,
 					NumBytes:  0,
 					Operation: "tls_handshake_done",
-					Proto:     "",
+					Proto:     "tcp",
 					T0:        time.Second.Seconds(),
 					T:         time.Second.Seconds(),
 					Tags:      []string{},

internal/minipipeline/analysis.go

@@ -51,21 +51,23 @@ func NewLinearWebAnalysis(input *WebObservationsContainer) (output []*WebObserva
 		output = append(output, entry)
 	}
 
-	// sort in descending order
+	// sort using complex sorting rule
 	sort.SliceStable(output, func(i, j int) bool {
 		left, right := output[i], output[j]
 
 		// We use -1 as the default value such that observations with undefined
 		// TagDepth sort at the end of the generated list.
 		if left.TagDepth.UnwrapOr(-1) > right.TagDepth.UnwrapOr(-1) {
 			return true
-		} else if left.TagDepth.UnwrapOr(-1) < right.TagDepth.UnwrapOr(-1) {
+		}
+		if left.TagDepth.UnwrapOr(-1) < right.TagDepth.UnwrapOr(-1) {
 			return false
 		}
 
 		if left.Type > right.Type {
 			return true
-		} else if left.Type < right.Type {
+		}
+		if left.Type < right.Type {
 			return false
 		}
 
@@ -74,7 +76,8 @@ func NewLinearWebAnalysis(input *WebObservationsContainer) (output []*WebObserva
 		const defaultFailureValue = "unknown_failure"
 		if left.Failure.UnwrapOr(defaultFailureValue) < right.Failure.UnwrapOr(defaultFailureValue) {
 			return true
-		} else if left.Failure.UnwrapOr(defaultFailureValue) > right.Failure.UnwrapOr(defaultFailureValue) {
+		}
+		if left.Failure.UnwrapOr(defaultFailureValue) > right.Failure.UnwrapOr(defaultFailureValue) {
 			return false
 		}