feat: clearly indicate which resolver we're using (#885)

See what we documented at ooni/spec#257

Reference issue: ooni/probe#2238

See also the related ooni/spec PR: ooni/spec#257

See also ooni/probe#2237

While there, bump webconnectivity@v0.5 version because this change
has an impact onto the generated data format.

The drop in coverage is unavoidable because we've written some
tests for `measurex` to ensure we deal with DNS resolvers and transport
names correctly depending on the splitting policy we use.

(However, `measurex` is only used for the `tor` experiment and, per
the step-by-step design document, new experiments should use
`measurexlite` instead, so this is hopefully fine(TM).)

While there, fix a broken integration test that does not run in `-short` mode.

internal/engine/experiment_integration_test.go

@@ -29,7 +29,18 @@ func TestCreateAll(t *testing.T) {
 		exp := builder.NewExperiment()
 		good := (exp.Name() == name)
 		if !good {
-			t.Fatal("unexpected experiment name")
+			// We have introduced the concept of versioned experiments in
+			// https://github.com/ooni/probe-cli/pull/882. This works like
+			// in brew: we append @vX.Y to the experiment name. So, here
+			// we're stripping the version specification and retry.
+			index := strings.Index(name, "@")
+			if index >= 0 {
+				name = name[:index]
+				if good := (exp.Name() == name); good {
+					continue
+				}
+			}
+			t.Fatal("unexpected experiment name", exp.Name(), name)
 		}
 	}
 }

internal/experiment/webconnectivity/measurer.go

@@ -36,7 +36,7 @@ func (m *Measurer) ExperimentName() string {
 
 // ExperimentVersion implements model.ExperimentMeasurer.
 func (m *Measurer) ExperimentVersion() string {
-	return "0.5.1"
+	return "0.5.2"
 }
 
 // Run implements model.ExperimentMeasurer.

internal/measurex/dnsx.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/tracex"
 )
 
 // WrapDNSXRoundTripper creates a new DNSXRoundTripper that
