Bug 1453312 - Record event for toolbox close r?yulia draft
authorMichael Ratcliffe <mratcliffe@mozilla.com>
Wed, 11 Apr 2018 14:18:44 +0100
changeset 780647 5e7d6784dc69a425b06edb35ee112ab12e537735
parent 780394 f6c3a0a19d82db25750d8badccd5cf37e79d028e
push id106062
push userbmo:mratcliffe@mozilla.com
push dateWed, 11 Apr 2018 18:32:33 +0000
reviewersyulia
bugs1453312
milestone61.0a1
Bug 1453312 - Record event for toolbox close r?yulia Changes: 1. The change in telemetry.js avoids an issue with overwriting pending properties. 2. The test in telemetry.md has been updated now that we have a working test. 3. Added the actual toolbox.close telemetry event and test. MozReview-Commit-ID: 9fOCkwCCwDx
devtools/client/framework/test/browser.ini
devtools/client/framework/test/browser_toolbox_telemetry_close.js
devtools/client/framework/toolbox.js
devtools/client/shared/telemetry.js
devtools/docs/frontend/telemetry.md
toolkit/components/telemetry/Events.yaml
--- a/devtools/client/framework/test/browser.ini
+++ b/devtools/client/framework/test/browser.ini
@@ -99,16 +99,17 @@ skip-if = e10s # Bug 1069044 - destroyIn
 [browser_toolbox_selectionchanged_event.js]
 [browser_toolbox_sidebar.js]
 [browser_toolbox_sidebar_events.js]
 [browser_toolbox_sidebar_existing_tabs.js]
 [browser_toolbox_sidebar_overflow_menu.js]
 [browser_toolbox_split_console.js]
 [browser_toolbox_target.js]
 [browser_toolbox_tabsswitch_shortcuts.js]
+[browser_toolbox_telemetry_close.js]
 [browser_toolbox_textbox_context_menu.js]
 [browser_toolbox_theme.js]
 [browser_toolbox_theme_registration.js]
 [browser_toolbox_toggle.js]
 [browser_toolbox_tool_ready.js]
 [browser_toolbox_tool_remote_reopen.js]
 [browser_toolbox_toolbar_overflow.js]
 [browser_toolbox_tools_per_toolbox_registration.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/framework/test/browser_toolbox_telemetry_close.js
@@ -0,0 +1,81 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const { Toolbox } = require("devtools/client/framework/toolbox");
+
+const URL = "data:text/html;charset=utf8,browser_toolbox_telemetry_close.js";
+const OPTOUT = Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTOUT;
+const { SIDE, BOTTOM } = Toolbox.HostType;
+const DATA = [
+  {
+    timestamp: null,
+    category: "devtools.main",
+    method: "close",
+    object: "tools",
+    value: null,
+    extra: {
+      host: "side",
+      width: "1440"
+    }
+  },
+  {
+    timestamp: null,
+    category: "devtools.main",
+    method: "close",
+    object: "tools",
+    value: null,
+    extra: {
+      host: "bottom",
+      width: "1440"
+    }
+  }
+];
+
+add_task(async function() {
+  // Let's reset the counts.
+  Services.telemetry.clearEvents();
+
+  // Ensure no events have been logged
+  const snapshot = Services.telemetry.snapshotEvents(OPTOUT, true);
+  ok(!snapshot.parent, "No events have been logged for the main process");
+
+  await openAndCloseToolbox("webconsole", SIDE);
+  await openAndCloseToolbox("webconsole", BOTTOM);
+
+  checkResults();
+});
+
+async function openAndCloseToolbox(toolId, host) {
+  const tab = await addTab(URL);
+  const target = TargetFactory.forTab(tab);
+  const toolbox = await gDevTools.showToolbox(target, toolId);
+
+  await toolbox.switchHost(host);
+  await toolbox.destroy();
+}
+
+function checkResults() {
+  const snapshot = Services.telemetry.snapshotEvents(OPTOUT, true);
+  const events = snapshot.parent.filter(event => event[1] === "devtools.main" &&
+                                                 event[2] === "close" &&
+                                                 event[3] === "tools" &&
+                                                 event[4] === null
+  );
+
+  for (let i in DATA) {
+    const [ timestamp, category, method, object, value, extra ] = events[i];
+    const expected = DATA[i];
+
+    // ignore timestamp
+    ok(timestamp > 0, "timestamp is greater than 0");
+    is(category, expected.category, "category is correct");
+    is(method, expected.method, "method is correct");
+    is(object, expected.object, "object is correct");
+    is(value, expected.value, "value is correct");
+
+    is(extra.host, expected.extra.host, "host is correct");
+    ok(extra.width > 0, "width is greater than 0");
+  }
+}
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -2751,16 +2751,20 @@ Toolbox.prototype = {
         button.teardown();
       }
     });
 
     // We need to grab a reference to win before this._host is destroyed.
     let win = this.win;
 
     this._telemetry.toolClosed("toolbox");
