1451734 - Application panel: add telemetry to track panel usage

Status: RESOLVED FIXED

(DevTools :: Application Panel, enhancement, P3)

Comment 1

[Patrick Brosset <:pbro>]

Mass move to the new application panel component.

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1451734 - Add basic telemetry for application panel (open count, active time);

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

Comment 3

[Julian Descottes [:jdescottes]]

Here is a first patch that simply adds open count and active time for the application panel. The logic is already handled by the toolbox with generic code, we just had to define entries for the application panel here.

Going further some metrics that could be interesting:
- clicks on links/buttons in the workers list ("Start"/"Debug"/"Unregister"/"about:debugging")
- time spent debugging: we could start a timer when opening the toolbox, and stop the timer when the toolbox closes (timer histogram)
- clicks on links in the empty screen

I think it makes sense to use event telemetry here, as it would be interesting to understand the workflow of users with the panel.

Comment 4

[Julian Descottes [:jdescottes]]

I could use some help to organize our event telemetry data for the application panel, particularly deciding what should go into category, method, object and value fields. 

I thought the category should be "devtools.application" or "devtools.application.serviceworkers" (since there will be other subcategories in the application panel, for instance "storage"). 

But apparently we are using "devtools.main" for actions specific to other panels (cf https://bugzilla.mozilla.org/show_bug.cgi?id=1463118 which adds the debugger action "pause" under devtools.main?). I feel like the actions I am going to add here will only ever make sense in the scope of the application panel, so I am not sure having them under devtools.main makes sense? 

Let's talk about instrumenting clicks in the list of workers for instance. Each action relates to a given worker, the "object" field should be "worker" or "serviceworker" and the "method" field should be related to the action performed by the click? 

Using the example of the start link, displayed next to a running service worker. The event could look like:
  { 
    category: devtools.application
    method: start,
    object: serviceworker,
    value: null,
    extra: {}
  }

With devtools.main as the category:
  {
    category: devtools.main
    method: start_serviceworker,
    object: application,
    value: null,
    extra: {}
  }

Maybe this is a spot where we should use value? I can imagine a variant of the event above as
  {
    category: devtools.application
    method: click,
    object: serviceworker,
    value: start_link,
    extra: {}
  }

or again, with devtools.main as the category:
  {
    category: devtools.main
    method: click_serviceworker,
    object: application,
    value: start_link,
    extra: {}
  }

Any idea which option would be the best for us here?

Comment 5

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

We use devtools.main as our category because it is from the main process... I would say we are better sticking with that name for the root category.

For the event to work in digitarald's tools value needs to be null.

For Harald properties are much easier to read within extra so you should use:
  {
    category: "devtools.main"
    method: "start_serviceworker",
    object: "application",
    value: null,
    extra: {
      start_link: "whatever the start link is"
    }
  }

Hope that helps.

We should probably enforce a value of zero for digitarald's sanity... I will log a bug for that.

Comment 6

[Julian Descottes [:jdescottes]]

Attachment: Bug 1451734 - Track application panel events with EventTelemetry

Events tracked here:
- service worker debug (with duration)
- service worker start
- service worker unregister
- click on aboutdebugging link at bottom of sw list
- click on any help link

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

Comment 7

[Julian Descottes [:jdescottes]]

Comment on attachment 8980593 [details]
Bug 1451734 - Add basic telemetry for application panel (open count, active time);

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

Comment 8

[Julian Descottes [:jdescottes]]

Comment on attachment 8981858 [details]
Bug 1451734 - Track application panel events with EventTelemetry

Mike, Harald: Can you have a look at this proposal to use event telemetry in the application panel, let me know if that makes sense or if I should rather use scalars :)

Thanks

Comment 9

[Belén [:ladybenko]]

Comment on attachment 8980593 [details]
Bug 1451734 - Add basic telemetry for application panel (open count, active time);

https://reviewboard.mozilla.org/r/246746/#review254702

LGTM! Thanks :) Btw, I'm seeing some DOM elements that are not localized in `WorkerList`, but maybe that's for a different bug.

Comment 10

[Julian Descottes [:jdescottes]]

Comment on attachment 8980593 [details]
Bug 1451734 - Add basic telemetry for application panel (open count, active time);

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

Comment 11

[Julian Descottes [:jdescottes]]

Attachment: bug_1451734_data_review.txt

Adding :francois to data-review for the basic devtools panel telemetry part.

Comment 12

[François Marier [:francois]]

Comment on attachment 8983747 [details]
bug_1451734_data_review.txt

Apologies for the delay.

This looks good, the only little thing that should get fixed since it's a little confusing is the answer to question 4:

> 4) Can current instrumentation answer these questions?
>
> Yes.

I think you meant "No", given your answer to question 3 (no instrumentation for this new panel). That's why these new probes are needed.

If you upload a new file with just this little fix, I'll be happy to give that an r+.

Comment 13

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

Comment on attachment 8980593 [details]
Bug 1451734 - Add basic telemetry for application panel (open count, active time);

https://reviewboard.mozilla.org/r/246746/#review258402

Looks good to me... r+ if you address the data review issue.

Comment 14

[Julian Descottes [:jdescottes]]

Attachment: bug_1451734_data_review_v2.txt

Thanks for the review! 

Updated Q4 answer. For some reason I read the question more as "are the current tools enough to instrument what you need here".

Comment 15

[François Marier [:francois]]

Comment on attachment 8987463 [details]
bug_1451734_data_review_v2.txt

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

Yes in Histograms.json and Scalars.yaml.

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

Yes, telemetry setting.

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

Not permanent.

4) Using the **[category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection)** on the Mozilla wiki, what collection type of data do the requested measurements fall under?  **

Category 2.

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

Default on, all channels.

6) Does the instrumentation include the addition of **any *new* identifiers** (whether anonymous or otherwise; e.g., username, random IDs, etc.  See the appendix for more details)?

No.

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

Yes.

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

No, telemetry alerts are fine.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c68575fa1be0
Add basic telemetry for application panel (open count, active time);r=ladybenko,miker

Comment 17

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/c68575fa1be0