Add telemetry probe to addon ui shim to track entry points and addon installations

RESOLVED FIXED in Firefox 56

Status

()

Firefox
Developer Tools
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: ochameau, Assigned: ochameau)

Tracking

unspecified
Firefox 56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

5 months ago
For now, we never tracked devtools entry points.
We don't know from where users get into devtools:
* menus
* key shotcuts
* right click, inspect element
* command line
* ...

Also we should track add-on installation to ensure we don't loose developers and also see how many people are mislead into devtools shortcuts.
Comment hidden (mozreview-request)
(Assignee)

Comment 2

4 months ago
Morphing this bug to only track entry point and ignore addon installation for now.
Assignee: nobody → poirot.alex
Depends on: 1359855
No longer depends on: 1361080
Comment hidden (mozreview-request)
(Assignee)

Comment 4

4 months ago
Note that for testing telemetry patches, artifacts builds won't work. You have to build everything...

Comment 5

4 months ago
mozreview-review
Comment on attachment 8884011 [details]
Bug 1378863 - Add telemetry probe to track DevTools entry points.  datareview=francois

https://reviewboard.mozilla.org/r/154964/#review165882

The code seems fine, but according to the documentation (https://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/telemetry/collection/histograms.html#changing-a-histogram) changing an existing histogram should not be done lightly. I think it would be valuable to identify some entry points from the DevToolsShim that would be nice to log too (thinking about the context menu's inspect element entry mostly). 

This means we need a small refactor of the DevToolsShim in order to reuse the DevToolsStartup's init code. What do you think?

::: toolkit/components/telemetry/Histograms.json:9317
(Diff revision 2)
> +  "DEVTOOLS_ENTRY_POINT": {
> +    "record_in_processes": ["main"],
> +    "bug_numbers": [1378863],
> +    "alert_emails": ["dev-developer-tools@lists.mozilla.org"],
> +    "expires_in_version": "60",
> +    "kind": "enumerated",

What is the reason behind using enumerated vs categorical (https://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/telemetry/collection/histograms.html#categorical) ?

::: toolkit/components/telemetry/Histograms.json:9320
(Diff revision 2)
> +    "alert_emails": ["dev-developer-tools@lists.mozilla.org"],
> +    "expires_in_version": "60",
> +    "kind": "enumerated",
> +    "n_values": 5,
> +    "releaseChannelCollection": "opt-out",
> +    "description": "Records how the user is trigerring Developer Tools installation. (0: Unknown, 1: Key shortcut, 2: System menu, 3: Hamburger menu, 4: Command line argument)"

nit: trigerring -> triggering
Attachment #8884011 - Flags: review?(jdescottes)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 8

4 months ago
(In reply to Julian Descottes [:jdescottes] from comment #5)
> Comment on attachment 8884011 [details]
> Bug 1378863 - Add telemetry probe to track DevTools entry points.
> 
> https://reviewboard.mozilla.org/r/154964/#review165882
> 
> The code seems fine, but according to the documentation
> (https://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/
> telemetry/collection/histograms.html#changing-a-histogram) changing an
> existing histogram should not be done lightly. I think it would be valuable
> to identify some entry points from the DevToolsShim that would be nice to
> log too (thinking about the context menu's inspect element entry mostly).

I added ContextMenu as a potential value.
There is some other possibilities, but I'm not sure they are worth tracking:
* scratchpad session restore
* anything that access DevToolsShim.gDevTools, in today's mozilla-central it seems to be only inspectNode and SDK/WebExt methods?
 
> This means we need a small refactor of the DevToolsShim in order to reuse
> the DevToolsStartup's init code. What do you think?

We can also manually call telemetry from there is that's any easier.

> ::: toolkit/components/telemetry/Histograms.json:9317
> (Diff revision 2)
> > +  "DEVTOOLS_ENTRY_POINT": {
> > +    "record_in_processes": ["main"],
> > +    "bug_numbers": [1378863],
> > +    "alert_emails": ["dev-developer-tools@lists.mozilla.org"],
> > +    "expires_in_version": "60",
> > +    "kind": "enumerated",
> 
> What is the reason behind using enumerated vs categorical
> (https://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/
> telemetry/collection/histograms.html#categorical) ?

I wasn't aware of the existence of categorical, I based my patch on an existing probe which was using enumerated...
I switched to categorical.
(Assignee)

Comment 9

4 months ago
Comment on attachment 8884011 [details]
Bug 1378863 - Add telemetry probe to track DevTools entry points.  datareview=francois

Francois, Could you review this new telemetry probe?
This one would help knowing how web developers toggle Developer Tools.
The precise user actions we are tracking are:
* any DevTools key shortcut like Ctrl/Cmd+Shift+I, Ctrl/Cmd+Shift+C, ...
* "Inspect Element" context menu
* any DevTools command line being passed like --jsconsole
* System menu: Tools > Web Developer
* Hamburger menu: Web Developer
* Customizable toolbar items: the wrench icon
Attachment #8884011 - Flags: feedback?(francois)

Comment 10

4 months ago
mozreview-review
Comment on attachment 8884011 [details]
Bug 1378863 - Add telemetry probe to track DevTools entry points.  datareview=francois

https://reviewboard.mozilla.org/r/154964/#review166998

Assuming that the question you want to answer is "what is the relative usage of these various install triggers", then it looks good.

This is Category 2 data (https://wiki.mozilla.org/Firefox/Data_Collection#Data_Collection_Categories). datareview+

::: toolkit/components/telemetry/Histograms.json:9332
(Diff revision 4)
>      "description": "Type of call: (Bitmask) Audio = 1, Video = 2, DataChannels = 4"
>    },
> +  "DEVTOOLS_ENTRY_POINT": {
> +    "record_in_processes": ["main"],
> +    "bug_numbers": [1378863],
> +    "alert_emails": ["dev-developer-tools@lists.mozilla.org"],

We are now asking for all probes to include the email address of a person responsible for it.

You can keep the team list in there, but please add your own email address (or whoever asked for this probe to be added if you prefer) to this array.
Attachment #8884011 - Flags: review+
Attachment #8884011 - Flags: feedback?(francois)

Comment 11

4 months ago
mozreview-review
Comment on attachment 8884011 [details]
Bug 1378863 - Add telemetry probe to track DevTools entry points.  datareview=francois

https://reviewboard.mozilla.org/r/154964/#review168292

Check my comment about guarding the telemetry call to check if the toolbox is already opened. Can be addressed in a follow up if you want.

::: devtools/client/devtools-startup.js:361
(Diff revision 4)
>     */
>    initialized: false,
>  
> -  initDevTools: function () {
> +  initDevTools: function (reason) {
> +    try {
> +      Services.telemetry.getHistogramById("DEVTOOLS_ENTRY_POINT")

Quickly testing this, I feel like the data will be hard to analyze. I think we should only update telemetry if devtools are closed when initDevTools() is called.

Example: I mainly use the keyboard to toggle devtools and switch tools. The counter will increase when I open the tools, everytime I switch to another panel, and when I close the tools.

In comparison, a mouse user will most likely open the tools from the menu, and then click on the tabs to switch tools and finally click on the X icon to close devtools. 

For a similar scenario, my flow would log multiple hits on telemetry, while a mouse user will only log it once. We can keep as is, but we should be very careful when comparing counts.

We have hasToolboxOpened(win) on devtoolsBrowser that we could call before updating telemetry?

::: devtools/client/devtools-startup.js:363
(Diff revision 4)
>  
> -  initDevTools: function () {
> +  initDevTools: function (reason) {
> +    try {
> +      Services.telemetry.getHistogramById("DEVTOOLS_ENTRY_POINT")
> +                        .add(reason);
> +    } catch(e) {

nit: eslint, space after catch
Attachment #8884011 - Flags: review?(jdescottes) → review+
Comment hidden (mozreview-request)
(Assignee)

Comment 13

4 months ago
(In reply to Julian Descottes [:jdescottes] from comment #11)
> Comment on attachment 8884011 [details]
> Bug 1378863 - Add telemetry probe to track DevTools entry points.
> 
> https://reviewboard.mozilla.org/r/154964/#review168292
> 
> Check my comment about guarding the telemetry call to check if the toolbox
> is already opened. Can be addressed in a follow up if you want.
> 
> ::: devtools/client/devtools-startup.js:361
> (Diff revision 4)
> >     */
> >    initialized: false,
> >  
> > -  initDevTools: function () {
> > +  initDevTools: function (reason) {
> > +    try {
> > +      Services.telemetry.getHistogramById("DEVTOOLS_ENTRY_POINT")
> 
> Quickly testing this, I feel like the data will be hard to analyze. I think
> we should only update telemetry if devtools are closed when initDevTools()
> is called.

As discussed during the meeting, I updated the patch to only bump the telemetry value if this.initialized is false,
so that we only count the very first action that forces the load of devtools. We end up ignoring all next actions.

I also added my email to the telemetry json file.

Comment 14

4 months ago
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3f9161013144
Add telemetry probe to track DevTools entry points. r=francois,jdescottes datareview=francois

Comment 15

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3f9161013144
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
You need to log in before you can comment on or make changes to this bug.