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)

enhancement

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.
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
Severity: normal → enhancement
Priority: -- → P2
Attached file data-review.txt (obsolete) —
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
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)
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 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+
(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)
(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.
Depends on: 1464796
No longer depends on: 1465500
Attached file data-review.txt (obsolete) —
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)
Attachment #8981965 - Attachment is obsolete: true
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+
(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.
Product: Firefox → DevTools
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.
Attached file data-review.txt
"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)
Attachment #8988775 - Flags: review?(rcaliman)
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 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+
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0340ab4b2be0
Added 3 histograms to test font editor usage assumptions; r=rcaliman
Silly mistake in Histgrams.json. I'll get it fixed.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Flags: needinfo?(pbrosset)
Pushed to try quickly just to be sure. Should be good now. Pushing to autoland.
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7c5a7b0b6bc6
Added 3 histograms to test font editor usage assumptions; r=rcaliman
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)
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
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)
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b2f2361ee317
Added 3 histograms to test font editor usage assumptions; r=rcaliman
https://hg.mozilla.org/mozilla-central/rev/b2f2361ee317
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Component: Inspector: Fonts → Inspector
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: