urlgetter.TestKeys: add extra fields to simplify analysis

We often need to check the status code of the last response in the chain
and/or to see the body of such response.

To simplify these use cases, introduce two new variables that contain such
values. These variables are not JSON serialised, because it does not make
sense for us to do that. One can always recompute these fields, and the point
of this patch is just to avoid duplicating code.

Added an extra integration test to make sure that the value of these new
variables is okay not only when the context is canceled but also when we're
not doing HTTP but something else, e.g., just TLS.

Part of #646.

We will use this new code to simplify post processing of Telegram results.

experiment/urlgetter/getter.go

@@ -47,6 +47,11 @@ func (g Getter) Get(ctx context.Context) (TestKeys, error) {
 	tk.Requests = append(
 		tk.Requests, archival.NewRequestList(begin, events)...,
 	)
+	if len(tk.Requests) > 0 {
+		// OONI's convention is that the last request appears first
+		tk.HTTPResponseStatus = tk.Requests[0].Response.Code
+		tk.HTTPResponseBody = tk.Requests[0].Response.Body.Value
+	}
 	tk.TCPConnect = append(
 		tk.TCPConnect, archival.NewTCPConnectList(begin, events)...,
 	)

experiment/urlgetter/getter_test.go

@@ -73,6 +73,12 @@ func TestGetterWithCancelledContextVanilla(t *testing.T) {
 	if tk.Tunnel != "" {
 		t.Fatal("not the Tunnel we expected")
 	}
+	if tk.HTTPResponseStatus != 0 {
+		t.Fatal("not the HTTPResponseStatus we expected")
+	}
+	if tk.HTTPResponseBody != "" {
+		t.Fatal("not the HTTPResponseBody we expected")
+	}
 }
 
 func TestGetterWithCancelledContextAndMethod(t *testing.T) {
@@ -135,6 +141,12 @@ func TestGetterWithCancelledContextAndMethod(t *testing.T) {
 	if tk.Tunnel != "" {
 		t.Fatal("not the Tunnel we expected")
 	}
+	if tk.HTTPResponseStatus != 0 {
+		t.Fatal("not the HTTPResponseStatus we expected")
+	}
+	if tk.HTTPResponseBody != "" {
+		t.Fatal("not the HTTPResponseBody we expected")
+	}
 }
 
 func TestGetterWithCancelledContextNoFollowRedirects(t *testing.T) {
@@ -199,6 +211,12 @@ func TestGetterWithCancelledContextNoFollowRedirects(t *testing.T) {
 	if tk.Tunnel != "" {
 		t.Fatal("not the Tunnel we expected")
 	}
+	if tk.HTTPResponseStatus != 0 {
+		t.Fatal("not the HTTPResponseStatus we expected")
+	}
+	if tk.HTTPResponseBody != "" {
+		t.Fatal("not the HTTPResponseBody we expected")
+	}
 }
 
 func TestGetterWithCancelledContextCannotStartTunnel(t *testing.T) {
@@ -247,6 +265,12 @@ func TestGetterWithCancelledContextCannotStartTunnel(t *testing.T) {
 	if tk.Tunnel != "" {
 		t.Fatal("not the Tunnel we expected")
 	}
+	if tk.HTTPResponseStatus != 0 {
+		t.Fatal("not the HTTPResponseStatus we expected")
+	}
+	if tk.HTTPResponseBody != "" {
+		t.Fatal("not the HTTPResponseBody we expected")
+	}
 }
 
 func TestGetterWithCancelledContextWithTunnel(t *testing.T) {
@@ -315,6 +339,12 @@ func TestGetterWithCancelledContextWithTunnel(t *testing.T) {
 	if tk.Tunnel != "psiphon" {
 		t.Fatal("not the Tunnel we expected")
 	}
+	if tk.HTTPResponseStatus != 0 {
+		t.Fatal("not the HTTPResponseStatus we expected")
+	}
+	if tk.HTTPResponseBody != "" {
+		t.Fatal("not the HTTPResponseBody we expected")
+	}
 }
 
 func TestGetterWithCancelledContextUnknownResolverURL(t *testing.T) {
@@ -364,9 +394,15 @@ func TestGetterWithCancelledContextUnknownResolverURL(t *testing.T) {
 	if tk.Tunnel != "" {
 		t.Fatal("not the Tunnel we expected")
 	}
+	if tk.HTTPResponseStatus != 0 {
+		t.Fatal("not the HTTPResponseStatus we expected")
+	}
+	if tk.HTTPResponseBody != "" {
+		t.Fatal("not the HTTPResponseBody we expected")
+	}
 }
 
-func TestGetterIntegration(t *testing.T) {
+func TestGetterIntegrationHTTPS(t *testing.T) {
 	ctx := context.Background()
 	g := urlgetter.Getter{
 		Config: urlgetter.Config{
@@ -477,4 +513,123 @@ func TestGetterIntegration(t *testing.T) {
 	if tk.Tunnel != "" {
 		t.Fatal("not the Tunnel we expected")
 	}
+	if tk.HTTPResponseStatus != 200 {
+		t.Fatal("not the HTTPResponseStatus we expected")
+	}
+	if len(tk.HTTPResponseBody) <= 0 {
+		t.Fatal("not the HTTPResponseBody we expected")
+	}
+}
+
+func TestGetterIntegrationTLSHandshake(t *testing.T) {
+	ctx := context.Background()
+	g := urlgetter.Getter{
+		Config: urlgetter.Config{
+			NoFollowRedirects: true, // reduce number of events
+		},
+		Session: &mockable.ExperimentSession{},
+		Target:  "tlshandshake://www.google.com:443",
+	}
+	tk, err := g.Get(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if tk.Agent != "agent" {
+		t.Fatal("not the Agent we expected")
+	}
+	if tk.BootstrapTime != 0 {
+		t.Fatal("not the BootstrapTime we expected")
+	}
+	if tk.FailedOperation != nil {
+		t.Fatal("not the FailedOperation we expected")
+	}
+	if tk.Failure != nil {
+		t.Fatal("not the Failure we expected")
+	}
+	var (
+		httpTransactionStart     bool
+		httpRequestMetadata      bool
+		resolveStart             bool
+		resolveDone              bool
+		connect                  bool
+		tlsHandshakeStart        bool
+		tlsHandshakeDone         bool
+		httpWroteHeaders         bool
+		httpWroteRequest         bool
+		httpFirstResponseByte    bool
+		httpResponseMetadata     bool
+		httpResponseBodySnapshot bool
+		httpTransactionDone      bool
+	)
+	for _, ev := range tk.NetworkEvents {
+		switch ev.Operation {
+		case "http_transaction_start":
+			httpTransactionStart = true
+		case "http_request_metadata":
+			httpRequestMetadata = true
+		case "resolve_start":
+			resolveStart = true
+		case "resolve_done":
+			resolveDone = true
+		case modelx.ConnectOperation:
+			connect = true
+		case "tls_handshake_start":
+			tlsHandshakeStart = true
+		case "tls_handshake_done":
+			tlsHandshakeDone = true
+		case "http_wrote_headers":
+			httpWroteHeaders = true
+		case "http_wrote_request":
+			httpWroteRequest = true
+		case "http_first_response_byte":
+			httpFirstResponseByte = true
+		case "http_response_metadata":
+			httpResponseMetadata = true
+		case "http_response_body_snapshot":
+			httpResponseBodySnapshot = true
+		case "http_transaction_done":
+			httpTransactionDone = true
+		}
+	}
+	ok := true
+	ok = ok && !httpTransactionStart
+	ok = ok && !httpRequestMetadata
+	ok = ok && resolveStart
+	ok = ok && resolveDone
+	ok = ok && connect
+	ok = ok && tlsHandshakeStart
+	ok = ok && tlsHandshakeDone
+	ok = ok && !httpWroteHeaders
+	ok = ok && !httpWroteRequest
+	ok = ok && !httpFirstResponseByte
+	ok = ok && !httpResponseMetadata
+	ok = ok && !httpResponseBodySnapshot
+	ok = ok && !httpTransactionDone
+	if !ok {
+		t.Fatal("not the NetworkEvents we expected")
+	}
+	if len(tk.Queries) != 2 {
+		t.Fatal("not the Queries we expected")
+	}
+	if len(tk.TCPConnect) != 1 {
+		t.Fatal("not the TCPConnect we expected")
+	}
+	if len(tk.Requests) != 0 {
+		t.Fatal("not the Requests we expected")
+	}
+	if tk.SOCKSProxy != "" {
+		t.Fatal("not the SOCKSProxy we expected")
+	}
+	if len(tk.TLSHandshakes) != 1 {
+		t.Fatal("not the TLSHandshakes we expected")
+	}
+	if tk.Tunnel != "" {
+		t.Fatal("not the Tunnel we expected")
+	}
+	if tk.HTTPResponseStatus != 0 {
+		t.Fatal("not the HTTPResponseStatus we expected")
+	}
+	if tk.HTTPResponseBody != "" {
+		t.Fatal("not the HTTPResponseBody we expected")
+	}
 }

experiment/urlgetter/urlgetter.go

@@ -31,6 +31,7 @@ type Config struct {
 
 // TestKeys contains the experiment's result.
 type TestKeys struct {
+	// The following fields are part of the typical JSON emitted by OONI.
 	Agent           string                     `json:"agent"`
 	BootstrapTime   float64                    `json:"bootstrap_time,omitempty"`
 	DNSCache        []string                   `json:"dns_cache,omitempty"`
@@ -43,6 +44,11 @@ type TestKeys struct {
 	SOCKSProxy      string                     `json:"socksproxy,omitempty"`
 	TLSHandshakes   []archival.TLSHandshake    `json:"tls_handshakes"`
 	Tunnel          string                     `json:"tunnel,omitempty"`
+
+	// The following fields are not serialised but are useful to simplify
+	// analysing the measurements in telegram, etc.
+	HTTPResponseStatus int64  `json:"-"`
+	HTTPResponseBody   string `json:"-"`
 }
 
 // RegisterExtensions registers the extensions used by the urlgetter

netx/archival/archival.go

@@ -304,7 +304,7 @@ func addheaders(
 
 // NewRequestList returns the list for "requests"
 func NewRequestList(begin time.Time, events []trace.Event) []RequestEntry {
-	// OONI wants the least request to appear first
+	// OONI wants the last request to appear first
 	var out []RequestEntry
 	tmp := newRequestList(begin, events)
 	for i := len(tmp) - 1; i >= 0; i-- {