From dae02ce5b647662638d2c846b8c501d19e70a7d3 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 30 Mar 2021 11:59:29 +0200 Subject: [PATCH] feat(ooniprobe): propagate the RunType CheckIn hint (#274) This diff propagates the RunType CheckIn hint such that we run using less URLs when running in the background. Part of https://github.com/ooni/probe/issues/1299. --- cmd/ooniprobe/internal/cli/run/run.go | 20 +++++++++++-------- cmd/ooniprobe/internal/nettests/nettests.go | 12 +++++++++-- cmd/ooniprobe/internal/nettests/run.go | 15 ++++++++++---- .../internal/nettests/web_connectivity.go | 10 +++++++--- 4 files changed, 40 insertions(+), 17 deletions(-) diff --git a/cmd/ooniprobe/internal/cli/run/run.go b/cmd/ooniprobe/internal/cli/run/run.go index be23822..446e43e 100644 --- a/cmd/ooniprobe/internal/cli/run/run.go +++ b/cmd/ooniprobe/internal/cli/run/run.go @@ -26,19 +26,23 @@ func init() { log.WithError(err).Error("failed to perform onboarding") return err } - if *noCollector == true { + if *noCollector { probe.Config().Sharing.UploadResults = false } return nil }) - functionalRun := func(pred func(name string, gr nettests.Group) bool) error { + functionalRun := func(runType string, pred func(name string, gr nettests.Group) bool) error { for name, group := range nettests.All { - if pred(name, group) != true { + if !pred(name, group) { continue } log.Infof("Running %s tests", color.BlueString(name)) - conf := nettests.RunGroupConfig{GroupName: name, Probe: probe} + conf := nettests.RunGroupConfig{ + GroupName: name, + Probe: probe, + RunType: runType, + } if err := nettests.RunGroup(conf); err != nil { log.WithError(err).Errorf("failed to run %s", name) } @@ -48,7 +52,7 @@ func init() { genRunWithGroupName := func(targetName string) func(*kingpin.ParseContext) error { return func(*kingpin.ParseContext) error { - return functionalRun(func(groupName string, gr nettests.Group) bool { + return functionalRun("manual", func(groupName string, gr nettests.Group) bool { return groupName == targetName }) } @@ -75,14 +79,14 @@ func init() { unattendedCmd := cmd.Command("unattended", "") unattendedCmd.Action(func(_ *kingpin.ParseContext) error { - return functionalRun(func(name string, gr nettests.Group) bool { - return gr.UnattendedOK == true + return functionalRun("timed", func(name string, gr nettests.Group) bool { + return gr.UnattendedOK }) }) allCmd := cmd.Command("all", "").Default() allCmd.Action(func(_ *kingpin.ParseContext) error { - return functionalRun(func(name string, gr nettests.Group) bool { + return functionalRun("manual", func(name string, gr nettests.Group) bool { return true }) }) diff --git a/cmd/ooniprobe/internal/nettests/nettests.go b/cmd/ooniprobe/internal/nettests/nettests.go index 14e788a..9907194 100644 --- a/cmd/ooniprobe/internal/nettests/nettests.go +++ b/cmd/ooniprobe/internal/nettests/nettests.go @@ -53,6 +53,10 @@ type Controller struct { // using the command line using the --input flag. Inputs []string + // RunType contains the run_type hint for the CheckIn API. If + // not set, the underlying code defaults to "timed". + RunType string + // numInputs is the total number of inputs numInputs int @@ -115,6 +119,8 @@ func (c *Controller) Run(builder *engine.ExperimentBuilder, inputs []string) err start := time.Now() c.ntStartTime = start for idx, input := range inputs { + // TODO(bassosimone): should we allow for interruption when running + // in unattended mode? Likewise, should we honor MaxRuntime? if c.Probe.IsTerminated() { log.Info("user requested us to terminate using Ctrl-C") break @@ -172,7 +178,7 @@ func (c *Controller) Run(builder *engine.ExperimentBuilder, inputs []string) err } } // We only save the measurement to disk if we failed to upload the measurement - if saveToDisk == true { + if saveToDisk { if err := exp.SaveMeasurement(measurement, msmt.MeasurementFilePath.String); err != nil { return errors.Wrap(err, "failed to save measurement on disk") } @@ -204,6 +210,8 @@ func (c *Controller) Run(builder *engine.ExperimentBuilder, inputs []string) err // OnProgress should be called when a new progress event is available. func (c *Controller) OnProgress(perc float64, msg string) { + // TODO(bassosimone): should we adjust this algorithm when we have a + // maximum runtime that we would like to honor? log.Debugf("OnProgress: %f - %s", perc, msg) var eta float64 eta = -1.0 @@ -213,7 +221,7 @@ func (c *Controller) OnProgress(perc float64, msg string) { step := 1.0 / float64(c.numInputs) perc = floor + perc*step if c.curInputIdx > 0 { - eta = (time.Now().Sub(c.ntStartTime).Seconds() / float64(c.curInputIdx)) * float64(c.numInputs-c.curInputIdx) + eta = (time.Since(c.ntStartTime).Seconds() / float64(c.curInputIdx)) * float64(c.numInputs-c.curInputIdx) } } if c.ntCount > 0 { diff --git a/cmd/ooniprobe/internal/nettests/run.go b/cmd/ooniprobe/internal/nettests/run.go index 216fd41..6450cdf 100644 --- a/cmd/ooniprobe/internal/nettests/run.go +++ b/cmd/ooniprobe/internal/nettests/run.go @@ -1,6 +1,7 @@ package nettests import ( + "sync" "time" "github.com/apex/log" @@ -12,9 +13,10 @@ import ( // RunGroupConfig contains the settings for running a nettest group. type RunGroupConfig struct { GroupName string - Probe *ooni.Probe InputFiles []string Inputs []string + Probe *ooni.Probe + RunType string // hint for check-in API } const websitesURLLimitRemoved = `WARNING: CONFIGURATION CHANGE REQUIRED: @@ -34,16 +36,20 @@ const websitesURLLimitRemoved = `WARNING: CONFIGURATION CHANGE REQUIRED: * Since 2022, we will start silently ignoring websites_url_limit ` +var deprecationWarningOnce sync.Once + // RunGroup runs a group of nettests according to the specified config. func RunGroup(config RunGroupConfig) error { if config.Probe.Config().Nettests.WebsitesURLLimit > 0 { - log.Warn(websitesURLLimitRemoved) if config.Probe.Config().Nettests.WebsitesMaxRuntime <= 0 { limit := config.Probe.Config().Nettests.WebsitesURLLimit maxRuntime := 5 * limit config.Probe.Config().Nettests.WebsitesMaxRuntime = maxRuntime } - time.Sleep(30 * time.Second) + deprecationWarningOnce.Do(func() { + log.Warn(websitesURLLimitRemoved) + time.Sleep(30 * time.Second) + }) } if config.Probe.IsTerminated() { @@ -90,7 +96,7 @@ func RunGroup(config RunGroupConfig) error { config.Probe.ListenForSignals() config.Probe.MaybeListenForStdinClosed() for i, nt := range group.Nettests { - if config.Probe.IsTerminated() == true { + if config.Probe.IsTerminated() { log.Debugf("context is terminated, stopping group.Nettests early") break } @@ -98,6 +104,7 @@ func RunGroup(config RunGroupConfig) error { ctl := NewController(nt, config.Probe, result, sess) ctl.InputFiles = config.InputFiles ctl.Inputs = config.Inputs + ctl.RunType = config.RunType ctl.SetNettestIndex(i, len(group.Nettests)) if err = nt.Run(ctl); err != nil { log.WithError(err).Errorf("Failed to run %s", group.Label) diff --git a/cmd/ooniprobe/internal/nettests/web_connectivity.go b/cmd/ooniprobe/internal/nettests/web_connectivity.go index 741572c..fc752b2 100644 --- a/cmd/ooniprobe/internal/nettests/web_connectivity.go +++ b/cmd/ooniprobe/internal/nettests/web_connectivity.go @@ -9,12 +9,15 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/model" ) -// TODO(bassosimone): we should propagate the kind of run -// to here such that we get the right runType. - func lookupURLs(ctl *Controller, categories []string) ([]string, map[int64]int64, error) { inputloader := &engine.InputLoader{ CheckInConfig: &model.CheckInConfig{ + // Setting Charging and OnWiFi to true causes the CheckIn + // API to return to us as much URL as possible with the + // given RunType hint. + Charging: true, + OnWiFi: true, + RunType: ctl.RunType, WebConnectivity: model.CheckInConfigWebConnectivity{ CategoryCodes: categories, }, @@ -24,6 +27,7 @@ func lookupURLs(ctl *Controller, categories []string) ([]string, map[int64]int64 SourceFiles: ctl.InputFiles, StaticInputs: ctl.Inputs, } + log.Infof("Calling CheckIn API with %s runType", ctl.RunType) testlist, err := inputloader.Load(context.Background()) var urls []string urlIDMap := make(map[int64]int64)