Closed
Bug 1388274
Opened 7 years ago
Closed 7 years ago
Adjust WebVR telemetry histogram's high bound for user time spend
Categories
(Core :: WebVR, defect)
Core
WebVR
Tracking
()
RESOLVED
FIXED
mozilla57
People
(Reporter: daoshengmu, Assigned: daoshengmu)
References
Details
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
francois
:
review+
kip
:
review+
gchang
:
approval-mozilla-beta+
|
Details |
Because we only define 50 sec for the high bound, in the use case of WebVR, it is very easy to over the upper bound.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → dmu
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•7 years ago
|
||
I realize our data from https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-08-06&keys=__none__!__none__!__none__&max_channel_version=nightly%252F57&measure=WEBVR_TIME_SPEND_FOR_VIEWING_IN_OCULUS&min_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-08-02&table=0&trim=1&use_submission_date=0, are around 50% beyond the high bound. I think it should make sense to adjust the high bound to 30 mins for user spend time, and we can ignore the spend time which is less than 3 seconds.
Assignee | ||
Updated•7 years ago
|
Attachment #8894834 -
Flags: review?(kgilbert)
Attachment #8894834 -
Flags: review?(francois)
Assignee | ||
Comment 4•7 years ago
|
||
:Francois, I just notice I need a data collection peer to review the telemetry patch at Bug 1306156. Please refer https://bugzilla.mozilla.org/show_bug.cgi?id=1306156#c5, give me some feedback. I am willing to do any adjustment here if I need to modify. Thanks.
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8894834 [details] Bug 1388274 - Adjust WebVR telemetry histogram's high bound for user time spent; https://reviewboard.mozilla.org/r/165988/#review171300 You likely need to create new probes for these changes (e.g. by renaming from `WEBVR_TIME_SPEND_FOR_VIEWING_IN_OCCULUS` to `WEBVR_TIME_SPEND_FOR_VIEWING_IN_OCCULUS_2`): https://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/telemetry/collection/histograms.html#changing-a-histogram In this case however, it should probably have been "TIME_SPENT_VIEWING_IN" instead of "TIME_SPEND_FOR_VIEWING_IN". So that would also work as a new name.
Attachment #8894834 -
Flags: review?(francois) → review-
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8894834 [details] Bug 1388274 - Adjust WebVR telemetry histogram's high bound for user time spent; https://reviewboard.mozilla.org/r/165988/#review171392 r=me with Francois' fixes addressed. Need to change "TIME_SPEND_VIEWING_IN" to "TIME_SPENT_VIEWING_IN" to create a new set of probes.
Attachment #8894834 -
Flags: review?(kgilbert) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•7 years ago
|
||
(In reply to Daosheng Mu[:daoshengmu] from comment #7) > Comment on attachment 8894834 [details] > Bug 1388274 - Adjust WebVR telemetry histogram's high bound for user time > spent; > > Review request updated; see interdiff: > https://reviewboard.mozilla.org/r/165988/diff/2-3/ After some internal discussion, we decide to change the high bound as 20 mins and don't set the low bound in order to collect the crash results when loading the content.
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8894834 [details] Bug 1388274 - Adjust WebVR telemetry histogram's high bound for user time spent; https://reviewboard.mozilla.org/r/165988/#review171916 datareview+
Attachment #8894834 -
Flags: review?(francois) → review+
Comment 10•7 years ago
|
||
Pushed by dmu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/67ade393270e Adjust WebVR telemetry histogram's high bound for user time spent; r=francois,kip
Assignee | ||
Comment 11•7 years ago
|
||
It needs to be uplift to FF 56 after confirming the functionality.
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/67ade393270e
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee | ||
Comment 13•7 years ago
|
||
Comment on attachment 8894834 [details] Bug 1388274 - Adjust WebVR telemetry histogram's high bound for user time spent; Approval Request Comment [Feature/Bug causing the regression]: Adjust WebVR telemetry histogram's high bound [User impact if declined]: The histogram would be too low to get the correct data. [Is this code covered by automated tests?]: nope. This is a telemetry function. I have tested in at about:telemetry. [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: nope. [List of other uplifts needed for the feature/fix]: nope. [Is the change risky?]: nope. [Why is the change risky/not risky?]: Just a telemetry, would not break anything. [String changes made/needed]:
Attachment #8894834 -
Flags: approval-mozilla-beta?
Updated•7 years ago
|
status-firefox56:
--- → affected
Comment 14•7 years ago
|
||
Comment on attachment 8894834 [details] Bug 1388274 - Adjust WebVR telemetry histogram's high bound for user time spent; Fix WebVR telemetry histogram's high bound issue. Beta56+. Should be in 56.0b3.
Attachment #8894834 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 15•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/3d1029848041
Comment 16•7 years ago
|
||
(In reply to Daosheng Mu[:daoshengmu] from comment #13) > [Is this code covered by automated tests?]: nope. This is a telemetry > function. I have tested in at about:telemetry. > [Has the fix been verified in Nightly?]: yes > [Needs manual test from QE? If yes, steps to reproduce]: nope. Setting qe-verify- based on Daosheng Mu's assessment on manual testing needs.
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•