Bug 2003478 - New lint: Warn against event metrics on the metrics or baseline ping

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- New lint: Warn against event metrics on the `metrics` or `baseline` ping ([#823](https://github.com/mozilla/glean_parser/pull/823))
+
 ## 18.1.1
 
 - Python server: Fix logging mechanism ([#820](https://github.com/mozilla/glean_parser/pull/820))

glean_parser/lint.py

@@ -300,6 +300,21 @@ def check_metric_on_events_lifetime(
         )
 
 
+def check_event_on_non_events_ping(
+    metric: metrics.Metric, parser_config: Dict[str, Any]
+) -> LintGenerator:
+    """
+    An event metric should usually go on the `events` ping or a custom ping,
+    not on a builtin ping.
+    """
+    disallowed_pings = set(pings.RESERVED_PING_NAMES) - {"default", "events"} | {"health"}
+    if metric.type == "event" and any([ping in disallowed_pings for ping in metric.send_in_pings]):
+        yield (
+            "An event metric should usually go on the `events` ping or a custom ping, "
+            + "not on a builtin ping."
+        )
+
+
 def check_unexpected_unit(
     metric: metrics.Metric, parser_config: Dict[str, Any]
 ) -> LintGenerator:
@@ -466,6 +481,7 @@ def check_name_too_similar(
     "EXPIRED": (check_expired_metric, CheckType.warning),
     "OLD_EVENT_API": (check_old_event_api, CheckType.warning),
     "METRIC_ON_EVENTS_LIFETIME": (check_metric_on_events_lifetime, CheckType.error),
+    "EVENT_ON_NON_EVENTS_PING": (check_event_on_non_events_ping, CheckType.warning),
     "UNEXPECTED_UNIT": (check_unexpected_unit, CheckType.warning),
     "EMPTY_DATAREVIEW": (check_empty_datareview, CheckType.warning),
     "HIGHER_DATA_SENSITIVITY_REQUIRED": (

tests/test_lint.py

@@ -875,3 +875,89 @@ def test_unknown_lint_warning(content, num_nits):
         assert set(["UNKNOWN_LINT"]) == set(
             v.check_name for v in nits
         )
+
+
+@pytest.mark.parametrize(
+    "content,num_nits",
+    [
+        (
+            {
+                "plain_event": {
+                    "type": "event",
+                }
+            },
+            0,
+        ),
+        (
+            {
+                "event_on_metrics": {
+                    "type": "event",
+                    "send_in_pings": ["metrics"],
+                }
+            },
+            1,
+        ),
+        (
+            {
+                "event_on_metrics_and_events": {
+                    "type": "event",
+                    "send_in_pings": ["metrics", "events"],
+                }
+            },
+            1,
+        ),
+        (
+            {
+                "event_on_baseline": {
+                    "type": "event",
+                    "send_in_pings": ["baseline"],
+                    "no_lint": ["BASELINE_PING"],
+                }
+            },
+            1,
+        ),
+        (
+            {
+                "event_on_baseline_and_metrics": {
+                    "type": "event",
+                    "send_in_pings": ["baseline", "metrics"],
+                    "no_lint": ["BASELINE_PING"],
+                }
+            },
+            1,
+        ),
+        (
+            {
+                "no_lint_events_on_metrics": {
+                    "type": "event",
+                    "send_in_pings": ["metrics", "events"],
+                    "no_lint": ["EVENT_ON_NON_EVENTS_PING"],
+                }
+            },
+            0,
+        ),
+        (
+            {
+                "default_ping": {
+                    "type": "event",
+                    "send_in_pings": ["default"],
+                }
+            },
+            0,
+        ),
+    ],
+)
+def test_events_on_metrics_ping(content, num_nits):
+    content = {"cat": content}
+    content = util.add_required(content)
+    all_metrics = parser.parse_objects(content)
+
+    errs = list(all_metrics)
+    assert len(errs) == 0
+
+    nits = lint.lint_metrics(all_metrics.value)
+    assert len(nits) == num_nits
+    if num_nits > 0:
+        assert set(["EVENT_ON_NON_EVENTS_PING"]) == set(
+            v.check_name for v in nits
+        )