Closed
Bug 1378863
Opened 7 years ago
Closed 7 years ago
Add telemetry probe to addon ui shim to track entry points and addon installations
Categories
(DevTools :: General, enhancement)
DevTools
General
Tracking
(firefox56 fixed)
RESOLVED
FIXED
Firefox 56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: ochameau, Assigned: ochameau)
References
Details
Attachments
(1 file)
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•7 years ago
|
||
Morphing this bug to only track entry point and ignore addon installation for now.
Assignee: nobody → poirot.alex
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
Note that for testing telemetry patches, artifacts builds won't work. You have to build everything...
Comment 5•7 years 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•7 years 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•7 years 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•7 years 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+
Updated•7 years ago
|
Attachment #8884011 -
Flags: feedback?(francois)
Comment 11•7 years 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•7 years 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•7 years 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•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•