diff --git a/CHANGELOG.md b/CHANGELOG.md index a062e804..e07d757f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Go server: Add support for `labeled_boolean` metrics with static labels ([AE-1250](https://mozilla-hub.atlassian.net/browse/AE-1250)) + ## 19.0.0 - Python server: Support optional metrics ([#828](https://github.com/mozilla/glean_parser/pull/828)) diff --git a/glean_parser/go_server.py b/glean_parser/go_server.py index af4b5e50..b6016535 100644 --- a/glean_parser/go_server.py +++ b/glean_parser/go_server.py @@ -38,6 +38,7 @@ "event", "datetime", "boolean", + "labeled_boolean", # static labels only; dynamic labels are not supported "string_list", ] @@ -87,6 +88,28 @@ def clean_string(s: str) -> str: return s.replace("\n", " ").rstrip() +def validate_labeled_boolean(metric: metrics.Metric) -> bool: + """ + Validate that a labeled_boolean metric has static labels defined. + + The Go server outputter requires labels to be listed in metrics.yaml + because it generates a Go struct with a field per label at build time. + Dynamic labels are not supported. + + Returns: + bool: True if valid, False otherwise + """ + if not getattr(metric, "ordered_labels", None): + print( + "❌ Ignoring labeled_boolean metric without static labels: " + + f"{metric.name}." + + " Define labels in metrics.yaml to use this metric type." + ) + return False + + return True + + def output_go( objs: metrics.ObjectTree, output_dir: Path, options: Optional[Dict[str, Any]] ) -> None: @@ -118,6 +141,9 @@ def output_go( # unique list of event metrics used in any ping event_metrics: List[metrics.Metric] = [] + # unique list of labeled_boolean metrics used in any ping + labeled_boolean_metrics: List[metrics.Metric] = [] + # 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. @@ -134,10 +160,22 @@ def output_go( ) continue + # Validate labeled_boolean metrics + if metric.type == "labeled_boolean" and not validate_labeled_boolean( + metric + ): + continue + for ping in metric.send_in_pings: if metric.type == "event" and metric not in event_metrics: event_metrics.append(metric) + if ( + metric.type == "labeled_boolean" + and metric not in labeled_boolean_metrics + ): + labeled_boolean_metrics.append(metric) + metrics_by_type = ping_to_metrics[ping] metrics_list = metrics_by_type.setdefault(metric.type, []) metrics_list.append(metric) @@ -156,6 +194,9 @@ def output_go( with filepath.open("w", encoding="utf-8") as fd: fd.write( template.render( - parser_version=__version__, pings=ping_to_metrics, events=event_metrics + parser_version=__version__, + pings=ping_to_metrics, + events=event_metrics, + labeled_booleans=labeled_boolean_metrics, ) ) diff --git a/glean_parser/templates/go_server.jinja2 b/glean_parser/templates/go_server.jinja2 index bdcb2625..320bfbfa 100644 --- a/glean_parser/templates/go_server.jinja2 +++ b/glean_parser/templates/go_server.jinja2 @@ -220,6 +220,18 @@ func (e {{ event|event_type_name }}) gleanEvent() gleanEvent { } {% endfor %} {% endif %} +{# Generate struct types for labeled_boolean metrics #} +{% if labeled_booleans %} +{% for metric in labeled_booleans %} + +// {{ metric.description|clean_string }} +type {{ metric|metric_argument_name }} struct { + {% for label in metric.ordered_labels %} + {{ label|event_extra_name }} bool // {{ label }} + {% endfor %} +} +{% endfor %} +{% endif %} {# struct & methods for submitting pings #} {% for ping, metrics_by_type in pings.items() %} {% if metrics_by_type['event'] %} @@ -240,7 +252,11 @@ type {{ ping|ping_type_name }} struct { {% for metric_type, metrics in metrics_by_type.items() %} {% if metric_type != 'event' %} {% for metric in metrics %} + {% if metric_type == 'labeled_boolean' %} + {{ metric|metric_argument_name }} {{ metric|metric_argument_name }} // {{ metric.description|clean_string }} + {% else %} {{ metric|metric_argument_name }} {{ metric_type|go_metric_type}} // {{ metric.description|clean_string }} + {% endif %} {% endfor %} {% endif %} {% endfor %} @@ -261,6 +277,12 @@ func (g GleanEventsLogger) Record{{ ping|ping_type_name }}( {% for metric in metrics %} {% if metric_type == 'datetime' %} "{{ metric|metric_name }}": params.{{ metric|metric_argument_name }}.Format("2006-01-02T15:04:05.000Z"), + {% elif metric_type == 'labeled_boolean' %} + "{{ metric|metric_name }}": map[string]bool{ + {% for label in metric.ordered_labels %} + "{{ label }}": params.{{ metric|metric_argument_name }}.{{ label|event_extra_name }}, + {% endfor %} + }, {% else %} "{{ metric|metric_name }}": params.{{ metric|metric_argument_name }}, {% endif %} diff --git a/tests/data/go_server_labeled_boolean_metrics.yaml b/tests/data/go_server_labeled_boolean_metrics.yaml new file mode 100644 index 00000000..f4bb729a --- /dev/null +++ b/tests/data/go_server_labeled_boolean_metrics.yaml @@ -0,0 +1,25 @@ +# Any copyright is dedicated to the Public Domain. +# https://creativecommons.org/publicdomain/zero/1.0/ + +--- +$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0 + +telemetry: + feature_flags: + type: labeled_boolean + description: > + Feature flags for A/B testing + labels: + - feature_one + - feature_two + - feature_three + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1234567 + data_reviews: + - https://example.com/review + notification_emails: + - telemetry@example.com + lifetime: application + expires: never + send_in_pings: + - events diff --git a/tests/test_go_server.py b/tests/test_go_server.py index 8790ff4e..24c99e4d 100644 --- a/tests/test_go_server.py +++ b/tests/test_go_server.py @@ -39,8 +39,6 @@ def test_parser_go_server_metrics_unsupported_type(tmp_path, capsys): captured = capsys.readouterr() assert "Ignoring unsupported metric type" in captured.out unsupported_types = [ - "boolean", - "labeled_boolean", "labeled_string", "timespan", "uuid", @@ -50,6 +48,49 @@ def test_parser_go_server_metrics_unsupported_type(tmp_path, capsys): assert t in captured.out +def test_parser_go_server_labeled_boolean_without_labels(tmp_path, capsys): + """Test that labeled_boolean without static labels is rejected.""" + translate.translate( + [ + ROOT / "data" / "go_server_metrics_unsupported.yaml", + ], + "go_server", + tmp_path, + ) + captured = capsys.readouterr() + assert "Ignoring labeled_boolean metric without static labels" in captured.out + + +def test_parser_go_server_labeled_boolean(tmp_path): + """Test that labeled_boolean metrics generate proper struct types.""" + translate.translate( + ROOT / "data" / "go_server_labeled_boolean_metrics.yaml", + "go_server", + tmp_path, + ) + + assert set(x.name for x in tmp_path.iterdir()) == set(["server_events.go"]) + + # Read generated file and verify struct is created + with (tmp_path / "server_events.go").open("r", encoding="utf-8") as fd: + content = fd.read() + + # Check that the labeled_boolean struct type was generated + assert "type TelemetryFeatureFlags struct {" in content + assert "FeatureOne bool" in content + assert "FeatureTwo bool" in content + assert "FeatureThree bool" in content + + # Check that it's used in the ping struct + assert "TelemetryFeatureFlags TelemetryFeatureFlags" in content + + # Check that serialization includes map creation + assert "map[string]bool{" in content + assert '"feature_one":' in content + assert '"feature_two":' in content + assert '"feature_three":' in content + + def test_parser_go_server_events_only(tmp_path): """Test that parser works for definitions that only use events ping""" translate.translate( @@ -344,6 +385,67 @@ def test_run_logging_nil_writer(tmp_path): assert logged_output == "writer not specified\n" +@pytest.mark.go_dependency +def test_run_logging_labeled_boolean(tmp_path): + glean_module_path = tmp_path / "glean" + + translate.translate( + [ + ROOT / "data" / "go_server_labeled_boolean_metrics.yaml", + ], + "go_server", + glean_module_path, + ) + + code = """ + _ = time.Now() // satisfy Go's unused import check for "time" + logger.RecordEventsPing( + glean.RequestInfo{ + UserAgent: "glean-test/1.0", + IpAddress: "127.0.0.1", + }, + glean.EventsPing{ + TelemetryFeatureFlags: glean.TelemetryFeatureFlags{ + FeatureOne: true, + FeatureTwo: false, + FeatureThree: true, + }, + }, + ) + """ + + logged_output = run_logger(tmp_path, code) + logged_output = json.loads(logged_output) + fields = logged_output["Fields"] + payload_str = fields["payload"] + payload = json.loads(payload_str) + + assert "glean-server-event" == logged_output["Type"] + assert "glean.test" == fields["document_namespace"] + assert "events" == fields["document_type"] + + # Validate payload against Glean schema + schema_url = ( + "https://raw.githubusercontent.com/mozilla-services/" + "mozilla-pipeline-schemas/main/" + "schemas/glean/glean/glean.1.schema.json" + ) + + input = io.StringIO(payload_str) + output = io.StringIO() + assert validate_ping.validate_ping(input, output, schema_url=schema_url) == 0, ( + output.getvalue() + ) + + # Check that labeled_boolean is properly serialized as a map + labeled_boolean_metrics = payload["metrics"]["labeled_boolean"] + assert "telemetry.feature_flags" in labeled_boolean_metrics + feature_flags = labeled_boolean_metrics["telemetry.feature_flags"] + assert feature_flags["feature_one"] is True + assert feature_flags["feature_two"] is False + assert feature_flags["feature_three"] is True + + @pytest.mark.go_dependency def test_run_logging_custom_ping_with_event(tmp_path): glean_module_path = tmp_path / "glean"