1429047 - Instrument Debugger's source load times

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement, P2)

Description

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

DEVTOOLS_DEBUGGER_LOAD_SOURCE: The time (in milliseconds) that it took to load a source for the user.

Comment 1

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

Attachment: Scalar.png

Comment 2

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

Attachment: Histogram.png

Comment 3

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

Attachment: Bug 1429047 - Add telemetry probes to Debugger

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

Comment 4

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

To make this work we had to add the fowwlowing to DEVTOOLS_DEBUGGER_LOAD_SOURCE:

"alert_emails": ["[e-mail]", "[e-mail]"],
"bug_numbers": [1429047],
...
"n_buckets": 100,

Comment 5

[:Harald Kirschner :digitarald]

Will probably come up in data review, timings usually have a _MS attached; so the probe should be named DEVTOOLS_DEBUGGER_LOAD_SOURCE_MS

Comment 6

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

Comment on attachment 8941464 [details]
Bug 1429047 - Add telemetry probes to Debugger

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

Comment 7

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

TODO

Add questions from this form and get a data collection peer to review it:
https://github.com/mozilla/data-review/blob/master/request.md

Comment 8

[:Harald Kirschner :digitarald]

Should be also fine to limit this probe to 3 releases and then reconsider if we keep it or have something better with Event Telemetry/Amplitude.

Comment 9

[:Harald Kirschner :digitarald]

Split out bugs to track follow up telemetry work

Comment 10

[Julian Descottes [:jdescottes]]

Attachment: bug-1429047-data-review.md

Here is an example of data review we could use for the DEVTOOLS_DEBUGGER_LOAD_SOURCE_MS (I assume we are only landing this probe for now).

Feel free to modify or discard it as you fit!

Regarding the current patch, the modification to Histograms.json from Mike's patch looks perfect, just need to set the expiry version to something like 63.

Comment 11

[Julian Descottes [:jdescottes]]

Attachment: Bug 1429047 - Add telemetry probe to measure source load time;,datareview=liuche

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

Comment 12

[Yulia Startsev [:yulia] OOO until July 2026]

Comment on attachment 8942151 [details]
Bug 1429047 - Add telemetry probe to measure source load time;,datareview=liuche

https://reviewboard.mozilla.org/r/212414/#review218148

This looks good! thanks!

Comment 13

[Yulia Startsev [:yulia] OOO until July 2026]

Comment on attachment 8941464 [details]
Bug 1429047 - Add telemetry probes to Debugger

https://reviewboard.mozilla.org/r/211742/#review218150

as mentioned on slack, we will do the github changes separately -- thanks mike!

Comment 14

[Yulia Startsev [:yulia] OOO until July 2026]

Comment on attachment 8942151 [details]
Bug 1429047 - Add telemetry probe to measure source load time;,datareview=liuche

https://reviewboard.mozilla.org/r/212414/#review218152

Comment 15

[Julian Descottes [:jdescottes]]

Comment on attachment 8941922 [details]
bug-1429047-data-review.md

As discussed by email, the implementation using this probe is being developed in parallel on GitHub at https://github.com/devtools-html/debugger.html/pull/4665.

Comment 16

[Julian Descottes [:jdescottes]]

Comment on attachment 8942151 [details]
Bug 1429047 - Add telemetry probe to measure source load time;,datareview=liuche

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

Comment 17

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

Comment on attachment 8941464 [details]
Bug 1429047 - Add telemetry probes to Debugger

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

Comment 18

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

@liuche: The data review form is at https://github.com/devtools-html/debugger.html/pull/4665#issuecomment-357358611

Comment 19

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

Not sure what happened here... I am guessing that jdescottes didn't realize this was assigned to me.

Comment 20

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

Ignore my comment... the data request form is attached to this bug.

Comment 21

[Chenxia Liu [:liuche]]

Comment on attachment 8941922 [details]
bug-1429047-data-review.md

data-review only

> Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate? (see here, here, and here for examples).

Yes, Histograms and on http://firefox-dev.tools/debugger.html/docs/local-development.html

> Is there a control mechanism that allows the user to turn the data collection on and off? (Note, for data collection not needed for security purposes, Mozilla provides such a control mechanism) Provide details as to the control mechanism available.

Yes, Firefox data controls

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

expires in 63

> Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under? **

Type 1, load time

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

> 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

> Is the data collection covered by the existing Firefox privacy notice?

yes 

    Does there need to be a check-in in the future to determine whether to renew the data? (Yes/No) (If yes, set a todo reminder or file a bug if appropriate)**

expires in 66

Comment 22

[Chenxia Liu [:liuche]]

Julian, one question: is there separate telemetry included in https://github.com/devtools-html/debugger.html that isn't controlled by Firefox data controls? (And I assume the telemetry doesn't work if it's not in Firefox.)

Comment 23

[Julian Descottes [:jdescottes]]

Thanks for the review!

(In reply to Chenxia Liu [:liuche] - not actively working on Fennec from comment #22)
> Julian, one question: is there separate telemetry included in
> https://github.com/devtools-html/debugger.html that isn't controlled by
> Firefox data controls? (And I assume the telemetry doesn't work if it's not
> in Firefox.)

We don't have any other telemetry setup for the debugger. When the debugger is not running in Firefox, the telemetry APIs are mocked and don't collect any data. The no-op implementation used is at https://github.com/devtools-html/devtools-core/blob/80fe5a5db96dff2bf3cc6ddb4820bb78a49367e6/packages/devtools-modules/src/Services.js#L563-L607

Comment 24

[Julian Descottes [:jdescottes]]

Comment on attachment 8942151 [details]
Bug 1429047 - Add telemetry probe to measure source load time;,datareview=liuche

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

Comment 25

[Julian Descottes [:jdescottes]]

(just updated the expiry in histograms.json to match version 66 mentioned in the data review)

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/acb786485419
Add telemetry probe to measure source load time;r=yulia,datareview=liuche

Comment 27

[Cristian Brindusan [:cbrindusan]]

Backed out changeset acb786485419 (bug 1429047) for Bustages on TelemetryHistogramEnums.h

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=acb786485419f2b5506b753681d25adf14dd4aaa&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=156895230&repo=autoland&lineNumber=3546

Backout: https://hg.mozilla.org/integration/autoland/rev/b0399c9ebb0c53ca57d117fe9d6acd003baa17f3

Comment 28

[Julian Descottes [:jdescottes]]

Comment on attachment 8942151 [details]
Bug 1429047 - Add telemetry probe to measure source load time;,datareview=liuche

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

Comment 29

[Julian Descottes [:jdescottes]]

Max bucket size is 100 by default.

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/55b95957d30b
Add telemetry probe to measure source load time;r=yulia,datareview=liuche

Comment 31

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/55b95957d30b