From 0f4221be9740101edac8bdb7773c7f814b07f336 Mon Sep 17 00:00:00 2001 From: Eduardo Filho Date: Tue, 7 Jul 2026 14:33:07 -0400 Subject: [PATCH 1/2] Prefer in-source metrics in case of filename collisions --- etl/glean_etl.py | 48 +++++++++++++++++++++++++++++++++++++---- etl_tests/test_glean.py | 41 ++++++++++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/etl/glean_etl.py b/etl/glean_etl.py index 73ca53343..7dd70ce13 100644 --- a/etl/glean_etl.py +++ b/etl/glean_etl.py @@ -1,6 +1,7 @@ import re import copy import json +import logging import os import shutil import tempfile @@ -99,6 +100,23 @@ def _normalize_metrics(name): return f"data_{metric_name}" +def _resolve_metric_collision(existing, candidate): + """ + Pick a winner when two metrics normalize to the same filename (see + `_normalize_metrics`). + """ + + def sort_key(metric): + return ( + 1 if metric.get("in_source") else 0, + metric.get("date_first_seen") or "", + ) + + if sort_key(candidate) > sort_key(existing): + return candidate, existing + return existing, candidate + + def _get_annotation(annotations_index, origin, item_type, identifier=None): if item_type == "app": return annotations_index.get(origin, {}) @@ -615,12 +633,34 @@ def write_glean_metadata(output_dir, functions_dir, app_names=None): extract_auto_event("glean.page_load", app_name, app_data, app_metrics) # write metrics, resorting the app-specific parts in user preference order + metrics_by_filename = {} for metric_data in app_metrics.values(): metric_data["variants"].sort(key=lambda v: USER_CHANNEL_PRIORITY[v["channel"]]) - open( - os.path.join(app_metrics_dir, f"{_normalize_metrics(metric_data['name'])}.json"), - "w", - ).write(dump_json(metric_data)) + filename = f"{_normalize_metrics(metric_data['name'])}.json" + existing = metrics_by_filename.get(filename) + if existing is None: + metrics_by_filename[filename] = metric_data + continue + # Resolve metric name collision. + winner, loser = _resolve_metric_collision(existing, metric_data) + metrics_by_filename[filename] = winner + logging.warning( + "Metric name collision for app %s: %r and %r both normalize to %s. " + "Keeping %r (in_source=%s); dropping %r (in_source=%s). " + "The dropped metric will not have its own page in the Glean " + "Dictionary or any consumer of its data (e.g. GLAM).", + app_name, + existing["name"], + metric_data["name"], + filename, + winner["name"], + winner.get("in_source"), + loser["name"], + loser.get("in_source"), + ) + + for filename, metric_data in metrics_by_filename.items(): + open(os.path.join(app_metrics_dir, filename), "w").write(dump_json(metric_data)) # write tag metadata (if any) if app_tags_for_objects: diff --git a/etl_tests/test_glean.py b/etl_tests/test_glean.py index fdaaf1cca..c384903e3 100644 --- a/etl_tests/test_glean.py +++ b/etl_tests/test_glean.py @@ -1,7 +1,12 @@ import pytest from etl.glean import GleanMetric -from etl.glean_etl import _get_metric_sample_data, _is_metric_in_ping +from etl.glean_etl import ( + _get_metric_sample_data, + _is_metric_in_ping, + _normalize_metrics, + _resolve_metric_collision, +) @pytest.fixture @@ -373,3 +378,37 @@ def test_is_metric_in_ping_info_section(metric, ping_data): metric["is_part_of_info_section"] = True ping_data["include_info_sections"] = False assert _is_metric_in_ping(metric, ping_data) is False + + +def test_normalize_metrics_collides_on_dots_vs_underscores(): + assert _normalize_metrics("search.suggestions_latency") == _normalize_metrics( + "search.suggestions.latency" + ) + + +def test_resolve_metric_collision_prefers_in_source(): + removed = { + "name": "search.suggestions_latency", + "in_source": False, + "date_first_seen": "2025-03-12 15:15:27", + } + active = { + "name": "search.suggestions.latency", + "in_source": True, + "date_first_seen": "2025-09-18 00:05:24", + } + + # the active metric wins regardless of iteration order + assert _resolve_metric_collision(removed, active) == (active, removed) + assert _resolve_metric_collision(active, removed) == (active, removed) + + +def test_resolve_metric_collision_breaks_ties_on_recency(): + older = {"name": "a.b", "in_source": True, "date_first_seen": "2024-01-01 00:00:00"} + newer = {"name": "a_b", "in_source": True, "date_first_seen": "2025-01-01 00:00:00"} + assert _resolve_metric_collision(older, newer)[0] is newer + assert _resolve_metric_collision(newer, older)[0] is newer + + old_removed = {"name": "a.b", "in_source": False, "date_first_seen": "2024-01-01 00:00:00"} + new_removed = {"name": "a_b", "in_source": False, "date_first_seen": "2025-01-01 00:00:00"} + assert _resolve_metric_collision(old_removed, new_removed)[0] is new_removed From 32ccc65e2a72785d0dafc011c7e44c4ac436d342 Mon Sep 17 00:00:00 2001 From: Eduardo Filho Date: Tue, 7 Jul 2026 14:59:45 -0400 Subject: [PATCH 2/2] Update local copy of Glean schema --- tests/fixtures/glean.1.schema.json | 47 ++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/tests/fixtures/glean.1.schema.json b/tests/fixtures/glean.1.schema.json index 3b1697675..c0c580f01 100644 --- a/tests/fixtures/glean.1.schema.json +++ b/tests/fixtures/glean.1.schema.json @@ -153,6 +153,43 @@ "name": { "type": "string" }, + "session": { + "additionalProperties": false, + "description": "Session metadata attached to this event. Absent for out-of-session events and events from before sessions were introduced.", + "properties": { + "event_seq": { + "description": "Per-session event counter, reset at each new session.", + "minimum": 0, + "type": "integer" + }, + "session_id": { + "description": "The unique UUID for this session.", + "type": "string" + }, + "session_sample_rate": { + "description": "The sampling rate in effect for this session.", + "maximum": 1.0, + "minimum": 0.0, + "type": "number" + }, + "session_seq": { + "description": "Monotonically increasing session counter, persisted across restarts.", + "minimum": 0, + "type": "integer" + }, + "session_start_time": { + "description": "Wall-clock timestamp at session start (RFC 3339). Absent on events from before this field was introduced.", + "type": "string" + } + }, + "required": [ + "session_id", + "session_seq", + "event_seq", + "session_sample_rate" + ], + "type": "object" + }, "timestamp": { "minimum": 0, "type": "integer" @@ -826,8 +863,8 @@ }, "description": "Map of metric identifiers (category.name) to boolean values indicating whether the metric is enabled", "propertyNames": { - "maxLength": 61, - "pattern": "^[a-z_][a-z0-9_]{0,29}(\\.[a-z_][a-z0-9_]{0,29})+$", + "maxLength": 111, + "pattern": "^[a-z_][a-z0-9_\\.]+$", "type": "string" }, "type": "object" @@ -843,6 +880,12 @@ "type": "string" }, "type": "object" + }, + "session_sample_rate": { + "description": "Remote override for the session sampling rate (0.0–1.0).", + "maximum": 1.0, + "minimum": 0.0, + "type": ["number", "null"] } }, "type": "object"