Fully remove support for the old events API

Author: badboy

CHANGELOG.md

@@ -5,6 +5,9 @@
 - BUGFIX: Remove internal-only fields from serialized metrics data ([#550](https://github.com/mozilla/glean_parser/pull/550))
 - FEATURE: New subcommand: `dump` to dump the metrics data as JSON ([#550](https://github.com/mozilla/glean_parser/pull/550))
 - BUGFIX: Kotlin: Generate enums with the right generic bound for ping reason codes ([#551](https://github.com/mozilla/glean_parser/pull/551)).
+- **BREAKING CHANGE:** Fully remove support for the old events API ([#549](https://github.com/mozilla/glean_parser/pull/549))
+  Adds a new lint `OLD_EVENT_API` to warn about missing `type` attributes on event extra keys.
+  Note that the Glean SDK already dropped support for the old events API.
 
 ## 6.4.0
 

glean_parser/kotlin.py

@@ -92,31 +92,20 @@ def type_name(obj: Union[metrics.Metric, pings.Ping]) -> str:
     """
     generate_enums = getattr(obj, "_generate_enums", [])
     if len(generate_enums):
-        template_args = []
+        generic = None
         for member, suffix in generate_enums:
             if len(getattr(obj, member)):
-                # Ugly hack to support the newer event extras API
-                # along the deprecated API.
-                # We need to specify both generic parameters,
-                # but only for event metrics.
-                # Plus `eventExtraKeys` use camelCase (lower),
-                # whereas proper class names should use CamelCase.
-                if suffix == "Extra":
-                    if isinstance(obj, metrics.Event):
-                        template_args.append("NoExtraKeys")
-                    template_args.append(util.Camelize(obj.name) + suffix)
+                if isinstance(obj, metrics.Event):
+                    generic = util.Camelize(obj.name) + suffix
                 else:
-                    template_args.append(util.camelize(obj.name) + suffix)
-                    if isinstance(obj, metrics.Event):
-                        template_args.append("NoExtras")
+                    generic = util.camelize(obj.name) + suffix
             else:
-                if suffix == "Keys":
-                    template_args.append("NoExtraKeys")
-                    template_args.append("NoExtras")
+                if isinstance(obj, metrics.Event):
+                    generic = "NoExtras"
                 else:
-                    template_args.append("No" + suffix)
+                    generic = "No" + suffix
 
-        return "{}<{}>".format(class_name(obj.type), ", ".join(template_args))
+        return "{}<{}>".format(class_name(obj.type), generic)
 
     return class_name(obj.type)
 

glean_parser/lint.py

@@ -273,6 +273,21 @@ def check_expired_metric(
         yield ("Metric has expired. Please consider removing it.")
 
 
+def check_old_event_api(
+    metric: metrics.Metric, parser_config: Dict[str, Any]
+) -> LintGenerator:
+    # Glean v52.0.0 removed the old events API.
+    # The metrics-2-0-0 schema still supports it.
+    # We want to warn about it.
+    # This can go when we introduce 3-0-0
+
+    if not isinstance(metric, metrics.Event):
+        return
+
+    if not all("type" in x for x in metric.extra_keys.values()):
+        yield ("The old event API is gone. Extra keys require a type.")
+
+
 def check_redundant_ping(
     pings: pings.Ping, parser_config: Dict[str, Any]
 ) -> LintGenerator:
@@ -318,6 +333,7 @@ def check_redundant_ping(
     "EXPIRATION_DATE_TOO_FAR": (check_expired_date, CheckType.warning),
     "USER_LIFETIME_EXPIRATION": (check_user_lifetime_expiration, CheckType.warning),
     "EXPIRED": (check_expired_metric, CheckType.warning),
+    "OLD_EVENT_API": (check_old_event_api, CheckType.warning),
 }
 
 

glean_parser/metrics.py

@@ -307,14 +307,11 @@ class Event(Metric):
 
     default_store_names = ["events"]
 
-    _generate_enums = [("allowed_extra_keys", "Keys")]
-
     def __init__(self, *args, **kwargs):
         self.extra_keys = kwargs.pop("extra_keys", {})
         self.validate_extra_keys(self.extra_keys, kwargs.get("_config", {}))
-        if self.has_extra_types:
-            self._generate_enums = [("allowed_extra_keys_with_types", "Extra")]
         super().__init__(*args, **kwargs)
+        self._generate_enums = [("allowed_extra_keys_with_types", "Extra")]
 
     @property
     def allowed_extra_keys(self):
@@ -329,14 +326,6 @@ def allowed_extra_keys_with_types(self):
             key=lambda x: x[0],
         )
 
-    @property
-    def has_extra_types(self):
-        """
-        If any extra key has a `type` specified,
-        we generate the new struct/object-based API.
-        """
-        return any("type" in x for x in self.extra_keys.values())
-
     @staticmethod
     def validate_extra_keys(extra_keys: Dict[str, str], config: Dict[str, Any]) -> None:
         if not config.get("allow_reserved") and any(

glean_parser/parser.py

@@ -150,22 +150,6 @@ def _get_schema_for_content(
     return _get_schema(schema_url, filepath)
 
 
-def get_parameter_doc(key: str) -> str:
-    """
-    Returns documentation about a specific metric parameter.
-    """
-    schema, _ = _get_schema(METRICS_ID)
-    return schema["definitions"]["metric"]["properties"][key]["description"]
-
-
-def get_ping_parameter_doc(key: str) -> str:
-    """
-    Returns documentation about a specific ping parameter.
-    """
-    schema, _ = _get_schema(PINGS_ID)
-    return schema["additionalProperties"]["properties"][key]["description"]
-
-
 def validate(
     content: Dict[str, util.JSONType], filepath: Union[str, Path] = "<input>"
 ) -> Generator[str, None, None]:

glean_parser/rust.py

@@ -103,16 +103,18 @@ def type_name(obj):
         return "LabeledMetric<{}>".format(class_name(obj.type))
     generate_enums = getattr(obj, "_generate_enums", [])  # Extra Keys? Reasons?
     if len(generate_enums):
+        generic = None
         for name, suffix in generate_enums:
-            if not len(getattr(obj, name)) and suffix == "Keys":
-                return class_name(obj.type) + "<NoExtraKeys>"
+            if len(getattr(obj, name)):
+                generic = util.Camelize(obj.name) + suffix
             else:
-                # we always use the `extra` suffix,
-                # because we only expose the new event API
-                suffix = "Extra"
-                return "{}<{}>".format(
-                    class_name(obj.type), util.Camelize(obj.name) + suffix
-                )
+                if isinstance(obj, metrics.Event):
+                    generic = "NoExtra"
+                else:
+                    generic = "No" + suffix
+
+        return "{}<{}>".format(class_name(obj.type), generic)
+
     return class_name(obj.type)
 
 

glean_parser/swift.py

@@ -94,29 +94,17 @@ def type_name(obj: Union[metrics.Metric, pings.Ping]) -> str:
     """
     generate_enums = getattr(obj, "_generate_enums", [])
     if len(generate_enums):
-        template_args = []
+        generic = None
         for member, suffix in generate_enums:
             if len(getattr(obj, member)):
-                # Ugly hack to support the newer event extras API
-                # along the deprecated API.
-                # We need to specify both generic parameters,
-                # but only for event metrics.
-                if suffix == "Extra":
-                    if isinstance(obj, metrics.Event):
-                        template_args.append("NoExtraKeys")
-                    template_args.append(util.Camelize(obj.name) + suffix)
-                else:
-                    template_args.append(util.Camelize(obj.name) + suffix)
-                    if isinstance(obj, metrics.Event):
-                        template_args.append("NoExtras")
+                generic = util.Camelize(obj.name) + suffix
             else:
-                if suffix == "Keys":
-                    template_args.append("NoExtraKeys")
-                    template_args.append("NoExtras")
+                if isinstance(obj, metrics.Event):
+                    generic = "NoExtras"
                 else:
-                    template_args.append("No" + suffix)
+                    generic = "No" + suffix
 
-        return "{}<{}>".format(class_name(obj.type), ", ".join(template_args))
+        return "{}<{}>".format(class_name(obj.type), generic)
 
     return class_name(obj.type)
 

glean_parser/templates/kotlin.jinja2

@@ -46,18 +46,6 @@ enum class {{ obj.name|camelize }}{{ suffix }} : ReasonCode {
 }
 {%- endmacro %}
 
-{%- macro enum_decl(obj, name, suffix) -%}
-@Suppress("ClassNaming", "EnumNaming")
-enum class {{ obj.name|camelize }}{{ suffix }} : EventExtraKey {
-{% for key in obj|attr(name) %}
-    {{ key|camelize }} {
-        override fun keyName(): String = "{{ key }}"
-    }{{ "," if not loop.last }}{{ ";" if loop.last }}
-
-{% endfor %}
-}
-{%- endmacro %}
-
 {%- macro struct_decl(obj, name, suffix) -%}
 @Suppress("ClassNaming", "EnumNaming")
 data class {{ obj.name|Camelize }}{{ suffix }}(
@@ -83,12 +71,10 @@ data class {{ obj.name|Camelize }}{{ suffix }}(
 package {{ namespace }}
 
 import {{ glean_namespace }}.private.CommonMetricData // ktlint-disable import-ordering no-unused-imports
-import {{ glean_namespace }}.private.EventExtraKey // ktlint-disable import-ordering no-unused-imports
 import {{ glean_namespace }}.private.EventExtras // ktlint-disable import-ordering no-unused-imports
 import {{ glean_namespace }}.private.HistogramType // ktlint-disable import-ordering no-unused-imports
 import {{ glean_namespace }}.private.Lifetime // ktlint-disable import-ordering no-unused-imports
 import {{ glean_namespace }}.private.MemoryUnit // ktlint-disable import-ordering no-unused-imports
-import {{ glean_namespace }}.private.NoExtraKeys // ktlint-disable import-ordering no-unused-imports
 import {{ glean_namespace }}.private.NoExtras // ktlint-disable import-ordering no-unused-imports
 import {{ glean_namespace }}.private.ReasonCode // ktlint-disable import-ordering no-unused-imports
 import {{ glean_namespace }}.private.NoReasonCodes // ktlint-disable import-ordering no-unused-imports
@@ -114,11 +100,7 @@ internal object {{ category_name|Camelize }} {
     {% if obj|attr("_generate_enums") %}
     {% for name, suffix in obj["_generate_enums"] %}
     {% if obj|attr(name)|length %}
-    {% if obj.has_extra_types %}
     {{ struct_decl(obj, name, suffix)|indent }}
-    {% else %}
-    {{ enum_decl(obj, name, suffix)|indent }}
-    {% endif %}
     {% endif %}
     {% endfor %}
     {% endif %}

glean_parser/templates/rust.jinja2

@@ -13,11 +13,7 @@ Jinja2 template is not. Please file bugs! #}
 {# we always use the `extra` suffix, because we only expose the new event API #}
 {% set suffix = "Extra" %}
 {% if obj|attr(name)|length %}
-    {% if obj.has_extra_types %}
     {{ extra_keys_with_types(obj, name, suffix)|indent }}
-    {% else %}
-    compile_error!("Untyped event extras not supported. Please annotate event extras with a type. See documentation for details. (Metric: {{obj.category}}.{{obj.name}}, defined in: {{obj.defined_in['filepath']}}:{{obj.defined_in['line']}})");
-    {% endif %}
 {% endif %}
 {% endfor %}
 {% endmacro %}

glean_parser/templates/swift.jinja2

@@ -24,18 +24,6 @@ Jinja2 template is not. Please file bugs! #}
     )
 {% endmacro %}
 
-{% macro enum_decl(obj, name, suffix) %}
-enum {{ obj.name|Camelize }}{{ suffix }}: Int32, ExtraKeys {
-        {% for key in obj|attr(name) %}
-        case {{ key|camelize|variable_name }} = {{ loop.index-1 }}
-        {% endfor %}
-
-        public func index() -> Int32 {
-            return self.rawValue
-        }
-    }
-{% endmacro %}
-
 {% macro struct_decl(obj, name, suffix) %}
 struct {{ obj.name|Camelize }}{{ suffix }}: EventExtras {
         {% for item, typ in obj|attr(name) %}
@@ -119,11 +107,7 @@ extension {{ namespace }} {
         {% if obj|attr("_generate_enums") %}
         {% for name, suffix in obj["_generate_enums"] %}
         {% if obj|attr(name)|length %}
-        {% if obj.has_extra_types %}
         {{ struct_decl(obj, name, suffix)|indent }}
-        {% else %}
-        {{ enum_decl(obj, name, suffix)|indent }}
-        {% endif %}
         {% endif %}
         {% endfor %}
         {% endif %}