diff --git a/CHANGELOG.md b/CHANGELOG.md index 4deb2f29b..06effca6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Python server: Support optional metrics ([#828](https://github.com/mozilla/glean_parser/pull/828)) + ## 18.2.0 - New lint: Warn against event metrics on the `metrics` or `baseline` ping ([#823](https://github.com/mozilla/glean_parser/pull/823)) diff --git a/glean_parser/templates/python_server.jinja2 b/glean_parser/templates/python_server.jinja2 index a5573e71c..79a7f072b 100644 --- a/glean_parser/templates/python_server.jinja2 +++ b/glean_parser/templates/python_server.jinja2 @@ -35,19 +35,20 @@ class {{ ping|camelize }}ServerEventLogger: def record( self, - user_agent: str, - ip_address: str, + user_agent: str | None = None, + ip_address: str | None = None, {% for metric_type, metrics in metrics_by_type.items() %} {% if metric_type != 'event' %} {% for metric in metrics %} - {{ metric.category }}_{{ metric.name }}: {{ metric.type|py_metric_type }}, + {{ metric.category }}_{{ metric.name }}: {{ metric.type|py_metric_type }} | None = None, {% endfor %} {% endif %} {% endfor %} - events: list[dict[str, Any]] + events: list[dict[str, Any]] | None = None, ) -> None: now = datetime.now(timezone.utc) timestamp = now.isoformat() + events = [] if events is None else events for event in events: event["timestamp"] = int(1000.0 * now.timestamp()) # Milliseconds since epoch event_payload = { @@ -55,9 +56,13 @@ class {{ ping|camelize }}ServerEventLogger: {% for metric_type, metrics in metrics_by_type.items() %} {% if metric_type != 'event' %} "{{ metric_type }}": { - {% for metric in metrics %} - "{{ metric.category }}.{{ metric.name }}": {{ metric.category }}_{{ metric.name }}, - {% endfor %} + key: value + for key, value in [ + {% for metric in metrics %} + ("{{ metric.category }}.{{ metric.name }}", {{ metric.category }}_{{ metric.name }}), + {% endfor %} + ] + if value is not None }, {% endif %} {% endfor %} @@ -117,17 +122,17 @@ class {{ ping|camelize }}ServerEventLogger: {% for event in metrics_by_type["event"] %} def {{ event|record_event_function_name }}( self, - user_agent: str, - ip_address: str, + user_agent: str | None = None, + ip_address: str | None = None, {% for metric_type, metrics in metrics_by_type.items() %} {% if metric_type != 'event' %} {% for metric in metrics %} - {{ metric.category }}_{{ metric.name }}: {{ metric.type|py_metric_type }}, + {{ metric.category }}_{{ metric.name }}: {{ metric.type|py_metric_type }} | None = None, {% endfor %} {% endif %} {% endfor %} {% for extra, metadata in event.extra_keys.items() %} - {{ extra }}: {{ metadata.type|py_metric_type }}, + {{ extra }}: {{ metadata.type|py_metric_type }} | None = None, {% endfor %} ) -> None: """ @@ -156,9 +161,13 @@ class {{ ping|camelize }}ServerEventLogger: "name": "{{ event.name }}", {% if event.extra_keys %} "extra": { - {% for extra, metadata in event.extra_keys.items() %} - "{{ extra }}": str({{ extra }}){% if 'bool' == metadata.type|py_metric_type %}.lower(){% endif %}, - {% endfor %} + key: value + for key, value in [ + {% for extra, metadata in event.extra_keys.items() %} + ("{{ extra }}", str({{ extra }}){% if 'bool' == metadata.type|py_metric_type %}.lower(){% endif %}), + {% endfor %} + ] + if value is not None }, {% endif %} } diff --git a/tests/test-py/test.py b/tests/test-py/test.py index ca880df43..5d70d5fc8 100644 --- a/tests/test-py/test.py +++ b/tests/test-py/test.py @@ -15,3 +15,7 @@ object_state="some_object_state", linking=True, ) + +logger.record_backend_object_update() + +logger.record() diff --git a/tests/test_python_server.py b/tests/test_python_server.py index 63398849a..b47d6a6b7 100644 --- a/tests/test_python_server.py +++ b/tests/test_python_server.py @@ -52,29 +52,30 @@ def test_logging(tmp_path): "utf-8" ) - logged_output = json.loads(logged_output) - fields = logged_output["Fields"] - payload = fields["payload"] - - # validate that ping envelope contains all the required fields - assert "glean-server-event" == logged_output["Type"] - assert "accounts_backend" == fields["document_namespace"] - assert "events" == fields["document_type"] - assert "1" == fields["document_version"] - assert "Mozilla/5.0 ..." == fields["user_agent"] - - schema_url = ( - "https://raw.githubusercontent.com/mozilla-services/" - "mozilla-pipeline-schemas/main/" - "schemas/glean/glean/glean.1.schema.json" - ) - - # validate that ping payload is valid against glean schema - input = io.StringIO(payload) - output = io.StringIO() - assert validate_ping.validate_ping(input, output, schema_url=schema_url) == 0, ( - output.getvalue() - ) + for log_line in logged_output.splitlines(): + json_line = json.loads(log_line) + fields = json_line["Fields"] + payload = fields["payload"] + + # validate that ping envelope contains all the required fields + assert "glean-server-event" == json_line["Type"] + assert "accounts_backend" == fields["document_namespace"] + assert "events" == fields["document_type"] + assert "1" == fields["document_version"] + assert fields["user_agent"] in ("Mozilla/5.0 ...", None) + + schema_url = ( + "https://raw.githubusercontent.com/mozilla-services/" + "mozilla-pipeline-schemas/main/" + "schemas/glean/glean/glean.1.schema.json" + ) + + # validate that ping payload is valid against glean schema + input = io.StringIO(payload) + output = io.StringIO() + assert validate_ping.validate_ping(input, output, schema_url=schema_url) == 0, ( + output.getvalue() + ) def test_parser_python_server_metrics_unsupported_type(tmp_path, capsys):