@@ -42,7 +43,7 @@ func (txp *dnsxRoundTripperDB) RoundTrip(
 	response, err := txp.DNSTransport.RoundTrip(ctx, query)
 	finished := time.Since(txp.begin).Seconds()
 	txp.db.InsertIntoDNSRoundTrip(&DNSRoundTripEvent{
-		Network:  txp.DNSTransport.Network(),
+		Network:  tracex.ResolverNetworkAdaptNames(txp.DNSTransport.Network()),
 		Address:  txp.DNSTransport.Address(),
 		Query:    txp.maybeQueryBytes(query),
 		Started:  started,

internal/measurex/dnsx_test.go

@@ -0,0 +1,47 @@
+package measurex
+
+import (
+	"context"
+	"testing"
+
+	"github.com/miekg/dns"
+	"github.com/ooni/probe-cli/v3/internal/model/mocks"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+)
+
+func TestDNSXModifiesStdlibTransportName(t *testing.T) {
+	// See https://github.com/ooni/spec/pull/257 for more information.
+	child := netxlite.NewDNSOverGetaddrinfoTransport()
+	mx := NewMeasurerWithDefaultSettings()
+	dbout := &MeasurementDB{}
+	txp := mx.WrapDNSXRoundTripper(dbout, child)
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel() // we want to fail immediately
+	query := &mocks.DNSQuery{
+		MockDomain: func() string {
+			return "dns.google"
+		},
+		MockType: func() uint16 {
+			return dns.TypeANY
+		},
+		MockBytes: func() ([]byte, error) {
+			return []byte{}, nil
+		},
+		MockID: func() uint16 {
+			return 1453
+		},
+	}
+	_, _ = txp.RoundTrip(ctx, query)
+	measurement := dbout.AsMeasurement()
+	var good int
+	for _, rtinfo := range measurement.DNSRoundTrip {
+		network := rtinfo.Network
+		if network != netxlite.StdlibResolverSystem {
+			t.Fatal("unexpected network", network)
+		}
+		good++
+	}
+	if good < 1 {
+		t.Fatal("no good entry seen")
+	}
+}

internal/measurex/resolver.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
+	"github.com/ooni/probe-cli/v3/internal/tracex"
 )
 
 // WrapResolver creates a new Resolver that saves events into the WritableDB.
@@ -105,7 +106,7 @@ func (r *resolverDB) LookupHost(ctx context.Context, domain string) ([]string, e
 func (r *resolverDB) saveLookupResults(domain string, started, finished float64,
 	err error, addrs []string, qtype string) {
 	ev := &DNSLookupEvent{
-		Network:   r.Resolver.Network(),
+		Network:   tracex.ResolverNetworkAdaptNames(r.Resolver.Network()),
 		Address:   r.Resolver.Address(),
 		Failure:   NewFailure(err),
 		Domain:    domain,
@@ -158,7 +159,7 @@ func (r *resolverDB) LookupHTTPS(ctx context.Context, domain string) (*model.HTT
 	https, err := r.Resolver.LookupHTTPS(ctx, domain)
 	finished := time.Since(r.begin).Seconds()
 	ev := &DNSLookupEvent{
-		Network:   r.Resolver.Network(),
+		Network:   tracex.ResolverNetworkAdaptNames(r.Resolver.Network()),
 		Address:   r.Resolver.Address(),
 		Domain:    domain,
 		QueryType: "HTTPS",

internal/measurex/resolver_test.go

@@ -0,0 +1,58 @@
+package measurex
+
+import (
+	"context"
+	"testing"
+
+	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+)
+
+func TestResolverModifiesStdlibResolverName(t *testing.T) {
+	// See https://github.com/ooni/spec/pull/257 for more information.
+
+	t.Run("for LookupHost", func(t *testing.T) {
+		child := netxlite.NewStdlibResolver(model.DiscardLogger)
+		mx := NewMeasurerWithDefaultSettings()
+		dbout := &MeasurementDB{}
+		txp := mx.WrapResolver(dbout, child)
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel() // we want to fail immediately
+		_, _ = txp.LookupHost(ctx, "dns.google")
+		measurement := dbout.AsMeasurement()
+		var good int
+		for _, rtinfo := range measurement.LookupHost {
+			network := rtinfo.Network
+			if network != netxlite.StdlibResolverSystem {
+				t.Fatal("unexpected network", network)
+			}
+			good++
+		}
+		if good < 1 {
+			t.Fatal("no good entry seen")
+		}
+	})
+
+	t.Run("for LookupHTTPS", func(t *testing.T) {
+		child := netxlite.NewStdlibResolver(model.DiscardLogger)
+		mx := NewMeasurerWithDefaultSettings()
+		dbout := &MeasurementDB{}
+		txp := mx.WrapResolver(dbout, child)
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel() // we want to fail immediately
+		_, _ = txp.LookupHTTPS(ctx, "dns.google")
+		measurement := dbout.AsMeasurement()
+		var good int
+		for _, rtinfo := range measurement.LookupHTTPSSvc {
+			network := rtinfo.Network
+			if network != netxlite.StdlibResolverSystem {
+				t.Fatal("unexpected network", network)
+			}
+			good++
+		}
+		if good < 1 {
+			t.Fatal("no good entry seen")
+		}
+	})
+
+}

internal/measurexlite/dns_test.go

@@ -301,8 +301,12 @@ func TestNewWrappedResolvers(t *testing.T) {
 		if resolvert.tx != trace {
 			t.Fatal("invalid trace")
 		}
-		if resolver.Network() != "system" {
-			t.Fatal("unexpected resolver network")
+		switch network := resolver.Network(); network {
+		case netxlite.StdlibResolverGetaddrinfo,
+			netxlite.StdlibResolverGolangNetResolver:
+		// ok
+		default:
+			t.Fatal("unexpected resolver network", network)
 		}
 	})
 }

internal/netxlite/dnsovergetaddrinfo.go

@@ -24,6 +24,11 @@ type dnsOverGetaddrinfoTransport struct {
 	testableLookupANY func(ctx context.Context, domain string) ([]string, string, error)
 }
 
+// NewDNSOverGetaddrinfoTransport creates a new dns-over-getaddrinfo transport.
+func NewDNSOverGetaddrinfoTransport() model.DNSTransport {
+	return &dnsOverGetaddrinfoTransport{}
+}
+
 var _ model.DNSTransport = &dnsOverGetaddrinfoTransport{}
 
 func (txp *dnsOverGetaddrinfoTransport) RoundTrip(

internal/netxlite/doc.go

@@ -47,7 +47,7 @@
 //
 // 1. establishing a TCP connection;
 //
-// 2. performing a domain name resolution with the "system" resolver
+// 2. performing a domain name resolution with the "stdlib" resolver
 // (i.e., getaddrinfo on Unix) or custom DNS transports (e.g., DoT, DoH);
 //
 // 3. performing the TLS handshake;

internal/netxlite/getaddrinfo.go

@@ -2,6 +2,23 @@ package netxlite
 
 import "errors"
 
+// Name of the resolver we use when we link with libc and use getaddrinfo directly.
+//
+// See https://github.com/ooni/spec/pull/257 for more info.
+const StdlibResolverGetaddrinfo = "getaddrinfo"
+
+// Name of the resolver we use when we don't link with libc and use net.Resolver.
+//
+// See https://github.com/ooni/spec/pull/257 for more info.
+const StdlibResolverGolangNetResolver = "golang_net_resolver"
+
+// Legacy name of the resolver we use when we're don't know whether we're using
+// getaddrinfo, but we're using net.Resolver, and we're splitting the answer
+// in two A and AAAA queries. Eventually will become deprecated.
+//
+// See https://github.com/ooni/spec/pull/257 for more info.
+const StdlibResolverSystem = "system"
+
 // ErrGetaddrinfo represents a getaddrinfo failure.
 type ErrGetaddrinfo struct {
 	// Err is the error proper.

internal/netxlite/getaddrinfo_cgo.go

@@ -28,10 +28,13 @@ import (
 // been used to implement the getaddrinfo resolver.
 //
 // This is the CGO_ENABLED=1 implementation of this function, which
-// always returns the string "system", because in this scenario
+// always returns the string [StdlibResolverGetaddrinfo], because in this scenario
 // we are actually calling the getaddrinfo libc function.
+//
+// See https://github.com/ooni/spec/pull/257 for more information on how
+// we evolved our naming of the "stdlib" resolver over time.
 func getaddrinfoResolverNetwork() string {
-	return "system"
+	return StdlibResolverGetaddrinfo
 }
 
 // getaddrinfoLookupANY attempts to perform an ANY lookup using getaddrinfo.

internal/netxlite/getaddrinfo_otherwise.go

@@ -11,13 +11,16 @@ import (
 // been used to implement the getaddrinfo resolver.
 //
 // This is the CGO_ENABLED=0 implementation of this function, which
-// always returns the string "go", because in this scenario we are actually
-// using whatever resolver is used under the hood by the stdlib.
+// always returns the string [StdlibResolverGolangNetResolver], because in this scenario
+// we are actually using whatever resolver is used under the hood by the stdlib.
 //
 // See https://github.com/ooni/probe/issues/2029#issuecomment-1140805266
-// for an explanation of why calling this resolver "netgo" is wrong.
+// for an explanation of why calling this resolver "netgo" was wrong.
+//
+// See https://github.com/ooni/spec/pull/257 for additional documentation
+// regarding using "golang_net_resolver" instead of "go".
 func getaddrinfoResolverNetwork() string {
-	return "go"
+	return StdlibResolverGolangNetResolver
 }
 
 // getaddrinfoLookupANY attempts to perform an ANY lookup using getaddrinfo.

internal/netxlite/resolvercore.go

@@ -20,11 +20,11 @@ import (
 
 // ErrNoDNSTransport is the error returned when you attempt to perform
 // a DNS operation that requires a custom DNSTransport (e.g., DNSOverHTTPSTransport)
-// but you are using the "system" resolver instead.
+// but you are using the "stdlib" resolver instead.
 var ErrNoDNSTransport = errors.New("operation requires a DNS transport")
 
 // NewStdlibResolver creates a new Resolver by combining WrapResolver
-// with an internal "system" resolver type. The list of optional wrappers
+// with an internal "stdlib" resolver type. The list of optional wrappers
 // allow to wrap the underlying getaddrinfo transport. Any nil wrapper
 // will be silently ignored by the code that performs the wrapping.
 func NewStdlibResolver(logger model.DebugLogger, wrappers ...model.DNSTransportWrapper) model.Resolver {
@@ -45,7 +45,7 @@ func NewParallelDNSOverHTTPSResolver(logger model.DebugLogger, URL string) model
 // implies, this function returns an unwrapped resolver.
 func NewUnwrappedStdlibResolver(wrappers ...model.DNSTransportWrapper) model.Resolver {
 	return &resolverSystem{
-		t: WrapDNSTransport(&dnsOverGetaddrinfoTransport{}, wrappers...),
+		t: WrapDNSTransport(NewDNSOverGetaddrinfoTransport(), wrappers...),
 	}
 }
 

internal/netxlite/resolvercore_test.go

@@ -347,7 +347,7 @@ func TestResolverLogger(t *testing.T) {
 						return expected, nil
 					},
 					MockNetwork: func() string {
-						return "system"
+						return StdlibResolverGetaddrinfo
 					},
 					MockAddress: func() string {
 						return ""
@@ -381,7 +381,7 @@ func TestResolverLogger(t *testing.T) {
 						return nil, expected
 					},
 					MockNetwork: func() string {
-						return "system"
+						return StdlibResolverGetaddrinfo
 					},
 					MockAddress: func() string {
 						return ""
@@ -420,7 +420,7 @@ func TestResolverLogger(t *testing.T) {
 						return expected, nil
 					},
 					MockNetwork: func() string {
-						return "system"
+						return StdlibResolverGetaddrinfo
 					},
 					MockAddress: func() string {
 						return ""
@@ -454,7 +454,7 @@ func TestResolverLogger(t *testing.T) {
 						return nil, expected
 					},
 					MockNetwork: func() string {
-						return "system"
+						return StdlibResolverGetaddrinfo
 					},
 					MockAddress: func() string {
 						return ""
@@ -509,7 +509,7 @@ func TestResolverLogger(t *testing.T) {
 						return expected, nil
 					},
 					MockNetwork: func() string {
-						return "system"
+						return StdlibResolverGetaddrinfo
 					},
 					MockAddress: func() string {
 						return ""
@@ -543,7 +543,7 @@ func TestResolverLogger(t *testing.T) {
 						return nil, expected
 					},
 					MockNetwork: func() string {
-						return "system"
+						return StdlibResolverGetaddrinfo
 					},
 					MockAddress: func() string {
 						return ""