Bug 1539101 - Force extra values to be strings in Uptake Telemetry r=glasserc

Force extra values to be strings in Uptake Telemetry

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

--HG--
extra : moz-landing-system : lando

services/common/tests/unit/test_uptake_telemetry.js

@@ -1,5 +1,6 @@
 const { ClientID } = ChromeUtils.import("resource://gre/modules/ClientID.jsm");
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
+const { TelemetryTestUtils } = ChromeUtils.import("resource://testing-common/TelemetryTestUtils.jsm");
 const { UptakeTelemetry } = ChromeUtils.import("resource://services-common/uptake-telemetry.js");
 
 const COMPONENT = "remotesettings";
@@ -44,6 +45,16 @@ add_task(async function test_unknown_status_is_not_reported() {
   checkUptakeTelemetry(startHistogram, endHistogram, expectedIncrements);
 });
 
+add_task(async function test_age_is_converted_to_string_and_reported() {
+  const status = UptakeTelemetry.STATUS.SUCCESS;
+  const age = 42;
+
+  await UptakeTelemetry.report(COMPONENT, status, { source: "s", age });
+
+  TelemetryTestUtils.assertEvents(
+    [["uptake.remotecontent.result", "uptake", COMPONENT, status, { source: "s", age: `${age}` }]]);
+});
+
 add_task(async function test_each_status_can_be_caught_in_snapshot() {
   const source = "some-source";
   const startHistogram = getUptakeTelemetrySnapshot(source);

services/common/uptake-telemetry.js

@@ -161,8 +161,13 @@ class UptakeTelemetry {
     const channel = Policy.getChannel();
     const shouldSendEvent = !["release", "esr"].includes(channel) || hash < gSampleRate;
     if (shouldSendEvent) {
+      // The Event API requires `extra` values to be of type string. Force it!
+      const extraStr = Object.keys(extra).reduce(( acc, k ) => {
+        acc[k] = extra[k].toString();
+        return acc;
+      }, {});
       Services.telemetry
-        .recordEvent(TELEMETRY_EVENTS_ID, "uptake", component, status, extra);
+        .recordEvent(TELEMETRY_EVENTS_ID, "uptake", component, status, extraStr);
     }
 
     // Report via histogram in main ping.

services/settings/RemoteSettingsClient.jsm

@@ -282,8 +282,8 @@ class RemoteSettingsClient extends EventEmitter {
    * @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, trigger: "manual" }) {
-    const { loadDump, trigger } = options;
+  async maybeSync(expectedTimestamp, options = {}) {
+    const { loadDump = true, trigger = "manual" } = options;
 
     let reportStatus = null;
     try {