Combine category/name -> identifier for Rust/Kotlin/Swift

glean_parser/__main__.py

@@ -231,7 +231,7 @@ def dump(input, allow_reserved, allow_missing_files, require_tags):
     assert len(errs) == 0
 
     metrics = {
-        metric.identifier(): metric.serialize()
+        metric.identifier: metric.serialize()
         for category, probes in results.value.items()
         for probe_name, metric in probes.items()
     }

glean_parser/javascript.py

@@ -121,7 +121,9 @@ def args(obj_type: str) -> Dict[str, object]:
     if obj_type == "ping":
         return {"common": util.ping_args, "extra": []}
 
-    return {"common": util.common_metric_args, "extra": util.extra_metric_args}
+    common = ["category", "name"] + util.common_metric_args
+    common.remove("identifier")
+    return {"common": common, "extra": util.extra_metric_args}
 
 
 def generate_build_date(date: Optional[str]) -> str:

glean_parser/lint.py

@@ -436,10 +436,10 @@ def check_name_too_similar(
         if check_name in metric.no_lint:
             continue
 
-        no_punc = metric.identifier().replace("_", "").replace(".", "")
+        no_punc = metric.identifier.replace("_", "").replace(".", "")
         if no_punc in seen_metrics:
-            msg = f"Metric `{metric.identifier()}`'s name is too similar to existing metric `{seen_metrics[no_punc].identifier()}`"
-            nit = GlinterNit(check_name, metric.identifier(), msg, check_type)
+            msg = f"Metric `{metric.identifier}`'s name is too similar to existing metric `{seen_metrics[no_punc].identifier}`"
+            nit = GlinterNit(check_name, metric.identifier, msg, check_type)
             yield nit
 
         seen_metrics[no_punc] = metric

glean_parser/markdown.py

@@ -231,7 +231,7 @@ def output_markdown(
     # in the docs and to make generated docs reproducible.
     for ping_name in metrics_by_pings:
         metrics_by_pings[ping_name] = sorted(
-            metrics_by_pings[ping_name], key=lambda x: x.identifier()
+            metrics_by_pings[ping_name], key=lambda x: x.identifier
         )
 
     project_title = options.get("project_title", "this project")

glean_parser/metrics.py

@@ -201,6 +201,7 @@ def _serialize_input(self) -> Dict[str, util.JSONType]:
         modified_dict = util.remove_output_params(d, "defined_in")
         return modified_dict
 
+    @property
     def identifier(self) -> str:
         """
         Create an identifier unique for this metric.

glean_parser/parser.py

@@ -242,7 +242,7 @@ def _instantiate_metrics(
                         )
                         metric_obj = None
                     elif (
-                        metric_obj.identifier()
+                        metric_obj.identifier
                         in ("glean.attribution.ext", "glean.distribution.ext")
                         and metric_obj.type != "object"
                     ):

glean_parser/pings.py

@@ -114,6 +114,7 @@ def _serialize_input(self) -> Dict[str, util.JSONType]:
         modified_dict = util.remove_output_params(modified_dict, "schedules_pings")
         return modified_dict
 
+    @property
     def identifier(self) -> str:
         """
         Used for the "generated from ..." comment in the output.

glean_parser/templates/javascript.jinja2

@@ -37,7 +37,7 @@ import {{ type|class_name }} from "@mozilla/glean/private/{{ type|import_path }}
 /**
  * {{ obj.description|wordwrap() | replace("\n", "\n * ") }}
  *
- * Generated from `{{ obj.identifier() }}`.
+ * Generated from `{{ obj.identifier }}`.
  */
 {% if obj.labeled %}
 {% if platform != "qt" %}export {% endif %}const {{ obj.name|camelize }} = {{ labeled_obj_declaration(obj) }};
@@ -49,7 +49,7 @@ import {{ type|class_name }} from "@mozilla/glean/private/{{ type|import_path }}
 {% for name, suffix in obj["_generate_enums"] %}
 {% if obj|attr(name)|length and name == "reason_codes" %}
 /**
- * Reason codes for `{{ obj.identifier() }}`.
+ * Reason codes for `{{ obj.identifier }}`.
  *
  * @readonly
  * @enum {string}

glean_parser/templates/kotlin.jinja2

@@ -13,7 +13,7 @@ Jinja2 template is not. Please file bugs! #}
 {% if (access != "private ") -%}
 @get:JvmName("{{ obj.name|camelize }}{{ suffix }}")
 {% endif -%}
-{{ access }}val {{ obj.name|camelize }}{{ suffix }}: {{ obj|type_name }}{% if lazy %} by lazy { {%- else %} ={% endif %} // generated from {{ obj.identifier() }}
+{{ access }}val {{ obj.name|camelize }}{{ suffix }}: {{ obj|type_name }}{% if lazy %} by lazy { {%- else %} ={% endif %} // generated from {{ obj.identifier }}
 {% if obj.type == 'ping' %}
     {{ obj|type_name }}(
         {% for arg_name in ping_args if obj[arg_name] is defined %}
@@ -346,7 +346,7 @@ internal object {{ category_name|Camelize }} {
     /**
      * {{ obj.description|wordwrap() | replace('\n', '\n        * ') }}
      */
-    val {{ obj.name|camelize }}: LabeledMetricType<{{ obj|type_name }}> by lazy { // generated from {{ obj.identifier() }}
+    val {{ obj.name|camelize }}: LabeledMetricType<{{ obj|type_name }}> by lazy { // generated from {{ obj.identifier }}
         LabeledMetricType(
             {% if obj.type == "labeled_custom_distribution" %}
             CustomDistributionLabeledMetricData(

glean_parser/templates/markdown.jinja2

@@ -70,7 +70,7 @@ In addition to those built-in metrics, the following metrics are added to the pi
 | Name | Type | Description | Data reviews | Extras | Expiration | [Data Sensitivity](https://wiki.mozilla.org/Firefox/Data_Collection) |
 | --- | --- | --- | --- | --- | --- | --- |
 {% for metric in metrics_by_pings[ping_name] %}
-| {{ metric.identifier() }} |
+| {{ metric.identifier }} |
 {{- '['}}{{ metric.type }}]({{ metric.type|metrics_docs }}) |
 {{- metric.description|replace("\n", " ") }} |
 {%- for data_review in metric.data_reviews %}

glean_parser/templates/rust.jinja2

@@ -97,8 +97,7 @@ impl ExtraKeys for {{ obj.name|Camelize }}{{ suffix }} {
 {% endmacro %}
 {% macro common_metric_data(obj) %}
 CommonMetricData {
-    category: {{ obj.category|rust }},
-    name: {{ obj.name|rust }},
+    identifier: {{ obj.identifier|rust }},
     send_in_pings: {{ obj.send_in_pings|rust }},
     lifetime: {{ obj.lifetime|rust }},
     disabled: {{ obj.is_disabled()|rust }},

glean_parser/templates/swift.jinja2

@@ -12,7 +12,7 @@ Jinja2 template is not. Please file bugs! #}
  * 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/. */
 {%- macro obj_declaration(obj, suffix='', access='') %}
-{{ access }}static let {{ obj.name|camelize|variable_name }}{{ suffix }} = {{ obj|type_name }}( // generated from {{ obj.identifier() }}
+{{ access }}static let {{ obj.name|camelize|variable_name }}{{ suffix }} = {{ obj|type_name }}( // generated from {{ obj.identifier }}
         CommonMetricData(
             {% for arg_name in common_metric_args if obj[arg_name] is defined %}
             {{ arg_name|camelize }}: {{ obj[arg_name]|swift }}{{ "," if not loop.last }}
@@ -216,7 +216,7 @@ extension {{ namespace }} {
         {% if obj.labeled %}
         {{ obj_declaration(obj, 'Label', 'private ') | indent }}
         /// {{ obj.description|wordwrap() | replace('\n', '\n        /// ') }}
-        static let {{ obj.name|camelize|variable_name }} = try! LabeledMetricType<{{ obj|type_name }}>( // generated from {{ obj.identifier() }}
+        static let {{ obj.name|camelize|variable_name }} = try! LabeledMetricType<{{ obj|type_name }}>( // generated from {{ obj.identifier }}
             {% if obj.type == "labeled_custom_distribution" %}
             .customDistribution(
                 cmd: CommonMetricData(

glean_parser/translate.py

@@ -87,7 +87,7 @@ def transform_metrics(objects):
         if category_name == "tags":
             continue
         for metric in category_val.values():
-            fqmn = metric.identifier()
+            fqmn = metric.identifier
             if getattr(metric, "type", None) == "counter":
                 counters[fqmn] = metric
             denominator_name = getattr(metric, "denominator_metric", None)

glean_parser/util.py

@@ -521,8 +521,7 @@ def remove_output_params(d, output_params):
 
 # Names of  parameters to pass to all metrics constructors constructors.
 common_metric_args = [
-    "category",
-    "name",
+    "identifier",
     "send_in_pings",
     "lifetime",
     "disabled",

tests/test_kotlin.py

@@ -84,7 +84,7 @@ def test_parser(tmp_path):
 
     with (tmp_path / "GleanInternalMetrics.kt").open("r", encoding="utf-8") as fd:
         content = fd.read()
-        assert 'category = ""' in content
+        assert 'identifier = "' in content
 
     with (tmp_path / "GleanBuildInfo.kt").open("r", encoding="utf-8") as fd:
         content = fd.read()
@@ -306,7 +306,7 @@ def test_arguments_are_generated_in_deterministic_order(tmp_path):
     with (tmp_path / "Event.kt").open("r", encoding="utf-8") as fd:
         content = fd.read()
         content = " ".join(content.split())
-        expected = 'EventMetricType<ExampleExtra> by lazy { // generated from event.example EventMetricType<ExampleExtra>( CommonMetricData( category = "event", name = "example", sendInPings = listOf("events"), lifetime = Lifetime.PING, disabled = false, inSession = true ), allowedExtraKeys = listOf("And1WithExtraCasing", "alice", "bob", "charlie")) } }'  # noqa
+        expected = 'EventMetricType<ExampleExtra> by lazy { // generated from event.example EventMetricType<ExampleExtra>( CommonMetricData( identifier = "event.example", sendInPings = listOf("events"), lifetime = Lifetime.PING, disabled = false, inSession = true ), allowedExtraKeys = listOf("And1WithExtraCasing", "alice", "bob", "charlie")) } }'  # noqa
         assert expected in content
 
 

tests/test_metrics.py

@@ -122,7 +122,7 @@ def test_identifier():
         expires="never",
     )
 
-    assert m.identifier() == "category.metric"
+    assert m.identifier == "category.metric"
 
 
 def test_identifier_glean_category():
@@ -140,7 +140,7 @@ def test_identifier_glean_category():
         expires="never",
     )
 
-    assert m.identifier() == "metric"
+    assert m.identifier == "metric"
 
 
 def test_reserved_extra_keys():

tests/test_parser.py

@@ -1188,7 +1188,7 @@ def test_allow_renaming_of_fields():
     assert len(errs) == 0
 
     metrics = {
-        metric.identifier(): metric.serialize(
+        metric.identifier: metric.serialize(
             rename_fields={"_generate_structure": "structure"}
         )
         for category, probes in results.value.items()
@@ -1274,7 +1274,7 @@ def test_no_internal_fields_exposed():
     assert len(errs) == 0
 
     metrics = {
-        metric.identifier(): metric.serialize()
+        metric.identifier: metric.serialize()
         for category, probes in results.value.items()
         for probe_name, metric in probes.items()
     }

tests/test_rust.py

@@ -58,7 +58,7 @@ def test_parser(tmp_path):
 
         assert "True if the user has set Firefox as the default browser." in content
         assert "جمع 搜集" in content
-        assert 'category: "telemetry"' in content
+        assert 'identifier: "telemetry.' in content
 
     # We don't have a cargo.toml, not sure what to do here aside from creating a fake
     # one for the purpose of running cargo fmt and cargo clippy
@@ -193,7 +193,7 @@ def test_order_of_fields(tmp_path):
         elif re.search("CommonMetricData {", line):
             found_metric = True
 
-    expected_fields = ["category", "name", "send_in_pings", "lifetime", "disabled"]
+    expected_fields = ["identifier", "send_in_pings", "lifetime", "disabled"]
 
     # We only check the limited list of always available fields.
     size = len(expected_fields)

tests/test_swift.py

@@ -46,7 +46,7 @@ def test_parser(tmp_path):
 
         assert "True if the user has set Firefox as the default browser." in content
         assert "جمع 搜集" in content
-        assert 'category: ""' in content
+        assert 'identifier: "' in content
         assert "class GleanBuild" in content
         assert "BuildInfo(buildDate:" in content
 
@@ -240,7 +240,7 @@ def test_order_of_fields(tmp_path):
         elif re.search("CommonMetricData", line):
             found_metric = True
 
-    expected_fields = ["category", "name", "sendInPings", "lifetime", "disabled"]
+    expected_fields = ["identifier", "sendInPings", "lifetime", "disabled"]
 
     # We only check the limited list of always available fields.
     size = len(expected_fields)