Bug 1523310 - Distinguish broadcast from scheduled sync in Remote Settings r=glasserc
authorMathieu Leplatre <mathieu@mozilla.com>
Thu, 07 Mar 2019 14:44:52 +0000
changeset 520764 41a911c24dd3c491da432881baa9b38fe0dce4d5
parent 520763 728658e02de5947d27e2aebd75b45de94b42e5d0
child 520765 0d69ee629ce50368c1dfead01fdd9878022380d9
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglasserc
bugs1523310
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 1523310 - Distinguish broadcast from scheduled sync in Remote Settings r=glasserc Distinguish broadcast from scheduled sync in Remote Settings Differential Revision: https://phabricator.services.mozilla.com/D22339
services/common/uptake-telemetry.js
services/settings/RemoteSettingsClient.jsm
services/settings/remote-settings.js
toolkit/components/telemetry/Events.yaml
toolkit/components/telemetry/docs/collection/uptake.rst
--- a/services/common/uptake-telemetry.js
+++ b/services/common/uptake-telemetry.js
@@ -82,16 +82,17 @@ class UptakeTelemetry {
 
   /**
    * Reports the uptake status for the specified source.
    *
    * @param {string} component     the component reporting the uptake (eg. "normandy").
    * @param {string} status        the uptake status (eg. "network_error")
    * @param {Object} extra         extra values to report
    * @param {string} extra.source  the update source (eg. "recipe-42").
+   * @param {string} extra.trigger what triggered the polling/fetching (eg. "broadcast", "timer").
    */
   static report(component, status, extra = {}) {
     const { source } = extra;
 
     if (!source) {
       throw new Error("`source` value is mandatory.");
     }
 
--- a/services/settings/RemoteSettingsClient.jsm
+++ b/services/settings/RemoteSettingsClient.jsm
@@ -277,17 +277,17 @@ class RemoteSettingsClient extends Event
    * @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)
    * @return {Promise}                 which rejects on sync or process failure.
    */
   async maybeSync(expectedTimestamp, options = { loadDump: true }) {
-    const { loadDump } = options;
+    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();
 
       // If there is no data currently in the collection, attempt to import
@@ -394,17 +394,17 @@ class RemoteSettingsClient extends Event
       }
       throw e;
     } finally {
       // No error was reported, this is a success!
       if (reportStatus === null) {
         reportStatus = UptakeTelemetry.STATUS.SUCCESS;
       }
       // Report success/error status to Telemetry.
-      UptakeTelemetry.report(TELEMETRY_COMPONENT, reportStatus, { source: this.identifier });
+      UptakeTelemetry.report(TELEMETRY_COMPONENT, reportStatus, { source: this.identifier, trigger });
     }
   }
 
   /**
    * Fetch the signature info from the collection metadata and verifies that the
    * local set of records has the same.
    *
    * @param {Array<Object>} remoteRecords   The list of changes to apply to the local database.
--- a/services/settings/remote-settings.js
+++ b/services/settings/remote-settings.js
@@ -151,25 +151,29 @@ function remoteSettingsFunction() {
   /**
    * 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.
    * @returns {Promise} or throws error if something goes wrong.
    */
   remoteSettings.pollChanges = async ({ expectedTimestamp } = {}) => {
+    const trigger = expectedTimestamp ? "broadcast" : "timer";
+    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);
       const remainingMilliseconds = parseInt(backoffReleaseTime, 10) - Date.now();
       if (remainingMilliseconds > 0) {
         // Backoff time has not elapsed yet.
-        UptakeTelemetry.report(TELEMETRY_COMPONENT,
-                               UptakeTelemetry.STATUS.BACKOFF,
-                               { source: TELEMETRY_SOURCE });
+        UptakeTelemetry.report(TELEMETRY_COMPONENT, UptakeTelemetry.STATUS.BACKOFF, telemetryArgs);
         throw new Error(`Server is asking clients to back off; retry in ${Math.ceil(remainingMilliseconds / 1000)}s.`);
       } else {
         gPrefs.clearUserPref(PREF_SETTINGS_SERVER_BACKOFF);
       }
     }
 
     Services.obs.notifyObservers(null, "remote-settings:changes-poll-start", JSON.stringify({ expectedTimestamp }));
 
@@ -189,27 +193,27 @@ function remoteSettingsFunction() {
         reportStatus = UptakeTelemetry.STATUS.SERVER_ERROR;
       } else if (/Timeout/.test(e.message)) {
         reportStatus = UptakeTelemetry.STATUS.TIMEOUT_ERROR;
       } else if (/NetworkError/.test(e.message)) {
         reportStatus = UptakeTelemetry.STATUS.NETWORK_ERROR;
       } else {
         reportStatus = UptakeTelemetry.STATUS.UNKNOWN_ERROR;
       }
-      UptakeTelemetry.report(TELEMETRY_COMPONENT, reportStatus, { source: TELEMETRY_SOURCE });
+      UptakeTelemetry.report(TELEMETRY_COMPONENT, reportStatus, telemetryArgs);
       // No need to go further.
       throw new Error(`Polling for changes failed: ${e.message}.`);
     }
 
     const {serverTimeMillis, changes, currentEtag, backoffSeconds} = pollResult;
 
     // Report polling success to Uptake Telemetry.
     const reportStatus = changes.length === 0 ? UptakeTelemetry.STATUS.UP_TO_DATE
                                               : UptakeTelemetry.STATUS.SUCCESS;
-    UptakeTelemetry.report(TELEMETRY_COMPONENT, reportStatus, { source: TELEMETRY_SOURCE });
+    UptakeTelemetry.report(TELEMETRY_COMPONENT, reportStatus, telemetryArgs);
 
     // Check if the server asked the clients to back off (for next poll).
     if (backoffSeconds) {
       const backoffReleaseTime = Date.now() + backoffSeconds * 1000;
       gPrefs.setCharPref(PREF_SETTINGS_SERVER_BACKOFF, backoffReleaseTime);
     }
 
     // Record new update time and the difference between local and server time.
@@ -231,17 +235,17 @@ function remoteSettingsFunction() {
 
       const client = await _client(bucket, collection);
       if (!client) {
         continue;
       }
       // Start synchronization! It will be a no-op if the specified `lastModified` equals
       // the one in the local database.
       try {
-        await client.maybeSync(last_modified, { loadDump });
+        await client.maybeSync(last_modified, { loadDump, trigger });
 
         // Save last time this client was successfully synced.
         Services.prefs.setIntPref(client.lastCheckTimePref, checkedServerTimeInSeconds);
       } catch (e) {
         if (!firstError) {
           firstError = e;
           firstError.details = change;
         }
--- a/toolkit/components/telemetry/Events.yaml
+++ b/toolkit/components/telemetry/Events.yaml
@@ -1004,16 +1004,19 @@ uptake.remotecontent.result:
       - uptake
     objects:
       - 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")
     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
@@ -74,16 +74,28 @@ Example:
    } catch (e) {
      status = /NetworkError/.test(e) ?
                  UptakeTelemetry.STATUS.NETWORK_ERROR :
                  UptakeTelemetry.STATUS.SERVER_ERROR ;
    }
    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"``)
+
+.. code-block:: js
+
+   UptakeTelemetry.report(component, status, { source, trigger: "timer" });
+
+
 Use-cases
 ---------
 
 The following remote data sources are already using this unified histogram.
 
 * remote settings changes monitoring
 * add-ons blocklist
 * gfx blocklist