945499 - Switch BrowserUITelemetry from using UITelemetry's event logging to just counting events

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

Conversations with geekboy in IRC and bug 945347, as well as vladan's concerns in bug 944115 and bug 944116 make me think it'd be better if we forgo UITelemetry's built-in event logging for something a little simpler: just counting events, and reporting the counts.

I think that approach is more privacy-centric / privacy-friendly, and will also reduce the potential packet-size for long-running sessions.

Comment 1

[Blake Winton (:bwinton) (:☕️)]

I'm fine with that, with the exception of the two cases we talked about earlier.
(Amount of time spent in customize mode, and clicking on the bookmarks menu after clicking on the the bookmarks button, iirc.)

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1.1

Comment 4

[:Gijs (he/him)]

Comment on attachment 8341880 [details] [diff] [review]
Patch v1.1

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

LGTM!

::: browser/modules/BrowserUITelemetry.jsm
@@ +213,5 @@
> +  _countableEvents: {},
> +  _countEvent: function(aCategory, aAction) {
> +    if (!(aCategory in this._countableEvents)) {
> +      this._countableEvents[aCategory] = {};
> +    }

At this point I would stuff the object in an intermediate, ie:

let categoryEvents = this._countableEvents[aCategory];

Just to make the rest of the fn easier to read. But that's just me, can just land as is if you're not bothered.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Landed on Holly as https://hg.mozilla.org/projects/holly/rev/a7d753c6f061

We'll be doing the same thing for the probes that land on mozilla-central, but I'll do the forward-port in one shot, rather than piecemeal.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8341880 [details] [diff] [review]
Patch v1.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 

This is to land at the same time as bug 942244. This fixes an issue brought up by the Privacy team where BrowserUITelemetry was using UITelemetry's internal sequential event logging mechanism which was deemed too privacy invasive. This patch switches from using that mechanism to simply counting events.


User impact if declined: 

If bug 942244 lands and this one does not, a sequential log of (whitelisted) user activities in the Firefox Appmenu and during customization will be sent in Telemetry packets, which is not great for privacy.


Testing completed (on m-c, etc.): 

Lots of manual testing on Holly, which has since merged to Aurora.


Risk to taking this patch (and alternatives if risky): 

None.


String or IDL/UUID changes made by this patch:

None.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Removing [Australis:P1] since these block a P1 already. Let's not be redundant and noisy.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Landed in mozilla-beta as https://hg.mozilla.org/releases/mozilla-beta/rev/71be3d716c22