DENG-1736 Add support for event metric type in server-side JavaScript outputter

2023-11-06 12:39:46 +01:00 · 2023-11-06 12:39:46 +01:00 · 05e54649d6
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -2,6 +2,8 @@

 ## Unreleased

+- Add support for event metric type in server-side JavaScript outputter ([DENG-1736](https://mozilla-hub.atlassian.net/browse/DENG-1736))
+
 ## 10.0.3

 - Warn about empty or TODO-tagged data reviews in the list ([#634](https://github.com/mozilla/glean_parser/pull/634))
--- a/glean_parser/javascript_server.py
+++ b/glean_parser/javascript_server.py
@ -17,6 +17,13 @@ pipeline to pick the messages up and process.

 Warning: this outputter supports limited set of metrics,
 see `SUPPORTED_METRIC_TYPES` below.
+
+Note on `event` metric type:
+It is advised to use `event` metric type in server-side environments. In this case API
+generated here will have a `record{event}` method per event metric.
+For historical reasons we also support custom pings-as-events pattern where event name
+is encoded in a String metric. In this case API generated here will have a single
+`record` method which takes event name as a parameter.
 """
 from collections import defaultdict
 from pathlib import Path
@ -28,11 +35,14 @@ from . import util

 # Adding a metric here will require updating the `generate_js_metric_type` function
 # and might require changes to the template.
-SUPPORTED_METRIC_TYPES = ["string"]
+SUPPORTED_METRIC_TYPES = ["string", "event"]


-def event_class_name(pingName: str) -> str:
-    return util.Camelize(pingName) + "ServerEvent"
+def event_class_name(pingName: str, event_metric_exists: bool) -> str:
+    # For compatibility with FxA codebase we don't want to add "Logger" suffix
+    # when custom pings without event metrics are used.
+    suffix = "Logger" if event_metric_exists else ""
+    return util.Camelize(pingName) + "ServerEvent" + suffix


 def generate_metric_name(metric: metrics.Metric) -> str:
@ -47,12 +57,20 @@ def generate_js_metric_type(metric: metrics.Metric) -> str:
    return metric.type


-def generate_metric_argument_description(metric: metrics.Metric) -> str:
-    return metric.description.replace("\n", " ").rstrip()
+def generate_ping_factory_method(ping: str, event_metric_exists: bool) -> str:
+    # `ServerEventLogger` better describes role of the class that this factory
+    # method generates, but for compatibility with existing FxA codebase
+    # we use `Event` suffix if no event metrics are defined.
+    suffix = "ServerEventLogger" if event_metric_exists else "Event"
+    return f"create{util.Camelize(ping)}{suffix}"


-def generate_ping_factory_method(ping: str) -> str:
-    return f"create{util.Camelize(ping)}Event"
+def generate_event_metric_record_function_name(metric: metrics.Metric) -> str:
+    return f"record{util.Camelize(metric.category)}{util.Camelize(metric.name)}"
+
+
+def clean_string(s: str) -> str:
+    return s.replace("\n", " ").rstrip()


 def output(
@ -79,8 +97,12 @@ def output(
            ("metric_name", generate_metric_name),
            ("metric_argument_name", generate_metric_argument_name),
            ("js_metric_type", generate_js_metric_type),
-            ("metric_argument_description", generate_metric_argument_description),
            ("factory_method", generate_ping_factory_method),
+            (
+                "event_metric_record_function_name",
+                generate_event_metric_record_function_name,
+            ),
+            ("clean_string", clean_string),
        ),
    )

@ -98,6 +120,8 @@ def output(
        print("❌ No ping definition found." + PING_METRIC_ERROR_MSG)
        return

+    EVENT_METRIC_EXISTS = False
+
    # Go through all metrics in objs and build a map of
    # ping->list of metric categories->list of metrics
    # for easier processing in the template.
@ -113,6 +137,10 @@ def output(
                        + " metric type."
                    )
                    continue
+                if metric.type == "event":
+                    # This is used in the template - generated code is slightly
+                    # different when event metric type is used.
+                    EVENT_METRIC_EXISTS = True
                for ping in metric.send_in_pings:
                    metrics_by_type = ping_to_metrics[ping]
                    metrics_list = metrics_by_type.setdefault(metric.type, [])
@ -129,6 +157,7 @@ def output(
            template.render(
                parser_version=__version__,
                pings=ping_to_metrics,
+                event_metric_exists=EVENT_METRIC_EXISTS,
                lang=lang,
            )
        )
--- a/glean_parser/templates/javascript_server.jinja2
+++ b/glean_parser/templates/javascript_server.jinja2
@ -17,19 +17,27 @@ import mozlog{% if lang == "typescript" %}, { Logger }{% endif %} from 'mozlog';
 const GLEAN_EVENT_MOZLOG_TYPE = 'glean-server-event';
 {% if lang == "typescript" %}
 type LoggerOptions = { app: string; fmt?: 'heka' };
+{% if event_metric_exists %}
+type Event = {
+  category: string;
+  name: string;
+  extra: Record<string, any>;
+  timestamp?: string;
+};
+{% endif %}
 {% endif %}

 let _logger{% if lang == "typescript" %}: Logger{% endif %};

 {% for ping, metrics_by_type in pings.items() %}
-class {{ ping|event_class_name }} {
+class {{ ping|event_class_name(event_metric_exists) }} {
  {% if lang == "typescript" %}
  _applicationId: string;
  _appDisplayVersion: string;
  _channel: string;
  {% endif %}
  /**
-   * Create {{ ping|event_class_name }} instance.
+   * Create {{ ping|event_class_name(event_metric_exists) }} instance.
   *
   * @param {string} applicationId - The application ID.
   * @param {string} appDisplayVersion - The application display version.
@ -64,6 +72,9 @@ class {{ ping|event_class_name }} {
      {% endif %}
    }
  }
+  {% if event_metric_exists %}
+  #record({
+  {% else %}
  /**
   * Record and submit a server event object.
   * Event is logged using internal mozlog logger.
@ -73,40 +84,59 @@ class {{ ping|event_class_name }} {
   *                              information and scrubbed at ingestion.
   {% for metric_type, metrics in metrics_by_type.items() %}
   {% for metric in metrics %}
-   * @param { {{-metric|js_metric_type-}} } {{ metric|metric_argument_name }} - {{ metric|metric_argument_description }}.
+   * @param { {{-metric|js_metric_type-}} } {{ metric|metric_argument_name }} - {{ metric.description|clean_string }}.
   {% endfor %}
   {% endfor %}
   */
  record({
+  {% endif %}
    user_agent,
    ip_address,
    {% for metric_type, metrics in metrics_by_type.items() %}
+    {% if metric_type != 'event' %}
    {% for metric in metrics %}
    {{ metric|metric_argument_name }},
    {% endfor %}
+    {% endif %}
    {% endfor %}
+    {% if event_metric_exists %}
+    event,
+    {% endif %}
  {% if lang == "typescript" %}
  }: {
    user_agent: string,
    ip_address: string,
    {% for metric_type, metrics in metrics_by_type.items() %}
+    {% if metric_type != 'event' %}
    {% for metric in metrics %}
-    {{ metric|metric_argument_name }}: {{ metric|js_metric_type }};
+    {{ metric|metric_argument_name }}: {{ metric|js_metric_type }},
    {% endfor %}
+    {% endif %}
    {% endfor %}
+    {% if event_metric_exists %}
+    event: Event
+    {% endif %}
  {% endif %}
  }) {
    const timestamp = new Date().toISOString();
+    {% if event_metric_exists %}
+    event.timestamp = timestamp;
+    {% endif %}
    const eventPayload = {
      metrics: {
        {% for metric_type, metrics in metrics_by_type.items() %}
+        {% if metric_type != 'event' %}
        {{ metric_type }}: {
          {% for metric in metrics %}
          '{{ metric|metric_name }}': {{ metric|metric_argument_name }},
          {% endfor %}
        },
+        {% endif %}
        {% endfor %}
      },
+      {% if event_metric_exists %}
+      events: [event],
+      {% endif %}
      ping_info: {
        seq: 0, // this is required, however doesn't seem to be useful in server context
        start_time: timestamp,
@ -140,11 +170,92 @@ class {{ ping|event_class_name }} {
    // this is similar to how FxA currently logs with mozlog: https://github.com/mozilla/fxa/blob/4c5c702a7fcbf6f8c6b1f175e9172cdd21471eac/packages/fxa-auth-server/lib/log.js#L289
    _logger.info(GLEAN_EVENT_MOZLOG_TYPE, ping);
  }
+  {% if event_metric_exists %}
+  {% for event in metrics_by_type["event"] %}
+  /**
+   * Record and submit a {{ event.category }}_{{ event.name }} event:
+   * {{ event.description|clean_string }}
+   * Event is logged using internal mozlog logger.
+   *
+   * @param {string} user_agent - The user agent.
+   * @param {string} ip_address - The IP address. Will be used to decode Geo
+   *                              information and scrubbed at ingestion.
+   {% for metric_type, metrics in metrics_by_type.items() %}
+   {% if metric_type != 'event' %}
+   {% for metric in metrics %}
+   * @param { {{-metric|js_metric_type-}} } {{ metric|metric_argument_name }} - {{ metric.description|clean_string }}.
+   {% endfor %}
+   {% endif %}
+   {% endfor %}
+   {% if event.extra_keys %}
+   {% for extra, metadata in event.extra_keys.items() %}
+   * @param { {{-metadata.type-}} } {{ extra }} - {{ metadata.description|clean_string }}.
+   {% endfor %}
+   {% endif %}
+   */
+  {{ event|event_metric_record_function_name }}({
+    user_agent,
+    ip_address,
+    {% for metric_type, metrics in metrics_by_type.items() %}
+    {% if metric_type != 'event' %}
+    {% for metric in metrics %}
+    {{ metric|metric_argument_name }},
+    {% endfor %}
+    {% endif %}
+    {% endfor %}
+    {% for extra, metadata in event.extra_keys.items() %}
+    {{ extra }},
+    {% endfor %}
+  {% if lang == "typescript" %}
+  }: {
+    user_agent: string,
+    ip_address: string,
+    {% for metric_type, metrics in metrics_by_type.items() %}
+    {% if metric_type != 'event' %}
+    {% for metric in metrics %}
+    {{ metric|metric_argument_name }}: {{ metric|js_metric_type }},
+    {% endfor %}
+    {% endif %}
+    {% endfor %}
+    {% for extra, metadata in event.extra_keys.items() %}
+    {{ extra }}: {{metadata.type}},
+    {% endfor %}
+  {% endif %}
+  }) {
+    let event = {
+      'category': '{{ event.category }}',
+      'name': '{{ event.name }}',
+      {% if event.extra_keys %}
+      'extra': {
+        {% for extra, metadata in event.extra_keys.items() %}
+        '{{ extra }}': {{ extra }},
+        {% endfor %}
+      },
+      {% endif %}
+    };
+    this.#record({
+      user_agent,
+      ip_address,
+      identifiers_fxa_account_id,
+      event
+    });
+  }
+  {% endfor %}
+  {% endif %}
 }
 {% endfor %}
 {% for ping in pings %}

-export const {{ ping|factory_method }} = function ({
+/**
+ * Factory function that creates an instance of Glean Server Event Logger to
+ * record `{{ ping }}` ping events.
+ * @param {string} applicationId - The application ID.
+ * @param {string} appDisplayVersion - The application display version.
+ * @param {string} channel - The channel.
+ * @param {Object} logger_options - The logger options.
+ * @returns {EventsServerEventLogger} An instance of EventsServerEventLogger.
+ */
+export const {{ ping|factory_method(event_metric_exists) }} = function ({
  applicationId,
  appDisplayVersion,
  channel,
@ -157,7 +268,7 @@ export const {{ ping|factory_method }} = function ({
  logger_options: LoggerOptions;
 {% endif %}
 }) {
-  return new {{ ping|event_class_name }}(
+  return new {{ ping|event_class_name(event_metric_exists) }}(
    applicationId,
    appDisplayVersion,
    channel,
--- a/tests/test_javascript_server.py
+++ b/tests/test_javascript_server.py
@ -71,7 +71,16 @@ def test_parser_js_server(tmpdir):
 def test_generate_ping_factory_method():
    ping = "accounts_events"
    expected_result = "createAccountsEventsEvent"
-    result = javascript_server.generate_ping_factory_method(ping)
+    result = javascript_server.generate_ping_factory_method(
+        ping, event_metric_exists=False
+    )
+    assert result == expected_result
+
+    ping = "accounts_events"
+    expected_result = "createAccountsEventsServerEventLogger"
+    result = javascript_server.generate_ping_factory_method(
+        ping, event_metric_exists=True
+    )
    assert result == expected_result