Bug 1633466 - Record usage of blocklist v3 in telemetry r=rpl,janerik

Although the data review request granted permission to keep the probes
for 6 months, I'm using a shorter expiration (81) so that all
blocklist-related telemetry can be coverd by one update request, in the
future.

"geckoview" is not listed yet, because blocklist v3 is not yet enabled
on mobile (bug 1639050). Once enabled, "geckoview" should be added to
every scalar from this commit.

Differential Revision: https://phabricator.services.mozilla.com/D76539

toolkit/components/telemetry/Scalars.yaml

@@ -4128,6 +4128,26 @@ blocklist:
     record_in_processes:
       - main
 
+  lastModified_rs_addons_mlbf:
+    bug_numbers:
+      - 1633466
+    description: >
+      Keep track of the last time the "addons-bloomfilters" remotesetting blocklist has been successfully
+      updated (as a datetime string in UTC format), set to "Missing Date" when the timestamp
+      couldn't be retrieved.
+    expires: "81"
+    kind: string
+    release_channel_collection: opt-out
+    notification_emails:
+      - addons-dev-internal@mozilla.com
+      - lgreco@mozilla.com
+      - awagner@mozilla.com
+      - rwu@mozilla.com
+    products:
+      - 'firefox'
+    record_in_processes:
+      - main
+
   lastModified_rs_plugins:
     bug_numbers:
       - 1572711
@@ -4149,6 +4169,107 @@ blocklist:
     record_in_processes:
       - main
 
+  mlbf_enabled:
+    bug_numbers:
+      - 1633466
+    description: >
+      Keep track of whether the addons blocklist engine uses bloom filters (blocklist v3).
+      If false, the blocklist v2 is used instead.
+    expires: "81"
+    kind: boolean
+    release_channel_collection: opt-out
+    notification_emails:
+      - addons-dev-internal@mozilla.com
+      - lgreco@mozilla.com
+      - awagner@mozilla.com
+      - rwu@mozilla.com
+    products:
+      - 'firefox'
+    record_in_processes:
+      - main
+
+  mlbf_stashes:
+    bug_numbers:
+      - 1633466
+    description: >
+      Keep track of whether the bloomfilter-based blocklist engine uses stashes from the
+      addons-bloomfilters collection, together with an infrequently updated base bloomfilter.
+      If false, the latest (full) bloom filter is downloaded on every update of the addon blocklist.
+      This metric is only meaningful if mlbf_enabled is true.
+    expires: "81"
+    kind: boolean
+    release_channel_collection: opt-out
+    notification_emails:
+      - addons-dev-internal@mozilla.com
+      - lgreco@mozilla.com
+      - awagner@mozilla.com
+      - rwu@mozilla.com
+    products:
+      - 'firefox'
+    record_in_processes:
+      - main
+
+  mlbf_generation_time:
+    bug_numbers:
+      - 1633466
+    description: >
+      Keep track of the generation time of the addon blocklist's bloom filter. This marks the
+      point in time until which signed add-ons are recognized by the selected bloom filter from the
+      addons-bloomfilters collection.
+      The value is a datetime string in UTC format, or  "Missing Date" when unavailable.
+    expires: "81"
+    kind: string
+    release_channel_collection: opt-out
+    notification_emails:
+      - addons-dev-internal@mozilla.com
+      - lgreco@mozilla.com
+      - awagner@mozilla.com
+      - rwu@mozilla.com
+    products:
+      - 'firefox'
+    record_in_processes:
+      - main
+
+  mlbf_stash_time_oldest:
+    bug_numbers:
+      - 1633466
+    description: >
+      Keep track of the timestamp of the oldest stash of the addons blocklist.
+      Only meaningful when mlbf_enabled and mlbf_stashes are true.
+      The value is a datetime string in UTC format, or "Missing Date" when unavailable.
+    expires: "81"
+    kind: string
+    release_channel_collection: opt-out
+    notification_emails:
+      - addons-dev-internal@mozilla.com
+      - lgreco@mozilla.com
+      - awagner@mozilla.com
+      - rwu@mozilla.com
+    products:
+      - 'firefox'
+    record_in_processes:
+      - main
+
+  mlbf_stash_time_newest:
+    bug_numbers:
+      - 1633466
+    description: >
+      Keep track of the timestamp of the most recent stash of the addons blocklist.
+      Only meaningful when mlbf_enabled and mlbf_stashes are true.
+      The value is a datetime string in UTC format, or "Missing Date" when unavailable.
+    expires: "81"
+    kind: string
+    release_channel_collection: opt-out
+    notification_emails:
+      - addons-dev-internal@mozilla.com
+      - lgreco@mozilla.com
+      - awagner@mozilla.com
+      - rwu@mozilla.com
+    products:
+      - 'firefox'
+    record_in_processes:
+      - main
+
 firstStartup:
   statusCode:
     bug_numbers:

