1166646 - WINDOW_ENABLED sharing telemetry probe not working

Status: RESOLVED FIXED

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

Description

[Mike de Boer [:mikedeboer]]

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.

Comment 1

[Mike de Boer [:mikedeboer]]

Attachment: Patch v1: WINDOW_ENABLED telemetry key is falsy, thus not working. Check for its type properly

Comment 2

[Mike de Boer [:mikedeboer]]

[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 3

[Mark Banner (:standard8)]

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.

Comment 4

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking for 39+  Please request uplift to mozilla-beta and aurora when you're ready.

Comment 5

[Mike de Boer [:mikedeboer]]

Attachment: Patch v2: WINDOW_ENABLED telemetry key is falsy, thus not working. Check for its type properly

Comment 6

[Mark Banner (:standard8)]

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

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/ddd02b236cc2

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ddd02b236cc2

Comment 9

[Mike de Boer [:mikedeboer]]

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.

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a2f619521d74

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f8bbcd803829