diff --git a/glean_parser/metrics.py b/glean_parser/metrics.py index 9ba944fe..fb3badab 100644 --- a/glean_parser/metrics.py +++ b/glean_parser/metrics.py @@ -61,6 +61,7 @@ def __init__( data_sensitivity: Optional[List[str]] = None, defined_in: Optional[Dict] = None, telemetry_mirror: Optional[str] = None, + in_session: bool = False, _config: Optional[Dict[str, Any]] = None, _validated: bool = False, ): @@ -98,6 +99,8 @@ def __init__( self.defined_in = defined_in if telemetry_mirror is not None: self.telemetry_mirror = telemetry_mirror + if in_session: + self.in_session = in_session # _validated indicates whether this metric has already been jsonschema # validated (but not any of the Python-level validation). @@ -312,10 +315,19 @@ class Event(Metric): def __init__(self, *args, **kwargs): self.extra_keys = kwargs.pop("extra_keys", {}) + in_session = kwargs.pop("in_session", False) + if in_session: + self.in_session = True + self.out_of_session = False self.validate_extra_keys(self.extra_keys, kwargs.get("_config", {})) super().__init__(*args, **kwargs) self._generate_enums = [("allowed_extra_keys_with_types", "Extra")] + def serialize(self) -> Dict[str, "util.JSONType"]: + d = super().serialize() + d.pop("out_of_session", None) + return d + @property def allowed_extra_keys(self): # Sort keys so that output is deterministic diff --git a/glean_parser/schemas/metrics.2-0-0.schema.yaml b/glean_parser/schemas/metrics.2-0-0.schema.yaml index 47086784..15773394 100644 --- a/glean_parser/schemas/metrics.2-0-0.schema.yaml +++ b/glean_parser/schemas/metrics.2-0-0.schema.yaml @@ -627,6 +627,18 @@ definitions: so glean_parser can find it. type: string + in_session: + title: In session + description: | + Whether this metric participates in session sampling. + When `true`, the metric is subject to session-level sampling and + session metadata is attached to it. + When `false` (default), the metric bypasses session sampling entirely. + + Only valid when `type`_ is `event`. + type: boolean + default: false + structure: title: A subset of a JSON schema definition description: | @@ -773,6 +785,18 @@ additionalProperties: description: | `denominator_metric` is only allowed for `rate`. maxLength: 0 + - if: + not: + properties: + type: + const: event + then: + properties: + in_session: + description: | + `in_session: true` is only allowed for `event` metrics. + Only event metrics support session sampling. + const: false - if: properties: diff --git a/glean_parser/templates/rust.jinja2 b/glean_parser/templates/rust.jinja2 index b6593e44..7ac300bb 100644 --- a/glean_parser/templates/rust.jinja2 +++ b/glean_parser/templates/rust.jinja2 @@ -102,6 +102,9 @@ CommonMetricData { send_in_pings: {{ obj.send_in_pings|rust }}, lifetime: {{ obj.lifetime|rust }}, disabled: {{ obj.is_disabled()|rust }}, + {% if obj.out_of_session is defined %} + out_of_session: {{ obj.out_of_session|rust }}, + {% endif %} ..Default::default() } {%- endmacro -%} diff --git a/glean_parser/util.py b/glean_parser/util.py index 794a8426..55c91f44 100644 --- a/glean_parser/util.py +++ b/glean_parser/util.py @@ -515,6 +515,7 @@ def remove_output_params(d, output_params): "send_in_pings", "lifetime", "disabled", + "out_of_session", ] diff --git a/tests/data/all_metrics.yaml b/tests/data/all_metrics.yaml index ec6c5431..c28acb18 100644 --- a/tests/data/all_metrics.yaml +++ b/tests/data/all_metrics.yaml @@ -107,6 +107,7 @@ all_metrics: event: <<: *defaults type: event + in_session: true extra_keys: source: description: Source of this event diff --git a/tests/test_kotlin.py b/tests/test_kotlin.py index 9d0e9329..d151da33 100644 --- a/tests/test_kotlin.py +++ b/tests/test_kotlin.py @@ -105,6 +105,10 @@ def test_parser_all_metrics(tmp_path): assert set(x.name for x in tmp_path.iterdir()) == set(["AllMetrics.kt"]) + with (tmp_path / "AllMetrics.kt").open("r", encoding="utf-8") as fd: + content = fd.read() + assert "outOfSession = false" in content + run_linters(tmp_path.glob("*.kt")) diff --git a/tests/test_metrics.py b/tests/test_metrics.py index aa6a75d4..8f743b89 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -190,6 +190,66 @@ def test_no_unit(): assert not event.unit +def test_in_session_on_event(): + """in_session: true on event produces out_of_session = False.""" + event = metrics.Event( + type="event", + category="category", + name="metric", + bugs=["http://bugzilla.mozilla.com/12345"], + notification_emails=["nobody@example.com"], + description="description...", + expires="never", + in_session=True, + ) + assert event.out_of_session is False + assert event.in_session is True + + +def test_in_session_default_on_event(): + """Omitting in_session defaults to out-of-session (no out_of_session attr).""" + event = metrics.Event( + type="event", + category="category", + name="metric", + bugs=["http://bugzilla.mozilla.com/12345"], + notification_emails=["nobody@example.com"], + description="description...", + expires="never", + ) + assert not hasattr(event, "out_of_session") + + +def test_in_session_true_on_non_event_rejected(): + """in_session: true on a non-event metric must raise a validation error.""" + with pytest.raises(ValueError): + metrics.Counter( + type="counter", + category="category", + name="metric", + bugs=["http://bugzilla.mozilla.com/12345"], + notification_emails=["nobody@example.com"], + description="description...", + expires="never", + in_session=True, + ) + + +def test_in_session_false_on_non_event_accepted(): + """in_session: false (explicit) on a non-event metric should be accepted.""" + counter = metrics.Counter( + type="counter", + category="category", + name="metric", + bugs=["http://bugzilla.mozilla.com/12345"], + notification_emails=["nobody@example.com"], + description="description...", + expires="never", + in_session=False, + ) + assert not hasattr(counter, "out_of_session") + + def test_jwe_is_rejected(): with pytest.raises(ValueError): metrics.Jwe( diff --git a/tests/test_rust.py b/tests/test_rust.py index ddd438e5..c387db93 100644 --- a/tests/test_rust.py +++ b/tests/test_rust.py @@ -289,6 +289,24 @@ def test_object_metric(tmp_path): assert "}" in content +def test_in_session_output(tmp_path): + """Assert that in_session: true produces out_of_session: false in Rust output.""" + translate.translate( + ROOT / "data" / "all_metrics.yaml", + "rust", + tmp_path, + {}, + {"allow_reserved": False}, + ) + + assert set(x.name for x in tmp_path.iterdir()) == set(["glean_metrics.rs"]) + + with (tmp_path / "glean_metrics.rs").open("r", encoding="utf-8") as fd: + content = fd.read() + + assert "out_of_session: false" in content + + def test_dual_labeled_counter_metric(tmp_path): """ Assert that a dual labeled counter metric is created. diff --git a/tests/test_swift.py b/tests/test_swift.py index 593a6d32..8fec3872 100644 --- a/tests/test_swift.py +++ b/tests/test_swift.py @@ -109,6 +109,10 @@ def test_parser_all_metrics(tmp_path): assert set(x.name for x in tmp_path.iterdir()) == set(["Metrics.swift"]) + with (tmp_path / "Metrics.swift").open("r", encoding="utf-8") as fd: + content = fd.read() + assert "outOfSession: false" in content + run_linters(tmp_path.glob("*.swift"))