diff --git a/.sampo/changesets/minimal-feature-flag-called-events.md b/.sampo/changesets/minimal-feature-flag-called-events.md new file mode 100644 index 00000000..92fe3d58 --- /dev/null +++ b/.sampo/changesets/minimal-feature-flag-called-events.md @@ -0,0 +1,11 @@ +--- +'pypi/posthog': minor +--- + +`$feature_flag_called` events are now minimized for non-experiment flags when the server enables it. When the `/flags` v2 response (`minimalFlagCalledEvents`) or the local-evaluation payload (`minimal_flag_called_events`) reports the gate as enabled and the evaluated flag has no linked experiment (`has_experiment` is `false`), the event's properties are reduced to a strict allowlist (`$feature_flag`, `$feature_flag_response`, `$feature_flag_has_experiment`, the `$feature_flag_*` debug scalars, `locally_evaluated`, `$groups`, `$process_person_profile`, `$session_id`, `$lib`, `$lib_version`, `$is_server`, `$geoip_disable`, `$os`, `$os_version`, `$os_distro`, `$python_runtime`, `$python_version`). Everything else — including super properties and custom event properties — is stripped from those events. + +If the server does not report the gate, if the flag's `has_experiment` signal is missing, or if the flag is linked to an experiment, the full property set is sent unchanged. There is no SDK-side configuration; the gate is controlled per-team by the server. For `evaluate_flags()` snapshots, the gate is pinned when the snapshot is created, so deferred flag accesses are shaped by the evaluation that produced them. + +Custom `flag_definition_cache` providers now receive an additional `minimal_flag_called_events` key in the definitions payload, so the gate survives external cache round-trips. + +When the server reports `has_experiment` for a flag, every `$feature_flag_called` event also carries a `$feature_flag_has_experiment` boolean property. diff --git a/posthog/args.py b/posthog/args.py index e871ea4b..313edc0f 100644 --- a/posthog/args.py +++ b/posthog/args.py @@ -1,4 +1,14 @@ -from typing import TYPE_CHECKING, TypedDict, Optional, Any, Dict, Union, Tuple, Type +from typing import ( + TYPE_CHECKING, + TypedDict, + Optional, + Any, + Dict, + FrozenSet, + Union, + Tuple, + Type, +) from types import TracebackType from typing_extensions import NotRequired # For Python < 3.11 compatibility from datetime import datetime @@ -50,6 +60,9 @@ class OptionalCaptureArgs(TypedDict): disable_geoip: NotRequired[ Optional[bool] ] # As above, optional so we can tell if the user is intentionally overriding a client setting or not + _property_allowlist: NotRequired[ + Optional[FrozenSet[str]] + ] # Internal: strict allowlist applied to the fully-enriched event properties. Used by minimal $feature_flag_called events. class OptionalSetArgs(TypedDict): diff --git a/posthog/client.py b/posthog/client.py index 64baf087..275ace1a 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -199,6 +199,58 @@ def wrapper(self, *args, **kwargs): return decorator +# Strict allowlist for minimal ``$feature_flag_called`` events, per the cross-SDK +# contract: everything else — customer-passed properties, super properties, context +# tags, and the richer parts of system context — is stripped from the +# fully-enriched properties dict. The static platform/runtime identity keys below +# are the exception: they're cheap and useful for debugging flag behavior by +# platform, so they survive minimization. +_MINIMAL_FLAG_CALLED_EVENT_PROPERTIES: frozenset[str] = frozenset( + { + # Identity + "$feature_flag", + "$feature_flag_response", + "$feature_flag_has_experiment", + # Evaluation debug + "$feature_flag_id", + "$feature_flag_version", + "$feature_flag_reason", + "$feature_flag_request_id", + "$feature_flag_evaluated_at", + "$feature_flag_error", + "locally_evaluated", + # Correctness-required + "$groups", + "$process_person_profile", + # Linkage / SDK identity + "$session_id", + "$lib", + "$lib_version", + "$is_server", + # Processing-control sentinel this SDK sets to deliver the event correctly + "$geoip_disable", + # Static platform/runtime identity: cheap, low-cardinality dimensions kept + # for platform/runtime breakdowns on flag-call debugging. + "$os", + "$os_version", + "$os_distro", + "$python_runtime", + "$python_version", + } +) + + +def _parse_has_experiment(value: Any) -> Optional[bool]: + """Server-reported experiment linkage; anything but an explicit bool means unknown.""" + return value if isinstance(value, bool) else None + + +def _metadata_has_experiment(metadata: Any) -> Optional[bool]: + """Server-reported experiment linkage from flag metadata; ``None`` when absent + (e.g. ``LegacyFlagMetadata``, which doesn't carry the field).""" + return metadata.has_experiment if isinstance(metadata, FlagMetadata) else None + + class Client(object): """ This is the SDK reference for the PostHog Python SDK. @@ -457,6 +509,10 @@ def __init__( self.exception_capture = None self.privacy_mode = privacy_mode self.enable_local_evaluation = enable_local_evaluation + # Server-controlled gate for minimal $feature_flag_called events, read from + # the /flags v2 response and the local-evaluation payload. False until the + # server reports it, so full events are the fail-safe. + self._minimal_flag_called_events: bool = False self.capture_exception_code_variables = capture_exception_code_variables self.code_variables_mask_patterns = ( @@ -967,7 +1023,14 @@ def _get_flags_decision( **request_data, ) - return normalize_flags_response(resp_data) + response = normalize_flags_response(resp_data) + # Server-controlled gate for minimal $feature_flag_called events. Only the + # v2 response shape carries it; absent (legacy shape, older server, team + # not gated) means False. + self._minimal_flag_called_events = ( + response.get("minimalFlagCalledEvents") is True + ) + return response @no_throw() def capture( @@ -1034,6 +1097,9 @@ def capture( flags_snapshot = kwargs.get("flags", None) send_feature_flags = kwargs.get("send_feature_flags", False) disable_geoip = kwargs.get("disable_geoip", None) + # Internal, set for minimal $feature_flag_called events: a strict allowlist + # applied to the fully-enriched properties dict just before enqueueing. + property_allowlist = kwargs.get("_property_allowlist", None) properties = {**(properties or {}), **system_context()} @@ -1147,7 +1213,7 @@ def capture( properties = {**extra_properties, **properties} msg["properties"] = properties - return self._enqueue(msg, disable_geoip) + return self._enqueue(msg, disable_geoip, property_allowlist=property_allowlist) def _parse_send_feature_flags(self, send_feature_flags) -> SendFeatureFlagsOptions: """ @@ -1622,7 +1688,7 @@ def _reinit_after_fork(self): reset_sessions() - def _enqueue(self, msg, disable_geoip): + def _enqueue(self, msg, disable_geoip, property_allowlist=None): # type: (...) -> Optional[str] """Push a new `msg` onto the queue, return `(success, msg)`""" @@ -1670,6 +1736,14 @@ def _enqueue(self, msg, disable_geoip): if self.is_server: msg["properties"]["$is_server"] = True + # Applied after every enrichment step (system context, context tags, super + # properties, $lib/$lib_version) so the final event shape is exactly the + # allowlist regardless of where a property came from. + if property_allowlist is not None: + msg["properties"] = { + k: v for k, v in msg["properties"].items() if k in property_allowlist + } + msg["distinct_id"] = stringify_id(msg.get("distinct_id", None)) msg = clean(msg) @@ -1875,6 +1949,11 @@ def _update_flag_state( self.feature_flags = data["flags"] self.group_type_mapping = data["group_type_mapping"] self.cohorts = data["cohorts"] + # Server-controlled gate for minimal $feature_flag_called events; the + # local-evaluation payload carries it as a top-level key. Absent means False. + self._minimal_flag_called_events = ( + data.get("minimal_flag_called_events") is True + ) # Invalidate evaluation cache if flag definitions changed if ( @@ -1980,6 +2059,7 @@ def _fetch_feature_flags_from_api(self): "flags": self.feature_flags or [], "group_type_mapping": self.group_type_mapping or {}, "cohorts": self.cohorts or {}, + "minimal_flag_called_events": self._minimal_flag_called_events, } ) ) @@ -2269,6 +2349,7 @@ def _get_feature_flag_result( request_id = None evaluated_at = None feature_flag_error: Optional[str] = None + remote_minimal_flag_called_events = False # Resolve device_id from context if not provided if device_id is None: @@ -2311,16 +2392,20 @@ def _get_feature_flag_result( ) else: try: - flag_details, request_id, evaluated_at, errors_while_computing = ( - self._get_feature_flag_details_from_server( - key, - distinct_id, - groups, - person_properties, - group_properties, - disable_geoip, - device_id=device_id, - ) + ( + flag_details, + request_id, + evaluated_at, + errors_while_computing, + remote_minimal_flag_called_events, + ) = self._get_feature_flag_details_from_server( + key, + distinct_id, + groups, + person_properties, + group_properties, + disable_geoip, + device_id=device_id, ) errors = [] if errors_while_computing: @@ -2369,14 +2454,18 @@ def _get_feature_flag_result( # remotely-evaluated flags carry it in the response metadata. None when # the server (older deployment) does not report it. has_experiment: Optional[bool] = None + # Source the gate the same way as has_experiment above; see + # _capture_feature_flag_called_if_needed for why. + minimal_flag_called_events = self._minimal_flag_called_events if flag_was_locally_evaluated: local_def = (self.feature_flags_by_key or {}).get(key) if isinstance(local_def, dict): - has_experiment = local_def.get("has_experiment") - elif isinstance(flag_details, FeatureFlag) and isinstance( - flag_details.metadata, FlagMetadata - ): - has_experiment = flag_details.metadata.has_experiment + has_experiment = _parse_has_experiment( + local_def.get("has_experiment") + ) + elif isinstance(flag_details, FeatureFlag): + has_experiment = _metadata_has_experiment(flag_details.metadata) + minimal_flag_called_events = remote_minimal_flag_called_events self._capture_feature_flag_called( distinct_id, @@ -2391,6 +2480,7 @@ def _get_feature_flag_result( flag_details, feature_flag_error, has_experiment, + minimal_flag_called_events, ) return flag_result @@ -2636,10 +2726,12 @@ def _get_feature_flag_details_from_server( group_properties: dict[str, dict[str, Any]], disable_geoip: Optional[bool], device_id: Optional[str] = None, - ) -> tuple[Optional[FeatureFlag], Optional[str], Optional[int], bool]: + ) -> tuple[Optional[FeatureFlag], Optional[str], Optional[int], bool, bool]: """ Calls /flags and returns the flag details, request id, evaluated at timestamp, - and whether there were errors while computing flags. + whether there were errors while computing flags, and this response's own + minimal-flag-called-events gate (see _capture_feature_flag_called_if_needed + for why the caller should use this over the client-wide gate). """ resp_data = self._get_flags_decision( distinct_id, @@ -2653,9 +2745,16 @@ def _get_feature_flag_details_from_server( request_id = resp_data.get("requestId") evaluated_at = resp_data.get("evaluatedAt") errors_while_computing = resp_data.get("errorsWhileComputingFlags", False) + minimal_flag_called_events = resp_data.get("minimalFlagCalledEvents") is True flags = resp_data.get("flags") flag_details = flags.get(key) if flags else None - return flag_details, request_id, evaluated_at, errors_while_computing + return ( + flag_details, + request_id, + evaluated_at, + errors_while_computing, + minimal_flag_called_events, + ) def _capture_feature_flag_called( self, @@ -2671,6 +2770,7 @@ def _capture_feature_flag_called( flag_details: Optional[FeatureFlag], feature_flag_error: Optional[str] = None, has_experiment: Optional[bool] = None, + minimal_flag_called_events: bool = False, ): properties: dict[str, Any] = { "$feature_flag": key, @@ -2706,6 +2806,7 @@ def _capture_feature_flag_called( groups=groups, disable_geoip=disable_geoip, has_experiment=has_experiment, + minimal_flag_called_events=minimal_flag_called_events, ) def _capture_feature_flag_called_if_needed( @@ -2718,17 +2819,27 @@ def _capture_feature_flag_called_if_needed( groups: Optional[Mapping[str, Union[str, int]]] = None, disable_geoip: Optional[bool] = None, has_experiment: Optional[bool] = None, + minimal_flag_called_events: bool = False, ) -> None: """Fire a ``$feature_flag_called`` event if the (distinct_id, flag, response, groups) tuple hasn't already been reported on this client. Group context is - included so that group-scoped flags fire a separate event for each group a - user is evaluated under. Shared by the single-flag evaluation path and + included so that group-scoped flags fire a separate event for each group a user + is evaluated under. Shared by the single-flag evaluation path and ``FeatureFlagEvaluations.is_enabled() / get_flag()`` so both paths dedupe identically. ``has_experiment`` is the server-reported signal for whether the flag is linked - to an experiment; when the server reported it, it is recorded on the event as - ``$feature_flag_has_experiment``. ``None`` (unknown) omits the property. + to an experiment (``None`` when the server did not report it). When the + server-controlled gate is on and the flag is known non-experiment, the event is + trimmed to a strict allowlist; any missing signal sends the full legacy shape, + and experiment-linked flags keep the full set for exposure analysis. + + ``minimal_flag_called_events`` is the gate as observed by the evaluation that + produced ``response``, not a fresh read of client-wide state: both callers + resolve it themselves (the snapshot pins it at construction; the single-flag + path reads it from the specific local/remote source that produced the value) + so a concurrent poller refresh or another ``/flags`` call can't reshape an + event after the fact. """ groups_key = ( tuple(sorted((str(k), str(v)) for k, v in groups.items())) if groups else () @@ -2743,17 +2854,30 @@ def _capture_feature_flag_called_if_needed( if feature_flag_reported_key in reported_flags: return - # Record the server's experiment signal so the server can optimize ingestion. - # Omitted when unknown (older servers that don't report has_experiment). + # Record the server's experiment signal when known, so minimization's impact + # can be measured by segmenting on it. if has_experiment is not None: properties["$feature_flag_has_experiment"] = has_experiment + # Minimize iff the server-controlled gate is on AND the flag is known to have + # no linked experiment. Any missing signal (gate absent, has_experiment + # missing) fails safe to the full legacy shape. + should_minimize = minimal_flag_called_events and has_experiment is False + # Only thread the internal allowlist through when minimizing, so the + # full-property path's capture() call signature stays unchanged. + extra_capture_kwargs: dict[str, Any] = {} + if should_minimize: + extra_capture_kwargs["_property_allowlist"] = ( + _MINIMAL_FLAG_CALLED_EVENT_PROPERTIES + ) + self.capture( "$feature_flag_called", distinct_id=distinct_id, properties=properties, groups=groups or {}, disable_geoip=disable_geoip, + **extra_capture_kwargs, ) reported_flags.add(feature_flag_reported_key) @@ -3036,6 +3160,11 @@ def evaluate_flags( errors_while_computing = False quota_limited = False locally_evaluated_keys: set[str] = set() + # Source the gate the same way as has_experiment below; see + # _capture_feature_flag_called_if_needed for why. Defaults to the poller's + # current state; a successful remote fallback overwrites it with that + # response's own field below. + minimal_flag_called_events = self._minimal_flag_called_events # Try local evaluation first when the poller has loaded definitions. local_person_properties = self._person_properties_for_local_evaluation( @@ -3065,7 +3194,7 @@ def evaluate_flags( version=None, reason="Evaluated locally", locally_evaluated=True, - has_experiment=flag_def.get("has_experiment"), + has_experiment=_parse_has_experiment(flag_def.get("has_experiment")), ) locally_evaluated_keys.add(key) @@ -3091,6 +3220,9 @@ def evaluate_flags( errors_while_computing = bool( response.get("errorsWhileComputingFlags", False) ) + minimal_flag_called_events = ( + response.get("minimalFlagCalledEvents") is True + ) for key, detail in response.get("flags", {}).items(): if key in locally_evaluated_keys: continue @@ -3128,11 +3260,7 @@ def evaluate_flags( else None ), locally_evaluated=False, - has_experiment=( - detail.metadata.has_experiment - if isinstance(detail.metadata, FlagMetadata) - else None - ), + has_experiment=_metadata_has_experiment(detail.metadata), ) except QuotaLimitError as e: self.log.warning(f"[FEATURE FLAGS] Quota limit exceeded: {e}") @@ -3152,6 +3280,7 @@ def evaluate_flags( evaluated_at=evaluated_at, errors_while_computing=errors_while_computing, quota_limited=quota_limited, + minimal_flag_called_events=minimal_flag_called_events, ) _feature_flag_evaluations_host_cache: Optional[_FeatureFlagEvaluationsHost] = None diff --git a/posthog/feature_flag_evaluations.py b/posthog/feature_flag_evaluations.py index 5f65299e..f622906b 100644 --- a/posthog/feature_flag_evaluations.py +++ b/posthog/feature_flag_evaluations.py @@ -71,6 +71,7 @@ def __init__( evaluated_at: Optional[int] = None, errors_while_computing: bool = False, quota_limited: bool = False, + minimal_flag_called_events: bool = False, accessed: Optional[Set[str]] = None, ) -> None: """Internal — instances are created by the SDK via ``Client.evaluate_flags()``.""" @@ -83,6 +84,10 @@ def __init__( self._evaluated_at = evaluated_at self._errors_while_computing = errors_while_computing self._quota_limited = quota_limited + # Pinned at snapshot creation: the gate value from the evaluation that produced + # these records. Deferred flag accesses fire events shaped by THIS evaluation's + # server response, not whatever the client-wide gate happens to be at send time. + self._minimal_flag_called_events = minimal_flag_called_events self._accessed: Set[str] = set(accessed) if accessed is not None else set() def is_enabled(self, key: str) -> bool: @@ -196,6 +201,7 @@ def _clone_with( evaluated_at=self._evaluated_at, errors_while_computing=self._errors_while_computing, quota_limited=self._quota_limited, + minimal_flag_called_events=self._minimal_flag_called_events, # Copy the accessed set so the child tracks further access independently # of the parent. Callers expect ``only_accessed()`` on the parent to reflect # only what the parent saw, not what happened on filtered views. @@ -262,4 +268,5 @@ def _record_access(self, key: str) -> None: disable_geoip=self._disable_geoip, properties=properties, has_experiment=flag.has_experiment if flag else None, + minimal_flag_called_events=self._minimal_flag_called_events, ) diff --git a/posthog/flag_definition_cache.py b/posthog/flag_definition_cache.py index e19b0e19..e1e6325c 100644 --- a/posthog/flag_definition_cache.py +++ b/posthog/flag_definition_cache.py @@ -29,7 +29,7 @@ runtime_checkable, ) -from typing_extensions import Required, TypedDict +from typing_extensions import NotRequired, Required, TypedDict class FlagDefinitionCacheData(TypedDict): @@ -40,11 +40,14 @@ class FlagDefinitionCacheData(TypedDict): flags: List of feature flag definition dictionaries from the API. group_type_mapping: Mapping of group type indices to group names. cohorts: Dictionary of cohort definitions for local evaluation. + minimal_flag_called_events: Server-controlled gate for minimal + ``$feature_flag_called`` events. Treated as False when absent. """ flags: Required[List[Dict[str, Any]]] group_type_mapping: Required[Dict[str, str]] cohorts: Required[Dict[str, Any]] + minimal_flag_called_events: NotRequired[bool] @runtime_checkable diff --git a/posthog/test/test_feature_flag_called_minimization.py b/posthog/test/test_feature_flag_called_minimization.py new file mode 100644 index 00000000..bf32d9d6 --- /dev/null +++ b/posthog/test/test_feature_flag_called_minimization.py @@ -0,0 +1,527 @@ +"""Tests for minimal ``$feature_flag_called`` events. + +Per the cross-SDK contract, a minimal event is sent iff the server-controlled gate is +on (top-level ``minimalFlagCalledEvents`` in the v2 ``/flags`` response, or top-level +``minimal_flag_called_events`` in the local-evaluation payload) AND the evaluated +flag's ``has_experiment`` is exactly ``False``. The minimal shape is a strict +allowlist applied to the fully-enriched properties dict; any missing signal fails safe +to the full legacy shape. +""" + +import unittest +from unittest import mock + +from parameterized import parameterized + +from posthog.client import _MINIMAL_FLAG_CALLED_EVENT_PROPERTIES, Client +from posthog.request import GetResponse +from posthog.test.test_utils import FAKE_TEST_API_KEY +from posthog.utils import system_context + + +def _flags_response(has_experiment, gate): + """Build a v2 ``/flags`` response for ``person-flag``. + + ``has_experiment=None`` omits the per-flag field; ``gate=None`` omits the + top-level ``minimalFlagCalledEvents`` field, simulating an ungated team or an + older server. + """ + metadata = {"id": 23, "version": 42, "payload": "300"} + if has_experiment is not None: + metadata["has_experiment"] = has_experiment + response = { + "flags": { + "person-flag": { + "key": "person-flag", + "enabled": True, + "variant": None, + "reason": {"description": "Matched condition set 1"}, + "metadata": metadata, + }, + }, + "requestId": "req-1", + "evaluatedAt": 1640995200000, + } + if gate is not None: + response["minimalFlagCalledEvents"] = gate + return response + + +def _local_flag_definition(has_experiment): + flag = { + "id": 1, + "name": "Beta Feature", + "key": "person-flag", + "active": True, + "filters": { + "groups": [{"properties": [], "rollout_percentage": 100}], + "payloads": {"true": "300"}, + }, + } + if has_experiment is not None: + flag["has_experiment"] = has_experiment + return flag + + +class _CapturedEventsMixin: + """Builds a non-sending client whose fully-enriched events are captured via + ``before_send``, so tests assert the exact wire shape after every enrichment + step (system context, super properties, $lib, ...).""" + + def _make_client(self, **kwargs): + captured = [] + + def before_send(msg): + captured.append(msg) + return msg + + client = Client( + FAKE_TEST_API_KEY, + send=False, + before_send=before_send, + super_properties={"app_version": "1.2.3"}, + **kwargs, + ) + return client, captured + + def _flag_called_properties(self, captured, index=0): + events = [m for m in captured if m["event"] == "$feature_flag_called"] + assert events, "no $feature_flag_called event captured" + return events[index]["properties"] + + +class TestMinimizationViaFlagsResponse(_CapturedEventsMixin, unittest.TestCase): + """Gate sourced from the top-level ``minimalFlagCalledEvents`` field of the v2 + ``/flags`` response.""" + + @mock.patch("posthog.client.flags") + def test_gated_non_experiment_flag_sends_exactly_the_allowlist(self, patch_flags): + patch_flags.return_value = _flags_response(has_experiment=False, gate=True) + client, captured = self._make_client() + + client.get_feature_flag_result("person-flag", "some-distinct-id") + + properties = self._flag_called_properties(captured) + # The event shape is exactly the allowlist intersection of what the full + # event would have carried: no $feature/, no payload, no super + # properties. Static platform/runtime identity (system_context()) survives. + self.assertEqual( + set(properties), + { + "$feature_flag", + "$feature_flag_response", + "$feature_flag_has_experiment", + "$feature_flag_id", + "$feature_flag_version", + "$feature_flag_reason", + "$feature_flag_request_id", + "$feature_flag_evaluated_at", + "locally_evaluated", + "$lib", + "$lib_version", + "$is_server", + "$geoip_disable", + } + | set(system_context()), + ) + self.assertLessEqual(set(properties), _MINIMAL_FLAG_CALLED_EVENT_PROPERTIES) + self.assertIs(properties["$is_server"], True) + self.assertEqual(properties["$feature_flag"], "person-flag") + self.assertEqual(properties["$feature_flag_response"], True) + self.assertIs(properties["$feature_flag_has_experiment"], False) + self.assertEqual(properties["$feature_flag_id"], 23) + self.assertEqual(properties["$feature_flag_version"], 42) + self.assertEqual(properties["$feature_flag_reason"], "Matched condition set 1") + self.assertEqual(properties["$feature_flag_request_id"], "req-1") + self.assertEqual(properties["$feature_flag_evaluated_at"], 1640995200000) + + @mock.patch("posthog.client.flags") + def test_gated_non_experiment_flag_with_groups_keeps_group_context( + self, patch_flags + ): + # $groups and $process_person_profile are "correctness-required" allowlist + # entries: this pins that they actually survive minimization rather than + # just being present in the allowlist with no test ever exercising them. + patch_flags.return_value = _flags_response(has_experiment=False, gate=True) + client, captured = self._make_client() + + client.get_feature_flag_result( + "person-flag", "some-distinct-id", groups={"organization": "org-1"} + ) + + properties = self._flag_called_properties(captured) + self.assertEqual(properties["$groups"], {"organization": "org-1"}) + self.assertLessEqual(set(properties), _MINIMAL_FLAG_CALLED_EVENT_PROPERTIES) + + @mock.patch("posthog.client.flags") + def test_remote_evaluation_uses_this_responses_gate_not_a_concurrent_update( + self, patch_flags + ): + # This response's gate is off. Simulate a concurrent poller refresh (or + # another /flags call on another thread) flipping the client-wide gate on, + # in the gap between this response landing and the event being captured. + # The single-flag path must shape the event from this response's own gate, + # not whatever the client-wide attribute reads at capture time. + patch_flags.return_value = _flags_response(has_experiment=False, gate=False) + client, captured = self._make_client() + + original_get_details = client._get_feature_flag_details_from_server + + def get_details_then_concurrent_gate_flip(*args, **kwargs): + result = original_get_details(*args, **kwargs) + client._minimal_flag_called_events = True + return result + + client._get_feature_flag_details_from_server = ( + get_details_then_concurrent_gate_flip + ) + + client.get_feature_flag_result("person-flag", "some-distinct-id") + + properties = self._flag_called_properties(captured) + # Had the single-flag path re-read the (now-flipped) client-wide gate at + # capture time, this would have minimized instead. + self.assertEqual(properties["$feature/person-flag"], True) + self.assertIn("$python_version", properties) + + @mock.patch("posthog.client.flags") + def test_gated_experiment_flag_sends_full_event(self, patch_flags): + patch_flags.return_value = _flags_response(has_experiment=True, gate=True) + client, captured = self._make_client() + + client.get_feature_flag_result("person-flag", "some-distinct-id") + + properties = self._flag_called_properties(captured) + self.assertEqual(properties["$feature/person-flag"], True) + self.assertEqual(properties["$feature_flag_payload"], 300) + self.assertIn("$python_version", properties) + self.assertEqual(properties["app_version"], "1.2.3") + self.assertIs(properties["$feature_flag_has_experiment"], True) + + @parameterized.expand( + [ + ("gate_absent", None), + ("gate_false", False), + ("gate_not_a_bool", "true"), + ] + ) + @mock.patch("posthog.client.flags") + def test_without_a_true_gate_sends_full_event(self, _name, gate, patch_flags): + patch_flags.return_value = _flags_response(has_experiment=False, gate=gate) + client, captured = self._make_client() + + client.get_feature_flag_result("person-flag", "some-distinct-id") + + self.assertIs(client._minimal_flag_called_events, False) + properties = self._flag_called_properties(captured) + self.assertEqual(properties["$feature/person-flag"], True) + self.assertEqual(properties["$feature_flag_payload"], 300) + self.assertIn("$python_version", properties) + self.assertEqual(properties["app_version"], "1.2.3") + # The experiment signal still rides along when the server reports it. + self.assertIs(properties["$feature_flag_has_experiment"], False) + + @mock.patch("posthog.client.flags") + def test_gated_flag_without_has_experiment_sends_full_event(self, patch_flags): + # Gate on, but the per-flag signal is missing (older deployment): fail safe + # to the full shape, and don't fabricate the has_experiment property. + patch_flags.return_value = _flags_response(has_experiment=None, gate=True) + client, captured = self._make_client() + + client.get_feature_flag_result("person-flag", "some-distinct-id") + + properties = self._flag_called_properties(captured) + self.assertEqual(properties["$feature/person-flag"], True) + self.assertEqual(properties["$feature_flag_payload"], 300) + self.assertNotIn("$feature_flag_has_experiment", properties) + + @mock.patch("posthog.client.flags") + def test_gate_persists_and_follows_the_latest_response(self, patch_flags): + patch_flags.side_effect = [ + _flags_response(has_experiment=False, gate=True), + _flags_response(has_experiment=False, gate=None), + ] + client, captured = self._make_client() + + client.get_feature_flag_result("person-flag", "user-1") + self.assertIs(client._minimal_flag_called_events, True) + + # A later response without the field flips the gate off and full events resume. + client.get_feature_flag_result("person-flag", "user-2") + self.assertIs(client._minimal_flag_called_events, False) + + minimal_properties = self._flag_called_properties(captured, index=0) + full_properties = self._flag_called_properties(captured, index=1) + self.assertNotIn("$feature/person-flag", minimal_properties) + self.assertEqual(full_properties["$feature/person-flag"], True) + + +class TestMinimizationViaLocalEvaluationPayload( + _CapturedEventsMixin, unittest.TestCase +): + """Gate sourced from the top-level ``minimal_flag_called_events`` key of the + local-evaluation payload, persisted alongside the polled flag definitions.""" + + def _load_definitions(self, client, patch_get, *responses): + patch_get.side_effect = list(responses) + client.load_feature_flags() + + def _definitions_payload(self, has_experiment, gate): + data = { + "flags": [_local_flag_definition(has_experiment)], + "group_type_mapping": {}, + "cohorts": {}, + } + if gate is not None: + data["minimal_flag_called_events"] = gate + return GetResponse(data=data, etag='"etag-1"') + + @mock.patch("posthog.client.Poller") + @mock.patch("posthog.client.get") + def test_gated_non_experiment_flag_sends_exactly_the_allowlist( + self, patch_get, _patch_poller + ): + client, captured = self._make_client(personal_api_key="personal-key") + self._load_definitions( + client, + patch_get, + self._definitions_payload(has_experiment=False, gate=True), + ) + + client.get_feature_flag_result("person-flag", "some-distinct-id") + + self.assertIs(client._minimal_flag_called_events, True) + properties = self._flag_called_properties(captured) + self.assertEqual( + set(properties), + { + "$feature_flag", + "$feature_flag_response", + "$feature_flag_has_experiment", + "locally_evaluated", + "$lib", + "$lib_version", + "$is_server", + "$geoip_disable", + } + | set(system_context()), + ) + self.assertLessEqual(set(properties), _MINIMAL_FLAG_CALLED_EVENT_PROPERTIES) + self.assertEqual(properties["$feature_flag_response"], True) + self.assertIs(properties["locally_evaluated"], True) + + @mock.patch("posthog.client.Poller") + @mock.patch("posthog.client.get") + def test_gated_experiment_flag_sends_full_event(self, patch_get, _patch_poller): + client, captured = self._make_client(personal_api_key="personal-key") + self._load_definitions( + client, + patch_get, + self._definitions_payload(has_experiment=True, gate=True), + ) + + client.get_feature_flag_result("person-flag", "some-distinct-id") + + properties = self._flag_called_properties(captured) + self.assertEqual(properties["$feature/person-flag"], True) + self.assertEqual(properties["$feature_flag_payload"], 300) + self.assertIn("$python_version", properties) + self.assertIs(properties["$feature_flag_has_experiment"], True) + + @mock.patch("posthog.client.Poller") + @mock.patch("posthog.client.get") + def test_gate_absent_from_payload_sends_full_event(self, patch_get, _patch_poller): + client, captured = self._make_client(personal_api_key="personal-key") + self._load_definitions( + client, + patch_get, + self._definitions_payload(has_experiment=False, gate=None), + ) + + client.get_feature_flag_result("person-flag", "some-distinct-id") + + self.assertIs(client._minimal_flag_called_events, False) + properties = self._flag_called_properties(captured) + self.assertEqual(properties["$feature/person-flag"], True) + self.assertEqual(properties["$feature_flag_payload"], 300) + + @mock.patch("posthog.client.Poller") + @mock.patch("posthog.client.get") + def test_gate_survives_not_modified_polls(self, patch_get, _patch_poller): + client, captured = self._make_client(personal_api_key="personal-key") + self._load_definitions( + client, + patch_get, + self._definitions_payload(has_experiment=False, gate=True), + GetResponse(data=None, etag='"etag-1"', not_modified=True), + ) + + # A 304 poll keeps the cached definitions and the gate. + client._load_feature_flags() + self.assertIs(client._minimal_flag_called_events, True) + + client.get_feature_flag_result("person-flag", "some-distinct-id") + properties = self._flag_called_properties(captured) + self.assertNotIn("$feature/person-flag", properties) + self.assertNotIn("$feature_flag_payload", properties) + + +class TestMinimizationViaEvaluateFlagsSnapshot(_CapturedEventsMixin, unittest.TestCase): + """The ``evaluate_flags()`` snapshot path fires ``$feature_flag_called`` on + access and must minimize identically to the single-flag path.""" + + def _snapshot_response(self, has_experiment, gate): + metadata = {"id": 2, "version": 23, "payload": '{"key": "value"}'} + if has_experiment is not None: + metadata["has_experiment"] = has_experiment + response = { + "flags": { + "variant-flag": { + "key": "variant-flag", + "enabled": True, + "variant": "variant-value", + "reason": {"code": "variant", "description": "Matched set 3"}, + "metadata": metadata, + }, + }, + "requestId": "request-id-1", + "evaluatedAt": 1640995200000, + } + if gate is not None: + response["minimalFlagCalledEvents"] = gate + return response + + @mock.patch("posthog.client.flags") + def test_gated_non_experiment_flag_sends_exactly_the_allowlist(self, patch_flags): + patch_flags.return_value = self._snapshot_response( + has_experiment=False, gate=True + ) + client, captured = self._make_client() + + flags = client.evaluate_flags("user-1") + flags.get_flag("variant-flag") + + properties = self._flag_called_properties(captured) + self.assertEqual( + set(properties), + { + "$feature_flag", + "$feature_flag_response", + "$feature_flag_has_experiment", + "$feature_flag_id", + "$feature_flag_version", + "$feature_flag_reason", + "$feature_flag_request_id", + "$feature_flag_evaluated_at", + "locally_evaluated", + "$lib", + "$lib_version", + "$is_server", + "$geoip_disable", + } + | set(system_context()), + ) + self.assertLessEqual(set(properties), _MINIMAL_FLAG_CALLED_EVENT_PROPERTIES) + self.assertEqual(properties["$feature_flag_response"], "variant-value") + self.assertIs(properties["$feature_flag_has_experiment"], False) + + @mock.patch("posthog.client.flags") + def test_gated_experiment_flag_sends_full_event(self, patch_flags): + patch_flags.return_value = self._snapshot_response( + has_experiment=True, gate=True + ) + client, captured = self._make_client() + + flags = client.evaluate_flags("user-1") + flags.get_flag("variant-flag") + + properties = self._flag_called_properties(captured) + self.assertEqual(properties["$feature/variant-flag"], "variant-value") + # The evaluate_flags path JSON-parses the payload before attaching it. + self.assertEqual(properties["$feature_flag_payload"], {"key": "value"}) + self.assertIn("$python_version", properties) + self.assertIs(properties["$feature_flag_has_experiment"], True) + + @mock.patch("posthog.client.flags") + def test_ungated_non_experiment_flag_sends_full_event(self, patch_flags): + patch_flags.return_value = self._snapshot_response( + has_experiment=False, gate=None + ) + client, captured = self._make_client() + + flags = client.evaluate_flags("user-1") + flags.get_flag("variant-flag") + + properties = self._flag_called_properties(captured) + self.assertEqual(properties["$feature/variant-flag"], "variant-value") + self.assertEqual(properties["$feature_flag_payload"], {"key": "value"}) + self.assertEqual(properties["app_version"], "1.2.3") + + @mock.patch("posthog.client.flags") + def test_snapshot_pins_gate_at_creation_not_at_access(self, patch_flags): + # The gate is pinned when the snapshot is created: a gate change learned from + # a LATER response must not reshape events for flags evaluated earlier. + patch_flags.return_value = self._snapshot_response( + has_experiment=False, gate=None + ) + client, captured = self._make_client() + + flags = client.evaluate_flags("user-1") + # A later response enables the gate before the snapshot is accessed. + client._minimal_flag_called_events = True + flags.get_flag("variant-flag") + + properties = self._flag_called_properties(captured) + self.assertEqual(properties["$feature/variant-flag"], "variant-value") + self.assertEqual(properties["app_version"], "1.2.3") + + @mock.patch("posthog.client.flags") + def test_snapshot_keeps_pinned_gate_when_later_response_disables_it( + self, patch_flags + ): + patch_flags.return_value = self._snapshot_response( + has_experiment=False, gate=True + ) + client, captured = self._make_client() + + flags = client.evaluate_flags("user-1") + # A later response disables the gate; this snapshot's evaluation authorized + # minimization, so its events stay minimal. + client._minimal_flag_called_events = False + flags.get_flag("variant-flag") + + properties = self._flag_called_properties(captured) + self.assertLessEqual(set(properties), _MINIMAL_FLAG_CALLED_EVENT_PROPERTIES) + self.assertIs(properties["$feature_flag_has_experiment"], False) + + @mock.patch("posthog.client.flags") + def test_snapshot_uses_this_responses_gate_not_a_concurrent_update_during_construction( + self, patch_flags + ): + # This response's gate is off. Simulate a concurrent poller refresh (or + # another /flags call) flipping the client-wide gate on, in the gap between + # the remote fallback landing and the snapshot being constructed at the end + # of evaluate_flags(). The snapshot must pin the gate from its own response, + # not whatever the client-wide attribute reads at construction time. + patch_flags.return_value = self._snapshot_response( + has_experiment=False, gate=False + ) + client, captured = self._make_client() + + original_get_flags_decision = client._get_flags_decision + + def get_flags_decision_then_concurrent_gate_flip(*args, **kwargs): + result = original_get_flags_decision(*args, **kwargs) + client._minimal_flag_called_events = True + return result + + client._get_flags_decision = get_flags_decision_then_concurrent_gate_flip + + flags = client.evaluate_flags("user-1") + flags.get_flag("variant-flag") + + properties = self._flag_called_properties(captured) + # Had the snapshot re-read the (now-flipped) client-wide gate at + # construction time instead of this response's own field, it would have + # minimized instead. + self.assertEqual(properties["$feature/variant-flag"], "variant-value") diff --git a/posthog/test/test_flag_definition_cache.py b/posthog/test/test_flag_definition_cache.py index 829f8454..8cee6618 100644 --- a/posthog/test/test_flag_definition_cache.py +++ b/posthog/test/test_flag_definition_cache.py @@ -425,6 +425,24 @@ def test_awaits_async_provider_for_cached_data(self, mock_get): client.join() + @mock.patch("posthog.client.get") + def test_awaits_async_provider_for_cached_data_with_gate_enabled(self, mock_get): + """A cache provider serving a stored gate=True round-trips it onto the + client, so an external cache hit doesn't silently disable minimization.""" + self.cache_provider.should_fetch_return_value = False + self.cache_provider.stored_data = { + **self.sample_flags_data, + "minimal_flag_called_events": True, + } + + client = self._create_client_with_cache() + client._load_feature_flags() + + mock_get.assert_not_called() + self.assertIs(client._minimal_flag_called_events, True) + + client.join() + @mock.patch("posthog.client.get") def test_awaits_async_provider_when_fetching_from_api(self, mock_get): """Async should_fetch and on_received methods are awaited.""" @@ -440,7 +458,12 @@ def test_awaits_async_provider_when_fetching_from_api(self, mock_get): self.assertEqual(self.cache_provider.should_fetch_call_count, 1) self.assertEqual(self.cache_provider.get_call_count, 0) self.assertEqual(self.cache_provider.on_received_call_count, 1) - self.assertEqual(self.cache_provider.stored_data, self.sample_flags_data) + # The stored payload carries the minimal $feature_flag_called gate alongside + # the flag definitions so it survives cache round-trips. + self.assertEqual( + self.cache_provider.stored_data, + {**self.sample_flags_data, "minimal_flag_called_events": False}, + ) self.assertEqual(len(set(self.cache_provider.loop_ids)), 1) client.join() diff --git a/posthog/types.py b/posthog/types.py index 8724e4e3..8e8d7a63 100644 --- a/posthog/types.py +++ b/posthog/types.py @@ -79,8 +79,8 @@ class FlagMetadata: payload: Payload configured for the matched flag value, if any. version: Feature flag version. description: Feature flag description. - has_experiment: Whether the flag has a linked experiment. ``None`` when the - server does not report the field (older deployments). + has_experiment: Whether the flag has a linked experiment. ``None`` when + the server does not report the field (older deployments). """ id: int @@ -93,12 +93,15 @@ class FlagMetadata: def from_json(cls, resp: Any) -> Union["FlagMetadata", LegacyFlagMetadata]: if not resp: return LegacyFlagMetadata(payload=None) + raw_has_experiment = resp.get("has_experiment") return cls( id=resp.get("id", 0), payload=resp.get("payload"), version=resp.get("version", 0), description=resp.get("description", ""), - has_experiment=resp.get("has_experiment"), + has_experiment=raw_has_experiment + if isinstance(raw_has_experiment, bool) + else None, ) @@ -167,6 +170,7 @@ class FlagsResponse(TypedDict, total=False): requestId: str quotaLimit: Optional[List[str]] evaluatedAt: Optional[int] + minimalFlagCalledEvents: bool class FlagsAndPayloads(TypedDict, total=True): diff --git a/references/public_api_snapshot.txt b/references/public_api_snapshot.txt index f7468453..5161f7cd 100644 --- a/references/public_api_snapshot.txt +++ b/references/public_api_snapshot.txt @@ -636,6 +636,7 @@ attribute posthog.feature_flags_request_timeout_seconds = 3 attribute posthog.flag_definition_cache.FlagDefinitionCacheData.cohorts: Required[Dict[str, Any]] attribute posthog.flag_definition_cache.FlagDefinitionCacheData.flags: Required[List[Dict[str, Any]]] attribute posthog.flag_definition_cache.FlagDefinitionCacheData.group_type_mapping: Required[Dict[str, str]] +attribute posthog.flag_definition_cache.FlagDefinitionCacheData.minimal_flag_called_events: NotRequired[bool] attribute posthog.flag_definition_cache_provider = None attribute posthog.host = None attribute posthog.in_app_modules = None @@ -775,6 +776,7 @@ attribute posthog.types.FlagsAndPayloads.featureFlags: Optional[dict[str, FlagVa attribute posthog.types.FlagsResponse.errorsWhileComputingFlags: bool attribute posthog.types.FlagsResponse.evaluatedAt: Optional[int] attribute posthog.types.FlagsResponse.flags: dict[str, FeatureFlag] +attribute posthog.types.FlagsResponse.minimalFlagCalledEvents: bool attribute posthog.types.FlagsResponse.quotaLimit: Optional[List[str]] attribute posthog.types.FlagsResponse.requestId: str attribute posthog.types.LegacyFlagMetadata.payload: Any @@ -869,7 +871,7 @@ class posthog.contexts.ContextScope(parent=None, fresh: bool = False, capture_ex class posthog.exception_capture.ExceptionCapture(client: Client, rate_limiting_enabled=False, bucket_size=DEFAULT_BUCKET_SIZE, refill_rate=DEFAULT_REFILL_RATE, refill_interval_seconds=DEFAULT_REFILL_INTERVAL_SECONDS) class posthog.exception_utils.AnnotatedValue(value, metadata) class posthog.exception_utils.VariableSizeLimiter(max_size=DEFAULT_TOTAL_VARIABLES_SIZE_LIMIT) -class posthog.feature_flag_evaluations.FeatureFlagEvaluations(host: _FeatureFlagEvaluationsHost, distinct_id: str, flags: Dict[str, _EvaluatedFlagRecord], groups: Optional[Mapping[str, Union[str, int]]] = None, disable_geoip: Optional[bool] = None, request_id: Optional[str] = None, evaluated_at: Optional[int] = None, errors_while_computing: bool = False, quota_limited: bool = False, accessed: Optional[Set[str]] = None) +class posthog.feature_flag_evaluations.FeatureFlagEvaluations(host: _FeatureFlagEvaluationsHost, distinct_id: str, flags: Dict[str, _EvaluatedFlagRecord], groups: Optional[Mapping[str, Union[str, int]]] = None, disable_geoip: Optional[bool] = None, request_id: Optional[str] = None, evaluated_at: Optional[int] = None, errors_while_computing: bool = False, quota_limited: bool = False, minimal_flag_called_events: bool = False, accessed: Optional[Set[str]] = None) class posthog.feature_flags.ConditionMatch class posthog.feature_flags.InconclusiveMatchError class posthog.feature_flags.RequiresServerEvaluation