Remove mask_id from email event data

api/tests/views_tests.py

@@ -97,14 +97,12 @@ def test_post_domainaddress_success(
     assert ret_data["full_address"].startswith("my-new-mask@premium.")
 
     assert (event := get_glean_event(caplog)) is not None
-    address = premium_user.domainaddress_set.get()
     expected_event = create_expected_glean_event(
         category="email_mask",
         name="created",
         user=premium_user,
         extra_items={
             "n_domain_masks": "1",
-            "mask_id": address.metrics_id,
             "is_random_mask": "false",
             "created_by_api": "true",
             "has_website": "false",
@@ -446,7 +444,6 @@ def test_patch_domainaddress(
             user=premium_user,
             extra_items={
                 "n_domain_masks": "1",
-                "mask_id": existing.metrics_id,
                 "is_random_mask": "false",
             },
             event_time=event["timestamp"],
@@ -605,7 +602,6 @@ def test_delete_domainaddress(
     prem_api_client: APIClient, premium_user: User, caplog: pytest.LogCaptureFixture
 ) -> None:
     existing = DomainAddress.objects.create(user=premium_user, address="my-doomed-mask")
-    existing_mask_id = existing.metrics_id
     url = reverse("domainaddress-detail", args=[existing.id])
     response = prem_api_client.delete(url)
     assert response.status_code == 204
@@ -618,7 +614,6 @@ def test_delete_domainaddress(
         user=premium_user,
         extra_items={
             "n_deleted_domain_masks": "1",
-            "mask_id": existing_mask_id,
             "is_random_mask": "false",
         },
         event_time=event["timestamp"],
@@ -639,14 +634,12 @@ def test_post_relayaddress_success(
     assert ret_data["enabled"]
 
     assert (event := get_glean_event(caplog)) is not None
-    address = free_user.relayaddress_set.get()
     expected_event = create_expected_glean_event(
         category="email_mask",
         name="created",
         user=free_user,
         extra_items={
             "n_random_masks": "1",
-            "mask_id": address.metrics_id,
             "is_random_mask": "true",
             "created_by_api": "true",
             "has_website": "false",
@@ -677,7 +670,6 @@ def test_post_relayaddress_with_generated_for_success(
             "n_random_masks": "1",
             "has_extension": "true",
             "date_got_extension": str(int(address.created_at.timestamp())),
-            "mask_id": address.metrics_id,
             "is_random_mask": "true",
             "created_by_api": "true",
             "has_website": "true",
@@ -773,7 +765,6 @@ def test_patch_relayaddress(
             user=free_user,
             extra_items={
                 "n_random_masks": "1",
-                "mask_id": existing.metrics_id,
                 "is_random_mask": "true",
             },
             event_time=event["timestamp"],
@@ -933,7 +924,6 @@ def test_delete_randomaddress(
     free_api_client: APIClient, free_user: User, caplog: pytest.LogCaptureFixture
 ) -> None:
     existing = RelayAddress.objects.create(user=free_user)
-    existing_mask_id = existing.metrics_id
     url = reverse("relayaddress-detail", args=[existing.id])
     response = free_api_client.delete(url)
     assert response.status_code == 204
@@ -946,7 +936,6 @@ def test_delete_randomaddress(
         user=free_user,
         extra_items={
             "n_deleted_random_masks": "1",
-            "mask_id": existing_mask_id,
             "is_random_mask": "true",
         },
         event_time=event["timestamp"],

api/views/__init__.py

@@ -140,14 +140,12 @@ class AddressViewSet(Generic[_Address], SaveToRequestUser, viewsets.ModelViewSet
             )
 
     def perform_destroy(self, instance: _Address) -> None:
-        mask_id = instance.metrics_id
         user = instance.user
         is_random_mask = isinstance(instance, RelayAddress)
         super().perform_destroy(instance)
         glean_logger().log_email_mask_deleted(
             request=self.request,
             user=user,
-            mask_id=mask_id,
             is_random_mask=is_random_mask,
         )
 

emails/tests/views_tests.py

@@ -498,7 +498,6 @@ class SNSNotificationIncomingTest(SNSNotificationTestBase):
         assert (event := get_glean_event(caplog)) is not None
         assert event["category"] == "email"
         assert event["name"] == "forwarded"
-        assert event["extra"]["mask_id"] == self.ra.metrics_id
 
     @override_settings(STATSD_ENABLED=True)
     def test_spamVerdict_FAIL_auto_block_doesnt_relay(self) -> None:
@@ -532,7 +531,6 @@ class SNSNotificationIncomingTest(SNSNotificationTestBase):
         assert mask_event is not None
         shared_extra_items = {
             "n_domain_masks": "1",
-            "mask_id": da.metrics_id,
             "is_random_mask": "false",
         }
         expected_mask_event = create_expected_glean_event(
@@ -792,7 +790,6 @@ class SNSNotificationRepliesTest(SNSNotificationTestBase):
         assert (event := get_glean_event(caplog)) is not None
         assert event["category"] == "email"
         assert event["name"] == "forwarded"
-        assert event["extra"]["mask_id"] == self.relay_address.metrics_id
         assert event["extra"]["is_reply"] == "true"
 
         self.mock_remove_message_from_s3.assert_called_once()
@@ -1366,7 +1363,6 @@ class SNSNotificationValidUserEmailsInS3Test(TestCase):
     ) -> dict[str, Any]:
         extra_items = {
             "n_random_masks": "1",
-            "mask_id": self.address.metrics_id,
             "is_random_mask": "true",
             "is_reply": "true" if is_reply else "false",
         }
@@ -1773,7 +1769,6 @@ class GetAddressTest(TestCase):
             extra_items={
                 "n_random_masks": "1",
                 "n_domain_masks": "2",
-                "mask_id": address.metrics_id,
                 "is_random_mask": "false",
                 "has_website": "false",
                 "created_by_api": "false",
@@ -1806,7 +1801,6 @@ class GetAddressTest(TestCase):
             extra_items={
                 "n_random_masks": "1",
                 "n_domain_masks": "2",
-                "mask_id": address.metrics_id,
                 "is_random_mask": "false",
                 "has_website": "false",
                 "created_by_api": "false",

privaterelay/glean/server_events.py

@@ -104,7 +104,6 @@ class EventsServerEventLogger:
         date_joined_premium: int,
         has_extension: bool,
         date_got_extension: int,
-        mask_id: str,
         is_random_mask: bool,
         is_reply: bool,
         reason: str,
@@ -128,7 +127,6 @@ class EventsServerEventLogger:
         :param int date_joined_premium: Timestamp for starting premium_status subscription, seconds since epoch, -1 if not subscribed
         :param bool has_extension: The user has the Relay Add-on
         :param int date_got_extension: Timestamp for adding Relay Add-on, seconds since epoch, -1 if not used
-        :param str mask_id: Mask ID, 'R' (random mask) or 'D' (domain mask) followed by a number.
         :param bool is_random_mask: The mask is a random mask, instead of a domain mask
         :param bool is_reply: The email is a reply from the Relay user
         :param str reason: Code describing why the email was blocked
@@ -148,7 +146,6 @@ class EventsServerEventLogger:
                 "date_joined_premium": str(date_joined_premium),
                 "has_extension": str(has_extension).lower(),
                 "date_got_extension": str(date_got_extension),
-                "mask_id": str(mask_id),
                 "is_random_mask": str(is_random_mask).lower(),
                 "is_reply": str(is_reply).lower(),
                 "reason": str(reason),
@@ -171,7 +168,6 @@ class EventsServerEventLogger:
         date_joined_premium: int,
         has_extension: bool,
         date_got_extension: int,
-        mask_id: str,
         is_random_mask: bool,
         is_reply: bool,
     ) -> None:
@@ -194,7 +190,6 @@ class EventsServerEventLogger:
         :param int date_joined_premium: Timestamp for starting premium_status subscription, seconds since epoch, -1 if not subscribed
         :param bool has_extension: The user has the Relay Add-on
         :param int date_got_extension: Timestamp for adding Relay Add-on, seconds since epoch, -1 if not used
-        :param str mask_id: Mask ID, 'R' (random mask) or 'D' (domain mask) followed by a number.
         :param bool is_random_mask: The mask is a random mask, instead of a domain mask
         :param bool is_reply: The email is a reply from the Relay user
         """
@@ -213,7 +208,6 @@ class EventsServerEventLogger:
                 "date_joined_premium": str(date_joined_premium),
                 "has_extension": str(has_extension).lower(),
                 "date_got_extension": str(date_got_extension),
-                "mask_id": str(mask_id),
                 "is_random_mask": str(is_random_mask).lower(),
                 "is_reply": str(is_reply).lower(),
             },
@@ -235,7 +229,6 @@ class EventsServerEventLogger:
         date_joined_premium: int,
         has_extension: bool,
         date_got_extension: int,
-        mask_id: str,
         is_random_mask: bool,
         created_by_api: bool,
         has_website: bool,
@@ -259,7 +252,6 @@ class EventsServerEventLogger:
         :param int date_joined_premium: Timestamp for starting premium_status subscription, seconds since epoch, -1 if not subscribed
         :param bool has_extension: The user has the Relay Add-on
         :param int date_got_extension: Timestamp for adding Relay Add-on, seconds since epoch, -1 if not used
-        :param str mask_id: Mask ID, 'R' (random mask) or 'D' (domain mask) followed by a number.
         :param bool is_random_mask: The mask is a random mask, instead of a domain mask
         :param bool created_by_api: The mask was created via the API, rather than an incoming email
         :param bool has_website: The mask was created by the Add-on or integration on a website
@@ -279,7 +271,6 @@ class EventsServerEventLogger:
                 "date_joined_premium": str(date_joined_premium),
                 "has_extension": str(has_extension).lower(),
                 "date_got_extension": str(date_got_extension),
-                "mask_id": str(mask_id),
                 "is_random_mask": str(is_random_mask).lower(),
                 "created_by_api": str(created_by_api).lower(),
                 "has_website": str(has_website).lower(),
@@ -302,7 +293,6 @@ class EventsServerEventLogger:
         date_joined_premium: int,
         has_extension: bool,
         date_got_extension: int,
-        mask_id: str,
         is_random_mask: bool,
     ) -> None:
         """
@@ -324,7 +314,6 @@ class EventsServerEventLogger:
         :param int date_joined_premium: Timestamp for starting premium_status subscription, seconds since epoch, -1 if not subscribed
         :param bool has_extension: The user has the Relay Add-on
         :param int date_got_extension: Timestamp for adding Relay Add-on, seconds since epoch, -1 if not used
-        :param str mask_id: Mask ID, 'R' (random mask) or 'D' (domain mask) followed by a number.
         :param bool is_random_mask: The mask is a random mask, instead of a domain mask
         """
         event = {
@@ -342,7 +331,6 @@ class EventsServerEventLogger:
                 "date_joined_premium": str(date_joined_premium),
                 "has_extension": str(has_extension).lower(),
                 "date_got_extension": str(date_got_extension),
-                "mask_id": str(mask_id),
                 "is_random_mask": str(is_random_mask).lower(),
             },
         }
@@ -363,7 +351,6 @@ class EventsServerEventLogger:
         date_joined_premium: int,
         has_extension: bool,
         date_got_extension: int,
-        mask_id: str,
         is_random_mask: bool,
     ) -> None:
         """
@@ -385,7 +372,6 @@ class EventsServerEventLogger:
         :param int date_joined_premium: Timestamp for starting premium_status subscription, seconds since epoch, -1 if not subscribed
         :param bool has_extension: The user has the Relay Add-on
         :param int date_got_extension: Timestamp for adding Relay Add-on, seconds since epoch, -1 if not used
-        :param str mask_id: Mask ID, 'R' (random mask) or 'D' (domain mask) followed by a number.
         :param bool is_random_mask: The mask is a random mask, instead of a domain mask
         """
         event = {
@@ -403,7 +389,6 @@ class EventsServerEventLogger:
                 "date_joined_premium": str(date_joined_premium),
                 "has_extension": str(has_extension).lower(),
                 "date_got_extension": str(date_got_extension),
-                "mask_id": str(mask_id),
                 "is_random_mask": str(is_random_mask).lower(),
             },
         }

privaterelay/glean_interface.py

@@ -113,22 +113,18 @@ class UserData(NamedTuple):
 class EmailMaskData(NamedTuple):
     """Extract and store data from a Relay email mask."""
 
-    mask_id: str
     is_random_mask: bool
     has_website: bool
 
     @classmethod
     def from_mask(cls, mask: RelayAddress | DomainAddress) -> EmailMaskData:
-        mask_id = mask.metrics_id
         if isinstance(mask, RelayAddress):
             is_random_mask = True
             has_website = bool(mask.generated_for)
         else:
             is_random_mask = False
             has_website = False
-        return EmailMaskData(
-            mask_id=mask_id, is_random_mask=is_random_mask, has_website=has_website
-        )
+        return EmailMaskData(is_random_mask=is_random_mask, has_website=has_website)
 
 
 class RelayGleanLogger(EventsServerEventLogger):
@@ -179,7 +175,6 @@ class RelayGleanLogger(EventsServerEventLogger):
             date_joined_premium=_opt_dt_to_glean(user_data.date_joined_premium),
             has_extension=user_data.has_extension,
             date_got_extension=_opt_dt_to_glean(user_data.date_got_extension),
-            mask_id=mask_data.mask_id,
             is_random_mask=mask_data.is_random_mask,
             created_by_api=created_by_api,
             has_website=mask_data.has_website,
@@ -211,7 +206,6 @@ class RelayGleanLogger(EventsServerEventLogger):
             date_joined_premium=_opt_dt_to_glean(user_data.date_joined_premium),
             has_extension=user_data.has_extension,
             date_got_extension=_opt_dt_to_glean(user_data.date_got_extension),
-            mask_id=mask_data.mask_id,
             is_random_mask=mask_data.is_random_mask,
         )
 
@@ -220,7 +214,6 @@ class RelayGleanLogger(EventsServerEventLogger):
         *,
         request: HttpRequest,
         user: User,
-        mask_id: str,
         is_random_mask: bool,
     ) -> None:
         """Log that a Relay email mask was deleted."""
@@ -242,7 +235,6 @@ class RelayGleanLogger(EventsServerEventLogger):
             date_joined_premium=_opt_dt_to_glean(user_data.date_joined_premium),
             has_extension=user_data.has_extension,
             date_got_extension=_opt_dt_to_glean(user_data.date_got_extension),
-            mask_id=mask_id,
             is_random_mask=is_random_mask,
         )
 
@@ -272,7 +264,6 @@ class RelayGleanLogger(EventsServerEventLogger):
             date_joined_premium=_opt_dt_to_glean(user_data.date_joined_premium),
             has_extension=user_data.has_extension,
             date_got_extension=_opt_dt_to_glean(user_data.date_got_extension),
-            mask_id=mask_data.mask_id,
             is_random_mask=mask_data.is_random_mask,
             is_reply=is_reply,
         )
@@ -304,7 +295,6 @@ class RelayGleanLogger(EventsServerEventLogger):
             date_joined_premium=_opt_dt_to_glean(user_data.date_joined_premium),
             has_extension=user_data.has_extension,
             date_got_extension=_opt_dt_to_glean(user_data.date_got_extension),
-            mask_id=mask_data.mask_id,
             is_random_mask=mask_data.is_random_mask,
             is_reply=is_reply,
             reason=reason,

privaterelay/tests/glean_tests.py

@@ -175,7 +175,6 @@ def test_email_mask_relay_address() -> None:
 
     mask_data = EmailMaskData.from_mask(mask)
 
-    assert mask_data.mask_id == mask.metrics_id
     assert mask_data.is_random_mask is True
     assert mask_data.has_website is False
 
@@ -199,7 +198,6 @@ def test_email_mask_domain_address() -> None:
 
     mask_data = EmailMaskData.from_mask(mask)
 
-    assert mask_data.mask_id == mask.metrics_id
     assert mask_data.is_random_mask is False
     assert mask_data.has_website is False
 
@@ -307,7 +305,6 @@ def test_log_email_mask_created(
         name="created",
         extra_items={
             "n_random_masks": "1",
-            "mask_id": address.metrics_id,
             "is_random_mask": "true",
             "has_website": "false",
             "created_by_api": "true",
@@ -367,7 +364,6 @@ def test_log_email_mask_label_updated(
         name="label_updated",
         extra_items={
             "n_random_masks": "1",
-            "mask_id": address.metrics_id,
             "is_random_mask": "true",
         },
         user=user,
@@ -408,13 +404,10 @@ def test_log_email_mask_deleted(
     """Check that log_email_mask_deleted results in a Glean server-side log."""
     user = make_free_test_user()
     address = baker.make(RelayAddress, user=user)
-    mask_id = address.metrics_id
     request = rf.delete(f"/api/v1/relayaddresses/{address.id}/")
     address.delete()  # Real request will delete the mask before glean event
 
-    glean_logger.log_email_mask_deleted(
-        user=user, mask_id=mask_id, is_random_mask=True, request=request
-    )
+    glean_logger.log_email_mask_deleted(user=user, is_random_mask=True, request=request)
     # Check the one glean-server-event log
     assert len(caplog.records) == 1
     record = caplog.records[0]
@@ -428,7 +421,6 @@ def test_log_email_mask_deleted(
         name="deleted",
         extra_items={
             "n_random_masks": "0",
-            "mask_id": mask_id,
             "is_random_mask": "true",
         },
         user=user,
@@ -448,14 +440,11 @@ def test_log_email_mask_deleted_with_opt_out(
 ) -> None:
     """A log is not emitted for mask deletion when the user has opted-out of metrics"""
     address = baker.make(RelayAddress, user=optout_user)
-    mask_id = address.metrics_id
     user = address.user
     request = rf.delete(f"/api/v1/relayaddresses/{address.id}/")
     address.delete()  # Real request will delete the mask before glean event
 
-    glean_logger.log_email_mask_deleted(
-        user=user, mask_id=mask_id, is_random_mask=True, request=request
-    )
+    glean_logger.log_email_mask_deleted(user=user, is_random_mask=True, request=request)
     assert len(caplog.records) == 0
 
 
@@ -485,7 +474,6 @@ def test_log_email_forwarded(
         name="forwarded",
         extra_items={
             "n_random_masks": "1",
-            "mask_id": address.metrics_id,
             "is_random_mask": "true",
             "is_reply": "true" if is_reply else "false",
         },
@@ -537,7 +525,6 @@ def test_log_email_blocked(
         name="blocked",
         extra_items={
             "n_random_masks": "1",
-            "mask_id": address.metrics_id,
             "is_random_mask": "true",
             "is_reply": "true" if is_reply else "false",
             "reason": reason,

telemetry/glean/relay-server-metrics.yaml

@@ -59,9 +59,6 @@ email_mask:
       date_got_extension:
         description: Timestamp for adding Relay Add-on, seconds since epoch, -1 if not used
         type: quantity
-      mask_id:
-        description: Mask ID, 'R' (random mask) or 'D' (domain mask) followed by a number.
-        type: string
       is_random_mask:
         description: The mask is a random mask, instead of a domain mask
         type: boolean