From f3d65462afd53d2ba3b1c99b54cb2e5a59eddf0f Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 7 Mar 2019 14:44:52 +0000 Subject: [PATCH] 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 --HG-- extra : moz-landing-system : lando --- services/common/uptake-telemetry.js | 1 + services/settings/RemoteSettingsClient.jsm | 4 ++-- services/settings/remote-settings.js | 16 ++++++++++------ toolkit/components/telemetry/Events.yaml | 3 +++ .../telemetry/docs/collection/uptake.rst | 12 ++++++++++++ 5 files changed, 28 insertions(+), 8 deletions(-) diff --git a/services/common/uptake-telemetry.js b/services/common/uptake-telemetry.js index 1e8ed3538373..7454143719de 100644 --- a/services/common/uptake-telemetry.js +++ b/services/common/uptake-telemetry.js @@ -87,6 +87,7 @@ class UptakeTelemetry { * @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; diff --git a/services/settings/RemoteSettingsClient.jsm b/services/settings/RemoteSettingsClient.jsm index f8eecf9c118c..1298fc6b9a31 100644 --- a/services/settings/RemoteSettingsClient.jsm +++ b/services/settings/RemoteSettingsClient.jsm @@ -282,7 +282,7 @@ class RemoteSettingsClient extends EventEmitter { * @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 { @@ -399,7 +399,7 @@ class RemoteSettingsClient extends EventEmitter { 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 }); } } diff --git a/services/settings/remote-settings.js b/services/settings/remote-settings.js index a4a69f8e7db7..83ac1962bfbd 100644 --- a/services/settings/remote-settings.js +++ b/services/settings/remote-settings.js @@ -156,15 +156,19 @@ function remoteSettingsFunction() { * @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); @@ -194,7 +198,7 @@ function remoteSettingsFunction() { } 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}.`); } @@ -204,7 +208,7 @@ function remoteSettingsFunction() { // 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) { @@ -236,7 +240,7 @@ function remoteSettingsFunction() { // 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); diff --git a/toolkit/components/telemetry/Events.yaml b/toolkit/components/telemetry/Events.yaml index 1a8c9d8381a9..6f9997218d63 100644 --- a/toolkit/components/telemetry/Events.yaml +++ b/toolkit/components/telemetry/Events.yaml @@ -1009,6 +1009,9 @@ uptake.remotecontent.result: 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"] diff --git a/toolkit/components/telemetry/docs/collection/uptake.rst b/toolkit/components/telemetry/docs/collection/uptake.rst index 62a660503283..b3c0a3e7ee65 100644 --- a/toolkit/components/telemetry/docs/collection/uptake.rst +++ b/toolkit/components/telemetry/docs/collection/uptake.rst @@ -79,6 +79,18 @@ 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"``) + +.. code-block:: js + + UptakeTelemetry.report(component, status, { source, trigger: "timer" }); + + Use-cases ---------