Bug 1466881 - Missing open events for some users r?nchevobbe draft
authorMichael Ratcliffe <mratcliffe@mozilla.com>
Mon, 16 Jul 2018 21:16:50 +0100
changeset 823554 e87d65dd31880ac4adf53472f88d25c09640cc76
parent 823408 4b2ff11aa39d7bc0c2b5c176e5377890d480479c
push id117718
push usermratcliffe@mozilla.com
push dateFri, 27 Jul 2018 15:42:44 +0000
reviewersnchevobbe
bugs1466881
milestone63.0a1
Bug 1466881 - Missing open events for some users r?nchevobbe Changes: - All files - Generally shuffled around telemetry events to fix issues with properties not been sent or being sent in the wrong order. - devtools/client/framework/browser-menus.js - Added telemetry open event to missed codepath. - devtools/startup/DevToolsShim.jsm - Added missing telemetry "shortcut" property. - devtools/startup/devtools-startup.js - Added comment to sendEntryPointTelemetry(). MozReview-Commit-ID: ELldeyF32rG
devtools/client/debugger/new/test/mochitest/browser_dbg-browser-content-toolbox.js
devtools/client/framework/browser-menus.js
devtools/client/framework/devtools.js
devtools/client/framework/toolbox.js
devtools/startup/DevToolsShim.jsm
devtools/startup/devtools-startup.js
--- a/devtools/client/debugger/new/test/mochitest/browser_dbg-browser-content-toolbox.js
+++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-browser-content-toolbox.js
@@ -44,18 +44,18 @@ add_task(async function() {
   clearDebuggerPreferences();
 
   info("Open a tab pointing to doc-scripts.html");
   await addTab(EXAMPLE_URL + "doc-scripts.html");
 
   info("Open the Browser Content Toolbox");
   let toolbox = await gDevToolsBrowser.openContentProcessToolbox(gBrowser);
 
-  info("Wait for the debugger to be ready");
-  await toolbox.getPanelWhenReady("jsdebugger");
+  info("Select the debugger");
+  await toolbox.selectTool("jsdebugger");
 
   let dbg = createDebuggerContext(toolbox);
   ok(dbg, "Debugger context is available");
 
   info("Create a breakpoint");
   await selectSource(dbg, "simple2");
   await addBreakpoint(dbg, "simple2", 3);
 
--- a/devtools/client/framework/browser-menus.js
+++ b/devtools/client/framework/browser-menus.js
@@ -13,16 +13,19 @@
  */
 
 const {Cu} = require("chrome");
 const {LocalizationHelper} = require("devtools/shared/l10n");
 const MENUS_L10N = new LocalizationHelper("devtools/client/locales/menus.properties");
 
 loader.lazyRequireGetter(this, "gDevTools", "devtools/client/framework/devtools", true);
 loader.lazyRequireGetter(this, "gDevToolsBrowser", "devtools/client/framework/devtools-browser", true);
+loader.lazyRequireGetter(this, "Telemetry", "devtools/client/shared/telemetry");
+
+let telemetry = null;
 
 // Keep list of inserted DOM Elements in order to remove them on unload
 // Maps browser xul document => list of DOM Elements
 const FragmentsCache = new Map();
 
 function l10n(key) {
   return MENUS_L10N.getStr(key);
 }
@@ -73,16 +76,17 @@ function createToolMenuElements(toolDefi
   // Prevent multiple entries for the same tool.
   if (doc.getElementById(menuId)) {
     return;
   }
 
   const oncommand = function(id, event) {
     const window = event.target.ownerDocument.defaultView;
     gDevToolsBrowser.selectToolCommand(window.gBrowser, id, Cu.now());
+    sendEntryPointTelemetry();
   }.bind(null, id);
 
   const menuitem = createMenuItem({
     doc,
     id: "menuitem_" + id,
     label: toolDefinition.menuLabel || toolDefinition.label,
     accesskey: toolDefinition.accesskey
   });
@@ -92,16 +96,36 @@ function createToolMenuElements(toolDefi
   menuitem.addEventListener("command", oncommand);
 
   return {
     menuitem
   };
 }
 
 /**
+ * Send entry point telemetry explaining how the devtools were launched when
+ * launched from the System Menu.. This functionality also lives inside
+ * `devtools/startup/devtools-startup.js` but that codepath is only used the
+ * first time a toolbox is opened for a tab.
+ */
+function sendEntryPointTelemetry() {
+  if (!telemetry) {
+    telemetry = new Telemetry();
+  }
+
+  telemetry.addEventProperty(
+    "devtools.main", "open", "tools", null, "shortcut", ""
+  );
+
+  telemetry.addEventProperty(
+    "devtools.main", "open", "tools", null, "entrypoint", "SystemMenu"
+  );
+}
+
+/**
  * Create xul menuitem, key elements for a given tool.
  * And then insert them into browser DOM.
  *
  * @param {XULDocument} doc
  *        The document to which the tool is to be registered.
  * @param {Object} toolDefinition
  *        Tool definition of the tool to register.
  * @param {Object} prevDef
--- a/devtools/client/framework/devtools.js
+++ b/devtools/client/framework/devtools.js
@@ -448,16 +448,17 @@ DevTools.prototype = {
    *        Optional, indicates the time at which the user event related to this toolbox
    *        opening started. This is a `Cu.now()` timing.
    *
    * @return {Toolbox} toolbox
    *        The toolbox that was opened
    */
   async showToolbox(target, toolId, hostType, hostOptions, startTime) {
     let toolbox = this._toolboxes.get(target);
+
     if (toolbox) {
       if (hostType != null && toolbox.hostType != hostType) {
         await toolbox.switchHost(hostType);
       }
 
       if (toolId != null && toolbox.currentToolId != toolId) {
         await toolbox.selectTool(toolId, "toolbox_show");
       }
@@ -472,51 +473,57 @@ DevTools.prototype = {
         return promise;
       }
       const toolboxPromise = this.createToolbox(target, toolId, hostType, hostOptions);
       this._creatingToolboxes.set(target, toolboxPromise);
       toolbox = await toolboxPromise;
       this._creatingToolboxes.delete(target);
 
       if (startTime) {
-        this.logToolboxOpenTime(toolbox.currentToolId, startTime);
+        this.logToolboxOpenTime(toolbox, startTime);
       }
       this._firstShowToolbox = false;
     }
 
+    // We send the "enter" width here to ensure it is always sent *after*
+    // the "open" event.
     const width = Math.ceil(toolbox.win.outerWidth / 50) * 50;
+    const panelName = this.makeToolIdHumanReadable(toolId || toolbox.defaultToolId);
     this._telemetry.addEventProperty(
-      "devtools.main", "open", "tools", null, "width", width);
+      "devtools.main", "enter", panelName, null, "width", width
+    );
 
     return toolbox;
   },
 
   /**
    * Log telemetry related to toolbox opening.
    * Two distinct probes are logged. One for cold startup, when we open the very first
    * toolbox. This one includes devtools framework loading. And a second one for all
    * subsequent toolbox opening, which should all be faster.
    * These two probes are indexed by Tool ID.
    *
-   * @param {String} toolId
-   *        The id of the opened tool.
+   * @param {String} toolbox
+   *        Toolbox instance.
    * @param {Number} startTime
    *        Indicates the time at which the user event related to the toolbox
    *        opening started. This is a `Cu.now()` timing.
    */
-  logToolboxOpenTime(toolId, startTime) {
+  logToolboxOpenTime(toolbox, startTime) {
+    const toolId = toolbox.currentToolId || toolbox.defaultToolId;
     const delay = Cu.now() - startTime;
+    const panelName = this.makeToolIdHumanReadable(toolId);
 
     const telemetryKey = this._firstShowToolbox ?
       "DEVTOOLS_COLD_TOOLBOX_OPEN_DELAY_MS" : "DEVTOOLS_WARM_TOOLBOX_OPEN_DELAY_MS";
     this._telemetry.getKeyedHistogramById(telemetryKey).add(toolId, delay);
 
     this._telemetry.addEventProperty(
-      "devtools.main", "open", "tools", null, "first_panel",
-      this.makeToolIdHumanReadable(toolId));
+      "devtools.main", "open", "tools", null, "first_panel", panelName
+    );
   },
 
   makeToolIdHumanReadable(toolId) {
     if (/^[0-9a-fA-F]{40}_temporary-addon/.test(toolId)) {
       return "temporary-addon";
     }
 
     let matches = toolId.match(
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -239,16 +239,20 @@ Toolbox.prototype = {
     return this._currentToolId;
   },
 
   set currentToolId(id) {
     this._currentToolId = id;
     this.component.setCurrentToolId(id);
   },
 
+  get defaultToolId() {
+    return this._defaultToolId;
+  },
+
   get panelDefinitions() {
     return this._panelDefinitions;
   },
 
   set panelDefinitions(definitions) {
     this._panelDefinitions = definitions;
     this._combineAndSortPanelDefinitions();
   },
@@ -1896,62 +1900,64 @@ Toolbox.prototype = {
     });
   },
 
   _pingTelemetrySelectTool(id, reason) {
     const width = Math.ceil(this.win.outerWidth / 50) * 50;
     const panelName = this.getTelemetryPanelNameOrOther(id);
     const prevPanelName = this.getTelemetryPanelNameOrOther(this.currentToolId);
     const cold = !this.getPanel(id);
-
-    this.telemetry.addEventProperties("devtools.main", "enter", panelName, null, {
-      "host": this._hostType,
-      "width": width,
-      "start_state": reason,
-      "panel_name": panelName,
-      "cold": cold,
-      // "session_id" is included at the end of this method.
-    });
+    const pending = ["host", "width", "start_state", "panel_name", "cold", "session_id"];
 
     // On first load this.currentToolId === undefined so we need to skip sending
     // a devtools.main.exit telemetry event.
     if (this.currentToolId) {
       this.telemetry.recordEvent("devtools.main", "exit", prevPanelName, null, {
         "host": this._hostType,
         "width": width,
         "panel_name": prevPanelName,
         "next_panel": panelName,
         "reason": reason,
         "session_id": this.sessionId
       });
     }
 
-    const pending = ["host", "width", "start_state", "panel_name", "cold", "session_id"];
+    this.telemetry.addEventProperties("devtools.main", "open", "tools", null, {
+      "width": width,
+      "session_id": this.sessionId
+    });
+
     if (id === "webconsole") {
       pending.push("message_count");
     }
 
     this.telemetry.preparePendingEvent("devtools.main", "enter", panelName, null, pending);
 
+    this.telemetry.addEventProperties("devtools.main", "enter", panelName, null, {
+      "host": this._hostType,
+      "start_state": reason,
+      "panel_name": panelName,
+      "cold": cold,
+      "session_id": this.sessionId
+    });
+
+    if (reason !== "initial_panel") {
+      const width = Math.ceil(this.win.outerWidth / 50) * 50;
+      this.telemetry.addEventProperty(
+        "devtools.main", "enter", panelName, null, "width", width
+      );
+    }
+
     // Cold webconsole event message_count is handled in
     // devtools/client/webconsole/webconsole-output-wrapper.js
     if (!cold && id === "webconsole") {
       this.telemetry.addEventProperty(
         "devtools.main", "enter", "webconsole", null, "message_count", 0);
     }
 
-    this.telemetry.addEventProperty(
-      "devtools.main", "open", "tools", null, "session_id", this.sessionId
-    );
-    // We send the "enter" session ID here to ensure it is always sent *after*
-    // the "open" session ID.
-    this.telemetry.addEventProperty(
-      "devtools.main", "enter", panelName, null, "session_id", this.sessionId
-    );
-
     this.telemetry.toolOpened(id);
   },
 
   /**
    * Focus a tool's panel by id
    * @param  {string} id
    *         The id of tool to focus
    */
@@ -2958,29 +2964,29 @@ Toolbox.prototype = {
 
     // We need to grab a reference to win before this._host is destroyed.
     const win = this.win;
     const host = this._getTelemetryHostString();
     const width = Math.ceil(win.outerWidth / 50) * 50;
     const prevPanelName = this.getTelemetryPanelNameOrOther(this.currentToolId);
 
     this.telemetry.toolClosed("toolbox");
-    this.telemetry.recordEvent("devtools.main", "close", "tools", null, {
-      "host": host,
-      "width": width,
-      "session_id": this.sessionId
-    });
     this.telemetry.recordEvent("devtools.main", "exit", prevPanelName, null, {
       "host": host,
       "width": width,
       "panel_name": this.getTelemetryPanelNameOrOther(this.currentToolId),
       "next_panel": "none",
       "reason": "toolbox_close",
       "session_id": this.sessionId
     });
+    this.telemetry.recordEvent("devtools.main", "close", "tools", null, {
+      "host": host,
+      "width": width,
+      "session_id": this.sessionId
+    });
 
     // Finish all outstanding tasks (which means finish destroying panels and
     // then destroying the host, successfully or not) before destroying the
     // target.
     this._destroyer = new Promise(resolve => {
       resolve(settleAll(outstanding)
         .catch(console.error)
         .then(() => {
--- a/devtools/startup/DevToolsShim.jsm
+++ b/devtools/startup/DevToolsShim.jsm
@@ -259,16 +259,19 @@ this.DevToolsShim = {
    */
   initDevTools: function(reason) {
     if (!this.isEnabled()) {
       throw new Error("DevTools are not enabled and can not be initialized.");
     }
 
     if (reason) {
       this.telemetry.addEventProperty(
+        "devtools.main", "open", "tools", null, "shortcut", ""
+      );
+      this.telemetry.addEventProperty(
         "devtools.main", "open", "tools", null, "entrypoint", reason
       );
     }
 
     if (!this.isInitialized()) {
       DevtoolsStartup.initDevTools(reason);
     }
   },
--- a/devtools/startup/devtools-startup.js
+++ b/devtools/startup/devtools-startup.js
@@ -836,16 +836,28 @@ DevToolsStartup.prototype = {
       dump("Unable to start debugger server on " + portOrPath + ": " + e);
     }
 
     if (cmdLine.state == Ci.nsICommandLine.STATE_REMOTE_AUTO) {
       cmdLine.preventDefault = true;
     }
   },
 
+  /**
+   * Send entry point telemetry explaining how the devtools were launched. This
+   * functionality also lives inside `devtools/client/framework/browser-menus.js`
+   * because this codepath is only used the first time a toolbox is opened for a
+   * tab.
+   *
+   * @param {String} reason
+   *        One of "KeyShortcut", "SystemMenu", "HamburgerMenu", "ContextMenu",
+   *        "CommandLine".
+   * @param {String} key
+   *        The key used by a key shortcut.
+   */
   sendEntryPointTelemetry(reason, key = "") {
     if (!reason) {
       return;
     }
 
     let keys = "";
 
     if (reason === "KeyShortcut") {