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

See what we documented at https://github.com/ooni/spec/pull/257 Reference issue: https://github.com/ooni/probe/issues/2238 See also the related ooni/spec PR: https://github.com/ooni/spec/pull/257 See also https://github.com/ooni/probe/issues/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.
2022-08-27 15:47:48 +02:00
parent c3964e43b3
commit 8a0c062844
19 changed files with 362 additions and 59 deletions
@@ -10,6 +10,7 @@ import (
 	"time"

 	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
 )

 // ResolverSaver is a resolver that saves events.
@@ -42,7 +43,7 @@ func (r *ResolverSaver) LookupHost(ctx context.Context, hostname string) ([]stri
 	r.Saver.Write(&EventResolveStart{&EventValue{
 		Address:  r.Resolver.Address(),
 		Hostname: hostname,
-		Proto:    r.Resolver.Network(),
+		Proto:    r.Network(),
 		Time:     start,
 	}})
 	addrs, err := r.Resolver.LookupHost(ctx, hostname)
@@ -53,14 +54,29 @@ func (r *ResolverSaver) LookupHost(ctx context.Context, hostname string) ([]stri
 		Duration:  stop.Sub(start),
 		Err:       NewFailureStr(err),
 		Hostname:  hostname,
-		Proto:     r.Resolver.Network(),
+		Proto:     r.Network(),
 		Time:      stop,
 	}})
 	return addrs, err
 }

+// ResolverNetworkAdaptNames makes sure we map the [netxlite.StdlibResolverGolangNetResolver] and
+// [netxlite.StdlibResolverGetaddrinfo] resolver names to [netxlite.StdlibResolverSystem]. You MUST
+// call this function when your resolver splits the "stdlib" resolver results into two fake AAAA
+// and A queries rather than faking a single ANY query.
+//
+// See https://github.com/ooni/spec/pull/257 for more information.
+func ResolverNetworkAdaptNames(input string) string {
+	switch input {
+	case netxlite.StdlibResolverGetaddrinfo, netxlite.StdlibResolverGolangNetResolver:
+		return netxlite.StdlibResolverSystem
+	default:
+		return input
+	}
+}
+
 func (r *ResolverSaver) Network() string {
-	return r.Resolver.Network()
+	return ResolverNetworkAdaptNames(r.Resolver.Network())
 }

 func (r *ResolverSaver) Address() string {
@@ -112,7 +128,7 @@ func (txp *DNSTransportSaver) RoundTrip(
 	txp.Saver.Write(&EventDNSRoundTripStart{&EventValue{
 		Address:  txp.DNSTransport.Address(),
 		DNSQuery: dnsMaybeQueryBytes(query),
-		Proto:    txp.DNSTransport.Network(),
+		Proto:    txp.Network(),
 		Time:     start,
 	}})
 	response, err := txp.DNSTransport.RoundTrip(ctx, query)
@@ -123,14 +139,14 @@ func (txp *DNSTransportSaver) RoundTrip(
 		DNSResponse: dnsMaybeResponseBytes(response),
 		Duration:    stop.Sub(start),
 		Err:         NewFailureStr(err),
-		Proto:       txp.DNSTransport.Network(),
+		Proto:       txp.Network(),
 		Time:        stop,
 	}})
 	return response, err
 }

 func (txp *DNSTransportSaver) Network() string {
-	return txp.DNSTransport.Network()
+	return ResolverNetworkAdaptNames(txp.DNSTransport.Network())
 }

 func (txp *DNSTransportSaver) Address() string {
@@ -9,6 +9,7 @@ import (
 	"testing"
 	"time"

+	"github.com/miekg/dns"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/model/mocks"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
@@ -75,7 +76,7 @@ func TestResolverSaver(t *testing.T) {
 			reso := saver.WrapResolver(newFakeResolverWithResult(expected))
 			addrs, err := reso.LookupHost(context.Background(), "www.google.com")
 			if err != nil {
-				t.Fatal("expected nil error here")
+				t.Fatal(err)
 			}
 			if !reflect.DeepEqual(addrs, expected) {
 				t.Fatal("not the result we expected")
@@ -112,19 +113,56 @@ func TestResolverSaver(t *testing.T) {
 				t.Fatal("the saved time is wrong")
 			}
 		})
+
+		t.Run("with stdlib resolver there's correct .Network remapping", func(t *testing.T) {
+			saver := &Saver{}
+			reso := saver.WrapResolver(netxlite.NewStdlibResolver(model.DiscardLogger))
+			ctx, cancel := context.WithCancel(context.Background())
+			cancel() // immediately fail the operation
+			_, _ = reso.LookupHost(ctx, "www.google.com")
+			// basically, we just want to ensure that the engine name is converted
+			ev := saver.Read()
+			if len(ev) != 2 {
+				t.Fatal("expected number of events")
+			}
+			if ev[0].Value().Proto != netxlite.StdlibResolverSystem {
+				t.Fatal("unexpected Proto")
+			}
+			if ev[1].Value().Proto != netxlite.StdlibResolverSystem {
+				t.Fatal("unexpected Proto")
+			}
+		})
 	})

 	t.Run("Network", func(t *testing.T) {
-		saver := &Saver{}
-		child := &mocks.Resolver{
-			MockNetwork: func() string {
-				return "x"
-			},
-		}
-		reso := saver.WrapResolver(child)
-		if reso.Network() != "x" {
-			t.Fatal("unexpected result")
-		}
+		t.Run("when using a custom resolver", func(t *testing.T) {
+			saver := &Saver{}
+			child := &mocks.Resolver{
+				MockNetwork: func() string {
+					return "x"
+				},
+			}
+			reso := saver.WrapResolver(child)
+			if reso.Network() != "x" {
+				t.Fatal("unexpected result")
+			}
+		})
+
+		t.Run("when using the stdlib resolver", func(t *testing.T) {
+			child := netxlite.NewStdlibResolver(model.DiscardLogger)
+			switch network := child.Network(); network {
+			case netxlite.StdlibResolverGetaddrinfo,
+				netxlite.StdlibResolverGolangNetResolver:
+				// ok
+			default:
+				t.Fatal("unexpected child resolver network", network)
+			}
+			saver := &Saver{}
+			reso := saver.WrapResolver(child)
+			if network := reso.Network(); network != netxlite.StdlibResolverSystem {
+				t.Fatal("unexpected wrapped resolver network", network)
+			}
+		})
 	})

 	t.Run("Address", func(t *testing.T) {
@@ -326,19 +364,70 @@ func TestDNSTransportSaver(t *testing.T) {
 				t.Fatal("the saved time is wrong")
 			}
 		})
