Closed Bug 1166646 Opened 5 years ago Closed 5 years ago

WINDOW_ENABLED sharing telemetry probe not working

Categories

(Hello (Loop) :: Client, defect, P1, blocker)

defect
Points:
1

Tracking

(firefox38 unaffected, firefox39+ fixed, firefox40+ fixed, firefox41 fixed)

RESOLVED FIXED
mozilla41
Iteration:
41.1 - May 25
Tracking Status
firefox38 --- unaffected
firefox39 + fixed
firefox40 + fixed
firefox41 --- fixed

People

(Reporter: mikedeboer, Assigned: mikedeboer)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

As you can see at http://telemetry.mozilla.org/#filter=aurora%2F39%2FLOOP_SHARING_STATE_CHANGE_1%2Fsaved_session%2FFirefox&aggregates=multiselect-all!Submissions&evoOver=Builds&locked=true&sanitize=true&renderhistogram=Graph all screen sharing probes are working, except the WINDOW_ENABLED one.

It's a silly bug in the client code.
Flags: qe-verify+
Flags: firefox-backlog+
Flags: qe-verify+ → qe-verify-
[Tracking Requested - why for this release]: The screen sharing telemetry probe is not working as expected right now, which is quite essential to have.
Comment on attachment 8608015 [details] [diff] [review]
Patch v1: WINDOW_ENABLED telemetry key is falsy, thus not working. Check for its type properly

Review of attachment 8608015 [details] [diff] [review]:
-----------------------------------------------------------------

The values for SHARING_STATE_CHANGE and TWO_WAY_MEDIA_CONN_LENGTH in otSdkDriver_test.js weren't updated in bug 1152733 - otherwise this would have been caught.

Hence, please update the tests to match, and could you also add a note in MozLoopService that the values are replicated in the tests so if one is updated, then they both should be.

::: browser/components/loop/content/shared/js/otSdkDriver.js
@@ +851,5 @@
>        }
>  
>        var bucket = this.mozLoop.SHARING_STATE_CHANGE[type.toUpperCase() + "_" +
>          (enabled ? "ENABLED" : "DISABLED")];
> +      if (typeof bucket == "undefined") {

nit: we're generally standardised on strict equality for content code.
Attachment #8608015 - Flags: review?(standard8) → review-
Blocks: 1152733
Keywords: regression
Tracking for 39+  Please request uplift to mozilla-beta and aurora when you're ready.
Comment on attachment 8610547 [details] [diff] [review]
Patch v2: WINDOW_ENABLED telemetry key is falsy, thus not working. Check for its type properly

Review of attachment 8610547 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. r=Standard8
Attachment #8610547 - Flags: review?(standard8) → review+
https://hg.mozilla.org/mozilla-central/rev/ddd02b236cc2
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment on attachment 8610547 [details] [diff] [review]
Patch v2: WINDOW_ENABLED telemetry key is falsy, thus not working. Check for its type properly

Approval Request Comment
[Feature/regressing bug #]: Bug 1152733
[User impact if declined]: The WINDOW_ENABLED telemetry value will not be recorded without this fix in, which currently has the result that the LOOP_SHARING_STATE_CHANGE_1 histogram is missing data.
[Describe test coverage new/current, TreeHerder]: landed on m-c, tests pass.
[Risks and why]: minor.
[String/UUID change made/needed]: n/a.
Attachment #8610547 - Flags: approval-mozilla-beta?
Attachment #8610547 - Flags: approval-mozilla-aurora?
Comment on attachment 8610547 [details] [diff] [review]
Patch v2: WINDOW_ENABLED telemetry key is falsy, thus not working. Check for its type properly

Approved for uplift to aurora and beta to fix a regression in 39+ that affects telemetry.
Attachment #8610547 - Flags: approval-mozilla-beta?
Attachment #8610547 - Flags: approval-mozilla-beta+
Attachment #8610547 - Flags: approval-mozilla-aurora?
Attachment #8610547 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.