Closed Bug 1466881 Opened 6 years ago Closed 6 years ago

Missing open events for some users

Categories

(DevTools :: General, enhancement, P1)

Product:
DevTools
Component:
General
Type:
enhancement

Tracking

(firefox63 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 63
Tracking Flags:
Tracking Status
firefox63 --- fixed

People

(Reporter: Harald, Assigned: miker)

References

Details

Attachments

(1 file)

Bug 1466881 - Missing open events for some users
59 bytes, text/x-review-board-request
nchevobbe
: review+
Details 
In Event Telemetry we see users that only have close events and panel enter/exit events without any open events.

Are there cases where our tracking misses open events?
This may (or may not!) be related to the observed discrepancy between the DEVTOOLS_TOOLBOX_OPENED_COUNT histogram and the event telemetry in a small number of pings, per https://dbc-caf9527b-e073.cloud.databricks.com/#notebook/12865/command/12876.
Product: Firefox → DevTools
This is making the M1 event telemetry impractical to analyze. We should land this soon, validate and uplift to Beta.
Re-using that bug to track this, not sure if this is related with the original issue but it does look similar.

Reasonable balance of open and close events (though in the wrong order): https://analytics.amplitude.com/mozilla-corp/project/200527/search/55890618694

Missing open events: https://analytics.amplitude.com/mozilla-corp/project/200527/search/58855321152

So far the hypothesis is that some entry path for opening devtools does not send open events.
Flags: needinfo?(mratcliffe)
I have managed to reproduce an issue where the open event throws an exception... investigating.
Flags: needinfo?(mratcliffe)
STR:

1. Open Firefox.
2. Tools -> Web Developer -> Inspector.
3. Select the debugger tab and then close the toolbox.
4. Tools -> Web Developer -> Inspector.

After step 4 no open events are fired.
Status: NEW → ASSIGNED
The problem is that when the toolbox is first initialized using `devtools/startup/devtools-startup.js` and `devtools/startup/DevToolsShim.jsm`. Inside these files we set the entrypoint and shortcut properties.

The second time the toolbox is opened it is already initialized so we skip these files and the event is never sent.

We can probably move these events into devtools/client/framework/browser-menus.js::createToolMenuElements()... still investigating.
Attachment #8992465 - Flags: review?(nchevobbe)
Comment on attachment 8992465 [details]
Bug 1466881 - Missing open events for some users

https://reviewboard.mozilla.org/r/257326/#review266958

::: commit-message-72ed9:1
(Diff revision 8)
> +Bug 1466881 - Missing open events for some users r?

change to r=nchevobbe

::: devtools/client/debugger/new/test/mochitest/browser_dbg-browser-content-toolbox.js:53
(Diff revision 8)
>  
>    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");

why do we make this change ?

::: devtools/client/framework/devtools.js:456
(Diff revision 8)
>     * @return {Toolbox} toolbox
>     *        The toolbox that was opened
>     */
>    async showToolbox(target, toolId, hostType, hostOptions, startTime) {
>      let toolbox = this._toolboxes.get(target);
> +    const panelName = this.makeToolIdHumanReadable(toolId);

nit: could we declare this just before sending the event to telemetry (l.491) ? I'd say we could even do without declaring a variable for it since we only use it once.

::: devtools/client/framework/devtools.js:525
(Diff revision 8)
> +    if (!toolId) {
> +      return "application";

I don't follow here. Is application the new application panel being worked on ?
If so, why isn't there a toolId being passed in this case ?
Comment on attachment 8992465 [details]
Bug 1466881 - Missing open events for some users

https://reviewboard.mozilla.org/r/257326/#review266958

> I don't follow here. Is application the new application panel being worked on ?
> If so, why isn't there a toolId being passed in this case ?

It was a workaround just so we have a tool id but the new fix is better.

Basically, if we open the content toolbox without specifying a tool id we have no tool id but then we use toolbox.defaultToolId so I have changed the code to make this clearer and act more appropriately.
Comment on attachment 8992465 [details]
Bug 1466881 - Missing open events for some users

https://reviewboard.mozilla.org/r/257326/#review267022
Attachment #8992465 - Flags: review?(nchevobbe) → review+
Pushed by mratcliffe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fde63f5ba297
Missing open events for some users r=nchevobbe
https://hg.mozilla.org/mozilla-central/rev/fde63f5ba297
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: