1613790 - Experiment: Measure and compare performance of TRR providers

Status: VERIFIED FIXED

(Firefox :: Security, task, P1)

Comment 1

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1613790 - Implement module to measure performance of TRRs and send telemetry. r=johannh!,dragana!,valentin!

Comment 2

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1613790 - Implement unit tests for TRRPerformance module. r=johannh!,dragana!,valentin!

Depends on D62845

Comment 3

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: data-review.md

The only answer I haven't filled out is the one about where the analysis will be shared. Tim can probably help with that. I'd appreciate any corrections or clarifications for the other answers as well. Thanks!

Comment 4

[(inactive) Nihanth Subramanya [:nhnt11]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=592e4e7de16baeca51484331cb522e383b4e25a6

Comment 5

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

Attachment: Data collection review

I fleshed it out a little bit. I'll leave this for chutten since I've been involved in the telemetry design.

Comment 6

[(inactive) Nihanth Subramanya [:nhnt11]]

Here are the prefs that control this module:

doh-rollout.trrRace.trrList: comma-separated list of TRR endpoints to test. e.g. "https://mozilla.cloudflare-dns.com/dns-query, https://trr.dns.nextdns.io/" (omit the quotes, obviously). Defaults to an empty list.

doh-rollout.trrRace.canonicalDomain: the canonical domain that we control. Subdomains will be generated against this. Defaults to "firefox-dns-perf-test.net", the sole purpose of this pref is to allow remotely overriding this if we need to for some reason.

doh-rollout.trrRace.randomSubdomainCount: Number of random subdomains to test per TRR. Defaults to 5.

doh-rollout.trrRace.popularDomains: Comma-separated list of "popular" domains - for cached lookups. Defaults to "google.com, facebook.com, apple.com" (without the quotes, obviously)

doh-rollout.trrRace.enabled: Causes the measurements to run when set, or if true at startup. Note: this has no effect if the complete pref is set to true! See below.

doh-rollout.trrRace.complete: This is set to true when we have completed our measurements and recorded telemetry. If this is set, the enabled pref will not cause us to re-run measurements.

Comment 7

[(inactive) Nihanth Subramanya [:nhnt11]]

I believe the debug failures in the previous try push can be fixed by removing nsISupports from the QueryInterface list. Pushed that idea to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8bdc37df3ff0960eace7a2caa5764626fc074657

Comment 8

[Chris H-C :chutten]

Comment on attachment 9126999 [details]
Data collection review

QUESTIONS:

Can we apply some sort of filter to make absolutely certain that the `domain` is only ever in a known list of possibilities? I have concerns about that aspect of the collection and would feel better if there was some sort of set-membership or even regex filter preventing accidental leakage.

DATA COLLECTION REVIEW RESPONSE:

    Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes. This collection is Telemetry so is documented in its definitions file [Events.yaml](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/Events.yaml) and the [Probe Dictionary](https://telemetry.mozilla.org/probe-dictionary/).

    Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.

    If the request is for permanent data collection, is there someone who will monitor the data over time?

No. This collection will expire in Firefox 80.

    Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, Technical.

    Is the data collection request for default-on or default-off?

Default on for all channels, when enabled by Normandy.

    Does the instrumentation include the addition of any new identifiers?

No.

    Is the data collection covered by the existing Firefox privacy notice?

Yes, pending answer to question.

    Does there need to be a check-in in the future to determine whether to renew the data?

Yes. :tdsmith is responsible for renewing or removing the collection before it expires in Firefox 80.

---
Result: datareview+, pending :tdsmith's answer

Comment 9

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

Thanks for the review. That works for me. I left a review comment in Phabricator.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a53749907dee
Implement module to measure performance of TRRs and send telemetry. r=johannh,dragana,valentin
https://hg.mozilla.org/integration/autoland/rev/e45f9aabdc9e
Implement unit tests for TRRPerformance module. r=johannh,dragana,valentin

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/a53749907dee
https://hg.mozilla.org/mozilla-central/rev/e45f9aabdc9e

Comment 12

[Johann Hofmann [:johannh]]

Comment on attachment 9126594 [details]
Bug 1613790 - Implement module to measure performance of TRRs and send telemetry. r=johannh!,dragana!,valentin!

Beta/Release Uplift Approval Request

• User impact if declined: This is the code for an experiment that is scheduled to run in 74 release, see https://experimenter.services.mozilla.com/experiments/doh-trr-selection-racing-experiment/

It should have no user-visible impact

• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: This experiment has its own QA team
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The experiment itself should not impact the user experience at all, even when turned on. When turned off, it doesn't do anything except listen for the pref to turn it on. Any bugs in the code that's being uplifted should only impact the experiment, which we could delay in case of issues.
• String changes made/needed: None

Comment 13

[Pascal Chevrel:pascalc]

Comment on attachment 9126594 [details]
Bug 1613790 - Implement module to measure performance of TRRs and send telemetry. r=johannh!,dragana!,valentin!

Needed for 74, uplift approved for 74 beta 9, thanks.

Comment 14

[Carmen Fat [:cfat] - Ecosystem QA]

@Johann, is this bug actionable at this point?
We manually added the experiment’s preference (doh-rollout.trrRace.enabled = true) in the latest Nightly build, but didn't notice any effect in the browser:

• the preferences mentioned in comment 6 were not added,
• the "security.doh.trrPerformance" telemetry event was not generated in the Events section from "about:telemetry".

Comment 15

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/25bb8c38a190
https://hg.mozilla.org/releases/mozilla-beta/rev/b7d80cd5aa86

Comment 16

[Johann Hofmann [:johannh]]

(In reply to Carmen Fat [:cfat] - Ecosystem QA from comment #14)

@Johann, is this bug actionable at this point?
We manually added the experiment’s preference (doh-rollout.trrRace.enabled = true) in the latest Nightly build, but didn't notice any effect in the browser:

• the preferences mentioned in comment 6 were not added,
• the "security.doh.trrPerformance" telemetry event was not generated in the Events section from "about:telemetry".

I replied to your email, let me know if you need anything else :)

Thanks!

Comment 17

[Robert Martin [:romartin], Ecosystem QA]

Multiple default TRR racing

Firefox RC 74.0

We have finished testing the Multiple default TRR racing experiment for Firefox 74.

Quality status: GREEN - SHIP IT

Why is this feature green?

• During testing, we haven’t found any issues.

Testing Summary:

• Test Suite: Test Rail

Tested Platforms:

• Windows 10 x64

• macOS 10.15.1

• Ubuntu 18.04 x64

Tested Firefox versions:

• Firefox RC 74.0 en-US

Regards,
Robert Martin