Extend WebRTC ICE Telemetry probes

RESOLVED FIXED in Firefox 53

Status

()

Core
WebRTC
P1
normal
Rank:
15
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: drno, Assigned: drno)

Tracking

53 Branch
mozilla53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Comment hidden (empty)
Comment hidden (mozreview-request)
(Assignee)

Comment 2

a year ago
I just realized that a whole bunch of WebRTC ICE related telemetry probes is about to expire.

:bsmedberg can we extend these from "53" to "never", or is "never" only a legacy value which can't be used in the future any more?
Flags: needinfo?(benjamin)
(Assignee)

Updated

a year ago
backlog: --- → webrtc/webaudio+
Rank: 15
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8812970 - Flags: review?(rvitillo)

Comment 4

a year ago
"never" does exist, and the bar for never is something like this:
* You have a clear question that is worth answering forever. 
* You have a monitoring plan in place with somebody identified. This could be a dashboard that you commit to looking at regularly, or some sort of alerting system.
Flags: needinfo?(benjamin)
I'd like to get a little more clarity on the second point. In many cases, we need this kind of data not because we are monitoring for problems but because when problems get reported through other channels we need to be able to go back and look at the data to answer questions. That's a pretty valuable activity and I think a legitimate one. Do you consider this an acceptable standard?
Flags: needinfo?(benjamin)

Comment 6

a year ago
In abstract terms yes, post-hoc diagnostics to solve a real product/business problem are a fine reason to collect data.

In concrete terms, the Firefox data stewards want to periodically review this sort of prospective data collection to make sure that it's still expected to provide value, and that the tradeoffs (potential diagnostic value) are worth the costs (both in terms of privacy/data risk as well as processing/storage cost).

IIRC the data risks here were small, mainly some fingerprinting of IP/network configurations.  At the time this was originally proposed this was primarily for Hello. Now that webrtc is gaining wider deployment, what is the actual volume of data we're receiving and does that match your expected cost profile? How do we expect that volume to change as webrtc gains more deployment? Are we vulnerable to volume spikes and costs if popular websites misdeploy webrtc? Have we successfully used this data to solve problems in the past?

In this particular case, I would expect monitoring to at least include the volume of incoming data in terms of cost analysis, although large volume also seems like a signal that engineering teams should investigate. I believe Georg already has a cost dashboard by ping type, so that might just mean your teams needs to monitor that periodically.
Flags: needinfo?(benjamin) → needinfo?(drno)
Attachment #8812970 - Flags: review?(rvitillo) → review?(benjamin)

Comment 7

a year ago
Comment on attachment 8812970 [details]
Bug 1319268: extend WebRTC ICE Telemetry probes from 53 to 58.

Review queue maintenance. Please reset review when questions answered.
Attachment #8812970 - Flags: review?(benjamin)
(Assignee)

Comment 8

a year ago
To partially answer the questions from comment #6:

No we don't have a dash board yet. But we are more then interested to build one.

The ICE Telemetry as pretty much all of WebRTC's data gets collected since way before Hello. We created separate buckets (?) for the same data with the LOOP prefix instead of the WEBRTC prefix to be able to have distinct data from the controlled Hello environment compared to the rest of the WebRTC wild wild west. But all of these separate buckets got removed when Hello got removed.

I don't see how any of the data collected by WebRTC Telemetry Histrograms could be used to fingerprint IP addresses or networks as we never submit any of these as part of these probes.

Hello had a separate "feature" which submitted ICE call failure logs in case of a call failure into the Telemetry storage backend. The anonymized data submitted there had the potential for fingerprinting. But this is gone since Hello is turned off now.

I'm pretty sure that the data has at least directed us into investigating problems more closely. Although I doubt that just the Telemetry data itself ever resulted in a bug fix.

I'm assuming the costs and/or the volume of WebRTC related Telemetry data is low compared to other areas since all of WebRTC probes only report something after WebRTC was used. Since WebRTC is not used that widely yet I assume that results in low volume. But we are happy to look and monitor any data Georg can provide.
Flags: needinfo?(drno) → needinfo?(gfritzsche)
(In reply to Nils Ohlmeier [:drno] from comment #8)
> But we are happy to look and monitor any data Georg can provide.

I'm not sure what data you are looking for that i could point you to.

If this is just about the histograms in this bug, then they are fine volume-wise.
It's a handful of opt-in histograms with lowish bucket counts and non-keyed.
Flags: needinfo?(gfritzsche)

Comment 10

11 months ago
Comment on attachment 8812970 [details]
Bug 1319268: extend WebRTC ICE Telemetry probes from 53 to 58.

I think I was confused, and most of my questions were about extended the ICE ping. If this is only about histograms, then most of the questions about cost monitoring and fingerprinting are irrelevant.  Does the ICE ping still exist and are you planning on extending that as well?

I'm worried that this kind of histogram telemetry is hardly ever useful for post-event diagnosis and is mainly useful for alerting when there are errors (and we're missing the alerting), but if your team thinks this is providing value that's ok. Note that none of these are opt-out, so they're only for the prerelease audience. Is that the only audience you care about? if so data-review=me for this patch.
Attachment #8812970 - Flags: feedback+
Comment hidden (mozreview-request)
(Assignee)

Comment 12

11 months ago
Lets extend to 58 for now to avoid going blind.

We will hopefully work on a dashboard hopefully soon.

Comment 13

11 months ago
mozreview-review
Comment on attachment 8812970 [details]
Bug 1319268: extend WebRTC ICE Telemetry probes from 53 to 58.

https://reviewboard.mozilla.org/r/94500/#review106696

These changes are straightforward and good. Better would be if you added bug_numbers fields containing 1319268 to the probes you changed (and then we can remove the probes from the "doesn't have bug_numbers" whitelist in histogram-whitelists.json) but that can be accomplished in a follow-up patch.
Attachment #8812970 - Flags: review?(chutten) → review+
Comment hidden (mozreview-request)

Comment 15

11 months ago
mozreview-review
Comment on attachment 8812970 [details]
Bug 1319268: extend WebRTC ICE Telemetry probes from 53 to 58.

https://reviewboard.mozilla.org/r/94500/#review106794

Comment 16

11 months ago
Pushed by drno@ohlmeier.org:
https://hg.mozilla.org/integration/autoland/rev/1a92810b34d2
extend WebRTC ICE Telemetry probes from 53 to 58. r=chutten

Comment 17

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1a92810b34d2
Status: NEW → RESOLVED
Last Resolved: 11 months ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.