feat(openvpn): add a top-level test key with bootstrap time and handshake failure

internal/experiment/openvpn/openvpn.go

@@ -17,7 +17,7 @@ import (
 
 const (
 	testName        = "openvpn"
-	testVersion     = "0.1.3"
+	testVersion     = "0.1.4"
 	openVPNProtocol = "openvpn"
 )
 
@@ -48,6 +48,9 @@ type TestKeys struct {
 	NetworkEvents    []*vpntracex.Event                      `json:"network_events"`
 	TCPConnect       []*model.ArchivalTCPConnectResult       `json:"tcp_connect,omitempty"`
 	OpenVPNHandshake []*model.ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"`
+	BootstrapTime    float64                                 `json:"bootstrap_time"`
+	Tunnel           string                                  `json:"tunnel"`
+	Failure          *string                                 `json:"failure"`
 }
 
 // NewTestKeys creates new openvpn TestKeys.
@@ -57,17 +60,20 @@ func NewTestKeys() *TestKeys {
 		NetworkEvents:    []*vpntracex.Event{},
 		TCPConnect:       []*model.ArchivalTCPConnectResult{},
 		OpenVPNHandshake: []*model.ArchivalOpenVPNHandshakeResult{},
+		BootstrapTime:    0,
+		Tunnel:           "openvpn",
+		Failure:          nil,
 	}
 }
 
 // SingleConnection contains the results of a single handshake.
 type SingleConnection struct {
+	BootstrapTime    float64
 	TCPConnect       *model.ArchivalTCPConnectResult       `json:"tcp_connect,omitempty"`
 	OpenVPNHandshake *model.ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"`
 	NetworkEvents    []*vpntracex.Event                    `json:"network_events"`
-	// TODO(ainghazal): make sure to document in the spec that these network events only cover the handshake.
 	// TODO(ainghazal): in the future, we will want to store more operations under this struct for a single connection,
-	// like pingResults or urlgetter calls.
+	// like pingResults or urlgetter calls. Be sure to modify the spec when that happens.
 }
 
 // AddConnectionTestKeys adds the result of a single OpenVPN connection attempt to the
@@ -79,6 +85,14 @@ func (tk *TestKeys) AddConnectionTestKeys(result *SingleConnection) {
 	}
 	tk.OpenVPNHandshake = append(tk.OpenVPNHandshake, result.OpenVPNHandshake)
 	tk.NetworkEvents = append(tk.NetworkEvents, result.NetworkEvents...)
+
+	// we assume one measurement has exactly one effective connection
+	tk.BootstrapTime = result.BootstrapTime
+
+	if result.OpenVPNHandshake.Failure != nil {
+		tk.Failure = result.OpenVPNHandshake.Failure
+		tk.BootstrapTime = 0
+	}
 }
 
 // AllConnectionsSuccessful returns true if all the registered handshakes have nil failures.
