bug 1645166 - Add support for rate metrics

Adds three new metric object model types:
* "rate" for rates that own both their numerator and denominator
* "numerator" for rates that have an external denominator
* "denominator" for counters that are external denominators

Author: chutten

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+- Add parser and object model support for `rate` metric type. ([bug 1645166](https://bugzilla.mozilla.org/show_bug.cgi?id=1645166))
+- Add parser and object model support for telemetry_mirror property. ([bug 1685406](https://bugzilla.mozilla.org/show_bug.cgi?id=1685406))
+
 ## 2.4.0 (2021-02-18)
 
 - **Experimental:** `glean_parser` has a new subcommand `coverage` to convert raw coverage reports

glean_parser/metrics.py

@@ -364,4 +364,12 @@ class LabeledCounter(Labeled, Counter):
     typename = "labeled_counter"
 
 
+class Rate(Metric):
+    typename = "rate"
+
+    def __init__(self, *args, **kwargs):
+        self.denominator_metric = kwargs.pop("denominator_metric", None)
+        super().__init__(*args, **kwargs)
+
+
 ObjectTree = Dict[str, Dict[str, Union[Metric, pings.Ping]]]

glean_parser/schemas/metrics.2-0-0.schema.yaml

@@ -130,6 +130,7 @@ definitions:
           - labeled_boolean
           - labeled_string
           - labeled_counter
+          - rate
 
       description:
         title: Description
@@ -479,6 +480,18 @@ definitions:
         type: string
         minLength: 6
 
+      denominator_metric:
+        title: The name of the denominator for this `rate` metric.
+        description: |
+          Denominators for `rate` metrics may be private and internal
+          or shared and external.
+          External denominators are `counter` metrics.
+          This field names the `counter` metric that serves as this
+          `rate` metric's external denominator.
+          The named denominator must be defined in this component
+          so glean_parser can find it.
+        type: string
+
     required:
       - type
       - bugs
@@ -605,3 +618,14 @@ additionalProperties:
             - decrypted_name
           description: |
             `jwe` is missing required parameter `decrypted_name`.
+      - if:
+          not:
+            properties:
+              type:
+                const: rate
+        then:
+          properties:
+            denominator_metric:
+              description: |
+                `denominator_metric` is only allowed for `rate`.
+              maxLength: 0

glean_parser/translate.py

@@ -11,6 +11,7 @@
 from pathlib import Path
 import os
 import shutil
+import sys
 import tempfile
 from typing import Any, Callable, Dict, Iterable, List, Optional
 
@@ -59,6 +60,39 @@ def __init__(
 }
 
 
+def transform_metrics(objects):
+    """
+    Transform the object model from one that represents the YAML definitions
+    to one that reflects the type specifics needed by code generators.
+
+    e.g. This will transform a `rate` to be a `numerator` if its denominator is
+    external.
+    """
+    counters = {}
+    numerators_by_denominator: Dict[str, Any] = {}
+    for category_val in objects.values():
+        for metric in category_val.values():
+            fqmn = metric.identifier()
+            if getattr(metric, "type", None) == "counter":
+                counters[fqmn] = metric
+            denominator_name = getattr(metric, "denominator_metric", None)
+            if denominator_name:
+                metric.type = "numerator"
+                numerators_by_denominator.setdefault(denominator_name, [])
+                numerators_by_denominator[denominator_name].append(fqmn)
+
+    for denominator_name, numerator_names in numerators_by_denominator.items():
+        if denominator_name not in counters:
+            print(
+                f"No `counter` named {denominator_name} found to be used as"
+                "denominator for {numerator_names}",
+                file=sys.stderr,
+            )
+            return 1
+        counters[denominator_name].type = "denominator"
+        counters[denominator_name].numerators = numerator_names
+
+
 def translate_metrics(
     input_filepaths: Iterable[Path],
     output_dir: Path,

glean_parser/util.py

@@ -422,6 +422,7 @@ def remove_output_params(d, output_params):
     "range_max",
     "range_min",
     "histogram_type",
+    "numerators",
 ]
 
 

tests/data/rate.yaml

@@ -0,0 +1,60 @@
+# 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
+
+testing.rates:
+  has_internal_denominator:
+    type: rate
+    lifetime: application
+    description: >
+      Test metric to ensure rates with internal denominators work.
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1645166
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1645166#c1
+    notification_emails:
+      - CHANGE-ME@example.com
+    expires: never
+
+  has_external_denominator:
+    type: rate
+    lifetime: application
+    description: >
+      Test metric to ensure rates with external denominators work.
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1645166
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1645166#c1
+    notification_emails:
+      - CHANGE-ME@example.com
+    expires: never
+    denominator_metric: testing.rates.the_denominator
+
+  also_has_external_denominator:
+    type: rate
+    lifetime: application
+    description: >
+      Test metric to ensure rates with shared external denominators work.
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1645166
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1645166#c1
+    notification_emails:
+      - CHANGE-ME@example.com
+    expires: never
+    denominator_metric: testing.rates.the_denominator
+
+  the_denominator:
+    type: counter
+    lifetime: application
+    description: >
+      Test denominator for rate metrics.
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1645166
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1645166#c1
+    notification_emails:
+      - CHANGE-ME@example.com
+    expires: never

tests/test_parser.py

@@ -647,3 +647,29 @@ def test_telemetry_mirror():
         all_metrics.value["telemetry.mirrored"]["parses_fine"].telemetry_mirror
         == "telemetry.test.string_kind"
     )
+
+
+def test_rates():
+    """
+    Ensure that `rate` metrics parse properly.
+    """
+
+    all_metrics = parser.parse_objects(
+        [ROOT / "data" / "rate.yaml"],
+        config={"allow_reserved": False},
+    )
+
+    errs = list(all_metrics)
+    assert len(errs) == 0
+
+    category = all_metrics.value["testing.rates"]
+    assert category["has_internal_denominator"].type == "rate"
+    assert (
+        category["has_external_denominator"].type == "rate"
+    )  # Hasn't been transformed to "numerator" yet
+    assert (
+        category["also_has_external_denominator"].type == "rate"
+    )  # Hasn't been transformed to "numerator" yet
+    assert (
+        category["the_denominator"].type == "counter"
+    )  # Hasn't been transformed to "denominator" yet

tests/test_translate.py

@@ -190,3 +190,39 @@ def test_getting_line_number():
 
     assert pings["custom-ping"].defined_in["line"] == 7
     assert metrics["core_ping"]["seq"].defined_in["line"] == 27
+
+
+def test_rates(tmpdir):
+    def external_translator(all_objects, output_dir, options):
+        assert (
+            all_objects["testing.rates"]["has_external_denominator"].type == "rate"
+        )  # Hasn't yet been transformed
+
+        translate.transform_metrics(all_objects)
+
+        category = all_objects["testing.rates"]
+        assert category["has_internal_denominator"].type == "rate"
+        assert category["has_external_denominator"].type == "numerator"
+        assert (
+            category["has_external_denominator"].denominator_metric
+            == "testing.rates.the_denominator"
+        )
+        assert category["also_has_external_denominator"].type == "numerator"
+        assert (
+            category["also_has_external_denominator"].denominator_metric
+            == "testing.rates.the_denominator"
+        )
+        assert category["the_denominator"].type == "denominator"
+        assert category["the_denominator"].numerators == [
+            "testing.rates.has_external_denominator",
+            "testing.rates.also_has_external_denominator",
+        ]
+
+    translate.translate_metrics(
+        ROOT / "data" / "rate.yaml",
+        Path(str(tmpdir)),
+        external_translator,
+        [],
+        options={"foo": "bar"},
+        parser_config={"allow_reserved": True},
+    )