+
+		t.Run("with getaddrinfo transport there's correct .Network remapping", func(t *testing.T) {
+			saver := &Saver{}
+			reso := saver.WrapDNSTransport(netxlite.NewDNSOverGetaddrinfoTransport())
+			ctx, cancel := context.WithCancel(context.Background())
+			cancel() // immediately fail the operation
+			query := &mocks.DNSQuery{
+				MockBytes: func() ([]byte, error) {
+					return []byte{}, nil
+				},
+				MockType: func() uint16 {
+					return dns.TypeANY
+				},
+				MockID: func() uint16 {
+					return 1453
+				},
+				MockDomain: func() string {
+					return "dns.google"
+				},
+			}
+			_, _ = reso.RoundTrip(ctx, query)
+			// basically, we just want to ensure that the engine name is converted
+			ev := saver.Read()
+			if len(ev) != 2 {
+				t.Fatal("expected number of events")
+			}
+			if ev[0].Value().Proto != netxlite.StdlibResolverSystem {
+				t.Fatal("unexpected Proto")
+			}
+			if ev[1].Value().Proto != netxlite.StdlibResolverSystem {
+				t.Fatal("unexpected Proto")
+			}
+		})
 	})

 	t.Run("Network", func(t *testing.T) {
-		saver := &Saver{}
-		child := &mocks.DNSTransport{
-			MockNetwork: func() string {
-				return "x"
-			},
-		}
-		txp := saver.WrapDNSTransport(child)
-		if txp.Network() != "x" {
-			t.Fatal("unexpected result")
-		}
+		t.Run("with custom child transport", func(t *testing.T) {
+			saver := &Saver{}
+			child := &mocks.DNSTransport{
+				MockNetwork: func() string {
+					return "x"
+				},
+			}
+			txp := saver.WrapDNSTransport(child)
+			if txp.Network() != "x" {
+				t.Fatal("unexpected result")
+			}
+		})
+
+		t.Run("when using the stdlib resolver", func(t *testing.T) {
+			child := netxlite.NewDNSOverGetaddrinfoTransport()
+			switch network := child.Network(); network {
+			case netxlite.StdlibResolverGetaddrinfo,
+				netxlite.StdlibResolverGolangNetResolver:
+				// ok
+			default:
+				t.Fatal("unexpected child resolver network", network)
+			}
+			saver := &Saver{}
+			reso := saver.WrapDNSTransport(child)
+			if network := reso.Network(); network != netxlite.StdlibResolverSystem {
+				t.Fatal("unexpected wrapped resolver network", network)
+			}
+		})
 	})

 	t.Run("Address", func(t *testing.T) {
@@ -429,3 +518,45 @@ func newFakeResolverWithResult(r []string) model.Resolver {
 		},
 	}
 }
+
+func TestResolverNetworkAdaptNames(t *testing.T) {
+	type args struct {
+		input string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{{
+		name: "with StdlibResolverGetaddrinfo",
+		args: args{
+			input: netxlite.StdlibResolverGetaddrinfo,
+		},
+		want: netxlite.StdlibResolverSystem,
+	}, {
+		name: "with StdlibResolverGolangNetResolver",
+		args: args{
+			input: netxlite.StdlibResolverGolangNetResolver,
+		},
+		want: netxlite.StdlibResolverSystem,
+	}, {
+		name: "with StdlibResolverSystem",
+		args: args{
+			input: netxlite.StdlibResolverSystem,
+		},
+		want: netxlite.StdlibResolverSystem,
+	}, {
+		name: "with any other name",
+		args: args{
+			input: "doh",
+		},
+		want: "doh",
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := ResolverNetworkAdaptNames(tt.args.input); got != tt.want {
+				t.Errorf("ResolverNetworkAdaptNames() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}