@@ -147,12 +161,19 @@ func (m *Measurer) connectAndHandshake(
 	handshakeEvents := handshakeTracer.Trace()
 	port, _ := strconv.Atoi(endpoint.Port)
 
-	t0, t, bootstrapTime := TimestampsFromHandshake(handshakeEvents)
+	t0, t, handshakeTime := TimestampsFromHandshake(handshakeEvents)
+
+	// the bootstrap time is defined to be zero if there's a handshake failure.
+	var bootstrapTime float64
+	if err == nil {
+		bootstrapTime = time.Since(zeroTime).Seconds()
+	}
 
 	return &SingleConnection{
-		TCPConnect: trace.FirstTCPConnectOrNil(),
+		BootstrapTime: bootstrapTime,
+		TCPConnect:    trace.FirstTCPConnectOrNil(),
 		OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{
-			BootstrapTime: bootstrapTime,
+			HandshakeTime: handshakeTime,
 			Endpoint:      endpoint.String(),
 			Failure:       measurexlite.NewFailure(err),
 			IP:            endpoint.IPAddr,
@@ -189,20 +210,6 @@ func TimestampsFromHandshake(events []*vpntracex.Event) (float64, float64, float
 	return t0, t, duration
 }
 
-// FetchProviderCredentials will extract credentials from the configuration we gathered for a given provider.
-func (m *Measurer) FetchProviderCredentials(
-	ctx context.Context,
-	sess model.ExperimentSession,
-	provider string) (*model.OOAPIVPNProviderConfig, error) {
-	// TODO(ainghazal): pass real country code, can be useful to orchestrate campaigns specific to areas.
-	// Since we have contacted the API previously, this call should use the cached info contained in the session.
-	config, err := sess.FetchOpenVPNConfig(ctx, provider, "XX")
-	if err != nil {
-		return nil, err
-	}
-	return config, nil
-}
-
 // Run implements model.ExperimentMeasurer.Run.
 // A single run expects exactly ONE input (endpoint), but we can modify whether
 // to test different transports by settings options.

internal/experiment/openvpn/openvpn_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	vpntracex "github.com/ooni/minivpn/pkg/tracex"
 	"github.com/ooni/probe-cli/v3/internal/experiment/openvpn"
+	"github.com/ooni/probe-cli/v3/internal/measurexlite"
 	"github.com/ooni/probe-cli/v3/internal/mocks"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/targetloading"
@@ -40,7 +41,7 @@ func TestNewExperimentMeasurer(t *testing.T) {
 	if m.ExperimentName() != "openvpn" {
 		t.Fatal("invalid ExperimentName")
 	}
-	if m.ExperimentVersion() != "0.1.3" {
+	if m.ExperimentVersion() != "0.1.4" {
 		t.Fatal("invalid ExperimentVersion")
 	}
 }
@@ -79,7 +80,7 @@ func TestAddConnectionTestKeys(t *testing.T) {
 				TransactionID: 1,
 			},
 			OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{
-				BootstrapTime:  1,
+				HandshakeTime:  1,
 				Endpoint:       "aa",
 				Failure:        nil,
 				IP:             "1.1.1.1",
@@ -111,7 +112,7 @@ func TestAddConnectionTestKeys(t *testing.T) {
 		sc := &openvpn.SingleConnection{
 			TCPConnect: nil,
 			OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{
-				BootstrapTime:  1,
+				HandshakeTime:  1,
 				Endpoint:       "aa",
 				Failure:        nil,
 				IP:             "1.1.1.1",
@@ -212,43 +213,6 @@ func TestBadTargetURLFailure(t *testing.T) {
 	}
 }
 
-func TestVPNInput(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skip test in short mode")
-	}
-	// TODO(ainghazal): do a real test, get credentials etc.
-}
-
-func TestMeasurer_FetchProviderCredentials(t *testing.T) {
-	t.Run("Measurer.FetchProviderCredentials calls method in session", func(t *testing.T) {
-		m := openvpn.NewExperimentMeasurer().(*openvpn.Measurer)
-
-		sess := makeMockSession()
-		_, err := m.FetchProviderCredentials(
-			context.Background(),
-			sess, "riseup")
-		if err != nil {
-			t.Fatal("expected no error")
-		}
-	})
-	t.Run("Measurer.FetchProviderCredentials raises error if API calls fail", func(t *testing.T) {
-		someError := errors.New("unexpected")
-
-		m := openvpn.NewExperimentMeasurer().(*openvpn.Measurer)
-
-		sess := makeMockSession()
-		sess.MockFetchOpenVPNConfig = func(context.Context, string, string) (*model.OOAPIVPNProviderConfig, error) {
-			return nil, someError
-		}
-		_, err := m.FetchProviderCredentials(
-			context.Background(),
-			sess, "riseup")
-		if !errors.Is(err, someError) {
-			t.Fatalf("expected error %v, got %v", someError, err)
-		}
-	})
-}
-
 func TestSuccess(t *testing.T) {
 	m := openvpn.NewExperimentMeasurer()
 	ctx := context.Background()
@@ -313,3 +277,108 @@ func TestTimestampsFromHandshake(t *testing.T) {
 		}
 	})
 }
+
+func TestBootstrapTimeWithNoFailure(t *testing.T) {
+	bootstrapTime := 1.2305
+	tk := openvpn.NewTestKeys()
+	sc := &openvpn.SingleConnection{
+		BootstrapTime: bootstrapTime,
+		TCPConnect: &model.ArchivalTCPConnectResult{
+			IP:   "1.1.1.1",
+			Port: 1194,
+			Status: model.ArchivalTCPConnectStatus{
+				Blocked: new(bool),
+				Failure: new(string),
+				Success: false,
+			},
+			T0:            0.1,
+			T:             0.9,
+			Tags:          []string{},
+			TransactionID: 1,
+		},
+		OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{
+			HandshakeTime:  1.20,
+			Endpoint:       "aa",
+			Failure:        nil,
+			IP:             "1.1.1.1",
+			Port:           1194,
+			Transport:      "tcp",
+			Provider:       "unknown",
+			OpenVPNOptions: model.ArchivalOpenVPNOptions{},
+			T0:             0.03,
+			T:              1.23,
+			Tags:           []string{},
+			TransactionID:  1,
+		},
+		NetworkEvents: []*vpntracex.Event{},
+	}
+	tk.AddConnectionTestKeys(sc)
+
+	if tk.Failure != nil {
+		t.Fatal("expected nil failure")
+	}
+	if tk.BootstrapTime != bootstrapTime {
+		t.Fatal("wrong bootstrap time")
+	}
+	if tk.Tunnel != "openvpn" {
+		t.Fatal("tunnel should be openvpn")
+	}
+}
+
+func TestBootstrapTimeWithFailure(t *testing.T) {
+	bootstrapTime := 6.1
+
+	handshakeError := errors.New("mocked error")
+	handshakeFailure := measurexlite.NewFailure(handshakeError)
+
+	tk := openvpn.NewTestKeys()
+	sc := &openvpn.SingleConnection{
+		BootstrapTime: bootstrapTime,
+		TCPConnect: &model.ArchivalTCPConnectResult{
+			IP:   "1.1.1.1",
+			Port: 1194,
+			Status: model.ArchivalTCPConnectStatus{
+				Blocked: new(bool),
+				Failure: new(string),
+				Success: false,
+			},
+			T0:            0.1,
+			T:             0.9,
+			Tags:          []string{},
+			TransactionID: 1,
+		},
+		OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{
+			HandshakeTime:  1.20,
+			Endpoint:       "aa",
+			Failure:        handshakeFailure,
+			IP:             "1.1.1.1",
+			Port:           1194,
+			Transport:      "tcp",
+			Provider:       "unknown",
+			OpenVPNOptions: model.ArchivalOpenVPNOptions{},
+			T0:             0.03,
+			T:              1.23,
+			Tags:           []string{},
+			TransactionID:  1,
+		},
+		NetworkEvents: []*vpntracex.Event{},
+	}
+	tk.AddConnectionTestKeys(sc)
+
+	if tk.Failure != handshakeFailure {
+		t.Fatalf("expected handshake failure, got %v", tk.Failure)
+	}
+	if tk.BootstrapTime != 0 {
+		t.Fatalf("wrong bootstrap time: expected 0, got %v", tk.BootstrapTime)
+	}
+	if tk.Tunnel != "openvpn" {
+		t.Fatal("tunnel should be openvpn")
+	}
+}
+
+func TestVPNInput(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skip test in short mode")
+	}
+	// TODO(ainghazal): do a real test, get credentials etc.
+}

internal/experiment/openvpn/richerinput_test.go

@@ -225,5 +225,4 @@ func TestTargetLoaderLoadFromBackend(t *testing.T) {
 	if targets[1].String() != "openvpn://target1" {
 		t.Fatal("expected openvpn://target1")
 	}
-
 }

internal/model/archival.go

@@ -399,9 +399,9 @@ type ArchivalNetworkEvent struct {
 
 // ArchivalOpenVPNHandshakeResult contains the result of a OpenVPN handshake.
 type ArchivalOpenVPNHandshakeResult struct {
-	BootstrapTime  float64                `json:"bootstrap_time,omitempty"`
 	Endpoint       string                 `json:"endpoint"`
 	Failure        *string                `json:"failure"`
+	HandshakeTime  float64                `json:"handshake_time,omitempty"`
 	IP             string                 `json:"ip"`
 	Port           int                    `json:"port"`
 	Transport      string                 `json:"transport"`