Closed
Bug 1465424
Opened 6 years ago
Closed 6 years ago
Track what the new Fonts panel gets used for with Event Telemetry
Categories
(DevTools :: Inspector, enhancement, P2)
DevTools
Inspector
Tracking
(firefox63 fixed)
RESOLVED
FIXED
Firefox 63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: pbro, Assigned: pbro)
References
Details
Attachments
(2 files, 2 obsolete files)
This is to validate product assumptions. We would like to know: - if the displayed font is variable vs. non variable - the number of variation axes - how many fonts the selected element required to render its content And we want to know this every time a new element is selected.
Assignee | ||
Comment 1•6 years ago
|
||
Let's depend on bug 1465500 since that bug will make it much more convenient to get this data from the redux store.
Depends on: 1465500
Assignee | ||
Updated•6 years ago
|
Severity: normal → enhancement
Priority: -- → P2
Assignee | ||
Comment 2•6 years ago
|
||
Events.yaml: devtools.main: font_editor_displayed: objects: ["inspector"] bug_numbers: [1465424] notification_emails: ["dev-developer-tools@lists.mozilla.org", "mbalfanz@mozilla.com"] record_in_processes: ["main"] description: The font editor was displayed as a result of a element being selected in the inspector release_channel_collection: opt-out expiry_version: 65 extra_keys: nb_fonts_used: Indicates how many fonts were needed to render the content of the selected element is_variable: Indicates whether the font editor UI is displayed for a variable or non-variable font nb_of_axes: Indicates how many variable axes does the font being edited has. 0 for non-variable fonts
Assignee | ||
Comment 3•6 years ago
|
||
Comment on attachment 8981965 [details]
data-review.txt
Hi Francois. I'd like to have a review from you on this proposal to add a new event telemetry to our DevTools.
Hopefully this data-review.txt file is clear enough, but let me know if you have any questions.
Attachment #8981965 -
Flags: review?(francois)
Comment 4•6 years ago
|
||
Chris, I'll assume you're ok with this Event telemetry because it's in devtools? Do you want to be NI'd for any further Event probes in devtools, or are those all good?
Flags: needinfo?(chutten)
Comment 5•6 years ago
|
||
Comment on attachment 8981965 [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, in 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?** 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.
Attachment #8981965 -
Flags: review?(francois) → review+
Comment 6•6 years ago
|
||
(In reply to François Marier [:francois] from comment #4) > Chris, I'll assume you're ok with this Event telemetry because it's in > devtools? Do you want to be NI'd for any further Event probes in devtools, > or are those all good? From a volume-perspective I'm ok with devtools.main.* on release. It's not the number of -types- of events I'm worried about, it's the number of times they're -recorded- :) So if you come across one that's recorded on mousemove or something, I'd definitely appreciate a ni? or something :) For this probe, though... Wanting the number and types of fonts... this doesn't need to be an Event, does it? It would more easily be analysed as a few scalars of the form devtools.main.font_editor_displayed_total, font_editor_displayed_font_count, font_editor_variable_font_count, font_editor_variable_axes_count, no?
Flags: needinfo?(chutten)
Assignee | ||
Comment 7•6 years ago
|
||
(In reply to Chris H-C :chutten from comment #6) > For this probe, though... Wanting the number and types of fonts... this > doesn't need to be an Event, does it? It would more easily be analysed as a > few scalars of the form devtools.main.font_editor_displayed_total, > font_editor_displayed_font_count, font_editor_variable_font_count, > font_editor_variable_axes_count, no? Sounds good to me, I'll verify with product.
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 8•6 years ago
|
||
New data review with 3 scalars this time, instead of an event. devtools.fonteditor: is_variable_font: bug_numbers: - 1465424 description: > Indicates whether the font editor UI is displayed for a variable or non-variable font. expires: "65" kind: boolean notification_emails: - dev-developer-tools@lists.mozilla.org - mbalfanz@mozilla.com record_in_processes: - 'main' variation_axes_count: bug_numbers: - 1465424 description: > Indicates the number of variation axes displayed in the font editor UI. expires: "65" kind: uint notification_emails: - dev-developer-tools@lists.mozilla.org - mbalfanz@mozilla.com record_in_processes: - 'main' rendered_fonts_count: bug_numbers: - 1465424 description: > Indicates how many fonts the selected element required to render its content. expires: "65" kind: uint notification_emails: - dev-developer-tools@lists.mozilla.org - mbalfanz@mozilla.com record_in_processes: - 'main'
Attachment #8984436 -
Flags: review?(francois)
Assignee | ||
Updated•6 years ago
|
Attachment #8981965 -
Attachment is obsolete: true
Comment 9•6 years ago
|
||
Comment on attachment 8984436 [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, in 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.
Attachment #8984436 -
Flags: review?(francois) → review+
Comment 10•6 years ago
|
||
(In reply to Patrick Brosset <:pbro> from comment #8) > devtools.fonteditor: > is_variable_font: > bug_numbers: > - 1465424 > description: > > Indicates whether the font editor UI is displayed for a variable or > non-variable font. > expires: "65" > kind: boolean > notification_emails: > - dev-developer-tools@lists.mozilla.org > - mbalfanz@mozilla.com > record_in_processes: > - 'main' > variation_axes_count: > bug_numbers: > - 1465424 > description: > > Indicates the number of variation axes displayed in the font editor UI. > expires: "65" > kind: uint > notification_emails: > - dev-developer-tools@lists.mozilla.org > - mbalfanz@mozilla.com > record_in_processes: > - 'main' > rendered_fonts_count: > bug_numbers: > - 1465424 > description: > > Indicates how many fonts the selected element required to render its > content. > expires: "65" > kind: uint > notification_emails: > - dev-developer-tools@lists.mozilla.org > - mbalfanz@mozilla.com > record_in_processes: > - 'main' It looks like you're missing `release_channel_collection: opt-out` in all of these probes BTW.
Updated•6 years ago
|
Product: Firefox → DevTools
Assignee | ||
Comment 11•6 years ago
|
||
I went back and forth on a few ideas, and finally talked to :chutten who helped me decide on the right type of data for this. I finally landed on histograms for all of this. So let me ask for a new data review. Sorry this is the 3rd time on this bug, but events weren't really adapted for this, and after reflection scalars either.
Assignee | ||
Comment 12•6 years ago
|
||
"DEVTOOLS_FONTEDITOR_N_FONT_AXES": { "record_in_processes": ["main"], "alert_emails": ["dev-developer-tools@lists.mozilla.org", "mbalfanz@mozilla.com"], "expires_in_version": 66, "bug_numbers": [1465424], "description": "Indicates the number of variation axes of the variable fonts that are displayed in the font editor UI", "releaseChannelCollection": "opt-out", "kind": "exponential", "high": 100, "n_buckets": 20 }, "DEVTOOLS_FONTEDITOR_N_FONTS_RENDERED": { "record_in_processes": ["main"], "alert_emails": ["dev-developer-tools@lists.mozilla.org", "mbalfanz@mozilla.com"], "expires_in_version": 66, "bug_numbers": [1465424], "description": "Indicates how many fonts the font editor displayed as being needed to render the selected element's content", "releaseChannelCollection": "opt-out", "kind": "exponential", "high": 50, "n_buckets": 10 }, "DEVTOOLS_FONTEDITOR_FONT_TYPE_DISPLAYED": { "record_in_processes": ["main"], "alert_emails": ["dev-developer-tools@lists.mozilla.org", "mbalfanz@mozilla.com"], "expires_in_version": 66, "bug_numbers": [1465424], "description": "Indicates if the font editor displayed its UI for a non-variable font or a variable font", "releaseChannelCollection": "opt-out", "kind": "categorical", "n_values": ["variable", "non-variable"] },
Attachment #8984436 -
Attachment is obsolete: true
Attachment #8988774 -
Flags: review?(francois)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8988775 -
Flags: review?(rcaliman)
Comment 14•6 years ago
|
||
mozreview-review |
Comment on attachment 8988775 [details] Bug 1465424 - Added 3 histograms to test font editor usage assumptions; https://reviewboard.mozilla.org/r/253962/#review260940 ::: devtools/client/inspector/fonts/fonts.js:487 (Diff revision 1) > + const editedFont = fontEditor.fonts[0]; > + if (!editedFont) { > + return; > + } > + > + const nbOfAxes = editedFont.variationAxes.length; Check that `variationAxes` exists before calling its length property. Inspecting older servers without the variable fonts implementation will throw an error because `variationAxes` may be undefined. ::: devtools/client/inspector/fonts/fonts.js:491 (Diff revision 1) > + > + const nbOfAxes = editedFont.variationAxes.length; > + telemetry.getHistogramById(HISTOGRAM_FONT_TYPE_DISPLAYED).add( > + !nbOfAxes ? "non-variable" : "variable"); > + if (nbOfAxes) { > + telemetry.getHistogramById(HISTOGRAM_N_FONT_AXES).add(nbOfAxes); Are we interested in also logging the font family or font name? Perhaps as an indicator of non-Latin font families that are prevalent and we should pay attention to. Just a thought, not a request. ::: toolkit/components/telemetry/Histograms.json:10152 (Diff revision 1) > "n_buckets": 29, > "bug_numbers": [1373483], > "description": "On page load, record the number of CSS Grid elements present on a page when the DevTools is open", > "releaseChannelCollection": "opt-out" > }, > + "DEVTOOLS_FONTEDITOR_N_FONT_AXES": { I don't know how to test the validity of these histogram configs. So I trust your judgement that they're correct.
Attachment #8988775 -
Flags: review?(rcaliman) → review+
Comment 15•6 years ago
|
||
Comment on attachment 8988774 [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, in Histograms.json. 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 in 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 should be fine.
Attachment #8988774 -
Flags: review?(francois) → review+
Comment hidden (mozreview-request) |
Comment 17•6 years ago
|
||
Pushed by pbrosset@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0340ab4b2be0 Added 3 histograms to test font editor usage assumptions; r=rcaliman
Comment 18•6 years ago
|
||
Backed out changeset 0340ab4b2be0 (bug 1465424) for causing build bustages. Backout: https://hg.mozilla.org/integration/autoland/rev/11b6d0429ff45f24d2b29cffebc7e1626b3f31bd Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0340ab4b2be0a13c7b4a16264a9a0609ff6ad981&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success&selectedJob=186788749 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=186788749&repo=autoland&lineNumber=1035
Flags: needinfo?(pbrosset)
Assignee | ||
Comment 19•6 years ago
|
||
Silly mistake in Histgrams.json. I'll get it fixed.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Flags: needinfo?(pbrosset)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 21•6 years ago
|
||
Pushed to try quickly just to be sure. Should be good now. Pushing to autoland.
Comment 22•6 years ago
|
||
Pushed by pbrosset@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7c5a7b0b6bc6 Added 3 histograms to test font editor usage assumptions; r=rcaliman
Comment 23•6 years ago
|
||
Backed out changeset 7c5a7b0b6bc6 (bug 1465424) for bustages in /builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/run_complete on a CLOSED TREE Problematic push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=7c5a7b0b6bc6123e774c8527c1a3cc5448b9fc12&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified Failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=186845730 Log: https://treeherder.mozilla.org/logviewer.html#?job_id=186845730&repo=autoland&lineNumber=1100 [task 2018-07-06T13:38:53.744Z] 13:38:53 INFO - checking for rustc... /builds/worker/workspace/build/src/rustc/bin/rustc [task 2018-07-06T13:38:53.744Z] 13:38:53 INFO - checking for cargo... /builds/worker/workspace/build/src/rustc/bin/cargo [task 2018-07-06T13:38:53.779Z] 13:38:53 INFO - checking rustc version... 1.28.0-beta.6 [task 2018-07-06T13:38:53.787Z] 13:38:53 INFO - checking cargo version... 1.28.0 [task 2018-07-06T13:38:53.975Z] 13:38:53 INFO - checking for rustdoc... /builds/worker/workspace/build/src/rustc/bin/rustdoc [task 2018-07-06T13:38:53.975Z] 13:38:53 INFO - checking for rustfmt... /builds/worker/workspace/build/src/rustc/bin/rustfmt [task 2018-07-06T13:38:53.975Z] 13:38:53 INFO - checking for nodejs... not found [task 2018-07-06T13:38:53.975Z] 13:38:53 INFO - WARNING: could not find Node.js executable; ensure `node` or `nodejs` is in PATH or set NODEJS in environment to point to an executable [task 2018-07-06T13:38:53.975Z] 13:38:53 INFO - WARNING: (This will become a fatal error in the near future.) [task 2018-07-06T13:38:53.978Z] 13:38:53 INFO - checking for tar... /bin/tar [task 2018-07-06T13:38:53.978Z] 13:38:53 INFO - checking for unzip... /usr/bin/unzip [task 2018-07-06T13:38:53.979Z] 13:38:53 INFO - checking for zip... /usr/bin/zip [task 2018-07-06T13:38:53.979Z] 13:38:53 INFO - checking for gn... not found [task 2018-07-06T13:38:53.979Z] 13:38:53 INFO - checking for the Mozilla API key... no [task 2018-07-06T13:38:53.979Z] 13:38:53 INFO - checking for the Google API key... no [task 2018-07-06T13:38:53.979Z] 13:38:53 INFO - checking for the Bing API key... no [task 2018-07-06T13:38:53.979Z] 13:38:53 INFO - checking for the Adjust SDK key... no [task 2018-07-06T13:38:53.980Z] 13:38:53 INFO - checking for the Leanplum SDK key... no [task 2018-07-06T13:38:53.980Z] 13:38:53 INFO - checking for the Pocket API key... no [task 2018-07-06T13:38:53.980Z] 13:38:53 INFO - checking for llvm-config... /builds/worker/workspace/build/src/clang/bin/llvm-config [task 2018-07-06T13:38:53.996Z] 13:38:53 INFO - checking for awk... /usr/bin/gawk [task 2018-07-06T13:38:53.996Z] 13:38:53 INFO - checking for perl... /usr/bin/perl [task 2018-07-06T13:38:53.996Z] 13:38:53 INFO - checking for minimum required perl version >= 5.006... 5.014002 [task 2018-07-06T13:38:53.999Z] 13:38:53 INFO - checking for full perl installation... yes [task 2018-07-06T13:38:53.999Z] 13:38:53 INFO - checking for gmake... /usr/bin/make [task 2018-07-06T13:38:54.000Z] 13:38:54 INFO - checking for watchman... not found [task 2018-07-06T13:38:54.000Z] 13:38:54 INFO - checking for xargs... /usr/bin/xargs [task 2018-07-06T13:38:54.000Z] 13:38:54 INFO - checking for dsymutil... /builds/worker/workspace/build/src/build/macosx/llvm-dsymutil [task 2018-07-06T13:38:54.000Z] 13:38:54 INFO - checking for mkfshfs... /builds/worker/workspace/build/src/hfsplus-tools/newfs_hfs [task 2018-07-06T13:38:54.001Z] 13:38:54 INFO - checking for hfs_tool... /builds/worker/workspace/build/src/dmg/hfsplus [task 2018-07-06T13:38:54.001Z] 13:38:54 INFO - checking for autoconf... /usr/bin/autoconf2.13 [task 2018-07-06T13:38:54.003Z] 13:38:54 INFO - Refreshing /builds/worker/workspace/build/src/old-configure with /usr/bin/autoconf2.13
Flags: needinfo?(pbrosset)
Comment 24•6 years ago
|
||
Backout by shindli@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9cfad7403969 Backed out changeset 7c5a7b0b6bc6 for bustages in /builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/run_complete on a CLOSED TREE
Assignee | ||
Comment 25•6 years ago
|
||
I haven't had the time to look into this for a while. This morning I fixed the issue (I thought my try pushes would have picked up the problems before landing, but they did not). So I did a full build locally, and was able to confirm the issues were fixed. I also rebased the patch (which I'll attach soon). And I'll land it again!
Flags: needinfo?(pbrosset)
Comment hidden (mozreview-request) |
Comment 27•6 years ago
|
||
Pushed by pbrosset@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b2f2361ee317 Added 3 histograms to test font editor usage assumptions; r=rcaliman
Comment 28•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b2f2361ee317
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Updated•3 years ago
|
Component: Inspector: Fonts → Inspector
You need to log in
before you can comment on or make changes to this bug.
Description
•