diff --git a/nettests/im/facebook_messenger.go b/nettests/im/facebook_messenger.go index 4f7d716..c0c237c 100644 --- a/nettests/im/facebook_messenger.go +++ b/nettests/im/facebook_messenger.go @@ -24,7 +24,7 @@ type FacebookMessengerTestKeys struct { } // GetTestKeys generates a summary for a test run -func (h FacebookMessenger) GetTestKeys(tk map[string]interface{}) interface{} { +func (h FacebookMessenger) GetTestKeys(tk map[string]interface{}) (interface{}, error) { var ( dnsBlocking bool tcpBlocking bool @@ -45,7 +45,7 @@ func (h FacebookMessenger) GetTestKeys(tk map[string]interface{}) interface{} { DNSBlocking: dnsBlocking, TCPBlocking: tcpBlocking, IsAnomaly: dnsBlocking || tcpBlocking, - } + }, nil } // LogSummary writes the summary to the standard output diff --git a/nettests/im/telegram.go b/nettests/im/telegram.go index 0ef4943..10cbf00 100644 --- a/nettests/im/telegram.go +++ b/nettests/im/telegram.go @@ -25,7 +25,7 @@ type TelegramTestKeys struct { } // GetTestKeys generates a summary for a test run -func (h Telegram) GetTestKeys(tk map[string]interface{}) interface{} { +func (h Telegram) GetTestKeys(tk map[string]interface{}) (interface{}, error) { var ( tcpBlocking bool httpBlocking bool @@ -53,7 +53,7 @@ func (h Telegram) GetTestKeys(tk map[string]interface{}) interface{} { HTTPBlocking: httpBlocking, WebBlocking: webBlocking, IsAnomaly: webBlocking || httpBlocking || tcpBlocking, - } + }, nil } // LogSummary writes the summary to the standard output diff --git a/nettests/im/whatsapp.go b/nettests/im/whatsapp.go index 19b2199..10f9f44 100644 --- a/nettests/im/whatsapp.go +++ b/nettests/im/whatsapp.go @@ -25,7 +25,7 @@ type WhatsAppTestKeys struct { } // GetTestKeys generates a summary for a test run -func (h WhatsApp) GetTestKeys(tk map[string]interface{}) interface{} { +func (h WhatsApp) GetTestKeys(tk map[string]interface{}) (interface{}, error) { var ( webBlocking bool registrationBlocking bool @@ -51,7 +51,7 @@ func (h WhatsApp) GetTestKeys(tk map[string]interface{}) interface{} { WebBlocking: webBlocking, EndpointsBlocking: endpointsBlocking, IsAnomaly: registrationBlocking || webBlocking || endpointsBlocking, - } + }, nil } // LogSummary writes the summary to the standard output diff --git a/nettests/middlebox/http_header_field_manipulation.go b/nettests/middlebox/http_header_field_manipulation.go index f5df162..6b6b335 100644 --- a/nettests/middlebox/http_header_field_manipulation.go +++ b/nettests/middlebox/http_header_field_manipulation.go @@ -1,6 +1,8 @@ package middlebox import ( + "errors" + "github.com/measurement-kit/go-measurement-kit" "github.com/ooni/probe-cli/nettests" ) @@ -22,19 +24,21 @@ type HTTPHeaderFieldManipulationTestKeys struct { } // GetTestKeys returns a projection of the tests keys needed for the views -func (h HTTPHeaderFieldManipulation) GetTestKeys(tk map[string]interface{}) interface{} { - tampering := false - for _, v := range tk["tampering"].(map[string]interface{}) { +func (h HTTPHeaderFieldManipulation) GetTestKeys(tk map[string]interface{}) (interface{}, error) { + testKeys := HTTPHeaderFieldManipulationTestKeys{IsAnomaly: false} + tampering, ok := tk["tampering"].(map[string]interface{}) + if !ok { + return testKeys, errors.New("tampering testkey is invalid") + } + for _, v := range tampering { t, ok := v.(bool) // Ignore non booleans in the tampering map if ok && t == true { - tampering = true + testKeys.IsAnomaly = true } } - return HTTPHeaderFieldManipulationTestKeys{ - IsAnomaly: tampering, - } + return testKeys, nil } // LogSummary writes the summary to the standard output diff --git a/nettests/middlebox/http_invalid_request_line.go b/nettests/middlebox/http_invalid_request_line.go index b815c94..0d7cc7e 100644 --- a/nettests/middlebox/http_invalid_request_line.go +++ b/nettests/middlebox/http_invalid_request_line.go @@ -1,6 +1,8 @@ package middlebox import ( + "errors" + "github.com/measurement-kit/go-measurement-kit" "github.com/ooni/probe-cli/nettests" ) @@ -22,12 +24,16 @@ type HTTPInvalidRequestLineTestKeys struct { } // GetTestKeys generates a summary for a test run -func (h HTTPInvalidRequestLine) GetTestKeys(tk map[string]interface{}) interface{} { - tampering := tk["tampering"].(bool) +func (h HTTPInvalidRequestLine) GetTestKeys(tk map[string]interface{}) (interface{}, error) { + testKeys := HTTPInvalidRequestLineTestKeys{IsAnomaly: false} - return HTTPInvalidRequestLineTestKeys{ - IsAnomaly: tampering, + tampering, ok := tk["tampering"].(bool) + if !ok { + return testKeys, errors.New("tampering is not bool") } + testKeys.IsAnomaly = tampering + + return testKeys, nil } // LogSummary writes the summary to the standard output diff --git a/nettests/nettests.go b/nettests/nettests.go index 603f68c..9713a7a 100644 --- a/nettests/nettests.go +++ b/nettests/nettests.go @@ -22,7 +22,7 @@ import ( // Nettest interface. Every Nettest should implement this. type Nettest interface { Run(*Controller) error - GetTestKeys(map[string]interface{}) interface{} + GetTestKeys(map[string]interface{}) (interface{}, error) LogSummary(string) error } @@ -299,10 +299,15 @@ func (c *Controller) OnEntry(idx int64, jsonStr string) { log.WithError(err).Error("failed to parse onEntry") return } - tk := c.nt.GetTestKeys(entry.TestKeys) + // XXX is it correct to just log the error instead of marking the whole + // measurement as failed? + tk, err := c.nt.GetTestKeys(entry.TestKeys) + if err != nil { + log.WithError(err).Error("failed to obtain testKeys") + } log.Debugf("Fetching: %s %v", idx, c.msmts[idx]) - err := database.AddTestKeys(c.Ctx.DB, c.msmts[idx], tk) + err = database.AddTestKeys(c.Ctx.DB, c.msmts[idx], tk) if err != nil { log.WithError(err).Error("failed to add test keys to summary") } diff --git a/nettests/performance/dash.go b/nettests/performance/dash.go index 3d63809..f1f5a18 100644 --- a/nettests/performance/dash.go +++ b/nettests/performance/dash.go @@ -1,6 +1,8 @@ package performance import ( + "github.com/pkg/errors" + "github.com/measurement-kit/go-measurement-kit" "github.com/ooni/probe-cli/nettests" ) @@ -20,21 +22,40 @@ func (d Dash) Run(ctl *nettests.Controller) error { // TODO: process 'receiver_data' to provide an array of performance for a chart. type DashTestKeys struct { Latency float64 `json:"connect_latency"` - Bitrate int64 `json:"median_bitrate"` + Bitrate float64 `json:"median_bitrate"` Delay float64 `json:"min_playout_delay"` IsAnomaly bool `json:"-"` } // GetTestKeys generates a summary for a test run -func (d Dash) GetTestKeys(tk map[string]interface{}) interface{} { - simple := tk["simple"].(map[string]interface{}) +func (d Dash) GetTestKeys(tk map[string]interface{}) (interface{}, error) { + var err error - return DashTestKeys{ - IsAnomaly: false, - Latency: simple["connect_latency"].(float64), - Bitrate: int64(simple["median_bitrate"].(float64)), - Delay: simple["min_playout_delay"].(float64), + testKeys := DashTestKeys{IsAnomaly: false} + + simple, ok := tk["simple"].(map[string]interface{}) + if !ok { + return testKeys, errors.New("simple key is not of the expected type") } + + latency, ok := simple["connect_latency"].(float64) + if !ok { + err = errors.Wrap(err, "connect_latency is invalid") + } + testKeys.Latency = latency + + bitrate, ok := simple["median_bitrate"].(float64) + if !ok { + err = errors.Wrap(err, "median_bitrate is invalid") + } + testKeys.Bitrate = bitrate + + delay, ok := simple["min_playout_delay"].(float64) + if !ok { + err = errors.Wrap(err, "min_playout_delay is invalid") + } + testKeys.Delay = delay + return testKeys, err } // LogSummary writes the summary to the standard output diff --git a/nettests/performance/ndt.go b/nettests/performance/ndt.go index 590b779..9d0ae00 100644 --- a/nettests/performance/ndt.go +++ b/nettests/performance/ndt.go @@ -3,6 +3,7 @@ package performance import ( "github.com/measurement-kit/go-measurement-kit" "github.com/ooni/probe-cli/nettests" + "github.com/pkg/errors" ) // NDT test implementation @@ -18,36 +19,75 @@ func (n NDT) Run(ctl *nettests.Controller) error { // NDTTestKeys for the test type NDTTestKeys struct { - Upload int64 `json:"upload"` - Download int64 `json:"download"` - Ping int64 `json:"ping"` + Upload float64 `json:"upload"` + Download float64 `json:"download"` + Ping float64 `json:"ping"` MaxRTT float64 `json:"max_rtt"` AvgRTT float64 `json:"avg_rtt"` MinRTT float64 `json:"min_rtt"` - MSS int64 `json:"mss"` - OutOfOrder int64 `json:"out_of_order"` + MSS float64 `json:"mss"` + OutOfOrder float64 `json:"out_of_order"` PacketLoss float64 `json:"packet_loss"` - Timeouts int64 `json:"timeouts"` + Timeouts float64 `json:"timeouts"` IsAnomaly bool `json:"-"` } // GetTestKeys generates a summary for a test run -func (n NDT) GetTestKeys(tk map[string]interface{}) interface{} { - simple := tk["simple"].(map[string]interface{}) - advanced := tk["advanced"].(map[string]interface{}) +func (n NDT) GetTestKeys(tk map[string]interface{}) (interface{}, error) { + var err error + testKeys := NDTTestKeys{IsAnomaly: false} - return NDTTestKeys{ - Upload: int64(simple["upload"].(float64)), - Download: int64(simple["download"].(float64)), - Ping: int64(simple["ping"].(float64)), - MaxRTT: advanced["max_rtt"].(float64), - AvgRTT: advanced["avg_rtt"].(float64), - MinRTT: advanced["min_rtt"].(float64), - MSS: int64(advanced["mss"].(float64)), - OutOfOrder: int64(advanced["out_of_order"].(float64)), - PacketLoss: advanced["packet_loss"].(float64), - Timeouts: int64(advanced["timeouts"].(float64)), + simple, ok := tk["simple"].(map[string]interface{}) + if !ok { + return testKeys, errors.New("simple key is invalid") } + advanced, ok := tk["advanced"].(map[string]interface{}) + if !ok { + return testKeys, errors.New("advanced key is invalid") + } + + // XXX there is likely a better pattern for this + testKeys.Upload, ok = simple["upload"].(float64) + if !ok { + err = errors.Wrap(err, "upload key invalid") + } + testKeys.Download, ok = simple["download"].(float64) + if !ok { + err = errors.Wrap(err, "download key invalid") + } + testKeys.Ping, ok = simple["ping"].(float64) + if !ok { + err = errors.Wrap(err, "ping key invalid") + } + testKeys.MaxRTT, ok = advanced["max_rtt"].(float64) + if !ok { + err = errors.Wrap(err, "max_rtt key invalid") + } + testKeys.AvgRTT, ok = advanced["avg_rtt"].(float64) + if !ok { + err = errors.Wrap(err, "avg_rtt key invalid") + } + testKeys.MinRTT, ok = advanced["min_rtt"].(float64) + if !ok { + err = errors.Wrap(err, "min_rtt key invalid") + } + testKeys.MSS, ok = advanced["mss"].(float64) + if !ok { + err = errors.Wrap(err, "mss key invalid") + } + testKeys.OutOfOrder, ok = advanced["out_of_order"].(float64) + if !ok { + err = errors.Wrap(err, "out_of_order key invalid") + } + testKeys.PacketLoss, ok = advanced["packet_loss"].(float64) + if !ok { + err = errors.Wrap(err, "packet_loss key invalid") + } + testKeys.Timeouts, ok = advanced["timeouts"].(float64) + if !ok { + err = errors.Wrap(err, "timeouts key invalid") + } + return testKeys, err } // LogSummary writes the summary to the standard output diff --git a/nettests/websites/web_connectivity.go b/nettests/websites/web_connectivity.go index 4121207..bfea69a 100644 --- a/nettests/websites/web_connectivity.go +++ b/nettests/websites/web_connectivity.go @@ -93,7 +93,7 @@ type WebConnectivityTestKeys struct { } // GetTestKeys generates a summary for a test run -func (n WebConnectivity) GetTestKeys(tk map[string]interface{}) interface{} { +func (n WebConnectivity) GetTestKeys(tk map[string]interface{}) (interface{}, error) { var ( blocked bool blocking string @@ -124,7 +124,7 @@ func (n WebConnectivity) GetTestKeys(tk map[string]interface{}) interface{} { Accessible: accessible, Blocking: blocking, IsAnomaly: blocked, - } + }, nil } // LogSummary writes the summary to the standard output