From f6a356601c0d100c967a43c8b296115c35f8ddf8 Mon Sep 17 00:00:00 2001 From: Chris H-C Date: Thu, 18 Apr 2024 16:42:25 -0400 Subject: [PATCH] bug 1886810 - Forbid (by lint) non-ping lifetimes for _distribution metrics --- CHANGELOG.md | 2 ++ glean_parser/lint.py | 16 ++++++++++++++++ tests/data/gecko.yaml | 4 ---- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 556ab5367..988f88beb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Changelog +- NEW LINT: Forbid (via lint) non-ping lifetimes for `*_distribution` metrics ([bug 1886810](https://bugzilla.mozilla.org/show_bug.cgi?id=1886810)) + ## 14.0.1 - BUGFIX: Fix missing `ping_arg` in util.py ([#687](https://github.com/mozilla/glean_parser/pull/687)) diff --git a/glean_parser/lint.py b/glean_parser/lint.py index 891d05ad6..be38bd849 100644 --- a/glean_parser/lint.py +++ b/glean_parser/lint.py @@ -385,6 +385,21 @@ def check_unknown_ping( yield nit +def check_distribution_lifetime( + metric: metrics.Metric, parser_config: Dict[str, Any] +) -> LintGenerator: + """*_distribution metrics should only be ping-lifetime, because otherwise + we will send the same samples more than once.""" + if ( + metric.type.endswith("_distribution") + and metric.lifetime != metrics.Lifetime.ping + ): + yield ( + "Distribution metrics ought to have ping lifetime. Otherwise, their" + " samples could be submitted multiple times." + ) + + # The checks that operate on an entire category of metrics: # {NAME: (function, is_error)} CATEGORY_CHECKS: Dict[ @@ -412,6 +427,7 @@ def check_unknown_ping( "METRIC_ON_EVENTS_LIFETIME": (check_metric_on_events_lifetime, CheckType.error), "UNEXPECTED_UNIT": (check_unexpected_unit, CheckType.warning), "EMPTY_DATAREVIEW": (check_empty_datareview, CheckType.warning), + "DISTRIBUTION_NON_PING_LIFETIME": (check_distribution_lifetime, CheckType.error), } diff --git a/tests/data/gecko.yaml b/tests/data/gecko.yaml index 2fc0cd684..552d35052 100644 --- a/tests/data/gecko.yaml +++ b/tests/data/gecko.yaml @@ -9,7 +9,6 @@ page.perf: type: timing_distribution gecko_datapoint: GV_PAGE_LOAD_MS time_unit: millisecond - lifetime: application description: > A sample timing distribution metric exported from Gecko. bugs: @@ -24,7 +23,6 @@ page.perf: type: timing_distribution gecko_datapoint: GV_PAGE_RELOAD_MS time_unit: millisecond - lifetime: application description: > Another sample timing distribution metric exported from Gecko. bugs: @@ -62,7 +60,6 @@ gfx.content.checkerboard: type: timing_distribution gecko_datapoint: CHECKERBOARD_DURATION time_unit: millisecond - lifetime: application description: > A sample timing distribution metric exported from Gecko. bugs: @@ -121,7 +118,6 @@ non.gecko.metrics: app_load_time: type: timing_distribution time_unit: millisecond - lifetime: application description: > A sample timing distribution that is not tied to a Gecko datapoint. bugs: