Enable the use of multiple categories for Gecko metrics (#79)

Dexterp37 · web-flow · commit 3879143d57cd · 2019-07-26T12:55:43.000+02:00
* Enable the use of multiple categories for Gecko metrics

This introduces a new template used for generating a
single mapping object. The object then calls in the
other generated metrics, if needed.

* Fix flake8 issues
diff --git a/glean_parser/kotlin.py b/glean_parser/kotlin.py
@@ -13,6 +13,7 @@
 
 from . import metrics
 from . import util
+from collections import defaultdict
 
 
 def kotlin_datatypes_filter(value):
@@ -91,6 +92,77 @@ def class_name(obj_type):
     return f'{util.Camelize(obj_type)}MetricType'
 
 
+def output_gecko_lookup(objs, output_dir, options={}):
+    """
+    Given a tree of objects, generate a Kotlin map between Gecko histograms and
+    Glean SDK metric types.
+
+    :param objects: A tree of objects (metrics and pings) as returned from
+    `parser.parse_objects`.
+    :param output_dir: Path to an output directory to write to.
+    :param options: options dictionary, with the following optional keys:
+
+        - `namespace`: The package namespace to declare at the top of the
+          generated files. Defaults to `GleanMetrics`.
+        - `glean_namespace`: The package namespace of the glean library itself.
+          This is where glean objects will be imported from in the generated
+          code.
+    """
+    template = util.get_jinja2_template(
+        'kotlin.geckoview.jinja2',
+        filters=(
+            ('kotlin', kotlin_datatypes_filter),
+            ('type_name', type_name),
+            ('class_name', class_name)
+        )
+    )
+
+    namespace = options.get('namespace', 'GleanMetrics')
+    glean_namespace = options.get(
+        'glean_namespace',
+        'mozilla.components.service.glean'
+    )
+
+    # Build a dictionary that contains data for metrics that are
+    # histogram-like and contain a gecko_datapoint, with this format:
+    #
+    # {
+    #  "category": [
+    #    {"gecko_datapoint": "the-datapoint", "name": "the-metric-name"},
+    #    ...
+    #  ],
+    #  ...
+    # }
+    gecko_metrics = defaultdict(list)
+
+    for category_key, category_val in objs.items():
+        # Support exfiltration of Gecko metrics from products using both the
+        # Glean SDK and GeckoView. See bug 1566356 for more context.
+        for metric in category_val.values():
+            if getattr(metric, 'gecko_datapoint', False):
+                gecko_metrics[category_key].append({
+                        'gecko_datapoint': metric.gecko_datapoint,
+                        'name': metric.name
+                    })
+
+    if not gecko_metrics:
+        # Bail out and don't create a file if no gecko metrics
+        # are found.
+        return
+
+    filepath = output_dir / "GleanGeckoHistogramMapping.kt"
+    with open(filepath, 'w', encoding='utf-8') as fd:
+        fd.write(
+            template.render(
+                gecko_metrics=gecko_metrics,
+                namespace=namespace,
+                glean_namespace=glean_namespace,
+            )
+        )
+        # Jinja2 squashes the final newline, so we explicitly add it
+        fd.write('\n')
+
+
 def output_kotlin(objs, output_dir, options={}):
     """
     Given a tree of objects, output Kotlin code to `output_dir`.
@@ -147,13 +219,6 @@ def output_kotlin(objs, output_dir, options={}):
             for metric in category_val.values()
         )
 
-        # Support exfiltration of Gecko metrics from products using both the
-        # Glean SDK and GeckoView. See bug 1566356 for more context.
-        has_gecko_datapoints = any(
-            getattr(metric, 'gecko_datapoint', False)
-            for metric in category_val.values()
-        )
-
         with open(filepath, 'w', encoding='utf-8') as fd:
             fd.write(
                 template.render(
@@ -163,9 +228,11 @@ def output_kotlin(objs, output_dir, options={}):
                     extra_args=extra_args,
                     namespace=namespace,
                     has_labeled_metrics=has_labeled_metrics,
-                    has_gecko_datapoints=has_gecko_datapoints,
                     glean_namespace=glean_namespace,
                 )
             )
             # Jinja2 squashes the final newline, so we explicitly add it
             fd.write('\n')
+
+    # TODO: Maybe this should just be a separate outputter?
+    output_gecko_lookup(objs, output_dir, options)
diff --git a/glean_parser/templates/kotlin.geckoview.jinja2 b/glean_parser/templates/kotlin.geckoview.jinja2
@@ -0,0 +1,34 @@
+// -*- mode: kotlin -*-
+
+/*
+ * AUTOGENERATED BY glean_parser.  DO NOT EDIT.
+ */
+
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+@file:Suppress("PackageNaming")
+package {{ namespace }}
+
+import {{ glean_namespace }}.private.HistogramBase // ktlint-disable import-ordering no-unused-imports
+
+/*
+ * This class performs the mapping between Gecko Histograms and Glean SDK
+ * metric types.
+ */
+internal object GleanGeckoHistogramMapping {
+    // Support exfiltration of Gecko metrics from products using both the
+    // Glean SDK and GeckoView. See bug 1566356 for more context.
+    operator fun get(geckoMetricName: String): HistogramBase? {
+        return when (geckoMetricName) {
+        {% for category in gecko_metrics.keys()|sort %}
+            // From {{ category|Camelize }}.kt
+            {% for metric in gecko_metrics[category] %}
+            "{{ metric.gecko_datapoint }}" -> {{ category|Camelize }}.{{ metric.name|camelize }}
+            {% endfor %}
+        {%- endfor %}
+            else -> null
+        }
+    }
+}
diff --git a/glean_parser/templates/kotlin.jinja2 b/glean_parser/templates/kotlin.jinja2
@@ -20,9 +20,6 @@
 @file:Suppress("PackageNaming")
 package {{ namespace }}
 
-{% if has_gecko_datapoints %}
-import {{ glean_namespace }}.private.HistogramBase
-{% endif %}
 import {{ glean_namespace }}.private.Lifetime // ktlint-disable no-unused-imports
 import {{ glean_namespace }}.private.NoExtraKeys // ktlint-disable no-unused-imports
 import {{ glean_namespace }}.private.TimeUnit // ktlint-disable no-unused-imports
@@ -67,18 +64,4 @@ internal object {{ category_name|Camelize }} {
     {{ obj_declaration(obj, lazy=obj.type != 'ping') }}
     {% endif %}
 {%- endfor %}
-{% if has_gecko_datapoints %}
-    // Support exfiltration of Gecko metrics from products using both the
-    // Glean SDK and GeckoView. See bug 1566356 for more context.
-    operator fun get(geckoMetricName: String): HistogramBase? {
-        return when (geckoMetricName) {
-        {% for obj in objs.values() %}
-            {% if obj.gecko_datapoint %}
-            "{{ obj.gecko_datapoint }}" -> {{ obj.name|camelize }}
-            {% endif %}
-        {%- endfor %}
-            else -> null
-        }
-    }
-{% endif %}
 }
diff --git a/tests/data/gecko.yaml b/tests/data/gecko.yaml
@@ -1,9 +1,9 @@
 $schema: moz://mozilla.org/schemas/glean/metrics/1-0-0
 
-gecko.imports:
-  page_load_time:
+page.perf:
+  load_time:
     type: timing_distribution
-    gecko_datapoint: FX_PAGE_LOAD_MS_2
+    gecko_datapoint: GV_PAGE_LOAD_MS
     time_unit: millisecond
     lifetime: application
     description: >
@@ -16,9 +16,9 @@ gecko.imports:
       - CHANGE-ME@example.com
     expires: 2100-01-01
 
-  startup_runtime:
+  reload_time:
     type: timing_distribution
-    gecko_datapoint: GV_STARTUP_RUNTIME_MS
+    gecko_datapoint: GV_PAGE_RELOAD_MS
     time_unit: millisecond
     lifetime: application
     description: >
@@ -31,6 +31,22 @@ gecko.imports:
       - CHANGE-ME@example.com
     expires: 2100-01-01
 
+gfx.content.checkerboard:
+  duration:
+    type: timing_distribution
+    gecko_datapoint: CHECKERBOARD_DURATION
+    time_unit: millisecond
+    lifetime: application
+    description: >
+      A sample timing distribution metric exported from Gecko.
+    bugs:
+      - 1566356
+    data_reviews:
+      - http://example.com/reviews
+    notification_emails:
+      - CHANGE-ME@example.com
+    expires: 2100-01-01
+
 non.gecko.metrics:
   app_load_time:
     type: timing_distribution
diff --git a/tests/test_kotlin.py b/tests/test_kotlin.py
@@ -188,16 +188,22 @@ def test_gecko_datapoints(tmpdir):
         {'allow_reserved': True}
     )
 
+    metrics_files = [
+        'GfxContentCheckerboard.kt',
+        'PagePerf.kt',
+        'NonGeckoMetrics.kt'
+    ]
     assert (
         set(x.name for x in tmpdir.iterdir()) ==
-        set([
-            'GeckoImports.kt',
-            'NonGeckoMetrics.kt'
-        ])
+        set(['GleanGeckoHistogramMapping.kt'] + metrics_files)
     )
 
     # Make sure descriptions made it in
-    with open(tmpdir / 'GeckoImports.kt', 'r', encoding='utf-8') as fd:
+    with open(
+        tmpdir / 'GleanGeckoHistogramMapping.kt',
+        'r',
+        encoding='utf-8'
+    ) as fd:
         content = fd.read()
         # Make sure we're adding the relevant Glean SDK import, once.
         assert content.count(
@@ -210,17 +216,21 @@ def test_gecko_datapoints(tmpdir):
         # changes, otherwise validation will fail.
         expected_operator = """    operator fun get(geckoMetricName: String): HistogramBase? {
         return when (geckoMetricName) {
-            "FX_PAGE_LOAD_MS_2" -> pageLoadTime
-            "GV_STARTUP_RUNTIME_MS" -> startupRuntime
+            // From GfxContentCheckerboard.kt
+            "CHECKERBOARD_DURATION" -> GfxContentCheckerboard.duration
+            // From PagePerf.kt
+            "GV_PAGE_LOAD_MS" -> PagePerf.loadTime
+            "GV_PAGE_RELOAD_MS" -> PagePerf.reloadTime
             else -> null
         }
     }"""
 
         assert expected_operator in content
 
-    with open(tmpdir / 'NonGeckoMetrics.kt', 'r', encoding='utf-8') as fd:
-        content = fd.read()
-        assert 'HistogramBase' not in content
+    for file_name in metrics_files:
+        with open(tmpdir / file_name, 'r', encoding='utf-8') as fd:
+            content = fd.read()
+            assert 'HistogramBase' not in content
 
     # Only run this test if ktlint is on the path
     if shutil.which('ktlint'):
