From c8e8ca2c9c1d7382d8a027e402ae38958a7bd425 Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Thu, 14 Sep 2023 11:30:55 +0000 Subject: [PATCH] Bug 1784258 - Add new telemetry probe for the DoH disablement reason r=necko-reviewers,kershaw This patch adds the following scalars: `networking.doh_heuristics_attempts` - contains the number of heuristic attempts we performed during a subsession. `networking.doh_heuristics_pass_count` - contains the number of times we passed heuristic during a subsession. `networking.doh_heuristics_result` - the result of the last heuristic run. `networking.doh_heuristic_ever_tripped` - a keyed scalar containing true if a heuristic was ever tripped during a subsession and false otherwise. Differential Revision: https://phabricator.services.mozilla.com/D187983 --- browser/components/doh/DoHController.sys.mjs | 90 ++++++++++++++++++- browser/components/doh/DoHHeuristics.sys.mjs | 49 ++++++++++ .../browser/browser_doorhangerUserReject.js | 19 ++++ .../test/browser/browser_platformDetection.js | 55 +++++++++++- .../test/browser/browser_policyOverride.js | 10 +++ .../test/browser/browser_providerSteering.js | 42 +++++++++ browser/components/doh/test/browser/head.js | 46 ++++++++++ toolkit/components/telemetry/Scalars.yaml | 84 +++++++++++++++++ 8 files changed, 392 insertions(+), 3 deletions(-) diff --git a/browser/components/doh/DoHController.sys.mjs b/browser/components/doh/DoHController.sys.mjs index b5a84c724a6a..2ed87c6b68ad 100644 --- a/browser/components/doh/DoHController.sys.mjs +++ b/browser/components/doh/DoHController.sys.mjs @@ -161,6 +161,15 @@ export const DoHController = { lazy.Preferences.observe(NATIVE_FALLBACK_WARNING_HEURISTIC_LIST_PREF, this); if (lazy.DoHConfigController.currentConfig.enabled) { + // At init time set these heuristics to false if we may run heuristics + for (let key of lazy.Heuristics.Telemetry.heuristicNames()) { + Services.telemetry.keyedScalarSet( + "networking.doh_heuristic_ever_tripped", + key, + false + ); + } + await this.maybeEnableHeuristics(); } else if (lazy.Preferences.get(FIRST_RUN_PREF, false)) { await this.rollback(); @@ -216,8 +225,32 @@ export const DoHController = { let policyResult = await lazy.Heuristics.checkEnterprisePolicy(); - if (["policy_without_doh", "disable_doh"].includes(policyResult)) { - await this.setState("policyDisabled"); + if (policyResult != "no_policy_set") { + switch (policyResult) { + case "policy_without_doh": + Services.telemetry.scalarSet( + "networking.doh_heuristics_result", + lazy.Heuristics.Telemetry.enterprisePresent + ); + await this.setState("policyDisabled"); + break; + case "disable_doh": + Services.telemetry.scalarSet( + "networking.doh_heuristics_result", + lazy.Heuristics.Telemetry.enterpriseDisabled + ); + await this.setState("policyDisabled"); + break; + case "enable_doh": + // The TRR mode has already been set, so theoretically we should not get here. + // XXX: should we skip heuristics or continue? + // TODO: Make sure we use the correct URL if the policy defines one. + Services.telemetry.scalarSet( + "networking.doh_heuristics_result", + lazy.Heuristics.Telemetry.enterpriseEnabled + ); + break; + } lazy.Preferences.set(SKIP_HEURISTICS_PREF, true); return; } @@ -326,6 +359,11 @@ export const DoHController = { async runHeuristics(evaluateReason) { let start = Date.now(); + Services.telemetry.scalarAdd("networking.doh_heuristics_attempts", 1); + Services.telemetry.scalarSet( + "networking.doh_heuristics_result", + lazy.Heuristics.Telemetry.incomplete + ); let results = await lazy.Heuristics.run(); if ( @@ -339,6 +377,10 @@ export const DoHController = { // during this heuristics run. We simply discard the results in this case. // Same thing if there was another heuristics run triggered or if we have // detected a locked captive portal while this one was ongoing. + Services.telemetry.scalarSet( + "networking.doh_heuristics_result", + lazy.Heuristics.Telemetry.ignored + ); return; } @@ -382,6 +424,11 @@ export const DoHController = { this.setHeuristicResult(Ci.nsITRRSkipReason.TRR_UNSET); if (decision === lazy.Heuristics.DISABLE_DOH) { + Services.telemetry.scalarSet( + "networking.doh_heuristics_result", + lazy.Heuristics.Telemetry.fromResults(results) + ); + let fallbackHeuristicTripped = undefined; if (lazy.Preferences.get(NATIVE_FALLBACK_WARNING_PREF, false)) { let heuristics = lazy.Preferences.get( @@ -411,6 +458,11 @@ export const DoHController = { await this.setState("disabled"); } else { + Services.telemetry.scalarSet( + "networking.doh_heuristics_result", + lazy.Heuristics.Telemetry.pass + ); + Services.telemetry.scalarAdd("networking.doh_heuristics_pass_count", 1); await this.setState("enabled"); } @@ -440,6 +492,14 @@ export const DoHController = { } else if (["vpn", "proxy", "nrpt"].includes(heuristicName)) { platform.push(heuristicName); } + + if (lazy.Heuristics.Telemetry.heuristicNames().includes(heuristicName)) { + Services.telemetry.keyedScalarSet( + "networking.doh_heuristic_ever_tripped", + heuristicName, + true + ); + } } resultsForTelemetry.canaries = canaries.join(","); @@ -488,6 +548,32 @@ export const DoHController = { state, "null" ); + + let modePref = lazy.Preferences.get(NETWORK_TRR_MODE_PREF); + if (state == "manuallyDisabled") { + if ( + modePref == Ci.nsIDNSService.MODE_TRRFIRST || + modePref == Ci.nsIDNSService.MODE_TRRONLY + ) { + Services.telemetry.scalarSet( + "networking.doh_heuristics_result", + lazy.Heuristics.Telemetry.manuallyEnabled + ); + } else if ( + lazy.Preferences.get("doh-rollout.doorhanger-decision", "") == + "UIDisabled" + ) { + Services.telemetry.scalarSet( + "networking.doh_heuristics_result", + lazy.Heuristics.Telemetry.optOut + ); + } else { + Services.telemetry.scalarSet( + "networking.doh_heuristics_result", + lazy.Heuristics.Telemetry.manuallyDisabled + ); + } + } }, async disableHeuristics(state) { diff --git a/browser/components/doh/DoHHeuristics.sys.mjs b/browser/components/doh/DoHHeuristics.sys.mjs index b858c8ff309f..122abb49173d 100644 --- a/browser/components/doh/DoHHeuristics.sys.mjs +++ b/browser/components/doh/DoHHeuristics.sys.mjs @@ -105,6 +105,55 @@ export const Heuristics = { } return Ci.nsITRRSkipReason.TRR_FAILED; }, + + // Keep this in sync with the description of networking.doh_heuristics_result + // defined in Scalars.yaml + Telemetry: { + incomplete: 0, + pass: 1, + optOut: 2, + manuallyDisabled: 3, + manuallyEnabled: 4, + enterpriseDisabled: 5, + enterprisePresent: 6, + enterpriseEnabled: 7, + vpn: 8, + proxy: 9, + nrpt: 10, + browserParent: 11, + modifiedRoots: 12, + thirdPartyRoots: 13, + google: 14, + youtube: 15, + zscalerCanary: 16, + canary: 17, + ignored: 18, + + heuristicNames() { + return [ + "google", + "youtube", + "zscalerCanary", + "canary", + "modifiedRoots", + "browserParent", + "thirdPartyRoots", + "policy", + "vpn", + "proxy", + "nrpt", + ]; + }, + + fromResults(results) { + for (let label of Heuristics.Telemetry.heuristicNames()) { + if (results[label] == Heuristics.DISABLE_DOH) { + return Heuristics.Telemetry[label]; + } + } + return Heuristics.Telemetry.pass; + }, + }, }; async function dnsLookup(hostname, resolveCanonicalName = false) { diff --git a/browser/components/doh/test/browser/browser_doorhangerUserReject.js b/browser/components/doh/test/browser/browser_doorhangerUserReject.js index 627ca48db251..f9468d46cd25 100644 --- a/browser/components/doh/test/browser/browser_doorhangerUserReject.js +++ b/browser/components/doh/test/browser/browser_doorhangerUserReject.js @@ -28,6 +28,14 @@ add_task(async function testDoorhangerUserReject() { await ensureTRRMode(2); await checkHeuristicsTelemetry("enable_doh", "startup"); + checkScalars([ + ["networking.doh_heuristics_attempts", { value: 1 }], + ["networking.doh_heuristics_pass_count", { value: 1 }], + ["networking.doh_heuristics_result", { value: Heuristics.Telemetry.pass }], + // All of the heuristics must be false. + falseExpectations([]), + ]); + prefPromise = TestUtils.waitForPrefChange( prefs.DOORHANGER_USER_DECISION_PREF ); @@ -52,6 +60,17 @@ add_task(async function testDoorhangerUserReject() { ensureNoHeuristicsTelemetry(); is(Preferences.get(prefs.BREADCRUMB_PREF), undefined, "Breadcrumb cleared."); + checkScalars([ + ["networking.doh_heuristics_attempts", { value: 1 }], + ["networking.doh_heuristics_pass_count", { value: 1 }], + [ + "networking.doh_heuristics_result", + { value: Heuristics.Telemetry.optOut }, + ], + // All of the heuristics must be false. + falseExpectations([]), + ]); + // Simulate a network change. simulateNetworkChange(); await ensureNoTRRModeChange(undefined); diff --git a/browser/components/doh/test/browser/browser_platformDetection.js b/browser/components/doh/test/browser/browser_platformDetection.js index bf97cab19602..025546ae9ec5 100644 --- a/browser/components/doh/test/browser/browser_platformDetection.js +++ b/browser/components/doh/test/browser/browser_platformDetection.js @@ -12,7 +12,6 @@ add_task(setup); add_task(async function testPlatformIndications() { // Check if the platform heuristics actually cause a "disable_doh" event - let { MockRegistrar } = ChromeUtils.importESModule( "resource://testing-common/MockRegistrar.sys.mjs" ); @@ -45,6 +44,14 @@ add_task(async function testPlatformIndications() { is(Preferences.get(prefs.BREADCRUMB_PREF), true, "Breadcrumb saved."); await checkHeuristicsTelemetry("enable_doh", "startup"); + checkScalars([ + ["networking.doh_heuristics_attempts", { value: 1 }], + ["networking.doh_heuristics_pass_count", { value: 1 }], + ["networking.doh_heuristics_result", { value: Heuristics.Telemetry.pass }], + // All of the heuristics must be false. + falseExpectations([]), + ]); + await ensureTRRMode(2); mockedLinkService.platformDNSIndications = @@ -52,22 +59,68 @@ add_task(async function testPlatformIndications() { simulateNetworkChange(); await ensureTRRMode(0); await checkHeuristicsTelemetry("disable_doh", "netchange"); + checkScalars( + [ + ["networking.doh_heuristics_attempts", { value: 2 }], + ["networking.doh_heuristics_pass_count", { value: 1 }], + ["networking.doh_heuristics_result", { value: Heuristics.Telemetry.vpn }], + ["networking.doh_heuristic_ever_tripped", { value: true, key: "vpn" }], + ].concat(falseExpectations(["vpn"])) + ); mockedLinkService.platformDNSIndications = Ci.nsINetworkLinkService.PROXY_DETECTED; simulateNetworkChange(); await ensureNoTRRModeChange(0); await checkHeuristicsTelemetry("disable_doh", "netchange"); + checkScalars( + [ + ["networking.doh_heuristics_attempts", { value: 3 }], + ["networking.doh_heuristics_pass_count", { value: 1 }], + [ + "networking.doh_heuristics_result", + { value: Heuristics.Telemetry.proxy }, + ], + ["networking.doh_heuristic_ever_tripped", { value: true, key: "vpn" }], // Was tripped earlier this session + ["networking.doh_heuristic_ever_tripped", { value: true, key: "proxy" }], + ].concat(falseExpectations(["vpn", "proxy"])) + ); mockedLinkService.platformDNSIndications = Ci.nsINetworkLinkService.NRPT_DETECTED; simulateNetworkChange(); await ensureNoTRRModeChange(0); await checkHeuristicsTelemetry("disable_doh", "netchange"); + checkScalars( + [ + ["networking.doh_heuristics_attempts", { value: 4 }], + ["networking.doh_heuristics_pass_count", { value: 1 }], + [ + "networking.doh_heuristics_result", + { value: Heuristics.Telemetry.nrpt }, + ], + ["networking.doh_heuristic_ever_tripped", { value: true, key: "vpn" }], // Was tripped earlier this session + ["networking.doh_heuristic_ever_tripped", { value: true, key: "proxy" }], // Was tripped earlier this session + ["networking.doh_heuristic_ever_tripped", { value: true, key: "nrpt" }], + ].concat(falseExpectations(["vpn", "proxy", "nrpt"])) + ); mockedLinkService.platformDNSIndications = Ci.nsINetworkLinkService.NONE_DETECTED; simulateNetworkChange(); await ensureTRRMode(2); await checkHeuristicsTelemetry("enable_doh", "netchange"); + checkScalars( + [ + ["networking.doh_heuristics_attempts", { value: 5 }], + ["networking.doh_heuristics_pass_count", { value: 2 }], + [ + "networking.doh_heuristics_result", + { value: Heuristics.Telemetry.pass }, + ], + ["networking.doh_heuristic_ever_tripped", { value: true, key: "vpn" }], // Was tripped earlier this session + ["networking.doh_heuristic_ever_tripped", { value: true, key: "proxy" }], // Was tripped earlier this session + ["networking.doh_heuristic_ever_tripped", { value: true, key: "nrpt" }], // Was tripped earlier this session + ].concat(falseExpectations(["vpn", "proxy", "nrpt"])) + ); }); diff --git a/browser/components/doh/test/browser/browser_policyOverride.js b/browser/components/doh/test/browser/browser_policyOverride.js index 25fd2c037ebe..02d851d9474a 100644 --- a/browser/components/doh/test/browser/browser_policyOverride.js +++ b/browser/components/doh/test/browser/browser_policyOverride.js @@ -47,6 +47,16 @@ add_task(async function testPolicyOverride() { await ensureNoTRRModeChange(undefined); ensureNoHeuristicsTelemetry(); + checkScalars( + [ + [ + "networking.doh_heuristics_result", + { value: Heuristics.Telemetry.enterprisePresent }, + ], + // All of the heuristics must be false. + ].concat(falseExpectations([])) + ); + // Simulate a network change. simulateNetworkChange(); await ensureNoTRRModeChange(undefined); diff --git a/browser/components/doh/test/browser/browser_providerSteering.js b/browser/components/doh/test/browser/browser_providerSteering.js index 069a823a073d..8e3b019ad0c6 100644 --- a/browser/components/doh/test/browser/browser_providerSteering.js +++ b/browser/components/doh/test/browser/browser_providerSteering.js @@ -83,6 +83,19 @@ add_task(async function testProviderSteering() { // Set enterprise roots enabled and ensure provider steering is disabled. Preferences.set("security.enterprise_roots.enabled", true); await testNetChangeResult(AUTO_TRR_URI, "disable_doh"); + checkScalars( + [ + [ + "networking.doh_heuristics_result", + { value: Heuristics.Telemetry.modifiedRoots }, + ], + [ + "networking.doh_heuristic_ever_tripped", + { value: true, key: "modifiedRoots" }, + ], + // All of the other heuristics must be false. + ].concat(falseExpectations(["modifiedRoots"])) + ); Preferences.reset("security.enterprise_roots.enabled"); // Check that provider steering is enabled again after we reset above. @@ -97,6 +110,20 @@ add_task(async function testProviderSteering() { await testNetChangeResult(AUTO_TRR_URI, "disable_doh"); gDNSOverride.clearHostOverride(googleDomain); gDNSOverride.addIPOverride(googleDomain, googleIP); + checkScalars( + [ + [ + "networking.doh_heuristics_result", + { value: Heuristics.Telemetry.google }, + ], + ["networking.doh_heuristic_ever_tripped", { value: true, key: "google" }], + [ + "networking.doh_heuristic_ever_tripped", + { value: true, key: "modifiedRoots" }, + ], + // All of the other heuristics must be false. + ].concat(falseExpectations(["modifiedRoots", "google"])) + ); // Check that provider steering is enabled again after we reset above. await testNetChangeResult(provider.uri, "enable_doh", provider.id); @@ -104,4 +131,19 @@ add_task(async function testProviderSteering() { // Finally, provider steering should be disabled once we clear the override. gDNSOverride.clearHostOverride(TEST_DOMAIN); await testNetChangeResult(AUTO_TRR_URI, "enable_doh"); + + checkScalars( + [ + [ + "networking.doh_heuristics_result", + { value: Heuristics.Telemetry.pass }, + ], + ["networking.doh_heuristic_ever_tripped", { value: true, key: "google" }], + [ + "networking.doh_heuristic_ever_tripped", + { value: true, key: "modifiedRoots" }, + ], + // All of the other heuristics must be false. + ].concat(falseExpectations(["modifiedRoots", "google"])) + ); }); diff --git a/browser/components/doh/test/browser/head.js b/browser/components/doh/test/browser/head.js index 20d8156cc0ac..3ba09e085e0f 100644 --- a/browser/components/doh/test/browser/head.js +++ b/browser/components/doh/test/browser/head.js @@ -4,9 +4,11 @@ ChromeUtils.defineESModuleGetters(this, { DoHConfigController: "resource:///modules/DoHConfig.sys.mjs", DoHController: "resource:///modules/DoHController.sys.mjs", DoHTestUtils: "resource://testing-common/DoHTestUtils.sys.mjs", + Heuristics: "resource:///modules/DoHHeuristics.sys.mjs", Preferences: "resource://gre/modules/Preferences.sys.mjs", Region: "resource://gre/modules/Region.sys.mjs", RegionTestUtils: "resource://testing-common/RegionTestUtils.sys.mjs", + TelemetryTestUtils: "resource://testing-common/TelemetryTestUtils.sys.mjs", RemoteSettings: "resource://services-settings/remote-settings.sys.mjs", }); @@ -70,6 +72,7 @@ async function setup() { let oldCanRecord = Services.telemetry.canRecordExtended; Services.telemetry.canRecordExtended = true; Services.telemetry.clearEvents(); + Services.telemetry.clearScalars(); // Enable the CFR. Preferences.set(CFR_PREF, JSON.stringify(CFR_JSON)); @@ -225,6 +228,49 @@ async function checkHeuristicsTelemetry( Services.telemetry.clearEvents(); } +// Generates an array of expectations for the ever_tripped scalar +// containing false and key, except for the keyes contained in +// the `except` parameter. +function falseExpectations(except) { + return Heuristics.Telemetry.heuristicNames() + .map(e => [ + "networking.doh_heuristic_ever_tripped", + { value: false, key: e }, + ]) + .filter(e => except && !except.includes(e[1].key)); +} + +function checkScalars(expectations) { + // expectations: [[scalarname: expectationObject]] + // expectationObject: {value, key} + let snapshot = TelemetryTestUtils.getProcessScalars("parent", false, false); + let keyedSnapshot = TelemetryTestUtils.getProcessScalars( + "parent", + true, + false + ); + for (let ex of expectations) { + let scalarName = ex[0]; + let exObject = ex[1]; + if (exObject.key) { + TelemetryTestUtils.assertKeyedScalar( + keyedSnapshot, + scalarName, + exObject.key, + exObject.value, + `${scalarName} expected to have ${exObject.value}, key: ${exObject.key}` + ); + } else { + TelemetryTestUtils.assertScalar( + snapshot, + scalarName, + exObject.value, + `${scalarName} expected to have ${exObject.value}` + ); + } + } +} + async function checkHeuristicsTelemetryMultiple(expectedEvaluateReasons) { let events; await TestUtils.waitForCondition(() => { diff --git a/toolkit/components/telemetry/Scalars.yaml b/toolkit/components/telemetry/Scalars.yaml index a9fd33970419..4250452910f6 100644 --- a/toolkit/components/telemetry/Scalars.yaml +++ b/toolkit/components/telemetry/Scalars.yaml @@ -6863,6 +6863,90 @@ networking: record_in_processes: - 'main' + doh_heuristics_attempts: + bug_numbers: + - 1784258 + description: > + The number of times we ran DoH heuristics. + kind: uint + expires: never # capturing continuously for monitoring purposes + release_channel_collection: opt-out + notification_emails: + - necko@mozilla.com + - vgosu@mozilla.com + products: + - 'firefox' + record_in_processes: + - main + + doh_heuristics_pass_count: + bug_numbers: + - 1784258 + description: > + The number of times we passed DoH heuristics. + kind: uint + expires: never # capturing continuously for monitoring purposes + release_channel_collection: opt-out + notification_emails: + - necko@mozilla.com + - vgosu@mozilla.com + products: + - 'firefox' + record_in_processes: + - main + + doh_heuristics_result: + bug_numbers: + - 1784258 + description: > + The value of this scalar indicates the result of the last heuristic run. + 0: "incomplete" + 1: "pass" + 2: "opt-out" + 3: "manually-disabled" + 4: "manually-enabled" + 5: "enterprise-disabled" + 6: "enterprise-present" + 7: "enterprise-enabled" + 8: "vpn" + 9: "proxy" + 10: "nrpt" + 11: "parental" + 12: "modifiedRoots" + 13: "thirdPartyRoots" + 14: "google" + 15: "youtube" + 16: "zscaler" + 17: "canary" + 18: "ignored" + kind: uint + expires: never # capturing continuously for monitoring purposes + release_channel_collection: opt-out + notification_emails: + - necko@mozilla.com + - vgosu@mozilla.com + products: + - 'firefox' + record_in_processes: + - main + + doh_heuristic_ever_tripped: + bug_numbers: + - 1784258 + description: > + True if this heuristic (key) was ever tripped during the session. + kind: boolean + keyed: true + expires: never # capturing continuously for monitoring purposes + release_channel_collection: opt-out + notification_emails: + - necko@mozilla.com + - vgosu@mozilla.com + products: + - 'firefox' + record_in_processes: + - main + blocklist: lastModified_rs_addons: bug_numbers: