engine: MVP of enabling experimental experiments using richer input #2553

bassosimone · 2023-10-10T14:36:26Z

This issue is about writing an MVP for enabling experimental experiments (currently riseupvpn and echcheck) conditionally through the check-in API. These experiments would always be disabled and only become enabled if the check-in API says so through feature flags (which were added as part of the richer input work).

This issue advocates for writing an MVP where we use the results of previous check-in API calls (typically performed, when running tests automatically, before invoking the Web Connectivity experiment). A more structured solution would modify the flow with which we run experiments to call the check-in API unconditionally once and at the beginning, so these feature flags are always up to date.

As a reminder, experimental experiments are experiments that we just added, so we're not 100% sure about them, or experiments that are historically flaky, so we want to be defensive.

Here's a functional specification of what to do:

when instantiating a new experiment
unless OONI_FORCE_ENABLE_EXPERIMENT=1 is set in the environment
if the experiment name is one of "riseupvpn" and "echecheck"
do not construct the experiment unless a feature flag like "${name}_enabled": true exists

This means that these experiments are disabled by default and we control whether to enable them with check-in.

The text was updated successfully, but these errors were encountered:

We have a class of experiments that I call experimental experiments. As a reminder, experimental experiments are the ones we have just added in a development cycle and which may potentially have issues down the line, so that we may want to disable them while working on an emergency release to fix them. Currently, riseupvpn and echechk fall into this class. The idea of this diff is to implement the functional specification described by issue ooni/probe#2553.

## Checklist - [x] I have read the [contribution guidelines](https://github.com/ooni/probe-cli/blob/master/CONTRIBUTING.md) - [x] reference issue for this pull request: ooni/probe#2553 - [x] if you changed anything related to how experiments work and you need to reflect these changes in the ooni/spec repository, please link to the related ooni/spec pull request: N/A - [x] if you changed code inside an experiment, make sure you bump its version number ## Description We have a class of experiments that I call experimental experiments. These are experiments that we just added, so we're not 100% sure about them, or experiments that are historically flaky, so we want to be defensive. Currently, riseupvpn and echechk fall into this class. The idea of this diff is to implement the functional specification described by issue ooni/probe#2553. While there, make sure we have more robust testing for the `./internal/registry` package. While there, make sure package creation logic lives in `./internal/registry` rather than in `./internal/engine`.

While there, emit some logs while running it. While there, make the warning emitted when a nettest is disabled much more specific and informative for the user. Part of ooni/probe#2547 Part of ooni/probe#2553

There may be upcoming changes in torsf which may cause it to fail consistently as it occurred during the 3.18 cycle. Therefore, be defensive and make it disabled by default. Part of ooni/probe#2553 While there, use slightly better naming for an echcheck function.

bassosimone · 2023-10-11T11:16:30Z

This work has now been done!

This diff backports #1355 to the release/3.19 branch. - [x] I have read the [contribution guidelines](https://github.com/ooni/probe-cli/blob/master/CONTRIBUTING.md) - [x] reference issue for this pull request: ooni/probe#2553 - [x] if you changed anything related to how experiments work and you need to reflect these changes in the ooni/spec repository, please link to the related ooni/spec pull request: N/A - [x] if you changed code inside an experiment, make sure you bump its version number We have a class of experiments that I call experimental experiments. These are experiments that we just added, so we're not 100% sure about them, or experiments that are historically flaky, so we want to be defensive. Currently, riseupvpn and echechk fall into this class. The idea of this diff is to implement the functional specification described by issue ooni/probe#2553. While there, make sure we have more robust testing for the `./internal/registry` package. While there, make sure package creation logic lives in `./internal/registry` rather than in `./internal/engine`.

This diff backports #1358 to the release/3.19 branch. While there, emit some logs while running it. While there, make the warning emitted when a nettest is disabled much more specific and informative for the user. Part of ooni/probe#2547 Part of ooni/probe#2553

This diff backports #1359 to the release/3.19 branch. There may be upcoming changes in torsf which may cause it to fail consistently as it occurred during the 3.18 cycle. Therefore, be defensive and make it disabled by default. Part of ooni/probe#2553 While there, use slightly better naming for an echcheck function.

The reference issue is ooni/probe#2553. When I implemented the disabled-by-default functionality, I missed that we should also disable-by-default vanilla_tor. The reason for doing that is that the experiment crashes on Android and possibly iOS due to ooni/probe#2406.