+    this._telemetry.recordEvent("devtools.main", "close", "tools", null, {
+      host: this._getTelemetryHostString(),
+      width: Math.ceil(win.outerWidth / 50) * 50
+    });
     this._telemetry.destroy();
 
     // Finish all outstanding tasks (which means finish destroying panels and
     // then destroying the host, successfully or not) before destroying the
     // target.
     deferred.resolve(settleAll(outstanding)
         .catch(console.error)
         .then(() => {
--- a/devtools/client/shared/telemetry.js
+++ b/devtools/client/shared/telemetry.js
@@ -480,19 +480,25 @@ class Telemetry {
    *        The pending property value
    */
   addEventProperty(category, method, object, value, pendingPropName, pendingPropValue) {
     const sig = `${category},${method},${object},${value}`;
 
     // If the pending event has not been created add the property to the pending
     // list.
     if (!PENDING_EVENTS.has(sig)) {
-      PENDING_EVENT_PROPERTIES.set(sig, {
-        [pendingPropName]: pendingPropValue
-      });
+      let props = PENDING_EVENT_PROPERTIES.get(sig);
+
+      if (props) {
+        props[pendingPropName] = pendingPropValue;
+      } else {
+        PENDING_EVENT_PROPERTIES.set(sig, {
+          [pendingPropName]: pendingPropValue
+        });
+      }
       return;
     }
 
     const { expected, extra } = PENDING_EVENTS.get(sig);
 
     if (expected.has(pendingPropName)) {
       extra[pendingPropName] = pendingPropValue;
 
--- a/devtools/docs/frontend/telemetry.md
+++ b/devtools/docs/frontend/telemetry.md
@@ -316,55 +316,97 @@ Warning: An attempt was made to write to
 
 So watch out for errors.
 
 #### Testing Event Telemetry
 
 This is best shown via an example:
 
 ```js
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const { Toolbox } = require("devtools/client/framework/toolbox");
+
+const URL = "data:text/html;charset=utf8,browser_toolbox_telemetry_close.js";
 const OPTOUT = Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTOUT;
-const expected = [
-  [null, "devtools.main", "open", "tools", null, {
-    entrypoint: "ContextMenu",
-    first_panel: "inspector",
-    host: "bottom",
-    splitconsole: "false",
-    width: "1440"
-  }],
-  [null, "devtools.main", "open", "tools", null, {
-    entrypoint: "KeyShortcut",
-    first_panel: "webconsole",
-    host: "bottom",
-    splitconsole: "false",
-    width: "1440"
-  }]
+const { SIDE, BOTTOM } = Toolbox.HostType;
+const DATA = [
+  {
+    timestamp: null,
+    category: "devtools.main",
+    method: "close",
+    object: "tools",
+    value: null,
+    extra: {
+      host: "side",
+      width: "1440"
+    }
+  },
+  {
+    timestamp: null,
+    category: "devtools.main",
+    method: "close",
+    object: "tools",
+    value: null,
+    extra: {
+      host: "bottom",
+      width: "1440"
+    }
+  }
 ];
 
-let snapshot = Services.telemetry.snapshotEvents(OPTOUT, true);
-is(snapshot.parent.length, 0, "No events have been logged");
+add_task(async function() {
+  // Let's reset the counts.
+  Services.telemetry.clearEvents();
 
-// Do something to trigger some event telemetry.
+  // Ensure no events have been logged
+  const snapshot = Services.telemetry.snapshotEvents(OPTOUT, true);
+  ok(!snapshot.parent, "No events have been logged for the main process");
 
-for (let i in snapshot.parent) {
-  let actual = snapshot.parent[i];
+  await openAndCloseToolbox("webconsole", SIDE);
+  await openAndCloseToolbox("webconsole", BOTTOM);
 
-  // ignore timestamp
-  is(actual.category, expected[i].category, "category is correct");
-  is(actual.method, expected[i].method, "method is correct");
-  is(actual.object, expected[i].object, "object is correct");
-  is(actual.value, expected[i].value, "value is correct");
+  checkResults();
+});
 
-  is(actual.extra.entrypoint, expected[i].extra.entrypoint, "entrypoint is correct");
-  is(actual.extra.first_panel, expected[i].extra.first_panel, "first_panel is correct");
-  is(actual.extra.host, expected[i].extra.host, "host is correct");
-  is(actual.extra.splitconsole, expected[i].extra.splitconsole, "splitconsole is correct");
-  is(actual.extra.width, expected[i].extra.width, "width is correct");
+async function openAndCloseToolbox(toolId, host) {
+  const tab = await addTab(URL);
+  const target = TargetFactory.forTab(tab);
+  const toolbox = await gDevTools.showToolbox(target, toolId);
+
+  await toolbox.switchHost(host);
+  await toolbox.destroy();
 }
 
+function checkResults() {
+  const snapshot = Services.telemetry.snapshotEvents(OPTOUT, true);
+  const events = snapshot.parent.filter(event => event[1] === "devtools.main" &&
+                                                 event[2] === "close" &&
+                                                 event[3] === "tools" &&
+                                                 event[4] === null
+  );
+
+  for (let i in DATA) {
+    const [ timestamp, category, method, object, value, extra ] = events[i];
+    const expected = DATA[i];
+
+    // ignore timestamp
+    ok(timestamp > 0, "timestamp is greater than 0");
+    is(category, expected.category, "category is correct");
+    is(method, expected.method, "method is correct");
+    is(object, expected.object, "object is correct");
+    is(value, expected.value, "value is correct");
+
+    is(extra.host, expected.extra.host, "host is correct");
+    ok(extra.width > 0, "width is greater than 0");
+  }
+}
 ```
 
 #### Compile it
 
 You need to do a full Firefox build if you have edited either `Histograms.json` or `Events.yaml`, as they are processed at build time, and various checks will be run on them to guarantee they are valid.
 
 ```bash
 ./mach build
--- a/toolkit/components/telemetry/Events.yaml
+++ b/toolkit/components/telemetry/Events.yaml
@@ -172,8 +172,19 @@ devtools.main:
     release_channel_collection: opt-out
     expiry_version: never
     extra_keys:
       entrypoint: How was the toolbox opened? CommandLine, ContextMenu, DeveloperToolbar, HamburgerMenu, KeyShortcut, SessionRestore or SystemMenu
       first_panel: The name of the first panel opened.
       host: "Toolbox host (positioning): bottom, side, window or other."
       splitconsole: Indicates whether the split console was open.
       width: Toolbox width rounded up to the nearest 50px.
+  close:
+    objects: ["tools"]
+    bug_numbers: [1453312]
+    notification_emails: ["dev-developer-tools@lists.mozilla.org", "hkirschner@mozilla.com"]
+    record_in_processes: ["main"]
+    description: User closes devtools toolbox.
+    release_channel_collection: opt-out
+    expiry_version: never
+    extra_keys:
+      host: "Toolbox host (positioning): bottom, side, window or other."
+      width: Toolbox width rounded up to the nearest 50px.