1556766 - Add telemetry for WebRTC video codecs used in calls

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, enhancement, P2)

Description

[Dan Minor [:dminor]]

We would like to collect telemetry on how frequently a particular codec type is being used in calls, e.g. how many calls are VP8 vs. VP9 vs. H.264. It would also be useful to collect telemetry on audio codec usage.

Comment 1

[Dan Minor [:dminor]]

From discussion with :drno, knowing video codec usage is the highest priority, so we won't worry about audio right now.

Comment 2

[Dan Minor [:dminor]]

Attachment: video-codec-use-request.txt

Comment 3

[Chris H-C :chutten]

Comment on attachment 9069938 [details]
video-codec-use-request.txt

Can you please expand upon what form this data collection will take? Will it be in Telemetry (and thus have expiry and documentation and user opt-out taken care of for you?), how wide is the domain that it covers (can codecs be arbitrary strings, or chosen from a predetermined list), and so forth?

Maybe it would be best to delay this review until the patch containing the collection is up for code review.

Comment 4

[Dan Minor [:dminor]]

Attachment: Bug 1556766 - Add telemetry for WebRTC video codecs used in calls; r=chutten

This adds a categorical histogram for the codecs used when receiving and
sending video in a WebRTC call.

Comment 5

[Dan Minor [:dminor]]

(In reply to Chris H-C :chutten from comment #3)

Comment on attachment 9069938 [details]
video-codec-use-request.txt

Can you please expand upon what form this data collection will take? Will it
be in Telemetry (and thus have expiry and documentation and user opt-out
taken care of for you?), how wide is the domain that it covers (can codecs
be arbitrary strings, or chosen from a predetermined list), and so forth?

Yes it will be in Telemetry. Codecs can be arbitrary strings, but we expect to see the values in this list [1]. The only expected values here are VP8, VP9 and H264, but seeing other values here would help us detect bugs in our signaling code. If it seems like we are gathering too much data, I'd be happy to make this a categorical with just VP8, VP9, H264, and Unknown.

[1] https://searchfox.org/mozilla-central/rev/153172de0c5bfca31ef861bd8fc0995f44cada6a/media/webrtc/trunk/webrtc/common_types.cc#166

Maybe it would be best to delay this review until the patch containing the
collection is up for code review.

Patch is up for review. Sorry, it seemed to me from the Telemetry documentation that one should ask for a data review prior to putting the patch up for review. Thanks for having a look!

Comment 6

[Chris H-C :chutten]

Comment on attachment 9069938 [details]
video-codec-use-request.txt

Preliminary note:

It is best practice to write automated tests for permanent collections to ensure new changes don't break the collection or change it unexpectedly.

DATA COLLECTION REVIEW RESPONSE:

    Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes. This collection is Telemetry so is documented in its definitions file [Scalars.yaml](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/Scalars.yaml) and the [Probe Dictionary](https://telemetry.mozilla.org/probe-dictionary/).

    Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.

    If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes, Dan Minor and Nils Olhlmeier are responsible.

    Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 2, Interaction. (Though the codec used is technical, the number of webrtc calls could maybe make the case that this is interaction. So let's call it Cat2 just to be prudent)

    Is the data collection request for default-on or default-off?

Default on for all channels.

    Does the instrumentation include the addition of any new identifiers?

No.

    Is the data collection covered by the existing Firefox privacy notice?

Yes.

    Does there need to be a check-in in the future to determine whether to renew the data?

No. This collection is permanent.

---
Result: datareview+

Comment 7

[Chris H-C :chutten]

Categorical would of course be preferable (fewer chances for strings to mess with us). Whether it's keyed uint scalar or categorical histogram doesn't change the intent of the collection so I proceeded with the data review.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a0577755db34
Add telemetry for WebRTC video codecs used in calls; r=chutten,drno

Comment 9

[Noemi Erli[:noemi_erli]]

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

Comment 10

[Dan Minor [:dminor]]

Comment on attachment 9070061 [details]
Bug 1556766 - Add telemetry for WebRTC video codecs used in calls; r=chutten

Beta/Release Uplift Approval Request

• User impact if declined: We will use this data in the short term to prioritize ongoing work on media codecs. Without data from Beta, we won't have a large enough user base to make meaningful decisions.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Low risk, this only adds two telemetry probes.
• String changes made/needed: None

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9070061 [details]
Bug 1556766 - Add telemetry for WebRTC video codecs used in calls; r=chutten

Needed to better understand WebRTC video codec usage on Fennec. Approved for Beta. Let's get this uplifted ASAP so we start getting Nightly & Beta Fennec telemetry results.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0199db54157108d082011d30ae4a6c9788a38e83