Bug 1639781 - record a new 'streamID' guid in sync-tab telemetry. r=rfkelly

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

services/fxaccounts/FxAccountsCommands.js

@@ -223,21 +223,24 @@ class SendTab {
     const flowID = this._fxai.telemetry.generateFlowID();
     const encoder = new TextEncoder("utf8");
     const data = { entries: [{ title: tab.title, url: tab.url }] };
-    const bytes = encoder.encode(JSON.stringify(data));
     const report = {
       succeeded: [],
       failed: [],
     };
     for (let device of to) {
       try {
+        const streamID = this._fxai.telemetry.generateFlowID();
+        const targetData = Object.assign({ flowID, streamID }, data);
+        const bytes = encoder.encode(JSON.stringify(targetData));
         const encrypted = await this._encrypt(bytes, device);
+        // TODO: remove flowID from the payload.
         const payload = { encrypted, flowID };
         await this._commands.invoke(COMMAND_SENDTAB, device, payload); // FxA needs an object.
         this._fxai.telemetry.recordEvent(
           "command-sent",
           COMMAND_SENDTAB_TAIL,
           this._fxai.telemetry.sanitizeDeviceId(device.id),
-          { flowID }
+          { flowID, streamID }
         );
         report.succeeded.push(device);
       } catch (error) {
@@ -261,19 +264,23 @@ class SendTab {
   }
 
   // Handle incoming send tab payload, called by FxAccountsCommands.
-  async handle(senderID, { encrypted, flowID }) {
+  async handle(senderID, { encrypted, flowID: deprecatedFlowID }) {
     const bytes = await this._decrypt(encrypted);
     const decoder = new TextDecoder("utf8");
     const data = JSON.parse(decoder.decode(bytes));
+    const { flowID, streamID, entries } = data;
     const current = data.hasOwnProperty("current")
       ? data.current
-      : data.entries.length - 1;
-    const { title, url: uri } = data.entries[current];
+      : entries.length - 1;
+    const { title, url: uri } = entries[current];
+    // `flowID` and `streamID` are in the top-level of the JSON, `entries` is
+    // an array of "tabs" with `current` being what index is the one we care
+    // about, or the last one if not specified.
     this._fxai.telemetry.recordEvent(
       "command-received",
       COMMAND_SENDTAB_TAIL,
       this._fxai.telemetry.sanitizeDeviceId(senderID),
-      { flowID }
+      { flowID: flowID || deprecatedFlowID, streamID }
     );
 
     return {

services/fxaccounts/tests/xpcshell/test_commands.js

@@ -86,6 +86,10 @@ add_task(async function test_sendtab_send() {
     { name: "Device 2" },
     { id: "dev3", name: "Device 3" },
   ];
+  // although we are sending to 3 devices, only 1 is successful - so there's
+  // only 1 streamID we care about. However, we've created IDs even for the
+  // failing items - so it's "4"
+  const expectedTelemetryStreamID = "4";
   const tab = { title: "Foo", url: "https://foo.bar/" };
   const report = await sendTab.send(to, tab);
   Assert.equal(report.succeeded.length, 1);
@@ -101,7 +105,7 @@ add_task(async function test_sendtab_send() {
       object: "command-sent",
       method: COMMAND_SENDTAB_TAIL,
       value: "dev3-san",
-      extra: { flowID: "1" },
+      extra: { flowID: "1", streamID: expectedTelemetryStreamID },
     },
   ]);
 });
@@ -182,6 +186,10 @@ add_task(async function test_sendtab_receive() {
 
   for (let { cmd, device, payload } of commands._invokes) {
     Assert.equal(cmd, COMMAND_SENDTAB);
+    // test we do the right thing with the "duplicated" flow ID.
+    Assert.equal(payload.flowID, "1");
+    // change it - ensure we still get what we expect in telemetry later.
+    payload.flowID = "ignore-me";
     Assert.deepEqual(await sendTab.handle(device.id, payload), {
       title: "tab title",
       uri: "http://example.com",
@@ -193,13 +201,40 @@ add_task(async function test_sendtab_receive() {
       object: "command-sent",
       method: COMMAND_SENDTAB_TAIL,
       value: "devid-san",
-      extra: { flowID: "1" },
+      extra: { flowID: "1", streamID: "2" },
     },
     {
       object: "command-received",
       method: COMMAND_SENDTAB_TAIL,
       value: "devid-san",
-      extra: { flowID: "1" },
+      extra: { flowID: "1", streamID: "2" },
+    },
+  ]);
+});
+
+// Test that a client which only sends the flowID in the envelope and not in the
+// encrypted body still gets recorded correctly.
+add_task(async function test_sendtab_receive_old_client() {
+  const fxai = FxaInternalMock();
+  const sendTab = new SendTab(null, fxai);
+  sendTab._decrypt = bytes => {
+    return bytes;
+  };
+  const data = { entries: [{ title: "title", url: "url" }] };
+  // No 'flowID' in the encrypted payload, no 'streamID' anywhere.
+  const payload = {
+    flowID: "flow-id",
+    encrypted: new TextEncoder("utf8").encode(JSON.stringify(data)),
+  };
+  await sendTab.handle("sender-id", payload);
+  Assert.deepEqual(fxai.telemetry._events, [
+    {
+      object: "command-received",
+      method: COMMAND_SENDTAB_TAIL,
+      value: "sender-id-san",
+      // deepEqual doesn't ignore undefined, but our telemetry code and
+      // JSON.stringify() do...
+      extra: { flowID: "flow-id", streamID: undefined },
     },
   ]);
 });

toolkit/components/telemetry/docs/data/sync-ping.rst

@@ -272,7 +272,11 @@ client, or opening a new URL.
 - extra: An object with the following attributes:
 
   - deviceID: A GUID which identifies the device the command is being sent to.
-  - flowID: A GUID which uniquely identifies this command invocation.
+  - flowID: A GUID which uniquely identifies this command invocation. This GUID
+            is the same for every device the tab is sent to.
+  - streamID: A GUID which uniquely identifies this command invocation's
+              specific target. This GUID is unique for every device the tab is
+              sent to (new in Firefox 79).
   - serverTime: (optional) Most recent server timestamp, as described above.
 
 processcommand
@@ -288,6 +292,9 @@ client. This is logically the "other end" of ``sendcommand``.
   - flowID: A GUID which uniquely identifies this command invocation. The value
             for this GUID will be the same as the flowID sent to the client via
             ``sendcommand``.
+  - streamID: A GUID which uniquely identifies this command invocation's
+              specific target. The value for this GUID will be the same as the
+              streamID sent to the client via ``sendcommand`` (new in Firefox 79).
   - serverTime: (optional) Most recent server timestamp, as described above.
 
 The ``migrations`` Array