toolkit/mozapps/extensions/Blocklist.jsm

@@ -180,6 +180,7 @@ const BlocklistTelemetry = {
    *
    * @param {string} blocklistType
    *        The blocklist type that has been updated (one of "addons" or "plugins",
+   *        or "addons_mlbf";
    *        the "gfx" blocklist is not covered by this telemetry).
    * @param {RemoteSettingsClient} remoteSettingsClient
    *        The RemoteSettings client to retrieve the lastModified timestamp from.
@@ -193,20 +194,29 @@ const BlocklistTelemetry = {
     }
 
     let lastModified = await remoteSettingsClient.getLastModified();
+    BlocklistTelemetry.recordTimeScalar(
+      "lastModified_rs_" + blocklistType,
+      lastModified
+    );
+  },
 
-    if (lastModified > 0) {
+  /**
+   * Record a timestamp in telemetry as a UTC string or "Missing Date" if the
+   * input is not a valid timestamp.
+   *
+   * @param {string} telemetryKey
+   *        The part of after "blocklist.", as defined in Scalars.yaml.
+   * @param {number} time
+   *        A timestamp to record. If invalid, "Missing Date" will be recorded.
+   */
+  recordTimeScalar(telemetryKey, time) {
+    if (time > 0) {
       // convert from timestamp in ms into UTC datetime string, so it is going
       // to be record in the same format previously used by blocklist.lastModified_xml.
-      lastModified = new Date(lastModified).toUTCString();
-      Services.telemetry.scalarSet(
-        `blocklist.lastModified_rs_${blocklistType}`,
-        lastModified
-      );
+      let dateString = new Date(time).toUTCString();
+      Services.telemetry.scalarSet("blocklist." + telemetryKey, dateString);
     } else {
-      Services.telemetry.scalarSet(
-        `blocklist.lastModified_rs_${blocklistType}`,
-        "Missing Date"
-      );
+      Services.telemetry.scalarSet("blocklist." + telemetryKey, "Missing Date");
     }
   },
 };
@@ -1444,9 +1454,10 @@ this.ExtensionBlocklistMLBF = {
           })
           // Sort by stash time - newest first.
           .sort((a, b) => b.stash_time - a.stash_time)
-          .map(({ stash }) => ({
+          .map(({ stash, stash_time }) => ({
             blocked: new Set(stash.blocked),
             unblocked: new Set(stash.unblocked),
+            stash_time,
           }));
       } else {
         mlbfRecord = mlbfRecords.find(
@@ -1471,6 +1482,7 @@ this.ExtensionBlocklistMLBF = {
       .then(() => {
         if (!isUpdateReplaced()) {
           this._updatePromise = null;
+          this._recordPostUpdateTelemetry();
         }
         return this._updatePromise;
       });
@@ -1478,6 +1490,29 @@ this.ExtensionBlocklistMLBF = {
     return updatePromise;
   },
 
+  // Update the telemetry of the blocklist. This is always called, even if
+  // the update request failed (e.g. due to network errors or data corruption).
+  _recordPostUpdateTelemetry() {
+    BlocklistTelemetry.recordRSBlocklistLastModified(
+      "addons_mlbf",
+      this._client
+    );
+    BlocklistTelemetry.recordTimeScalar(
+      "mlbf_generation_time",
+      this._mlbfData?.generationTime
+    );
+    // stashes has conveniently already been sorted by stash_time, newest first.
+    let stashes = this._stashes || [];
+    BlocklistTelemetry.recordTimeScalar(
+      "mlbf_stash_time_oldest",
+      stashes[stashes.length - 1]?.stash_time
+    );
+    BlocklistTelemetry.recordTimeScalar(
+      "mlbf_stash_time_newest",
+      stashes[0]?.stash_time
+    );
+  },
+
   ensureInitialized() {
     if (!gBlocklistEnabled || this._initialized) {
       return;
@@ -1497,6 +1532,7 @@ this.ExtensionBlocklistMLBF = {
       PREF_BLOCKLIST_USE_MLBF_STASHES,
       false
     );
+    Services.telemetry.scalarSet("blocklist.mlbf_stashes", this.stashesEnabled);
   },
 
   shutdown() {
@@ -1576,6 +1612,8 @@ this.ExtensionBlocklistMLBF = {
       }
     }
 
+    // signedDate is a Date if the add-on is signed, null if not signed,
+    // undefined if it's an addon update descriptor instead of an addon wrapper.
     let { signedDate } = addon;
     if (!signedDate) {
       // The MLBF does not apply to unsigned add-ons.
@@ -1787,6 +1825,10 @@ let Blocklist = {
               ExtensionBlocklistMLBF._initialized &&
               !ExtensionBlocklistMLBF._didShutdown
             ) {
+              Services.telemetry.scalarSet(
+                "blocklist.mlbf_stashes",
+                ExtensionBlocklistMLBF.stashesEnabled
+              );
               ExtensionBlocklistMLBF._onUpdate();
             }
             break;
@@ -1823,8 +1865,10 @@ let Blocklist = {
   _chooseExtensionBlocklistImplementationFromPref() {
     if (Services.prefs.getBoolPref(PREF_BLOCKLIST_USE_MLBF, false)) {
       this.ExtensionBlocklist = ExtensionBlocklistMLBF;
+      Services.telemetry.scalarSet("blocklist.mlbf_enabled", true);
     } else {
       this.ExtensionBlocklist = ExtensionBlocklistRS;
+      Services.telemetry.scalarSet("blocklist.mlbf_enabled", false);
     }
   },
 

toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/test_blocklist_mlbf_telemetry.js

@@ -0,0 +1,181 @@
+/* Any copyright is dedicated to the Public Domain.
+ * https://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+Services.prefs.setBoolPref("extensions.blocklist.useMLBF", true);
+Services.prefs.setBoolPref("extensions.blocklist.useMLBF.stashes", true);
+
+createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1");
+
+const { Downloader } = ChromeUtils.import(
+  "resource://services-settings/Attachments.jsm"
+);
+
+const { TelemetryController } = ChromeUtils.import(
+  "resource://gre/modules/TelemetryController.jsm"
+);
+const { TelemetryTestUtils } = ChromeUtils.import(
+  "resource://testing-common/TelemetryTestUtils.jsm"
+);
+
+const OLDEST_STASH = { stash: { blocked: [], unblocked: [] }, stash_time: 2e6 };
+const NEWEST_STASH = { stash: { blocked: [], unblocked: [] }, stash_time: 5e6 };
+const RECORDS_WITH_STASHES_AND_MLBF = [MLBF_RECORD, OLDEST_STASH, NEWEST_STASH];
+
+const ExtensionBlocklistMLBF = getExtensionBlocklistMLBF();
+
+function assertTelemetryScalars(expectedScalars) {
+  let scalars = TelemetryTestUtils.getProcessScalars("parent");
+
+  for (const scalarName of Object.keys(expectedScalars || {})) {
+    equal(
+      scalars[scalarName],
+      expectedScalars[scalarName],
+      `Got the expected value for ${scalarName} scalar`
+    );
+  }
+}
+
+function toUTC(time) {
+  return new Date(time).toUTCString();
+}
+
+add_task(async function setup() {
+  await TelemetryController.testSetup();
+  await promiseStartupManager();
+
+  // Disable the packaged record and attachment to make sure that the test
+  // will not fall back to the packaged attachments.
+  Downloader._RESOURCE_BASE_URL = "invalid://bogus";
+});
+
+add_task(async function test_initialization() {
+  ExtensionBlocklistMLBF.ensureInitialized();
+  assertTelemetryScalars({
+    "blocklist.mlbf_enabled": true,
+    "blocklist.mlbf_stashes": true,
+    // In other parts of this test, this value is not checked any more.
+    // test_blocklist_telemetry.js already checks lastModified_rs_addons_mlbf.
+    "blocklist.lastModified_rs_addons_mlbf": undefined,
+    "blocklist.mlbf_generation_time": undefined,
+    "blocklist.mlbf_stash_time_oldest": undefined,
+    "blocklist.mlbf_stash_time_newest": undefined,
+  });
+});
+
+// Test what happens if there is no blocklist data at all.
+add_task(async function test_without_mlbf() {
+  await AddonTestUtils.loadBlocklistRawData({ extensionsMLBF: [] });
+  assertTelemetryScalars({
+    "blocklist.mlbf_enabled": true,
+    "blocklist.mlbf_stashes": true,
+    "blocklist.mlbf_generation_time": "Missing Date",
+    "blocklist.mlbf_stash_time_oldest": "Missing Date",
+    "blocklist.mlbf_stash_time_newest": "Missing Date",
+  });
+});
+
+// Test the telemetry that would be recorded in the common case.
+add_task(async function test_common_good_case_with_stashes() {
+  // The exact content of the attachment does not matter in this test, as long
+  // as the data is valid.
+  await ExtensionBlocklistMLBF._client.db.saveAttachment(
+    ExtensionBlocklistMLBF.RS_ATTACHMENT_ID,
+    { record: MLBF_RECORD, blob: await load_mlbf_record_as_blob() }
+  );
+  await AddonTestUtils.loadBlocklistRawData({
+    extensionsMLBF: RECORDS_WITH_STASHES_AND_MLBF,
+  });
+  assertTelemetryScalars({
+    "blocklist.mlbf_enabled": true,
+    "blocklist.mlbf_stashes": true,
+    "blocklist.mlbf_generation_time": toUTC(MLBF_RECORD.generation_time),
+    "blocklist.mlbf_stash_time_oldest": toUTC(OLDEST_STASH.stash_time),
+    "blocklist.mlbf_stash_time_newest": toUTC(NEWEST_STASH.stash_time),
+  });
+
+  // The records and cached attachment carries over to the next tests.
+});
+
+add_task(async function test_toggle_stash_pref() {
+  // The previous test had imported RECORDS_WITH_STASHES_AND_MLBF.
+  // Verify that toggling the pref causes those stashes to be ignored.
+  await toggleStashPref(false, () => {
+    // Telemetry should be updated immediately after setting the pref.
+    assertTelemetryScalars({
+      "blocklist.mlbf_enabled": true,
+      "blocklist.mlbf_stashes": false,
+    });
+  });
+  assertTelemetryScalars({
+    "blocklist.mlbf_enabled": true,
+    "blocklist.mlbf_stashes": false,
+    "blocklist.mlbf_generation_time": toUTC(MLBF_RECORD.generation_time),
+    "blocklist.mlbf_stash_time_oldest": "Missing Date",
+    "blocklist.mlbf_stash_time_newest": "Missing Date",
+  });
+
+  await toggleStashPref(true, () => {
+    // Telemetry should be updated immediately after setting the pref.
+    assertTelemetryScalars({
+      "blocklist.mlbf_enabled": true,
+      "blocklist.mlbf_stashes": true,
+    });
+  });
+  assertTelemetryScalars({
+    "blocklist.mlbf_enabled": true,
+    "blocklist.mlbf_stashes": true,
+    "blocklist.mlbf_generation_time": toUTC(MLBF_RECORD.generation_time),
+    "blocklist.mlbf_stash_time_oldest": toUTC(OLDEST_STASH.stash_time),
+    "blocklist.mlbf_stash_time_newest": toUTC(NEWEST_STASH.stash_time),
+  });
+});
+
+// Test what happens when there are no stashes in the collection itself.
+add_task(async function test_without_stashes() {
+  await AddonTestUtils.loadBlocklistRawData({ extensionsMLBF: [MLBF_RECORD] });
+  assertTelemetryScalars({
+    "blocklist.mlbf_enabled": true,
+    "blocklist.mlbf_stashes": true,
+    "blocklist.mlbf_generation_time": toUTC(MLBF_RECORD.generation_time),
+    "blocklist.mlbf_stash_time_oldest": "Missing Date",
+    "blocklist.mlbf_stash_time_newest": "Missing Date",
+  });
+});
+
+// Test that the mlbf_enabled and mlbf_stashes scalars are updated in response
+// to preference changes.
+add_task(async function test_toggle_preferences() {
+  // Disable the blocklist, to prevent the v2 blocklist from initializing.
+  // We only care about scalar updates in response to preference changes.
+  Services.prefs.setBoolPref("extensions.blocklist.enabled", false);
+  // Sanity check: scalars haven't changed.
+  assertTelemetryScalars({
+    "blocklist.mlbf_enabled": true,
+    "blocklist.mlbf_stashes": true,
+    "blocklist.mlbf_generation_time": toUTC(MLBF_RECORD.generation_time),
+  });
+
+  Services.prefs.setBoolPref("extensions.blocklist.useMLBF.stashes", false);
+  assertTelemetryScalars({
+    "blocklist.mlbf_enabled": true,
+    "blocklist.mlbf_stashes": false,
+    "blocklist.mlbf_generation_time": toUTC(MLBF_RECORD.generation_time),
+  });
+
+  Services.prefs.setBoolPref("extensions.blocklist.useMLBF", false);
+  assertTelemetryScalars({
+    "blocklist.mlbf_enabled": false,
+    "blocklist.mlbf_stashes": false,
+    "blocklist.mlbf_generation_time": toUTC(MLBF_RECORD.generation_time),
+  });
+
+  Services.prefs.setBoolPref("extensions.blocklist.useMLBF.stashes", true);
+  assertTelemetryScalars({
+    "blocklist.mlbf_enabled": false,
+    // The mlbf_stashes scalar is only updated when useMLBF is true.
+    "blocklist.mlbf_stashes": false,
+    "blocklist.mlbf_generation_time": toUTC(MLBF_RECORD.generation_time),
+  });
+});

toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/test_blocklist_telemetry.js

@@ -42,6 +42,7 @@ add_task(async function test_blocklist_lastModified_rs_scalars() {
 
   const lastEntryTimes = {
     addons: now - 5000,
+    addons_mlbf: now - 4000,
     plugins: now - 3000,
   };
 
@@ -54,6 +55,7 @@ add_task(async function test_blocklist_lastModified_rs_scalars() {
   const {
     BlocklistTelemetry,
     ExtensionBlocklistRS,
+    ExtensionBlocklistMLBF,
     PluginBlocklistRS,
   } = ChromeUtils.import("resource://gre/modules/Blocklist.jsm", null);
 
@@ -87,6 +89,7 @@ add_task(async function test_blocklist_lastModified_rs_scalars() {
 
   assertTelemetryScalars({
     "blocklist.lastModified_rs_addons": undefined,
+    "blocklist.lastModified_rs_addons_mlbf": undefined,
     "blocklist.lastModified_rs_plugins": lastEntryTimesUTC.plugins,
   });
 
@@ -100,6 +103,22 @@ add_task(async function test_blocklist_lastModified_rs_scalars() {
 
   assertTelemetryScalars({
     "blocklist.lastModified_rs_addons": lastEntryTimesUTC.addons,
+    "blocklist.lastModified_rs_addons_mlbf": undefined,
+    "blocklist.lastModified_rs_plugins": lastEntryTimesUTC.plugins,
+  });
+
+  await ExtensionBlocklistMLBF.ensureInitialized();
+  await Promise.all([
+    promiseScalarRecorded(),
+    fakeRemoteSettingsSync(
+      ExtensionBlocklistMLBF._client,
+      lastEntryTimes.addons_mlbf
+    ),
+  ]);
+
+  assertTelemetryScalars({
+    "blocklist.lastModified_rs_addons": lastEntryTimesUTC.addons,
+    "blocklist.lastModified_rs_addons_mlbf": lastEntryTimesUTC.addons_mlbf,
     "blocklist.lastModified_rs_plugins": lastEntryTimesUTC.plugins,
   });
 });

toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/xpcshell.ini

@@ -21,6 +21,7 @@ skip-if = os == "android"
 skip-if = os == "android" # bug 1639050
 [test_blocklist_mlbf_fetch.js]
 [test_blocklist_mlbf_stashes.js]
+[test_blocklist_mlbf_telemetry.js]
 [test_blocklist_mlbf_update.js]
 [test_blocklist_osabi.js]
 # Bug 676992: test consistently hangs on Android