Bug 1855604 - Back out putting provenance attribution in installation.first_seen r=nalexander

Backs out Bug 1821189's patch: https://hg.mozilla.org/mozilla-central/rev/c4f8098f81bb Differential Revision: https://phabricator.services.mozilla.com/D189591
2023-10-03 23:40:24 +00:00
parent 9bbff3983b
commit e194d9f28b
5 changed files with 14 additions and 164 deletions
--- a/browser/components/attribution/ProvenanceData.sys.mjs
+++ b/browser/components/attribution/ProvenanceData.sys.mjs
@@ -441,15 +441,6 @@ export var ProvenanceData = {
  /**
   * Only submits telemetry once, no matter how many times it is called.
   * Has no effect on OSs where provenance data is not supported.
-   *
-   * @returns An object indicating the values submitted. Keys may not match the
-   *          Scalar names since the returned object is intended to be suitable
-   *          for use as a Telemetry Event's `extra` object, which has shorter
-   *          limits for extra key names than the limits for Scalar names.
-   *          Values will be converted to strings since Telemetry Event's
-   *          `extra` objects must have string values.
-   *          On platforms that do not support provenance data, this will always
-   *          return an empty object.
   */
  async submitProvenanceTelemetry() {
    if (gTelemetryPromise) {
@@ -457,31 +448,21 @@ export var ProvenanceData = {
    }
    gTelemetryPromise = (async () => {
      const errorValue = "error";
-
-      let extra = {};
-
      let provenance = await this.readZoneIdProvenanceFile();
      if (!provenance) {
-        return extra;
+        return;
      }

-      let setTelemetry = (scalarName, extraKey, value) => {
-        Services.telemetry.scalarSet(scalarName, value);
-        extra[extraKey] = value.toString();
-      };
-
-      setTelemetry(
+      Services.telemetry.scalarSet(
        "attribution.provenance.data_exists",
-        "data_exists",
        !provenance.readProvenanceError
      );
      if (provenance.readProvenanceError) {
-        return extra;
+        return;
      }

-      setTelemetry(
+      Services.telemetry.scalarSet(
        "attribution.provenance.file_system",
-        "file_system",
        provenance.fileSystem ?? errorValue
      );

@@ -494,50 +475,42 @@ export var ProvenanceData = {
          provenance.readZoneIdError == "openFile" &&
          provenance.readZoneIdErrorCode == ERROR_FILE_NOT_FOUND
        );
-      setTelemetry(
+      Services.telemetry.scalarSet(
        "attribution.provenance.ads_exists",
-        "ads_exists",
        ads_exists
      );
      if (!ads_exists) {
-        return extra;
+        return;
      }

-      setTelemetry(
+      Services.telemetry.scalarSet(
        "attribution.provenance.security_zone",
-        "security_zone",
        "zoneId" in provenance ? provenance.zoneId.toString() : errorValue
      );

      let haveReferrerUrl = URL.isInstance(provenance.referrerUrl);
-      setTelemetry(
+      Services.telemetry.scalarSet(
        "attribution.provenance.referrer_url_exists",
-        "refer_url_exist",
        haveReferrerUrl
      );
      if (haveReferrerUrl) {
-        setTelemetry(
+        Services.telemetry.scalarSet(
          "attribution.provenance.referrer_url_is_mozilla",
-          "refer_url_moz",
          provenance.referrerUrlIsMozilla
        );
      }

      let haveHostUrl = URL.isInstance(provenance.hostUrl);
-      setTelemetry(
+      Services.telemetry.scalarSet(
        "attribution.provenance.host_url_exists",
-        "host_url_exist",
        haveHostUrl
      );
      if (haveHostUrl) {
-        setTelemetry(
+        Services.telemetry.scalarSet(
          "attribution.provenance.host_url_is_mozilla",
-          "host_url_moz",
          provenance.hostUrlIsMozilla
        );
      }
-
-      return extra;
    })();
    return gTelemetryPromise;
  },
--- a/browser/components/attribution/test/xpcshell/test_zoneId_parsing.js
+++ b/browser/components/attribution/test/xpcshell/test_zoneId_parsing.js
@@ -27,20 +27,6 @@ add_setup(function setup() {
 // If `iniFileContents` is passed as `null`, we will simulate an error reading
 // the INI.
 async function testProvenance(iniFileContents, testFn, telemetryTestFn) {
-  // The extra data returned by `ProvenanceData.submitProvenanceTelemetry` uses
-  // names that don't actually match the scalar names due to name length
-  // restrictions.
-  let scalarToExtraKeyMap = {
-    "attribution.provenance.data_exists": "data_exists",
-    "attribution.provenance.file_system": "file_system",
-    "attribution.provenance.ads_exists": "ads_exists",
-    "attribution.provenance.security_zone": "security_zone",
-    "attribution.provenance.referrer_url_exists": "refer_url_exist",
-    "attribution.provenance.referrer_url_is_mozilla": "refer_url_moz",
-    "attribution.provenance.host_url_exists": "host_url_exist",
-    "attribution.provenance.host_url_is_mozilla": "host_url_moz",
-  };
-
  if (iniFileContents == null) {
    AttributionIOUtils.readUTF8 = async path => {
      throw new Error("test error: simulating provenance file read error");
@@ -54,15 +40,13 @@ async function testProvenance(iniFileContents, testFn, telemetryTestFn) {
  }
  if (telemetryTestFn) {
    Services.telemetry.clearScalars();
-    let extras = await ProvenanceData.submitProvenanceTelemetry();
+    await ProvenanceData.submitProvenanceTelemetry();
    let scalars = Services.telemetry.getSnapshotForScalars(
      "new-profile",
      false /* aClear */
    ).parent;
    let checkScalar = (scalarName, expectedValue) => {
      TelemetryTestUtils.assertScalar(scalars, scalarName, expectedValue);
-      let extraKey = scalarToExtraKeyMap[scalarName];
-      Assert.equal(extras[extraKey], expectedValue.toString());
    };
    telemetryTestFn(checkScalar);
  }
--- a/browser/modules/BrowserUsageTelemetry.sys.mjs
+++ b/browser/modules/BrowserUsageTelemetry.sys.mjs
@@ -1283,9 +1283,8 @@ export let BrowserUsageTelemetry = {
      return;
    }

-    let provenanceExtra = {};
    try {
-      provenanceExtra = await lazy.ProvenanceData.submitProvenanceTelemetry();
+      await lazy.ProvenanceData.submitProvenanceTelemetry();
    } catch (ex) {
      console.warn(
        "reportInstallationTelemetry - submitProvenanceTelemetry failed",
@@ -1422,13 +1421,6 @@ export let BrowserUsageTelemetry = {
      null,
      extra
    );
-    Services.telemetry.recordEvent(
-      "installation",
-      "first_seen_prov_ext",
-      installer_type,
-      null,
-      provenanceExtra
-    );
  },
 };

--- a/browser/modules/test/unit/test_InstallationTelemetry.js
+++ b/browser/modules/test/unit/test_InstallationTelemetry.js
@@ -6,9 +6,6 @@
 const { AppConstants } = ChromeUtils.importESModule(
  "resource://gre/modules/AppConstants.sys.mjs"
 );
-const { AttributionIOUtils } = ChromeUtils.importESModule(
-  "resource:///modules/AttributionCode.sys.mjs"
-);
 const { BrowserUsageTelemetry } = ChromeUtils.importESModule(
  "resource:///modules/BrowserUsageTelemetry.sys.mjs"
 );
@@ -72,33 +69,8 @@ async function runReport(
    expectExtra
      ? [{ object: installType, value: null, extra: expectExtra }]
      : [],
-    { category: "installation", method: "first_seen" },
-    { clear: false }
+    { category: "installation", method: "first_seen" }
  );
-  // Provenance Data is currently only supported on Windows.
-  if (AppConstants.platform == "win") {
-    let provenanceExtra = {
-      data_exists: "true",
-      file_system: "NTFS",
-      ads_exists: "true",
-      security_zone: "3",
-      refer_url_exist: "true",
-      refer_url_moz: "true",
-      host_url_exist: "true",
-      host_url_moz: "true",
-    };
-    TelemetryTestUtils.assertEvents(
-      expectExtra
-        ? [{ object: installType, value: null, extra: provenanceExtra }]
-        : [],
-      { category: "installation", method: "first_seen_prov_ext" }
-    );
-  } else {
-    TelemetryTestUtils.assertEvents(
-      expectExtra ? [{ object: installType, value: null, extra: {} }] : [],
-      { category: "installation", method: "first_seen_prov_ext" }
-    );
-  }

  // Check timestamp
  if (typeof expectTS == "string") {
@@ -106,28 +78,6 @@ async function runReport(
  }
 }

-add_setup(function setup() {
-  let origReadUTF8 = AttributionIOUtils.readUTF8;
-  registerCleanupFunction(() => {
-    AttributionIOUtils.readUTF8 = origReadUTF8;
-  });
-  AttributionIOUtils.readUTF8 = async path => {
-    return `
-[Mozilla]
-fileSystem=NTFS
-zoneIdFileSize=194
-zoneIdBufferLargeEnough=true
-zoneIdTruncated=false
-
-[MozillaZoneIdentifierStartSentinel]
-[ZoneTransfer]
-ZoneId=3
-ReferrerUrl=https://mozilla.org/
-HostUrl=https://download-installer.cdn.mozilla.net/pub/firefox/nightly/latest-mozilla-central-l10n/Firefox%20Installer.en-US.exe
-`;
-  };
-});
-
 let condition = {
  skip_if: () =>
    AppConstants.platform !== "win" ||
--- a/toolkit/components/telemetry/Events.yaml
+++ b/toolkit/components/telemetry/Events.yaml
@@ -3516,55 +3516,6 @@ installation:
      - application-update-telemetry-alerts@mozilla.com
      - rtestard@mozilla.com
    expiry_version: never
-  first_seen_prov_ext:
-    description: >
-      This is an extension of the `installation.first_seen` event. It will always be recorded at
-      the same time as that event, so an event ping that contains one will generally contain the
-      other (the exception being if the event ping splits between the two and one of the two pings
-      fails to be received properly, which should be exceedingly rare). The reason for this event
-      to exist is that we want to include more than 10 `extra_keys`, but the limit is 10. The
-      specific extra keys that we want to include are related to provenance data (see Bug 1814968).
-    objects:
-      - full # if the full installer was run directly
-      - stub # if the stub installer was used
-      - msix # if the installation was done through an MSIX package
-    release_channel_collection: opt-out
-    record_in_processes: ["main"]
-    products: ["firefox"]
-    operating_systems: ["windows"]
-    extra_keys:
-      data_exists: >
-        Will be "true" if the "zoneIdProvenanceData" file is present in the installation directory
-        and we were able to successfully read it. This key is only present on platforms that
-        support provenance data.
-      file_system: >
-        The file system that the installer resided on at installation time. Possible values are:
-        "NTFS", "FAT32", "other". In error cases, it can also be "missing", "error" or
-        "readIniError". This key is only present if `data_exists` is "true".
-      ads_exists: >
-        Will always be false if `data_exists` is false. Will be "true" if the provenance data
-        indicates that the :Zone.Identifier Alternate Data Stream existed on the installer.
-      security_zone: >
-        The zone identifier in the installer's :Zone.Identifier ADS. Possible values are integers
-        between 0 and 4, inclusive (encoded as strings). In error cases, it can also be
-        "unexpected", "missing", "error", or "readIniError". This key is only present if
-        `ads_exists` is "true".
-      refer_url_exist: >
-        Will be "true" if the zone identifier ADS contained a referrer URL. Will be false if a
-        referrer URL is specified, but it isn't a valid URL. Only sent if `ads_exists` is "true".
-      refer_url_moz: >
-        Will be "true" if the referrer URL from the zone identifier ADS appeared to be a Mozilla
-        URL. Only sent if `refer_url_exist` is "true".
-      host_url_exist: >
-        Will be "true" if the zone identifier ADS contained a host URL. Will be false if a host URL
-        is specified, but it isn't a valid URL. Only sent if `ads_exists` is "true".
-      host_url_moz: >
-        Will be "true" if the host URL from the zone identifier ADS appeared to be a Mozilla URL.
-        Only sent if `host_url_exists` is "true".
-    bug_numbers: [1814968, 1821189]
-    notification_emails:
-      - application-update-telemetry-alerts@mozilla.com
-    expiry_version: "121"

 contextservices.quicksuggest:
  data_collect_toggled: