Integrate further with ooni/probe-engine: episode two (#46)

* utils/geoip.go: use github.com/ooni/probe-engine

Let's start using the engine by rewriting utils/geoip.go to
be just a thin wrapper around the engine functionality.

* Ready for review

* Checkpoint: the im tests are converted

Still have some doubts with respect to the variables that
are passed to MK via probe-engine. Will double check.

* fix(i/c/r/run.go): write the correct logic

* nettests: one more comment and also fix a format string

* Tweak previous

* progress

* Fix doofus

* better comment

* XXX => actionable comment

* Add glue to simplify test keys management

Making the concept of measurement more abstract in the engine is
not feasible because, when submitting a measurement, we need to
modify it to update the report ID and the measurement ID. Therefore,
returning a serialized measurement is not a good idea. We will
keep using a model.Measurement in the engine.

Changing model.Measurement.TestKeys's type from a `interface{}`
pointing to a well defined data structure to `map[string]interface{}`
is a regression because means that we are moving from code that
has a clear and defined structure to code that is more complicated
to parse and validate. Since we're already suffering havily from
the lack of a good schema, I'm not going to make the situation
worst by worsening the engine. At least for ndt7 and psiphon, we
now have a good schema and I don't want to lose that.

However, the current code in this repository is expecting the
test keys to be a `map[string]interface{}`. This choice was
dictated by the fact that we receive a JSON from Measurement Kit
and by the fact that there's not a clear schema.

To solve this tension, in this commit I am going to write glue
adapter code that makes sure that the TestKeys of a Measurement
are converted to `map[string]interface{}`. This will be done
using a type cast where possible and JSON serialization and parsing
otherwise. In a perfect world, glue is not a good idea, but in a
real world it may actually be useful.

When all tests in the engine will have a clear Go data structure,
we'll then remove the glue and just cast to the proper data
structure from `interface{}` where required.

* nettests/performance: use probe-engine

* go.{mod,sum}: upgrade to latest probe-engine

* nettests/middlebox: use ooni/probe-engine

* Update to the latest probe-engine

* web_connectivity: rewrite to use probe-engine

* Cosmetic change suggested by @hellais

* nettests/nettests.go: remove unused code

* nettests/nettests.go: fix progress

* nettests/nettests.go: remove go-measurement-kit code

* We don't depend on go-measurement-kit anymore

* Improve non-verbose output where possible

See also: measurement-kit/measurement-kit#1856

* Make web_connectivity output pleasant

* Update to the latest probe-engine

* nettests/nettests.go: honour sharing settings

* Update to the latest probe-engine

* Use log.WithFields for probe-engine

* Update go.mod go.sum

* Revert "Update go.mod go.sum"

This reverts commit 5ecd38d.

* Revert "Revert "Update go.mod go.sum""

This reverts commit 6114b31.

* Upgrade ooni/probe-engine

* Unset GOPATH before running go build commands

* Dockefile: fix linux build by using latest

* Update to the latest ooni/probe-engine

```
go get -u github.com/ooni/probe-engine
go mod tidy
```

* Repair build

Dockerfile

@@ -1,3 +1,3 @@
-FROM openobservatory/mk-alpine:20190509
+FROM openobservatory/mk-alpine:latest
 RUN apk add --no-progress git go
 ADD . /oonibuild

Makefile

@@ -1,4 +1,4 @@
-GO ?= go
+GO ?= GOPATH="" go
 
 install-dev-deps:
 	@$(GO) get golang.org/x/tools/cmd/cover

go.mod

@@ -4,24 +4,24 @@ go 1.12
 
 require (
 	github.com/alecthomas/kingpin v2.2.6+incompatible
-	github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc // indirect
-	github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf // indirect
-	github.com/apex/log v1.1.0
+	github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751 // indirect
+	github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4 // indirect
+	github.com/apex/log v1.1.1
 	github.com/certifi/gocertifi v0.0.0-20180118203423-deb3ae2ef261 // indirect
 	github.com/fatih/color v1.7.0
 	github.com/getsentry/raven-go v0.0.0-20190419175539-919484f041ea
 	github.com/go-sql-driver/mysql v1.4.1 // indirect
 	github.com/gobuffalo/packr v1.25.0 // indirect
+	github.com/kr/pty v1.1.8 // indirect
 	github.com/lib/pq v1.1.1 // indirect
-	github.com/mattn/go-colorable v0.0.9
-	github.com/mattn/go-isatty v0.0.7 // indirect
+	github.com/mattn/go-colorable v0.1.2
 	github.com/mattn/go-sqlite3 v1.10.0 // indirect
-	github.com/measurement-kit/go-measurement-kit v0.0.0-20190521082856-635e836bbb9d
-	github.com/ooni/probe-engine v0.0.0-20190522105151-0c60545f09ea
+	github.com/ooni/probe-engine v0.0.0-20190814091928-473884e88632
+	github.com/oschwald/maxminddb-golang v1.3.1 // indirect
 	github.com/pkg/errors v0.8.1
 	github.com/rubenv/sql-migrate v0.0.0-20190327083759-54bad0a9b051
 	github.com/ziutek/mymysql v1.5.4 // indirect
-	google.golang.org/appengine v1.5.0 // indirect
+	google.golang.org/appengine v1.6.1 // indirect
 	gopkg.in/AlecAivazis/survey.v1 v1.8.4
 	gopkg.in/gorp.v1 v1.7.2 // indirect
 	upper.io/db.v3 v3.5.7+incompatible

internal/cli/run/run.go

@@ -1,6 +1,7 @@
 package run
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"strings"
@@ -95,6 +96,22 @@ func init() {
 			log.WithError(err).Error("Failed to create the network row")
 			return err
 		}
+		if ctx.Config.Advanced.BouncerURL != "" {
+			ctx.Session.AddAvailableHTTPSBouncer(ctx.Config.Advanced.BouncerURL)
+		}
+		if err := ctx.Session.MaybeLookupBackends(context.Background()); err != nil {
+			log.WithError(err).Warn("Failed to discover available test helpers")
+			// Rationale for falling through: some tests may be able to complete
+			// with no test helpers, so stopping may be excessive here.
+		}
+		if ctx.Config.Sharing.UploadResults {
+			if ctx.Config.Advanced.CollectorURL != "" {
+				ctx.Session.AddAvailableHTTPSCollector(ctx.Config.Advanced.CollectorURL)
+			} else if err := ctx.Session.MaybeLookupCollectors(context.Background()); err != nil {
+				log.WithError(err).Error("Failed to discover available collectors")
+				return err
+			}
+		}
 
 		if *nettestGroup == "" {
 			log.Infof("Running %s tests", color.BlueString("all"))

internal/enginex/enginex.go

@@ -0,0 +1,38 @@
+// Package enginex contains ooni/probe-engine extensions.
+package enginex
+
+import (
+	"encoding/json"
+
+	"github.com/apex/log"
+	"github.com/ooni/probe-engine/model"
+)
+
+// Logger is the logger used by the engine.
+var Logger = log.WithFields(log.Fields{
+  "type": "engine",
+})
+
+// MakeGenericTestKeys casts the m.TestKeys to a map[string]interface{}.
+//
+// Ideally, all tests should have a clear Go structure, well defined, that
+// will be stored in m.TestKeys as an interface. This is not already the
+// case and it's just valid for tests written in Go. Until all tests will
+// be written in Go, we'll keep this glue here to make sure we convert from
+// the engine format to the cli format.
+//
+// This function will first attempt to cast directly to map[string]interface{},
+// which is possible for MK tests, and then use JSON serialization and
+// de-serialization only if that's required.
+func MakeGenericTestKeys(m model.Measurement) (map[string]interface{}, error) {
+	if result, ok := m.TestKeys.(map[string]interface{}); ok {
+		return result, nil
+	}
+	data, err := json.Marshal(m.TestKeys)
+	if err != nil {
+		return nil, err
+	}
+	var result map[string]interface{}
+	err = json.Unmarshal(data, &result)
+	return result, err
+}

internal/log/handlers/cli/cli.go

@@ -104,6 +104,9 @@ func logTable(w io.Writer, f log.Fields) error {
 // TypedLog is used for handling special "typed" logs to the CLI
 func (h *Handler) TypedLog(t string, e *log.Entry) error {
 	switch t {
+	case "engine":
+		fmt.Fprintf(h.Writer, "[engine] %s\n", e.Message)
+		return nil
 	case "progress":
 		perc := e.Fields.Get("percentage").(float64) * 100
 		s := fmt.Sprintf("   %s %-25s",

nettests/im/facebook_messenger.go

@@ -1,8 +1,8 @@
 package im
 
 import (
-	"github.com/measurement-kit/go-measurement-kit"
 	"github.com/ooni/probe-cli/nettests"
+	"github.com/ooni/probe-engine/experiment/fbmessenger"
 )
 
 // FacebookMessenger test implementation
@@ -11,9 +11,10 @@ type FacebookMessenger struct {
 
 // Run starts the test
 func (h FacebookMessenger) Run(ctl *nettests.Controller) error {
-	mknt := mk.NewNettest("FacebookMessenger")
-	ctl.Init(mknt)
-	return mknt.Run()
+	experiment := fbmessenger.NewExperiment(ctl.Ctx.Session, fbmessenger.Config{
+		LogLevel: "INFO",
+	})
+	return ctl.Run(experiment, []string{""})
 }
 
 // FacebookMessengerTestKeys for the test

nettests/im/telegram.go

@@ -1,8 +1,8 @@
 package im
 
 import (
-	"github.com/measurement-kit/go-measurement-kit"
 	"github.com/ooni/probe-cli/nettests"
+	"github.com/ooni/probe-engine/experiment/telegram"
 )
 
 // Telegram test implementation
@@ -11,9 +11,10 @@ type Telegram struct {
 
 // Run starts the test
 func (h Telegram) Run(ctl *nettests.Controller) error {
-	mknt := mk.NewNettest("Telegram")
-	ctl.Init(mknt)
-	return mknt.Run()
+	experiment := telegram.NewExperiment(ctl.Ctx.Session, telegram.Config{
+		LogLevel: "INFO",
+	})
+	return ctl.Run(experiment, []string{""})
 }
 
 // TelegramTestKeys for the test

nettests/im/whatsapp.go

@@ -1,8 +1,8 @@
 package im
 
 import (
-	"github.com/measurement-kit/go-measurement-kit"
 	"github.com/ooni/probe-cli/nettests"
+	"github.com/ooni/probe-engine/experiment/whatsapp"
 )
 
 // WhatsApp test implementation
@@ -11,9 +11,10 @@ type WhatsApp struct {
 
 // Run starts the test
 func (h WhatsApp) Run(ctl *nettests.Controller) error {
-	mknt := mk.NewNettest("Whatsapp")
-	ctl.Init(mknt)
-	return mknt.Run()
+	experiment := whatsapp.NewExperiment(ctl.Ctx.Session, whatsapp.Config{
+		LogLevel: "INFO",
+	})
+	return ctl.Run(experiment, []string{""})
 }
 
 // WhatsAppTestKeys for the test

nettests/middlebox/http_header_field_manipulation.go

@@ -3,8 +3,8 @@ package middlebox
 import (
 	"errors"
 
-	"github.com/measurement-kit/go-measurement-kit"
 	"github.com/ooni/probe-cli/nettests"
+	"github.com/ooni/probe-engine/experiment/hhfm"
 )
 
 // HTTPHeaderFieldManipulation test implementation
@@ -13,9 +13,10 @@ type HTTPHeaderFieldManipulation struct {
 
 // Run starts the test
 func (h HTTPHeaderFieldManipulation) Run(ctl *nettests.Controller) error {
-	mknt := mk.NewNettest("HttpHeaderFieldManipulation")
-	ctl.Init(mknt)
-	return mknt.Run()
+	experiment := hhfm.NewExperiment(ctl.Ctx.Session, hhfm.Config{
+		LogLevel: "INFO",
+	})
+	return ctl.Run(experiment, []string{""})
 }
 
 // HTTPHeaderFieldManipulationTestKeys for the test

nettests/middlebox/http_invalid_request_line.go

@@ -3,8 +3,8 @@ package middlebox
 import (
 	"errors"
 
-	"github.com/measurement-kit/go-measurement-kit"
 	"github.com/ooni/probe-cli/nettests"
+	"github.com/ooni/probe-engine/experiment/hirl"
 )
 
 // HTTPInvalidRequestLine test implementation
@@ -13,9 +13,10 @@ type HTTPInvalidRequestLine struct {
 
 // Run starts the test
 func (h HTTPInvalidRequestLine) Run(ctl *nettests.Controller) error {
-	mknt := mk.NewNettest("HttpInvalidRequestLine")
-	ctl.Init(mknt)
-	return mknt.Run()
+	experiment := hirl.NewExperiment(ctl.Ctx.Session, hirl.Config{
+		LogLevel: "INFO",
+	})
+	return ctl.Run(experiment, []string{""})
 }
 
 // HTTPInvalidRequestLineTestKeys for the test