Closed
Bug 1515132
Opened 6 years ago
Closed 6 years ago
Add telemetry for FirstContentfulPaint
Categories
(Core :: Performance, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla67
People
(Reporter: jesup, Assigned: jesup)
Details
Attachments
(2 files)
3.53 KB,
patch
|
mstange
:
review+
chutten
:
review+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
3.00 KB,
text/plain
|
chutten
:
review+
|
Details |
We need to add telemetry for FirstContentfulPaint.
There should be no privacy or fingerprinting implications.
Assignee | ||
Comment 1•6 years ago
|
||
Mirrored on the NonBlankPaint telemetry. Does not include the NETOPT variants added by Honza for trialing network optimizations; we can add that as a separate bug if needed
Attachment #9036432 -
Flags: review?(mstange)
Attachment #9036432 -
Flags: review?(chutten)
Comment 2•6 years ago
|
||
Comment on attachment 9036432 [details] [diff] [review]
Add telemetry for FirstContentfulPaint data-review=chutten
Review of attachment 9036432 [details] [diff] [review]:
-----------------------------------------------------------------
This new data collection will require [Data Collection Review](https://wiki.mozilla.org/Firefox/Data_Collection) before it lands.
From a "technical use of Firefox Telemetry" POV it looks fine.
To me 100s looks like an awfully long time to be waiting for a contentful paint. The increased resolution a high of 60000 would bring ([high of 100s](https://telemetry.mozilla.org/histogram-simulator/#low=1&high=100000&n_buckets=100&kind=exponential&generate=log-normal) vs [high of 60s](https://telemetry.mozilla.org/histogram-simulator/#low=1&high=60000&n_buckets=100&kind=exponential&generate=log-normal)) might be worth the lack of being able to distinguish between "really bad" (60s) and "horrifyingly bad" (100s) but I'm not a domain expert and it seems as though there might me some benefit to having it be consistent.
I bring it up in case it might be helpful. It's likely fine for most purposes as-is.
Attachment #9036432 -
Flags: review?(chutten) → review+
Updated•6 years ago
|
Attachment #9036432 -
Flags: review?(mstange) → review+
Assignee | ||
Comment 3•6 years ago
|
||
Attachment #9041871 -
Flags: review?(chutten)
Updated•6 years ago
|
Attachment #9041871 -
Attachment mime type: application/octet-stream → text/plain
Comment 4•6 years ago
|
||
Comment on attachment 9041871 [details]
FirstContentfulPaint data collection
DATA COLLECTION REVIEW RESPONSE:
Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?
Yes. This collection is Telemetry so is documented in its definitions file ([Histograms.json](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/Histograms.json)) and the [Probe Dictionary](https://telemetry.mozilla.org/probe-dictionary/).
Is there a control mechanism that allows the user to turn the data collection on and off?
Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.
If the request is for permanent data collection, is there someone who will monitor the data over time?
Yes, two: Randell Jesup and Vicky Chin.
Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?
Category 2, Interaction. (insofar as the count of these paints is the count of pages loaded)
Is the data collection request for default-on or default-off?
Default on for all channels.
Does the instrumentation include the addition of any new identifiers?
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?
No. This collection is permanent.
---
Result: datareview+
Attachment #9041871 -
Flags: review?(chutten) → review+
Pushed by rjesup@wgate.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/134b85ae973c
Add telemetry for FirstContentfulPaint data-review=chutten r=mstange,a=chutten
Comment 6•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox67:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Assignee | ||
Comment 7•6 years ago
|
||
Comment on attachment 9036432 [details] [diff] [review]
Add telemetry for FirstContentfulPaint data-review=chutten
Beta/Release Uplift Approval Request
Feature/Bug causing the regression
None
User impact if declined
Less data on FCP for a release
Is this code covered by automated tests?
Yes
Has the fix been verified in Nightly?
Yes
Needs manual test from QE?
No
If yes, steps to reproduce
List of other uplifts needed
None
Risk to taking this patch
Low
Why is the change risky/not risky? (and alternatives if risky)
Just adds a telemetry point
String changes made/needed
none
Attachment #9036432 -
Flags: approval-mozilla-beta?
Comment 8•6 years ago
|
||
If we uplift this, we should also uplift bug 1506976, otherwise we won't be able to compare numbers between Beta and Nightly.
Comment 9•6 years ago
|
||
Comment on attachment 9036432 [details] [diff] [review]
Add telemetry for FirstContentfulPaint data-review=chutten
Support for measuring page load/rendering times, OK for uplift to beta 8.
Attachment #9036432 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 10•6 years ago
|
||
bugherder uplift |
status-firefox66:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•