Closed Bug 1504173 Opened 2 years ago Closed 2 years ago

[remote-dbg-next] record usage metrics to telemetry

Categories

(DevTools :: about:debugging, enhancement, P1)

enhancement

Tracking

(firefox67 fixed)

RESOLVED FIXED
Firefox 67
Tracking Status
firefox67 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

(Blocks 3 open bugs)

Details

(Whiteboard: old-remote-debugging-ng-m3)

Attachments

(4 files, 3 obsolete files)

We should monitor usage of the new about:debugging with telemetry (regular or event telemetry).

Interactions to instrument:
- open about debugging
- click on inspect button
- open connect page
- enable USB from connect page
- disable USB from connect page
- add a network location
- remove a network location
- click on connect in sidebar
- select remote runtime in sidebar
- disable connection prompt
- load temporary extension

It would also be interesting to record how much time users spend on each page. Maybe differentiate between time spent while tab has focus and while tab is in background.

We could also monitor the usage of about:devtools-toolbox coming from about:debugging inspect-buttons.

This bug might be a meta.
filter on remote-debugging-next-move-m3-to-m2
filter on remote-debugging-next-move-m3-to-m2
filter on remote-debugging-next-move-m3-to-m2
No longer blocks: remote-debugging-ng-m3
Priority: -- → P2
Whiteboard: old-remote-debugging-ng-m3
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Priority: P2 → P1
https://docs.google.com/document/d/1uG61H2JR1QG_gYmqGabnCZInfDhn_591JLYWfnVP_54/edit#

Harald: I listed all the telemetry probes and events I could think about in the document above. Could you have a look and let me know what you think? Maybe we should review it with Tim as well?
Flags: needinfo?(hkirschner)

Did an initial pass. We can probably scope it down somewhat for a first implementation.

Flags: needinfo?(hkirschner)

Thanks for the feedback! Some thoughts on the current things needed to implement

Telemetry Session ID

Each about debugging needs to have its own session_id. When starting about:debugging we should generate the session ID and possibly instanciate a Telemetry instance (the instance is absolutely useless actually, this can completely be migrated to a static module)

We can either have a middleware that knows how to send record events from actions. We would probably need new actions then, at least for opening and closing about:debugging, which don't have associated actions at the moment

Or we can have the sessionId (and telemetry instance) available globally. Some options

  • set on the window object
  • in the ui state
  • in a shared module

In all cases the session id should be generated by aboutdebugging.js when the application is opened. Either send it in an action to pass it to the middleware. Or set it anywhere we globally decided to store this information.

Forward the Telemetry Session ID to debugging toolboxes

Since we have 3 ways to open toolboxes to inspect targets, we will need to forward the session id to the toolbox in 3 different ways.

Opening addon toolbox for this-firefox in separate process:

create@resource://devtools/client/framework/toolbox-host-manager.js:68:17
asynccreateToolbox@resource://devtools/client/framework/devtools.js:544:27
async
showToolbox@resource://devtools/client/framework/devtools.js:473:30
asyncopenToolbox@chrome://devtools/content/framework/toolbox-process-window.js:164:25
async
connect@chrome://devtools/content/framework/toolbox-process-window.js:93:11
async*@chrome://devtools/content/framework/toolbox-process-window.js:128:11
EventListener.handleEvent*@chrome://devtools/content/framework/toolbox-process-window.js:116:1

Opening tab toolbox in about:devtools-toolbox tab

create@resource://devtools/client/framework/toolbox-host-manager.js:68:17
asynccreateToolbox@resource://devtools/client/framework/devtools.js:544:27
async
showToolbox@resource://devtools/client/framework/devtools.js:473:30
async*@chrome://devtools/content/framework/toolbox-init.js:80:11
async*@chrome://devtools/content/framework/toolbox-init.js:47:10

Opening worker toolbox in new window same process:

create@resource://devtools/client/framework/toolbox-host-manager.js:68:17
asynccreateToolbox@resource://devtools/client/framework/devtools.js:544:27
async
showToolbox@resource://devtools/client/framework/devtools.js:473:30
asyncopenWorkerToolbox@resource://devtools/client/framework/devtools-browser.js:384:27
async
inspectDebugTarget/<@resource://devtools/client/aboutdebugging-new/src/actions/debug-targets.js:75:9

Opening extension toolbox for remote firefox in same process:

create@resource://devtools/client/framework/toolbox-host-manager.js:68:17
asynccreateToolbox@resource://devtools/client/framework/devtools.js:544:27
async
showToolbox@resource://devtools/client/framework/devtools.js:473:30
asyncexports.debugRemoteAddon@resource://devtools/client/aboutdebugging-new/src/modules/extensions-helper.js:70:30
async
inspectDebugTarget/<@resource://devtools/client/aboutdebugging-new/src/actions/debug-targets.js:65:17

Those stacks end in toolbox-host-manager::create() where we instanciate the toolbox

