Bug 1533668 - Distinguish manual sync from sync via .get() in uptake Telemetry r=glasserc
authorMathieu Leplatre <mathieu@mozilla.com>
Fri, 08 Mar 2019 19:57:46 +0000
changeset 521330 b64cd61cd5ad1808e19ce76748e9cd44f13cfd95
parent 521329 7b2ae2ea04957e8f18250ffc62563f301e5335f8
child 521331 8076b05b8631a55cc9a9922821e975c4ea4434af
push id10866
push usernerli@mozilla.com
push dateTue, 12 Mar 2019 18:59:09 +0000
treeherdermozilla-beta@445c24a51727 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglasserc
bugs1533668
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1533668 - Distinguish manual sync from sync via .get() in uptake Telemetry r=glasserc Distinguish manual sync from sync via .get() in uptake Telemetry Differential Revision: https://phabricator.services.mozilla.com/D22661
services/settings/RemoteSettingsClient.jsm
services/settings/RemoteSettingsComponents.jsm
services/settings/remote-settings.js
toolkit/components/telemetry/Events.yaml
toolkit/components/telemetry/docs/collection/uptake.rst
--- a/services/settings/RemoteSettingsClient.jsm
+++ b/services/settings/RemoteSettingsClient.jsm
@@ -26,18 +26,18 @@ ChromeUtils.defineModuleGetter(this, "Ut
 
 XPCOMUtils.defineLazyGlobalGetters(this, ["fetch"]);
 
 // IndexedDB name.
 const DB_NAME = "remote-settings";
 
 const TELEMETRY_COMPONENT = "remotesettings";
 
-const INVALID_SIGNATURE = "Invalid content signature";
-const MISSING_SIGNATURE = "Missing signature";
+const INVALID_SIGNATURE_MSG = "Invalid content signature";
+const MISSING_SIGNATURE_MSG = "Missing signature";
 
 XPCOMUtils.defineLazyPreferenceGetter(this, "gServerURL",
                                       "services.settings.server");
 XPCOMUtils.defineLazyPreferenceGetter(this, "gChangesPath",
                                       "services.settings.changes.path");
 XPCOMUtils.defineLazyPreferenceGetter(this, "gVerifySignature",
                                       "services.settings.verify_signature", true);
 
@@ -80,17 +80,17 @@ class ClientEnvironment extends ClientEn
  * @returns {Promise<{String, String}>}
  */
 async function fetchCollectionSignature(bucket, collection, expectedTimestamp) {
   const client = new KintoHttpClient(gServerURL);
   const { signature: signaturePayload } = await client.bucket(bucket)
     .collection(collection)
     .getData({ query: { _expected: expectedTimestamp } });
   if (!signaturePayload) {
-    throw new Error(MISSING_SIGNATURE);
+    throw new Error(MISSING_SIGNATURE_MSG);
   }
   const { x5u, signature } = signaturePayload;
   const certChainResponse = await fetch(x5u);
   const certChain = await certChainResponse.text();
 
   return { signature, certChain };
 }
 
@@ -263,102 +263,101 @@ class RemoteSettingsClient extends Event
       },
     });
     if (changes.length === 0) {
       throw new Error(`Unknown collection "${this.identifier}"`);
     }
     // According to API, there will be one only (fail if not).
     const [{ last_modified: expectedTimestamp }] = changes;
 
-    return this.maybeSync(expectedTimestamp, options);
+    return this.maybeSync(expectedTimestamp, { ...options, trigger: "forced" });
   }
 
   /**
    * Synchronize the local database with the remote server, **only if necessary**.
    *
    * @param {int}    expectedTimestamp the lastModified date (on the server) for the remote collection.
    *                                   This will be compared to the local timestamp, and will be used for
    *                                   cache busting if local data is out of date.
    * @param {Object} options           additional advanced options.
    * @param {bool}   options.loadDump  load initial dump from disk on first sync (default: true)
+   * @param {string} options.trigger   label to identify what triggered this sync (eg. ``"timer"``, default: `"manual"`)
    * @return {Promise}                 which rejects on sync or process failure.
    */
