1237401 - Update youtube rewrite telemetry probes to cover all cases (YOUTUBE_NONREWRITABLE_EMBED_SEEN)

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Once bug 769117 lands, it'd be nice to know how many flash youtube embeds we're rewriting, and how many we're leaving alone. We should expand our telemetry probes to cover all of these cases.

Comment 1

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v1) - Add telemetry probe for non-rewritable youtube flash embeds; r=cpeterson

Will put up for review once bug 769117 lands, as this patch could change based on those reviews.

Comment 2

[Chris Peterson [:cpeterson]]

hi Kyle, is your YOUTUBE_NONREWRITABLE_EMBED_SEEN patch ready for review? Our YouTube contact estimated that only 5% of Flash embeds are scripted (enablejsapi=1), so it will be interesting to compare with our telemetry probe.

Comment 3

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

I think it should be ok now.

Comment 4

[Chris Peterson [:cpeterson]]

Comment on attachment 8704913 [details] [diff] [review]
Patch 1 (v1) - Add telemetry probe for non-rewritable youtube flash embeds; r=cpeterson

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

LGTM!

Comment 5

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8704913 [details] [diff] [review]
Patch 1 (v1) - Add telemetry probe for non-rewritable youtube flash embeds; r=cpeterson

Forgot to have a metrics peer review.

Comment 6

[Vladan Djeric (:vladan)]

Comment on attachment 8704913 [details] [diff] [review]
Patch 1 (v1) - Add telemetry probe for non-rewritable youtube flash embeds; r=cpeterson

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

::: toolkit/components/telemetry/Histograms.json
@@ +10033,5 @@
>    },
> +  "YOUTUBE_NONREWRITABLE_EMBED_SEEN": {
> +    "alert_emails": ["[e-mail]"],
> +    "expires_in_version": "48",
> +    "kind": "flag",

This probe will log False in all sessions where a non-rewriteable YouTube embed was NOT seen, even sessions that didn't visit YouTube. Is that the # you're after?

Also note that this probe will collect data from Release channel on an opt-in basis.

Comment 7

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

(In reply to Vladan Djeric (:vladan) -- please needinfo from comment #6)
> Comment on attachment 8704913 [details] [diff] [review]
> Patch 1 (v1) - Add telemetry probe for non-rewritable youtube flash embeds;
> r=cpeterson
> 
> Review of attachment 8704913 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/telemetry/Histograms.json
> @@ +10033,5 @@
> >    },
> > +  "YOUTUBE_NONREWRITABLE_EMBED_SEEN": {
> > +    "alert_emails": ["[e-mail]"],
> > +    "expires_in_version": "48",
> > +    "kind": "flag",
> 
> This probe will log False in all sessions where a non-rewriteable YouTube
> embed was NOT seen, even sessions that didn't visit YouTube. Is that the #
> you're after?

Yeah, we're after the total percentage of sessions that see a youtube embed with enablejsapi=1. We expect this to be fairly tiny.

> 
> Also note that this probe will collect data from Release channel on an
> opt-in basis.

ni?'ing :cpeterson. Not sure how widespread we want this.

Comment 8

[Chris Peterson [:cpeterson]]

(In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #7)
> (In reply to Vladan Djeric (:vladan) -- please needinfo from comment #6)
> > Also note that this probe will collect data from Release channel on an
> > opt-in basis.
> 
> ni?'ing :cpeterson. Not sure how widespread we want this.

I think that is OK. We don't care about exact count of Flash embeds, just the scale of the problem.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c525c5164cdd

Comment 10

[Carsten Book [:Tomcat]]

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