From 6a41f2e4aeb846569faa9a155f27d1105dfb09d2 Mon Sep 17 00:00:00 2001 From: Arkadiusz Komarzewski Date: Mon, 31 Aug 2026 14:13:43 +0200 Subject: [PATCH] feat(js_server): Add support for the server-deletion-request ping (DENG-4079) --- CHANGELOG.md | 2 + glean_parser/javascript_server.py | 36 +++++----- server_telemetry/server-side-pings.yaml | 16 +++++ .../data/server_deletion_request_metrics.yaml | 21 ++++++ tests/test_javascript_server.py | 68 +++++++++++++++++++ 5 files changed, 127 insertions(+), 16 deletions(-) create mode 100644 tests/data/server_deletion_request_metrics.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d082c3f7..553bb52af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- JS/TS server: Add support for `server-deletion-request` ping. Server apps can now add `server-deletion-request` to `send_in_pings` on identifier metrics (e.g. user IDs) ([DENG-4079](https://mozilla-hub.atlassian.net/browse/DENG-4079)) + ## 21.0.1 - Swift: Fix argument labels in labeled *distributions ([#859](https://github.com/mozilla/glean_parser/issues/859)) diff --git a/glean_parser/javascript_server.py b/glean_parser/javascript_server.py index dfa542c00..402df2adf 100644 --- a/glean_parser/javascript_server.py +++ b/glean_parser/javascript_server.py @@ -41,6 +41,10 @@ # and might require changes to the template. SUPPORTED_METRIC_TYPES = ["string", "event"] +# Pings defined in the glean-server library (server_telemetry/server-side-pings.yaml) +# are available to all server apps without needing a local pings.yaml definition. +KNOWN_SERVER_PINGS = {"events", "server-deletion-request"} + def event_class_name( ping_name: str, metrics_by_type: Dict[str, List[metrics.Metric]] @@ -164,24 +168,24 @@ def output( + " parser doesn't generate individual metric files. Make sure to pass all" + " your ping and metric definitions in a single invocation of the parser." ) - if "pings" not in objs: - # If events are meant to be sent in custom pings, we need to make sure they - # are defined. Otherwise we won't have destination tables defined and - # submissions won't pass validation at ingestion. - if event_metric_exists: - if "events" not in ping_to_metrics: - # Event metrics can be sent in standard `events` ping - # or in custom pings. - print( - "❌ " - + PING_METRIC_ERROR_MSG - + "\n You need to either send your event metrics in standard" - + " `events` ping or define a custom one." - ) - return - else: + # If custom pings are targeted, they must be explicitly defined in a + # pings.yaml - otherwise destination tables aren't set up and submissions + # fail ingestion. Pings from the glean-server library are exempt: they're + # centrally defined and available to all server apps. + if "pings" not in objs and not all( + p in KNOWN_SERVER_PINGS for p in ping_to_metrics + ): + if not event_metric_exists: print("❌ No ping definition found." + PING_METRIC_ERROR_MSG) return + if "events" not in ping_to_metrics: + print( + "❌ " + + PING_METRIC_ERROR_MSG + + "\n You need to either send your event metrics in standard" + + " `events` ping or define a custom one." + ) + return if not ping_to_metrics: print("❌ No pings with metrics found." + PING_METRIC_ERROR_MSG) diff --git a/server_telemetry/server-side-pings.yaml b/server_telemetry/server-side-pings.yaml index bde660ec9..cf3d0fca9 100644 --- a/server_telemetry/server-side-pings.yaml +++ b/server_telemetry/server-side-pings.yaml @@ -21,3 +21,19 @@ events: - https://bugzilla.mozilla.org/show_bug.cgi?id=1512938#c3 notification_emails: - glean-team@mozilla.com + +server-deletion-request: + description: | + This ping is submitted when a server application needs to request + deletion of a user's telemetry data from the warehouse. + Unlike the SDK's `deletion-request` ping, server applications have no + Glean-managed `client_id`, so this ping carries application-defined + identifier metrics that the pipeline uses to find the data to delete. + include_client_id: false + send_if_empty: true + bugs: + - https://mozilla-hub.atlassian.net/browse/DENG-4079 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1512938#c3 + notification_emails: + - glean-team@mozilla.com diff --git a/tests/data/server_deletion_request_metrics.yaml b/tests/data/server_deletion_request_metrics.yaml new file mode 100644 index 000000000..11f42cbc3 --- /dev/null +++ b/tests/data/server_deletion_request_metrics.yaml @@ -0,0 +1,21 @@ +# Any copyright is dedicated to the Public Domain. +# https://creativecommons.org/publicdomain/zero/1.0/ + +--- +$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0 + +account: + user_id: + type: string + description: | + The firefox/mozilla account id + bugs: + - TBD + data_reviews: + - TBD + notification_emails: + - fake@fake.com + expires: never + lifetime: application + send_in_pings: + - server-deletion-request diff --git a/tests/test_javascript_server.py b/tests/test_javascript_server.py index 5a0545fcb..67bc00d2b 100644 --- a/tests/test_javascript_server.py +++ b/tests/test_javascript_server.py @@ -44,6 +44,19 @@ def test_parser_js_server_metrics_no_ping(tmp_path): assert all(False for _ in tmp_path.iterdir()) +def test_parser_js_server_event_metrics_custom_ping_no_pings_file(tmp_path): + """Test that no files are generated when event metrics target only a + custom ping and that ping isn't defined. Event metrics are allowed + without a pings.yaml only when they go in the standard `events` ping.""" + translate.translate( + ROOT / "data" / "go_server_custom_ping_only_metrics.yaml", + "javascript_server", + tmp_path, + ) + + assert all(False for _ in tmp_path.iterdir()) + + def test_parser_js_server(tmp_path): """Test that no files are generated if only metric definitions are provided without pings.""" @@ -143,6 +156,61 @@ def test_logging_custom_ping_as_events(tmp_path): ) +@pytest.mark.parametrize( + "outputter,ext,extra_assertions", + [ + ( + "javascript_server", + "js", + ["'account.user_id'", "document_type: 'server-deletion-request'"], + ), + ("typescript_server", "ts", ["account_user_id: string"]), + ], +) +def test_parser_server_deletion_request_standalone( + tmp_path, outputter, ext, extra_assertions +): + """Test that server-deletion-request ping code is generated when metrics + target it, even without a pings.yaml file.""" + translate.translate( + ROOT / "data" / "server_deletion_request_metrics.yaml", + outputter, + tmp_path, + ) + + assert set(x.name for x in tmp_path.iterdir()) == {f"server_events.{ext}"} + + content = (tmp_path / f"server_events.{ext}").read_text(encoding="utf-8") + assert "ServerDeletionRequestServerEvent" in content + assert "createServerDeletionRequestEvent" in content + for expected in extra_assertions: + assert expected in content + + +def test_parser_js_server_deletion_request_with_other_pings(tmp_path): + """Test that server-deletion-request ping is generated alongside regular + pings when metrics target both.""" + translate.translate( + [ + ROOT / "data" / "fxa-server-pings.yaml", + ROOT / "data" / "fxa-server-metrics.yaml", + ROOT / "data" / "server_deletion_request_metrics.yaml", + ], + "javascript_server", + tmp_path, + ) + + assert set(x.name for x in tmp_path.iterdir()) == set(["server_events.js"]) + + with (tmp_path / "server_events.js").open("r", encoding="utf-8") as fd: + content = fd.read() + # Both ping classes should be generated + assert "AccountsEventsServerEvent" in content + assert "ServerDeletionRequestServerEvent" in content + assert "createAccountsEventsEvent" in content + assert "createServerDeletionRequestEvent" in content + + @pytest.mark.node_dependency def test_logging_events_ping_with_event_metrics(tmp_path): translate.translate(