diff --git a/.sampo/changesets/local-snapshot-tracking-metadata.md b/.sampo/changesets/local-snapshot-tracking-metadata.md new file mode 100644 index 000000000..82b003845 --- /dev/null +++ b/.sampo/changesets/local-snapshot-tracking-metadata.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +Retain the minimal-event gate and experiment metadata from the local definitions used by `evaluate_flags()`, including first-use loading and definition refreshes during evaluation. diff --git a/posthog/client.py b/posthog/client.py index d74ea56e3..4089bc02c 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -3250,6 +3250,7 @@ def _local_evaluation_snapshot(self) -> _LocalEvaluationSnapshot: "group_type_mapping": self.group_type_mapping or {}, "cohorts": self.cohorts or {}, "property_matching_version": self._property_matching_version, + "minimal_flag_called_events": self._minimal_flag_called_events, "flag_definition_version": self.flag_definition_version, } @@ -4450,7 +4451,7 @@ def get_all_flags_and_payloads( local_person_properties = self._person_properties_for_local_evaluation( distinct_id, person_properties ) - response, fallback_to_flags = self._get_all_flags_and_payloads_locally( + response, fallback_to_flags, _ = self._get_all_flags_and_payloads_locally( distinct_id, groups=groups, person_properties=local_person_properties, @@ -4587,26 +4588,27 @@ 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( distinct_id, person_properties ) - local_result, fallback_to_server = self._get_all_flags_and_payloads_locally( - distinct_id, - groups=dict(groups), - person_properties=local_person_properties, - group_properties=group_properties, - flag_keys_to_evaluate=flag_keys, - device_id=device_id, + local_result, fallback_to_server, definition_snapshot = ( + self._get_all_flags_and_payloads_locally( + distinct_id, + groups=dict(groups), + person_properties=local_person_properties, + group_properties=group_properties, + flag_keys_to_evaluate=flag_keys, + device_id=device_id, + ) ) - feature_flags_by_key: Dict[str, Any] = self.feature_flags_by_key or {} + # Pin local tracking metadata to the definitions used for these values. + # A successful remote fallback below supplies its own gate instead. + minimal_flag_called_events = definition_snapshot.get( + "minimal_flag_called_events", False + ) + feature_flags_by_key = definition_snapshot["flags_by_key"] local_flags = local_result.get("featureFlags") or {} local_payloads = local_result.get("featureFlagPayloads") or {} if requested_keys and not requested_keys.issubset(local_flags): @@ -4730,7 +4732,7 @@ def _get_all_flags_and_payloads_locally( warn_on_unknown_groups=False, flag_keys_to_evaluate: Optional[list[str]] = None, device_id: Optional[str] = None, - ) -> tuple[FlagsAndPayloads, bool]: + ) -> tuple[FlagsAndPayloads, bool, _LocalEvaluationSnapshot]: person_properties = person_properties or {} group_properties = group_properties or {} @@ -4781,10 +4783,14 @@ def _get_all_flags_and_payloads_locally( else: fallback_to_flags = True - return { - "featureFlags": flags, - "featureFlagPayloads": payloads, - }, fallback_to_flags + return ( + { + "featureFlags": flags, + "featureFlagPayloads": payloads, + }, + fallback_to_flags, + snapshot, + ) def _initialize_flag_cache(self, cache_url): """Initialize feature flag cache for graceful degradation during service outages. diff --git a/posthog/test/test_feature_flag_called_minimization.py b/posthog/test/test_feature_flag_called_minimization.py index c32b4ea27..32310beca 100644 --- a/posthog/test/test_feature_flag_called_minimization.py +++ b/posthog/test/test_feature_flag_called_minimization.py @@ -391,6 +391,92 @@ def _snapshot_response(self, has_experiment, gate): response["minimalFlagCalledEvents"] = gate return response + @parameterized.expand([("api",), ("cache",)]) + @mock.patch("posthog.client.flags") + @mock.patch("posthog.client.get") + def test_lazy_local_snapshot_retains_loaded_minimal_gate( + self, source, patch_get, patch_flags + ): + definitions = { + "flags": [_local_flag_definition(False)], + "group_type_mapping": {}, + "cohorts": {}, + "minimal_flag_called_events": True, + } + patch_get.return_value = GetResponse(data=definitions, etag='"etag-1"') + provider = None + if source == "cache": + provider = mock.Mock() + provider.should_fetch_flag_definitions.return_value = False + provider.get_flag_definitions.return_value = definitions + provider.shutdown.return_value = None + client, captured = self._make_client( + secret_key="personal-key", + enable_local_evaluation=False, + flag_definition_cache_provider=provider, + ) + self.addCleanup(client.shutdown) + + snapshot = client.evaluate_flags("user-1") + self.assertTrue(snapshot.get_flag("person-flag")) + + properties = self._flag_called_properties(captured) + self.assertLessEqual(set(properties), _MINIMAL_FLAG_CALLED_EVENT_PROPERTIES) + self.assertNotIn("app_version", properties) + self.assertIs(properties["locally_evaluated"], True) + patch_flags.assert_not_called() + if source == "cache": + patch_get.assert_not_called() + else: + patch_get.assert_called_once() + + @parameterized.expand([("gate_on", True), ("gate_off", False)]) + @mock.patch("posthog.client.flags") + @mock.patch("posthog.client.get") + def test_local_snapshot_retains_metadata_when_definitions_refresh_during_evaluation( + self, _name, gate, patch_get, patch_flags + ): + definitions = { + "flags": [_local_flag_definition(False)], + "group_type_mapping": {}, + "cohorts": {}, + "minimal_flag_called_events": gate, + } + patch_get.return_value = GetResponse(data=definitions, etag='"etag-1"') + client, captured = self._make_client( + secret_key="personal-key", enable_local_evaluation=False + ) + self.addCleanup(client.shutdown) + original_compute = client._compute_flag_locally + + def evaluate_then_refresh(*args, **kwargs): + value = original_compute(*args, **kwargs) + # A poll publishes new tracking metadata while this evaluation is + # still using the previous definition snapshot. + client._update_flag_state( + { + **definitions, + "flags": [_local_flag_definition(True)], + "minimal_flag_called_events": not gate, + } + ) + return value + + with mock.patch.object( + client, "_compute_flag_locally", side_effect=evaluate_then_refresh + ): + snapshot = client.evaluate_flags("user-1") + self.assertTrue(snapshot.get_flag("person-flag")) + + properties = self._flag_called_properties(captured) + self.assertIs(properties["$feature_flag_has_experiment"], False) + if gate: + self.assertLessEqual(set(properties), _MINIMAL_FLAG_CALLED_EVENT_PROPERTIES) + self.assertNotIn("app_version", properties) + else: + self.assertEqual(properties["app_version"], "1.2.3") + patch_flags.assert_not_called() + @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( diff --git a/posthog/test/test_property_matching_version.py b/posthog/test/test_property_matching_version.py index 640efacb5..f6f1f8774 100644 --- a/posthog/test/test_property_matching_version.py +++ b/posthog/test/test_property_matching_version.py @@ -238,7 +238,7 @@ def reload_during_evaluation(*args, **kwargs): "posthog.client.match_feature_flag_properties", side_effect=reload_during_evaluation, ): - result, fallback = client._get_all_flags_and_payloads_locally( + result, fallback, _ = client._get_all_flags_and_payloads_locally( "user", groups={"company": "company-id"}, person_properties={"value": "banana"},