From 3b0d7cbb2bf629c6e4789b63325fb3be5be7021a Mon Sep 17 00:00:00 2001 From: Glenda Leonard Date: Thu, 15 Oct 2026 18:11:13 -0400 Subject: [PATCH] Added support for LabeledQuantity to go_server. --- CHANGELOG.md | 1 + glean_parser/go_server.py | 33 ++++ glean_parser/templates/go_server.jinja2 | 40 +++++ .../go_server_labeled_quantity_metrics.yaml | 21 +++ tests/data/go_server_metrics_unsupported.yaml | 17 ++ tests/test_go_server.py | 146 ++++++++++++++++++ 6 files changed, 258 insertions(+) create mode 100644 tests/data/go_server_labeled_quantity_metrics.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 5209fe57..148d6994 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - JS/TS server: Add support for `server-deletion-request` ping. Server apps can now add `server-deletion-request` to `send_in_pings` on identifier metrics (e.g. user IDs) ([DENG-4079](https://mozilla-hub.atlassian.net/browse/DENG-4079)) - JS: Ensure imports are sorted ([#865](https://github.com/mozilla/glean_parser/pull/865)) - BREAKING CHANGE: Require pings defined requiring an "ohttp" uploader capability to have `metadata.include_info_sections: false` ([bug 2074263](https://bugzilla.mozilla.org/show_bug.cgi?id=2074263)) +- Go server: Add support for `labeled_quantity` metrics with dynamic labels; labels over 111 bytes are recorded under `__other__` ([TILES-259](https://mozilla-hub.atlassian.net/browse/TILES-259)) ## 21.0.1 diff --git a/glean_parser/go_server.py b/glean_parser/go_server.py index aecc97ba..1045df55 100644 --- a/glean_parser/go_server.py +++ b/glean_parser/go_server.py @@ -45,6 +45,7 @@ "boolean", "labeled_boolean", # static labels only; dynamic labels are not supported "string_list", + "labeled_quantity", ] @@ -83,6 +84,8 @@ def generate_metric_type(metric_type: str) -> str: return "time.Time" elif metric_type == "string_list": return "[]string" + elif metric_type == "labeled_quantity": + return "map[string]int64" else: print("❌ Unable to generate Go type from metric type: " + metric_type) exit @@ -115,6 +118,27 @@ def validate_labeled_boolean(metric: metrics.Metric) -> bool: return True +def validate_labeled_quantity(metric: metrics.Metric) -> bool: + """ + Validate that a labeled_quantity metric does not have static labels defined. + + The Go server outputter does not support static labels. It generates a Go map[string]int64 + Dynamic labels are supported. + + Returns: + bool: True if valid, False otherwise + """ + if getattr(metric, "ordered_labels", None): + print( + "❌ Ignoring labeled_quantity metric with static labels: " + + f"{metric.name}." + + " Do not define labels in metrics.yaml to use this metric type." + ) + return False + + return True + + def output_go( objs: metrics.ObjectTree, output_dir: Path, @@ -155,6 +179,8 @@ def output_go( # unique list of labeled_boolean metrics used in any ping labeled_boolean_metrics: List[metrics.Metric] = [] + has_labeled_quantity: bool = 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. @@ -177,6 +203,12 @@ def output_go( ): continue + # Validate labeled_quantity metrics + if metric.type == "labeled_quantity": + if not validate_labeled_quantity(metric): + continue + has_labeled_quantity = True + for ping in metric.send_in_pings: if metric.type == "event" and metric not in event_metrics: event_metrics.append(metric) @@ -209,6 +241,7 @@ def output_go( pings=ping_to_metrics, events=event_metrics, labeled_booleans=labeled_boolean_metrics, + has_labeled_quantity=has_labeled_quantity, transport=transport, ) ) diff --git a/glean_parser/templates/go_server.jinja2 b/glean_parser/templates/go_server.jinja2 index 4f524099..5f5bab4f 100644 --- a/glean_parser/templates/go_server.jinja2 +++ b/glean_parser/templates/go_server.jinja2 @@ -21,6 +21,9 @@ import ( {% endif %} "fmt" "io" +{% if has_labeled_quantity %} + "sort" +{% endif %} {% if include_logging %} "strconv" {% endif %} @@ -185,6 +188,39 @@ func createPingInfo() pingInfo { } } +{% if has_labeled_quantity %} +// maxLabelLength is the maximum label length, in bytes, accepted by the +// Glean ingestion schema. Longer labels cause the entire ping to be rejected. +const maxLabelLength = 111 + +// otherLabel is the label Glean uses for values recorded under an invalid label. +// Monitoring can be added for any recorded metrics with the label "__other__" +const otherLabel = "__other__" + +// sanitizeLabeledQuantity returns a copy of m with any label longer than +// maxLabelLength recorded under otherLabel, matching the Glean client SDKs. +// Labels are processed in sorted order so the value kept under otherLabel is +// deterministic when several labels are invalid. A nil map is returned as an +// empty map so it serializes as {} rather than null. +func sanitizeLabeledQuantity(m map[string]int64) map[string]int64 { + labels := make([]string, 0, len(m)) + for label := range m { + labels = append(labels, label) + } + sort.Strings(labels) + + out := make(map[string]int64, len(m)) + for _, label := range labels { + value := m[label] + if len(label) > maxLabelLength { + label = otherLabel + } + out[label] = value + } + return out +} + +{% endif %} // resolveDocumentID returns supplied if non-empty, otherwise a fresh UUID. // Set DocumentID on the ping params struct to share an ID across transports // (e.g., dual-write verification during a logging->pubsub migration). @@ -346,6 +382,10 @@ func (p {{ ping|ping_type_name }}) buildPayload(appDisplayVersion, appChannel st } {% endfor %} {% endif %} + {% for metric in metrics_by_type['labeled_quantity'] %} + // Remap invalid labels to __other__ and ensure nil serializes as {}, not null + p.{{ metric|metric_argument_name }} = sanitizeLabeledQuantity(p.{{ metric|metric_argument_name }}) + {% endfor %} metrics := metrics{ {% for metric_type, metrics in metrics_by_type.items() %} {% if metric_type != 'event' %} diff --git a/tests/data/go_server_labeled_quantity_metrics.yaml b/tests/data/go_server_labeled_quantity_metrics.yaml new file mode 100644 index 00000000..b7509eae --- /dev/null +++ b/tests/data/go_server_labeled_quantity_metrics.yaml @@ -0,0 +1,21 @@ +# 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: + test_labeled_quantity: + type: labeled_quantity + description: > + Labeled Quantity metric + 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/data/go_server_metrics_unsupported.yaml b/tests/data/go_server_metrics_unsupported.yaml index e0e475e3..5c0b193b 100644 --- a/tests/data/go_server_metrics_unsupported.yaml +++ b/tests/data/go_server_metrics_unsupported.yaml @@ -36,6 +36,23 @@ metric: send_in_pings: - events + labeled_quantity: + type: labeled_quantity + description: > + labeled quantity + bugs: + - TBD + data_reviews: + - TBD + notification_emails: + - CHANGE-ME@example.com + lifetime: application + expires: never + send_in_pings: + - events + labels: + - sample_feature + object: type: object description: > diff --git a/tests/test_go_server.py b/tests/test_go_server.py index 10866112..ad5a1589 100644 --- a/tests/test_go_server.py +++ b/tests/test_go_server.py @@ -85,6 +85,38 @@ def test_parser_go_server_labeled_boolean(tmp_path): assert "TelemetryFeatureFlags TelemetryFeatureFlags" in content +def test_parser_go_server_labeled_quantity_with_labels(tmp_path, capsys): + """Test that labeled_quantity with static labels is rejected.""" + translate.translate( + [ + ROOT / "data" / "go_server_metrics_unsupported.yaml", + ], + "go_server", + tmp_path, + ) + captured = capsys.readouterr() + assert "Ignoring labeled_quantity metric with static labels" in captured.out + + +def test_parser_go_server_labeled_quantity(tmp_path): + """Test that labeled_quantity metrics generate proper map[string]int64.""" + translate.translate( + ROOT / "data" / "go_server_labeled_quantity_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 it's used in the ping struct + assert "TelemetryTestLabeledQuantity map[string]int64" in content + assert "func sanitizeLabeledQuantity" in content + + def test_parser_go_server_events_only(tmp_path): """Test that parser works for definitions that only use events ping""" translate.translate( @@ -504,6 +536,120 @@ def test_run_logging_labeled_boolean_omitted(tmp_path): assert feature_flags == {} +@pytest.mark.go_dependency +def test_run_logging_labeled_quantity(tmp_path): + glean_module_path = tmp_path / "glean" + + translate.translate( + [ + ROOT / "data" / "go_server_labeled_quantity_metrics.yaml", + ], + "go_server", + glean_module_path, + ) + + code = f""" + _ = 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{{ + TelemetryTestLabeledQuantity: map[string]int64{{ + "tile": 5, + "{"a" * 111}": 9, + "{"b" * 112}": 7, + }}, + }}, + ) + """ + + 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() + ) + + # Labels over 111 bytes are recorded under __other__; 111 bytes is kept + value = payload["metrics"]["labeled_quantity"]["telemetry.test_labeled_quantity"] + assert value == {"tile": 5, "a" * 111: 9, "__other__": 7} + + +@pytest.mark.go_dependency +def test_run_logging_labeled_quantity_omitted(tmp_path): + """Test that completely omitted labeled_quantity metrics serialize correctly.""" + glean_module_path = tmp_path / "glean" + + translate.translate( + [ + ROOT / "data" / "go_server_labeled_quantity_metrics.yaml", + ], + "go_server", + glean_module_path, + ) + + # This code logs an events ping without setting the labeled_quantity metric at all, not even to nil. + 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{ + // TelemetryTestLabeledQuantity completely omitted (zero value) + }, + ) + """ + + 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() + validation_result = validate_ping.validate_ping( + input, output, schema_url=schema_url + ) + + assert validation_result == 0, f"Validation failed. Output: {output.getvalue()}" + + # Check how labeled_quantity is serialized when omitted + labeled_quantity_metrics = payload["metrics"]["labeled_quantity"] + assert labeled_quantity_metrics["telemetry.test_labeled_quantity"] == {} + + @pytest.mark.go_dependency def test_run_logging_nil_string_list(tmp_path): """Test that nil string_list metrics serialize as empty arrays, not null."""