1647762 - Interaction telemetry includes add-on IDs

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Dave Townsend [:mossop]]

It looks like the interaction telemetry gathered in bug 1620358 is including add-on IDs when the intention was to mask those. It looks like they are coming from keyboard shortcuts.

What is the correct course of action when we find we are gathering more data than intended?

Comment 1

[Chris H-C :chutten]

We ask Trust.

To guide our thinking there are a few engineering things I've seen happen in the past:

• We immediately ship a fix to stop collecting the extra data (this is a good first step regardless of other things we do)
• We bring in Data Pipeline folks to write a filter so that data coming in from affected clients is transformed to not have the extra data
• We brainstorm ways to keep this from happening in the future

There may also be other things to do, but agray's the one who'll tell us what to do.

Comment 2

[Julien Cristau [:jcristau]]

Should we back out 1620358 from 78 and respin a release candidate? It'd be good to know that today.

Comment 3

[Dave Townsend [:mossop]]

Attachment: Bug 1647762: Strip add-on identifiers from keyboard shortcut interaction telemetry. r=Gijs!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a021e03c7d44
Strip add-on identifiers from keyboard shortcut interaction telemetry. r=Gijs

Comment 5

[Dave Townsend [:mossop]]

(In reply to Julien Cristau [:jcristau] from comment #2)

Should we back out 1620358 from 78 and respin a release candidate? It'd be good to know that today.

Well I know we really want the data we'd be losing by backing out for the Flourish project so my preference would be to uplift the straightforward patch in this bug and spin an RC with that but I realise we're getting pretty late here. I'd also like to hear what Trust have to say about this.

Comment 6

[Alicia Gray]

Hi all, Trust review/guidance as follows:

1. The original collection is Cat 2 so we don't need to delete the ID collection in totality.
2. Prepare a fix for the on-going collection
3. Mask what we've already collected or set a filter such as :chutten mentioned in Comment 1
4. Update the data collection documentation to accurately reflect what was collected and when.
5. Take steps to make sure that part of the process going forward includes a way that could help spot identifier errors earlier

Let me know if there are more questions I can help with.

Comment 7

[Dave Townsend [:mossop]]

My read of that is that we don't need to back-out and respin here.

Comment 8

[Alicia Gray]

(In reply to Dave Townsend [:mossop] (he/him) from comment #7)

My read of that is that we don't need to back-out and respin here.

Yes, correct. Definitely put in a fix going forward for new/on-going collection. Where we've already collected the add-on ID, that should be sanitized or filtered. You do not have to go back and delete what has already been collected.

Comment 9

[Noemi Erli[:noemi_erli]]

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

Comment 10

[Julien Cristau [:jcristau]]

We're going to build 78 rc2 today anyway, so assuming we're getting what we expect from nightly now, feel free to request uplift here and I'll include this patch so we don't have that extra collection on release.

Comment 11

[Dave Townsend [:mossop]]

Comment on attachment 9158986 [details]
Bug 1647762: Strip add-on identifiers from keyboard shortcut interaction telemetry. r=Gijs!

Beta/Release Uplift Approval Request

• User impact if declined: We will submit interaction data including add-on identifiers from users on release. This was not intended in the original data review approval.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: Bug 1648251
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Mostly the code is just filtering a pattern of identifiers. I have verified it in my local nightly and used a telemetry query to verify that no builds produced with the fix are submitting identifiers of that pattern. Bug 1648251 is an optional additional fix that will stop test-verify runs from failing.
• String changes made/needed:

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 9158986 [details]
Bug 1647762: Strip add-on identifiers from keyboard shortcut interaction telemetry. r=Gijs!

approved for 78 rc2

Comment 13

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/37d625eb4f58

Comment 14

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/37d625eb4f58