1463092 - Instrument inspection of "Jump to Source" in the Web Console with event telemetry

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P2)

Comment 1

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

Attachment: data-review.txt

Events.yaml:
devtools.main:
  jump_to_source:
    objects: ["webconsole"],
    bug_numbers: [1463092]
    notification_emails: ["[e-mail]", "[e-mail]"]
    record_in_processes: ["main"]
    description: User has clicked a link to a source file in the web console.
    release_channel_collection: opt-out
    expiry_version: never

Usage:
this.telemetry.recordEvent("devtools.main", "jump_to_source", "webconsole");

Comment 2

[François Marier [:francois]]

Comment on attachment 8979202 [details]
data-review.txt

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

Yes, Events.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?**

Yes, Harald Kirschner.

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, permanent.

Comment 3

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

Attachment: Bug 1463092 - Instrument inspection of 'Jump to Source' in the Web Console with event telemetry

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

Comment 4

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

Try run failed because we still need a full build to see new events inside Events.yaml.

Comment 5

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

Comment on attachment 8987112 [details]
Bug 1463092 - Instrument inspection of 'Jump to Source' in the Web Console with event telemetry

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

Comment 6

[Julian Descottes [:jdescottes]]

Comment on attachment 8987112 [details]
Bug 1463092 - Instrument inspection of 'Jump to Source' in the Web Console with event telemetry

https://reviewboard.mozilla.org/r/252340/#review259740

Thanks for the patch Mike!

Can't always rely on this.toolbox here, would like to have a look at a new version of the patch fixing this.

I remember Harald said that we should actually not use devtools.main for tool specific events, so let's confirm the naming of the event before landing.

::: devtools/client/webconsole/webconsole-output-wrapper.js:37
(Diff revision 2)
>    this.queuedMessageAdds = [];
>    this.queuedMessageUpdates = [];
>    this.queuedRequestUpdates = [];
>    this.throttledDispatchPromise = null;
>  
> -  this._telemetry = new Telemetry();
> +  this.telemetry = this.toolbox.telemetry;

this.toolbox is not always available (eg for the BrowserConsole). This is the reason for the try failures.

::: devtools/client/webconsole/webconsole-output-wrapper.js:167
(Diff revision 2)
>        if (this.toolbox) {
>          Object.assign(serviceContainer, {
>            onViewSourceInDebugger: frame => {
> -            this.toolbox.viewSourceInDebugger(frame.url, frame.line).then(() =>
> -              this.hud.emit("source-in-debugger-opened")
> +            this.toolbox.viewSourceInDebugger(frame.url, frame.line).then(() => {
> +              this.telemetry.recordEvent("devtools.main", "jump_to_source", "webconsole",
> +                                         null, { "session_id": this.toolbox.sessionId }

Do we want to keep track of the type of source ? 
- script for debugger
- css for style-editor
- scratchpad

Comment 7

[Julian Descottes [:jdescottes]]

Harald, I remember we had a discussion about the naming for tool-specific events. Did we decide whether we should always use "devtools.main" or not?

Comment 8

[:Harald Kirschner :digitarald]

Double-checking with :chutten and :sunahsuh, we will stick with devtools.main as there are only minor pros and cons in changing.

Comment 9

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

Comment on attachment 8987112 [details]
Bug 1463092 - Instrument inspection of 'Jump to Source' in the Web Console with event telemetry

https://reviewboard.mozilla.org/r/252340/#review260050

::: devtools/client/webconsole/webconsole-output-wrapper.js:37
(Diff revision 2)
>    this.queuedMessageAdds = [];
>    this.queuedMessageUpdates = [];
>    this.queuedRequestUpdates = [];
>    this.throttledDispatchPromise = null;
>  
> -  this._telemetry = new Telemetry();
> +  this.telemetry = this.toolbox.telemetry;

Yup, so I see... fixed.

Comment 10

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

Comment on attachment 8987112 [details]
Bug 1463092 - Instrument inspection of 'Jump to Source' in the Web Console with event telemetry

https://reviewboard.mozilla.org/r/252340/#review259740

I have spoken to Harald and he agreed that we will continue to use devtools.main for now.

> Do we want to keep track of the type of source ? 
> - script for debugger
> - css for style-editor
> - scratchpad

Maybe in the future... it is really easy to do but I have been politely asked not to add all the things ;)

Harald has been having us add the more specific telemetry in later patches so lets stick to his plan.

Comment 11

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

Comment on attachment 8987112 [details]
Bug 1463092 - Instrument inspection of 'Jump to Source' in the Web Console with event telemetry

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

Comment 12

[Julian Descottes [:jdescottes]]

Comment on attachment 8987112 [details]
Bug 1463092 - Instrument inspection of 'Jump to Source' in the Web Console with event telemetry

https://reviewboard.mozilla.org/r/252340/#review260056

Thanks! Works for me if try is green

Comment 13

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

Comment on attachment 8987112 [details]
Bug 1463092 - Instrument inspection of 'Jump to Source' in the Web Console with event telemetry

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

Comment 14

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

Comment on attachment 8987112 [details]
Bug 1463092 - Instrument inspection of 'Jump to Source' in the Web Console with event telemetry

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/252340/diff/4-5/

Comment 15

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

(In reply to Julian Descottes [:jdescottes][:julian] from comment #12)
> Comment on attachment 8987112 [details]
> Bug 1463092 - Instrument inspection of 'Jump to Source' in the Web Console
> with event telemetry
> 
> https://reviewboard.mozilla.org/r/252340/#review260056
> 
> Thanks! Works for me if try is green

Which it is.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce4cec449d65
Instrument inspection of 'Jump to Source' in the Web Console with event telemetry r=jdescottes

Comment 17

[Cristian Brindusan [:cbrindusan]]

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