diff --git a/glean_parser/__main__.py b/glean_parser/__main__.py index 86a9fce5..9a559ca7 100755 --- a/glean_parser/__main__.py +++ b/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() } diff --git a/glean_parser/javascript.py b/glean_parser/javascript.py index db091d6f..f1e32871 100644 --- a/glean_parser/javascript.py +++ b/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: diff --git a/glean_parser/lint.py b/glean_parser/lint.py index 3c7f045c..68665ca8 100644 --- a/glean_parser/lint.py +++ b/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 diff --git a/glean_parser/markdown.py b/glean_parser/markdown.py index 7bf48ba2..45732d58 100644 --- a/glean_parser/markdown.py +++ b/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") diff --git a/glean_parser/metrics.py b/glean_parser/metrics.py index 2a945613..26881db3 100644 --- a/glean_parser/metrics.py +++ b/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. diff --git a/glean_parser/parser.py b/glean_parser/parser.py index 98489e3e..8faa94e8 100644 --- a/glean_parser/parser.py +++ b/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" ): diff --git a/glean_parser/pings.py b/glean_parser/pings.py index 937bb48f..13ebc5f4 100644 --- a/glean_parser/pings.py +++ b/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. diff --git a/glean_parser/templates/javascript.jinja2 b/glean_parser/templates/javascript.jinja2 index 4036e892..cd4f7cb2 100644 --- a/glean_parser/templates/javascript.jinja2 +++ b/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} diff --git a/glean_parser/templates/kotlin.jinja2 b/glean_parser/templates/kotlin.jinja2 index 97a4a3dc..4097087e 100644 --- a/glean_parser/templates/kotlin.jinja2 +++ b/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( diff --git a/glean_parser/templates/markdown.jinja2 b/glean_parser/templates/markdown.jinja2 index 93703112..93c6b9d9 100644 --- a/glean_parser/templates/markdown.jinja2 +++ b/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 %} diff --git a/glean_parser/templates/rust.jinja2 b/glean_parser/templates/rust.jinja2 index 6a2c74ac..16efd1ed 100644 --- a/glean_parser/templates/rust.jinja2 +++ b/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 }}, diff --git a/glean_parser/templates/swift.jinja2 b/glean_parser/templates/swift.jinja2 index d23c2bb6..2e5baba5 100644 --- a/glean_parser/templates/swift.jinja2 +++ b/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( diff --git a/glean_parser/translate.py b/glean_parser/translate.py index e9c35ae4..96008496 100644 --- a/glean_parser/translate.py +++ b/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) diff --git a/glean_parser/util.py b/glean_parser/util.py index 4f35b615..c7cf4d88 100644 --- a/glean_parser/util.py +++ b/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", diff --git a/tests/test_kotlin.py b/tests/test_kotlin.py index dcdb532c..b357b4d3 100644 --- a/tests/test_kotlin.py +++ b/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 by lazy { // generated from event.example EventMetricType( CommonMetricData( category = "event", name = "example", sendInPings = listOf("events"), lifetime = Lifetime.PING, disabled = false, inSession = true ), allowedExtraKeys = listOf("And1WithExtraCasing", "alice", "bob", "charlie")) } }' # noqa + expected = 'EventMetricType by lazy { // generated from event.example EventMetricType( 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 diff --git a/tests/test_metrics.py b/tests/test_metrics.py index af68d20b..e8c2477b 100644 --- a/tests/test_metrics.py +++ b/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(): diff --git a/tests/test_parser.py b/tests/test_parser.py index 9801add2..eb8bf5ba 100644 --- a/tests/test_parser.py +++ b/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() } diff --git a/tests/test_rust.py b/tests/test_rust.py index dc224ce0..d0e1f2c7 100644 --- a/tests/test_rust.py +++ b/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) diff --git a/tests/test_swift.py b/tests/test_swift.py index a4567744..6d2ce37b 100644 --- a/tests/test_swift.py +++ b/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)