const msSinceProcessStart = parseInt(this.telemetry.msSinceProcessStart(), 10);
const toolbox = new Toolbox(this.target, toolId, this.host.type,
                            this.host.frame.contentWindow, this.frameId,
                            msSinceProcessStart);

msSinceProcessStart here is actually used as the telemetry session id.

The common part of all those stacks is devtools::showToolbox() -> devtools::createToolbox() -> toolbox-host-manager::create(), which means we need at least to be able to pass the "telemetry session id" down this chain.
Additionally we will need to modify:

  • devtools-browser::openWorkerToolbox() to cover worker toolboxes
  • extensions-helper::debugRemoteAddon() to cover remote addon toolboxes

As well as adding a new URL param passed to about:devtools-toolbox, that will be processed in toolbox-init.js. And adding a new environment variable (similar to MOZ_BROWSER_TOOLBOX_ADDONID) that will be read in toolbox-process-window.js

Before I ask for data review, I would like to check that the events make sense for us

Harald, can you have a look at the events defined in https://phabricator.services.mozilla.com/D16330 . I reused the open and close events, and defined two new events in this first iteration. More will come to cover the rest of the document.

Thanks.

Flags: needinfo?(hkirschner)
Depends on D16331
Attached file data-review.txt (obsolete) —

Can you take a look Harald? I'd like to know if you are ok with reusing open & close here or if you prefer dedicated events. Thanks.

Flags: needinfo?(hkirschner)
Attachment #9037923 - Flags: feedback?(hkirschner)
Blocks: 1521497
Blocks: 1521507
Blocks: 1521511
Blocks: 1521514
Attachment #9036996 - Attachment is obsolete: true
Attachment #9036997 - Attachment is obsolete: true

Reusing open/close events is probably not the best choice, as the analysis will be a lot harder. Sending more kinds of events is not a bad practice and is already done for other non-toolbox "modes", like RDM and split console (active/deactivate). We probably want to do the same here; especially as toolbox and about:debugging usage patterns are very different user flows.

If we feel that the events are too similar we can merge later; but first we should gravitate towards clarity in the data.

Ok thanks! I will try to find other names, but with open/close/start/stop/activate/deactivate already taken and the limitation on number of characters for the names, it is getting difficult. I might end up just namespacing with something cryptic like adbg_open/adbg_close ....

Attached file data-review-v2.txt

Here is an updated version with dedicated events for aboutdebugging open/close (named open_adbg and close_adbg)

Attachment #9037923 - Attachment is obsolete: true
Attachment #9037923 - Flags: feedback?(hkirschner)
Attachment #9038550 - Flags: review?(hkirschner)
Attachment #9038550 - Flags: review?(chutten)
Attachment #9035959 - Attachment description: Bug 1504173 - Record open, close and page_select events in new about:debugging;r=ladybenko,digitarald → Bug 1504173 - Record open, close and page_select events in new about:debugging;r=ladybenko,datareview=chutten
Comment on attachment 9038550 [details]
data-review-v2.txt

DATA COLLECTION REVIEW RESPONSE:

    Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes. These collections are Telemetry so are documented in their definitions file ([Events.yaml](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/Events.yaml)), the [Probe Dictionary](https://telemetry.mozilla.org/probe-dictionary/), and on telemetry.mozilla.org's [Measurement Dashboards](https://telemetry.mozilla.org/new-pipeline/dist.html).

    Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. These collections are Telemetry so can be controlled through Firefox's Preferences.

    If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes. :digitarald is responsible.

    Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 2, Interaction.

    Is the data collection request for default-on or default-off?

Default on for all channels.

    Does the instrumentation include the addition of any new identifiers?

No.

    Is the data collection covered by the existing Firefox privacy notice?

Yes.

    Does there need to be a check-in in the future to determine whether to renew the data?

No. These collections are permanent.

---
Result: datareview+
Attachment #9038550 - Flags: review?(chutten) → review+
Comment on attachment 9038550 [details]
data-review-v2.txt

nit: devtools.main::inspect is a little too generic for devtools, as many panels have inspect actions. Maybe this could be namespaced (later).
Attachment #9038550 - Flags: review?(hkirschner) → review+
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3f15c4c80604
Reorder devtools events alphabetically in Events.yaml;r=miker
https://hg.mozilla.org/integration/autoland/rev/81f2225b7b01
Record open, close and page_select events in new about:debugging;r=ladybenko,janerik,datareview=chutten
https://hg.mozilla.org/integration/autoland/rev/8e29660cccf7
Test aboutdebugging telemetry events for open, close, select_page and inspect;r=ladybenko

This is permafailing on -sw (serviceworker refactor builds) since before this bug landed:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=5f877600f76fd123d08f288412ac187666a04b6a&selectedJob=224575641

and is already tracked by https://bugzilla.mozilla.org/show_bug.cgi?id=1523454.

Can we keep this bug as closed?

Flags: needinfo?(jdescottes) → needinfo?(nerli)
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Depends on: 1523454
Flags: needinfo?(nerli)
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
You need to log in before you can comment on or make changes to this bug.