-  async maybeSync(expectedTimestamp, options = { loadDump: true }) {
+  async maybeSync(expectedTimestamp, options = { loadDump: true, trigger: "manual" }) {
     const { loadDump, trigger } = options;
 
     let reportStatus = null;
     try {
-      const collection = await this.openCollection();
-      // Synchronize remote data into a local Sqlite DB.
-      let collectionLastModified = await collection.db.getLastModified();
+      // Synchronize remote data into a local DB using Kinto.
+      const kintoCollection = await this.openCollection();
+      let collectionLastModified = await kintoCollection.db.getLastModified();
 
       // If there is no data currently in the collection, attempt to import
       // initial data from the application defaults.
       // This allows to avoid synchronizing the whole collection content on
       // cold start.
       if (!collectionLastModified && loadDump) {
         try {
           await RemoteSettingsWorker.importJSONDump(this.bucketName, this.collectionName);
-          collectionLastModified = await collection.db.getLastModified();
+          collectionLastModified = await kintoCollection.db.getLastModified();
         } catch (e) {
           // Report but go-on.
           Cu.reportError(e);
         }
       }
 
       // If the data is up to date, there's no need to sync. We still need
       // to record the fact that a check happened.
       if (expectedTimestamp <= collectionLastModified) {
         reportStatus = UptakeTelemetry.STATUS.UP_TO_DATE;
         return;
       }
 
-      // If there is a `signerName` and collection signing is enforced, add a
-      // hook for incoming changes that validates the signature.
+      // If signature verification is enabled, then add a synchronization hook
+      // for incoming changes that validates the signature.
       if (this.signerName && gVerifySignature) {
-        collection.hooks["incoming-changes"] = [async (payload, collection) => {
+        kintoCollection.hooks["incoming-changes"] = [async (payload, collection) => {
           await this._validateCollectionSignature(payload.changes,
                                                   payload.lastModified,
                                                   collection,
                                                   { expectedTimestamp });
           // In case the signature is valid, apply the changes locally.
           return payload;
         }];
       }
 
-      // Fetch changes from server.
       let syncResult;
       try {
-        // Server changes have priority during synchronization.
+        // Fetch changes from server, and make sure we overwrite local data.
         const strategy = Kinto.syncStrategy.SERVER_WINS;
-        syncResult = await collection.sync({ remote: gServerURL, strategy, expectedTimestamp });
-        const { ok } = syncResult;
-        if (!ok) {
+        syncResult = await kintoCollection.sync({ remote: gServerURL, strategy, expectedTimestamp });
+        if (!syncResult.ok) {
           // With SERVER_WINS, there cannot be any conflicts, but don't silent it anyway.
           throw new Error("Synced failed");
         }
       } catch (e) {
-        if (e.message.includes(INVALID_SIGNATURE)) {
+        if (e.message.includes(INVALID_SIGNATURE_MSG)) {
           // Signature verification failed during synchronization.
           reportStatus = UptakeTelemetry.STATUS.SIGNATURE_ERROR;
           // If sync fails with a signature error, it's likely that our
           // local data has been modified in some way.
           // We will attempt to fix this by retrieving the whole
           // remote collection.
           try {
-            syncResult = await this._retrySyncFromScratch(collection, expectedTimestamp);
+            syncResult = await this._retrySyncFromScratch(kintoCollection, expectedTimestamp);
           } catch (e) {
             // If the signature fails again, or if an error occured during wiping out the
             // local data, then we report it as a *signature retry* error.
             reportStatus = UptakeTelemetry.STATUS.SIGNATURE_RETRY_ERROR;
             throw e;
           }
         } else {
           // The sync has thrown, it can be related to metadata, network or a general error.
-          if (e.message == MISSING_SIGNATURE) {
+          if (e.message == MISSING_SIGNATURE_MSG) {
             // Collection metadata has no signature info, no need to retry.
             reportStatus = UptakeTelemetry.STATUS.SIGNATURE_ERROR;
           } else if (/unparseable/.test(e.message)) {
             reportStatus = UptakeTelemetry.STATUS.PARSE_ERROR;
           } else if (/NetworkError/.test(e.message)) {
             reportStatus = UptakeTelemetry.STATUS.NETWORK_ERROR;
           } else if (/Timeout/.test(e.message)) {
             reportStatus = UptakeTelemetry.STATUS.TIMEOUT_ERROR;
@@ -367,18 +366,18 @@ class RemoteSettingsClient extends Event
           } else if (/Backoff/.test(e.message)) {
             reportStatus = UptakeTelemetry.STATUS.BACKOFF;
           } else {
             reportStatus = UptakeTelemetry.STATUS.SYNC_ERROR;
           }
           throw e;
         }
       }
-
-      const filteredSyncResult = await this._filterSyncResult(collection, syncResult);
+      // Filter the synchronization results using `filterFunc` (ie. JEXL).
+      const filteredSyncResult = await this._filterSyncResult(kintoCollection, syncResult);
       // If every changed entry is filtered, we don't even fire the event.
       if (filteredSyncResult) {
         try {
           await this.emit("sync", { data: filteredSyncResult });
         } catch (e) {
           reportStatus = UptakeTelemetry.STATUS.APPLY_ERROR;
           throw e;
         }
@@ -434,17 +433,17 @@ class RemoteSettingsClient extends Event
                                                                      remoteRecords,
                                                                      timestamp);
     const verifier = Cc["@mozilla.org/security/contentsignatureverifier;1"]
       .createInstance(Ci.nsIContentSignatureVerifier);
     if (!verifier.verifyContentSignature(serialized,
                                          "p384ecdsa=" + signature,
                                          certChain,
                                          this.signerName)) {
-      throw new Error(INVALID_SIGNATURE + ` (${bucket}/${collection})`);
+      throw new Error(`${INVALID_SIGNATURE_MSG} (${bucket}/${collection})`);
     }
   }
 
   /**
    * Fetch the whole list of records from the server, verify the signature again
    * and then compute a synchronization result as if the diff-based sync happened.
    * And eventually, wipe out the local data.
    *
--- a/services/settings/RemoteSettingsComponents.jsm
+++ b/services/settings/RemoteSettingsComponents.jsm
@@ -14,12 +14,12 @@ ChromeUtils.defineModuleGetter(this, "Re
 var RemoteSettingsTimer = function() {};
 RemoteSettingsTimer.prototype = {
   QueryInterface: ChromeUtils.generateQI([Ci.nsITimerCallback]),
   classID: Components.ID("{5e756573-234a-49ea-bbe4-59ec7a70657d}"),
   contractID: "@mozilla.org/services/settings;1",
 
   // By default, this timer fires once every 24 hours. See the "services.settings.poll_interval" pref.
   notify(timer) {
-    RemoteSettings.pollChanges()
+    RemoteSettings.pollChanges({ trigger: "timer" })
       .catch(e => Cu.reportError(e));
   },
 };
--- a/services/settings/remote-settings.js
+++ b/services/settings/remote-settings.js
@@ -148,20 +148,20 @@ function remoteSettingsFunction() {
     return null;
   }
 
   /**
    * Main polling method, called by the ping mechanism.
    *
    * @param {Object} options
 .  * @param {Object} options.expectedTimestamp (optional) The expected timestamp to be received — used by servers for cache busting.
+   * @param {string} options.trigger           (optional) label to identify what triggered this sync (eg. ``"timer"``, default: `"manual"`)
    * @returns {Promise} or throws error if something goes wrong.
    */
-  remoteSettings.pollChanges = async ({ expectedTimestamp } = {}) => {
-    const trigger = expectedTimestamp ? "broadcast" : "timer";
+  remoteSettings.pollChanges = async ({ expectedTimestamp, trigger = "manual" } = {}) => {
     const telemetryArgs = {
       source: TELEMETRY_SOURCE,
       trigger,
     };
 
     // Check if the server backoff time is elapsed.
     if (gPrefs.prefHasUserValue(PREF_SETTINGS_SERVER_BACKOFF)) {
       const backoffReleaseTime = gPrefs.getCharPref(PREF_SETTINGS_SERVER_BACKOFF);
@@ -219,17 +219,17 @@ function remoteSettingsFunction() {
     // Record new update time and the difference between local and server time.
     // Negative clockDifference means local time is behind server time
     // by the absolute of that value in seconds (positive means it's ahead)
     const clockDifference = Math.floor((Date.now() - serverTimeMillis) / 1000);
     gPrefs.setIntPref(PREF_SETTINGS_CLOCK_SKEW_SECONDS, clockDifference);
     const checkedServerTimeInSeconds = Math.round(serverTimeMillis / 1000);
     gPrefs.setIntPref(PREF_SETTINGS_LAST_UPDATE, checkedServerTimeInSeconds);
 
-
+    // Should the clients try to load JSON dump? (mainly disabled in tests)
     const loadDump = gPrefs.getBoolPref(PREF_SETTINGS_LOAD_DUMP, true);
 
     // Iterate through the collections version info and initiate a synchronization
     // on the related remote settings client.
     let firstError;
     for (const change of changes) {
       const { bucket, collection, last_modified } = change;
 
@@ -321,11 +321,11 @@ function remoteSettingsFunction() {
 
   return remoteSettings;
 }
 
 var RemoteSettings = remoteSettingsFunction();
 
 var remoteSettingsBroadcastHandler = {
   async receivedBroadcastMessage(data, broadcastID) {
-    return RemoteSettings.pollChanges({ expectedTimestamp: data });
+    return RemoteSettings.pollChanges({ expectedTimestamp: data, trigger: "broadcast" });
   },
 };
--- a/toolkit/components/telemetry/Events.yaml
+++ b/toolkit/components/telemetry/Events.yaml
@@ -1006,17 +1006,17 @@ uptake.remotecontent.result:
       - remotesettings
       - normandy
     extra_keys:
       source: >
         A label to distinguish what is being pulled or updated in the component (eg. recipe id,
         settings collection name, ...).
       trigger: >
         A label to distinguish what triggered the polling/fetching of remote content (eg. "broadcast",
-        "timer")
+        "timer", "forced", "manual")
     bug_numbers:
       - 1517469
     record_in_processes: ["main"]
     release_channel_collection: opt-out
     expiry_version: never
     notification_emails:
       - mleplatre@mozilla.com
       - bens-directs@mozilla.com
--- a/toolkit/components/telemetry/docs/collection/uptake.rst
+++ b/toolkit/components/telemetry/docs/collection/uptake.rst
@@ -79,17 +79,17 @@ Example:
    UptakeTelemetry.report(COMPONENT, status, { source: UPDATE_SOURCE });
 
 
 Additional Event Info
 '''''''''''''''''''''
 
 The Event API allows to report additional information. We support the following optional fields:
 
-- ``trigger``: A label to distinguish what triggered the polling/fetching of remote content (eg. ``"broadcast"``, ``"timer"``)
+- ``trigger``: A label to distinguish what triggered the polling/fetching of remote content (eg. ``"broadcast"``, ``"timer"``, ``"forced"``, ``"manual"``)
 
 .. code-block:: js
 
    UptakeTelemetry.report(component, status, { source, trigger: "timer" });
 
 
 Use-cases
 ---------