Adjust WebVR telemetry histogram's high bound for user time spend

RESOLVED FIXED in Firefox 56

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: daoshengmu, Assigned: daoshengmu)

Tracking

unspecified
mozilla57
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox56 fixed, firefox57 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
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

2 years ago
Assignee: nobody → dmu
(Assignee)

Updated

2 years ago
Blocks: 1306156
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8894834 - Flags: review?(kgilbert)
Attachment #8894834 - Flags: review?(francois)
(Assignee)

Comment 4

2 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

2 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

2 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

2 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

2 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

2 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

2 years ago
It needs to be uplift to FF 56 after confirming the functionality.

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/67ade393270e
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
(Assignee)

Comment 13

2 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?
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+
(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.