ooni / probe

OONI Probe network measurement tool for detecting internet censorship
https://ooni.org/install
BSD 3-Clause "New" or "Revised" License
754 stars 142 forks source link

oonimkall: investigate using richer input #2752

Closed bassosimone closed 3 months ago

bassosimone commented 3 months ago

This issue is about investigating what it would take for oonimkall to use richer input.

bassosimone commented 3 months ago

I discussed this topic extensively with @DecFox. Here's a summary of the conversation:

  1. Running experiments from mobile uses ./pkg/oonimkall/task.go, which is still quite close to the original C interface exposed by Measurement Kit. Under this API, we start tasks using JSON configuration, read JSON events emitted by the task, and stop when the task signals that it is done. The configuration only includes an Input []string. This means that we are not able to correctly run OONI Run v2 descriptors containing options.

  2. Mobile code uses a separate Go-Java bridge generated using go mobile for fetching Web Connectivity URLs. This really only happens for Web Connectivity and all other tests are executed without any input. This means that, if an experiment needs input and there is no input, the oonimkall code will then be responsible for somehow fetching such input.

  3. In fact, oonimkall has specific code that uses the experiment's input policy to determine what to do. This code does not currently invoke any Loader, as explained in a comment, because we were performing this change close to a release and we did not want to introduce too many breaking changes at the time. However, because Web Connectivity already provides URLs, it seems reasonable that we just use a loader there if there's no other input already.

  4. By doing this, we will unlock the possibility of fetching richer input for oonimkall experiments except for Web Connectivity, which uses a different pattern where the app is responsible for fetching inputs.

Because the change seems safe on paper, I propose we implement and evaluate it. Then, we should definitely document as technical debt that the mobile app uses a different pattern than the CLI and that the mobile app does not allow to pass options from OONI Run v2 to experiments. I see both problems as being linked, even though they are different problems, because they break the following narrative, which I think is very good to reason about OONI:

  1. OONI Run v2 is a way to automate running miniooni from the command line.

  2. Richer input is a way to fetch inputs and options when OONI Run v2 does not contain inputs and options.

  3. Experiment execution uses an experiment name, input, and options.

Instead, mobile is kind of different and does not really follow this code organization. However, it is also true that the issues I am describing are not impacting OONI operationally right now. Hence, my suggestion that, for now, we should just focus on unlocking using richer input for dnscheck et al., and document the rest as technical debt.

bassosimone commented 3 months ago

So, after https://github.com/ooni/probe/issues/2752#issuecomment-2180430243, we were left with the reckoning that there are two richer-input and/or OONI-Run-v2 lingering issues in the ./pkg/oonimkall codebase. These issues are:

Issue 1: It's not possible to pass structured options inside an OONI Run v2 descriptor to an experiment through the ./pkg/oonimkall API, because there's only room for Input []string.

Issue 2: that the pattern for executing experiments used by the mobile apps is not the same used by the CLI or miniooni and we end up not being able to obtain richer input for Web Connectivity easily. This happens because richer input is an interface inside the ./internal part of the codebase but the experiment needs to cast such an interface to the proper type containing richer input. The mobile app would not easily be able to construct the proper type due to difficulties caused by using JSON as the way to pass structures between the mobile app and the engine proper.

Issue 1 can be easily solved by introducing a json.RawMessage field on the receiver side that can receive a raw JSON message produced by the mobile app based on the content of the OONI Run v2 descriptor. So, the solution to this problem is actually quite simple and it should not require too much work.

Issue 2 is more complex to solve. While discussing this topic, @aanorbel proposed what I consider a very good solution of this problem that we should definitely explore. Without changing much in the mobile app, we can just take advantage of the fact that the status.measurement_start event provides the URL we're about to measure to the mobile app. Now, the mobile app also needs to now the category code and the country code, which we can easily supply because they are part of the richer input model used for any richer input. Thus, if we enrich the status.measurement_start event with category code and country code, the mobile app does not need to invoke check-in itself and we can instead invoke check-in inside the ./pkg/oonimkall codebase through a Loader. By implementing this chance, we would factor common code, simplify the mobile apps implementations, and streamline how we execute experiments without any functional changes.

bassosimone commented 3 months ago

I also need to remember to add a custom timeout for fetching targets in oonimkall when I modify it.

bassosimone commented 3 months ago

So, wrapping up and preparing for closing this issue. Here's what should happen next:

We can close this issue when all these three issues have been created. Luckily all three activities seem ~simple. 💥

bassosimone commented 3 months ago

All follow-up issues created. We can close this issue now.