1347517 - Add telemetry for GCLI commands used

Status: RESOLVED FIXED

(DevTools Graveyard :: Graphic Commandline and Toolbar, enhancement, P2)

Description

[Bryan Clark (DevTools PM) [:clarkbw]]

We have no idea what are the most popular commands being used in GCLI.  Some of these commands would be better promoted as tools in other places and some commands could be improved or removed.  Lets add some telemetry to collect the used GCLI commands and how often they are used.

I believe we need a keyed Telemetry probe https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Adding_a_new_Telemetry_probe#Keyed_Histograms which will collect the commands and add a count for the usage of each command.  Resulting data will look something like this:

{
  screenshot: 3,
  security: 1
}

For commands unused we don't need to pass a zero value, we can keep the data smaller and assume non-usage on the collection side.

Understanding the options passed to the commands would be useful but is something we could do in a next step as we need to understand the commands better.

Comment 1

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

We have a very large amount of users (around 750,000) that open the developer toolbar for "some reason" but we have no idea what that reason is... it could be to type GCLI commands but it could also be to have access to the page error count or toolbox toggle button.

The first thing we need to do to establish why the toolbar is open is to see which GCLI commands are being typed.

We also need to bare in mind that *some* commands still don't work with E10S enabled so this will give us a way to focus on which commands we should focus on, keep, move or remove.

I have worked hard to find a way to log telemetry for all valid typed commands so that, whether they have executed successfully or not we know that somebody typed the command and pressed <RETURN>.

Telemetry is not logged for toolbox button presses, e.g. the screenshot button, because the command was not typed.

Comment 2

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Bug 1347517 - Add telemetry for GCLI commands used

Review commit: https://reviewboard.mozilla.org/r/125520/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/125520/

Comment 3

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8853420 [details]
Bug 1347517 - Add telemetry for GCLI commands used

@jwalker: Sorry to ask you to review this but nobody else knows GCLI like you. At first I was trying to log the telemetry when the commands were executed but that was a nightmare because of the GCLI E10S implementation. In the end I settled on sending the telemetry ping only if the user types a valid command and presses <RETURN>.

@bsmedberg: Comment 2 pretty much summarizes why we need this... the histogram is defined as:

```
    "DEVTOOLS_GCLI_COMMANDS_KEYED": {
      "bug_numbers": [1347517],
      "alert_emails": ["[e-mail]"],
      "expires_in_version": "never",
      "keyed": true,
      "kind": "count",
      "description": "Reports the command name used in GCLI e.g. 'screenshot'"
    },
```

Comment 4

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

@bsmedberg: s/Comment 2/Comment 1/

Comment 5

[Joe Walker [:jwalker]]

Comment on attachment 8853420 [details]
Bug 1347517 - Add telemetry for GCLI commands used

https://reviewboard.mozilla.org/r/125520/#review128448

::: devtools/shared/gcli/source/lib/gcli/mozui/inputter.js:566
(Diff revision 1)
>    // Deny RETURN unless the command might work
>    if (this.requisition.status === Status.VALID) {
>      this._scrollingThroughHistory = false;
>      this.history.add(this.element.value);
>  
> +    let name = this.requisition.commandAssignment.conversion.value.name;

I'm fairly sure you can do:

    let name = this.requisition.commandAssignment.value.name;

(i.e. miss out 'conversion.' [1])

[1]: https://github.com/joewalker/gcli/blob/master/lib/gcli/cli.js#L155

Comment 6

[Joe Walker [:jwalker]]

Comment on attachment 8853420 [details]
Bug 1347517 - Add telemetry for GCLI commands used

Looks good to me.

Comment 7

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8853420 [details]
Bug 1347517 - Add telemetry for GCLI commands used

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/125520/diff/1-2/

Comment 8

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8853420 [details]
Bug 1347517 - Add telemetry for GCLI commands used

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/125520/diff/2-3/

Comment 9

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8853420 [details]
Bug 1347517 - Add telemetry for GCLI commands used

https://reviewboard.mozilla.org/r/125518/#review128868

Comment 10

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8853420 [details]
Bug 1347517 - Add telemetry for GCLI commands used

https://reviewboard.mozilla.org/r/125520/#review128870

Change was Joe's suggestion... makes no sense for him to review a change he said I should make.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3e544c215cd1
Add telemetry for GCLI commands used r=miker

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3e544c215cd1