VOOZH about

URL: https://phabricator.services.mozilla.com/D293953

⇱ ⚙ D293953 Bug 2031308: Add a country field to the pageload domain event that is filled in for some countries on release. r=denispal


Bug 2031308: Add a country field to the pageload domain event that is filled in for some countries on release. r=denispal
ClosedPublic

Authored by bas on Apr 13 2026, 6:05 PM.
Referenced Files
Unknown Object (File)
Wed, Jun 3, 6:35 PM
Unknown Object (File)
Wed, Jun 3, 12:50 PM
Unknown Object (File)
Wed, Jun 3, 8:18 AM
Unknown Object (File)
Wed, Jun 3, 3:31 AM
Unknown Object (File)
Wed, Jun 3, 3:28 AM
Unknown Object (File)
Wed, Jun 3, 2:49 AM
Unknown Object (File)
Tue, Jun 2, 5:16 PM
Unknown Object (File)
Tue, Jun 2, 5:14 PM
Subscribers

Details

Summary

This adds country data to our pageload events with domain information. Since this information can be highly fingerprintable for situations with few users in a country/channel combination, we only collect this on release, for users in specific countries where we know we have a good amount of users. This should bring the majority of the value with no risk of accidentally collecting user data.

Diff Detail

Event Timeline

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: secure-revision.
Comment Actions

This revision changes how Firefox collects data, so it needs a #data-classification tag.

If you (the patch author) are adding new or modifying existing data collection, you and your reviewer(s) should judge how sensitive this data is, using the data collection categories Mozilla uses. If the data you are collecting fits in either the Category 1 “technical data” or Category 2 “interaction data” described there, please add the #data-classification-low tag. If it’s any other category, or you and your reviewer(s) disagree, please add the #data-classification-high tag, and go through the sensitive data collection review process. If you think that the data in question fits in “technical” or “interaction” data but would benefit from additional review, you can also explicitly choose to use the #data-classification-high tag and thereby opt in to the sensitive data collection review process.

When using Glean for the data collection, the data classification of the new or expanded data collections should match the property in the metric definitions.

If you are removing data collection, or are making mechanical changes to these files that do not add or modify what data is collected, change when or how it is collected, please add the data-classification-unnecessary tag.

If you are unsure or feel uncomfortable making this assessment yourself, please ask the data-stewards group for help.

Whichever tag is used, please leave a comment explaining this choice. Note that Lando will not be able to land this revision until the revision has one of these tags and you remove the #needs-data-classification tag.

Finally, you as a patch author are encouraged to add the right tags yourself, but your reviewers are responsible for making sure the right tag is used.

bas retitled this revision from Bug 2030147: Add a country field to the pageload domain event that is filled in for some countries on release. r=denispal to Bug 2031308: Add a country field to the pageload domain event that is filled in for some countries on release. r=denispal.
bas changed the Bugzilla Bug ID from 2030147 to 2031308.
This comment was removed by jfkthame.
Comment Actions

Code analysis found 1 defect in diff 1247840:

  • 1 defect found by clang-format (Mozlint)
WARNING: Found 1 defect (warning level) that can be dismissed.

You can run this analysis locally with:


If you see a problem in this automated review, please report it here.

You can view these defects in the Diff Detail section of Phabricator diff 1247840.

bas edited projects, added Restricted Project; removed Restricted Project.Apr 13 2026, 8:08 PM
Comment Actions

This revision is now going through the sensitive data review process. As a patch author, please follow the steps described in that process by:

  1. Documenting the details of the implementation, intended use, and value to users for future reference and efficient review.
  1. Attach a filled in copy of the request template to the bug, marking the content type as text/plain.
  1. Emailing the data-review@mozilla.com list with the details from step (1) and (2)

More details on the rest of the process are on the wiki.

Someone from the data-stewards group should approve this revision for landing if/when the review process is complete and approval is given.

Comment Actions

LGTM, but I added a couple comments to consider.

tools/performance/PageloadEvent.cpp
9

You might need a here or it could fail some non-unified builds.

336

Do we expect this value to change during a browser session? Can we cache it instead in a or something?

tools/performance/PageloadEvent.cpp
336

It seems to normally be set once, although it can be updated. Is there an advantage here to caching it separately? Preferences::GetCString should be fast? Although if you have different experiences I can definitely cache it somehow. (although using a static char[3] is something I'd be pretty careful with from a security perspective)

tools/performance/PageloadEvent.cpp
336

The advantage would be just so we don't look this up every page load. But if it's fast enough, then I guess it's fine.

Comment Actions

Code analysis found 1 defect in diff 1253231:

  • 1 defect found by clang-format (Mozlint)
WARNING: Found 1 defect (warning level) that can be dismissed.

You can run this analysis locally with:


If you see a problem in this automated review, please report it here.

You can view these defects in the Diff Detail section of Phabricator diff 1253231.

Roger added a subscriber: Roger.
Comment Actions

Accept data classification high as data steward. This collection has been approved through the sensitive data collection review process.

This revision is now accepted and ready to land.Apr 29 2026, 1:33 PM
Comment Actions

This revision requires a Testing Policy Project Tag to be set before landing. Please apply one of , , , , . Tip: this Firefox add-on makes it easy!

bas marked 2 inline comments as done.
bas marked an inline comment as done.

Revision Contents

PathSize
dom/
7 lines
tools/
performance/
22 lines
CommitTreeParentsAuthorSummaryDate
0e291258508220972a35d39bBas Schouten
Bug 2031308: Add a country field to the pageload domain event that is filled in… (Show More…)

Diff 1261666

dom/metrics.yaml

Loading...

tools/performance/PageloadEvent.cpp

Loading...