This is the counterpart of ooni/probe-cli#1374. Reference issue: ooni/probe#2553.

The reference issue is ooni/probe#2553. When I implemented the disabled-by-default functionality, I missed that we should also disable-by-default vanilla_tor. The reason for doing that is that the experiment crashes on Android and possibly iOS due to ooni/probe#2406.

This diff backports #1374 to release/3.19. The reference issue is ooni/probe#2553. When I implemented the disabled-by-default functionality, I missed that we should also disable-by-default vanilla_tor. The reason for doing that is that the experiment crashes on Android and possibly iOS due to ooni/probe#2406.

This is the counterpart of ooni/probe-cli#1374. Reference issue: ooni/probe#2553.

## Checklist - [x] I have read the [contribution guidelines](https://github.com/ooni/probe-cli/blob/master/CONTRIBUTING.md) - [x] reference issue for this pull request: ooni/probe#2553 - [x] if you changed anything related to how experiments work and you need to reflect these changes in the ooni/spec repository, please link to the related ooni/spec pull request: N/A - [x] if you changed code inside an experiment, make sure you bump its version number ## Description We have a class of experiments that I call experimental experiments. These are experiments that we just added, so we're not 100% sure about them, or experiments that are historically flaky, so we want to be defensive. Currently, riseupvpn and echechk fall into this class. The idea of this diff is to implement the functional specification described by issue ooni/probe#2553. While there, make sure we have more robust testing for the `./internal/registry` package. While there, make sure package creation logic lives in `./internal/registry` rather than in `./internal/engine`.

While there, emit some logs while running it. While there, make the warning emitted when a nettest is disabled much more specific and informative for the user. Part of ooni/probe#2547 Part of ooni/probe#2553

There may be upcoming changes in torsf which may cause it to fail consistently as it occurred during the 3.18 cycle. Therefore, be defensive and make it disabled by default. Part of ooni/probe#2553 While there, use slightly better naming for an echcheck function.

The reference issue is ooni/probe#2553. When I implemented the disabled-by-default functionality, I missed that we should also disable-by-default vanilla_tor. The reason for doing that is that the experiment crashes on Android and possibly iOS due to ooni/probe#2406.

bassosimone added enhancement improving existing code or new feature priority/high interrupt ooni/probe-engine funder/drl2022-2024 releaseBlocker This issue blocks releasing labels Oct 10, 2023

bassosimone self-assigned this Oct 10, 2023

bassosimone mentioned this issue Oct 10, 2023

engine: enable/disable experimental experiments using richer input #2554

Closed

bassosimone mentioned this issue Oct 10, 2023

feat: mvp of conditionally enabling experiments ooni/probe-cli#1355

Merged

4 tasks

bassosimone mentioned this issue Oct 11, 2023

fix(ooniprobe): make sure we actually run echcheck ooni/probe-cli#1358

Merged

bassosimone mentioned this issue Oct 11, 2023

fix(registry): mark torsf as disabled by default ooni/probe-cli#1359

Merged

bassosimone closed this as completed Oct 11, 2023

bassosimone mentioned this issue Oct 19, 2023

fix(registry): disable vanilla_tor by default ooni/probe-cli#1374

Merged

bassosimone added a commit to ooni/backend that referenced this issue Oct 19, 2023

chore(check-in): force vanilla_tor to be enabled

996cfef

This is the counterpart of ooni/probe-cli#1374. Reference issue: ooni/probe#2553.

bassosimone mentioned this issue Oct 19, 2023

chore(check-in): force vanilla_tor to be enabled ooni/backend#751

Merged

bassosimone added a commit to ooni/backend that referenced this issue Oct 20, 2023

chore(check-in): force vanilla_tor to be enabled (#751)

7935162

This is the counterpart of ooni/probe-cli#1374. Reference issue: ooni/probe#2553.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

engine: MVP of enabling experimental experiments using richer input #2553

engine: MVP of enabling experimental experiments using richer input #2553

bassosimone commented Oct 10, 2023 •

edited

Loading

bassosimone commented Oct 11, 2023

engine: MVP of enabling experimental experiments using richer input #2553

engine: MVP of enabling experimental experiments using richer input #2553

Comments

bassosimone commented Oct 10, 2023 • edited Loading

bassosimone commented Oct 11, 2023

bassosimone commented Oct 10, 2023 •

edited

Loading