diff --git a/posthog/client.py b/posthog/client.py index 8ada4e1..04dc17b 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -479,17 +479,15 @@ def capture( extra_properties: dict[str, Any] = {} feature_variants: Optional[dict[str, Union[bool, str]]] = {} - if send_feature_flags: - try: - feature_variants = self.get_feature_variants(distinct_id, groups, disable_geoip=disable_geoip) - except Exception as e: - self.log.exception(f"[FEATURE FLAGS] Unable to get feature variants: {e}") - - elif self.feature_flags and event != "$feature_flag_called": + if self.feature_flags and event != "$feature_flag_called": # Local evaluation is enabled, flags are loaded, so try and get all flags we can without going to the server feature_variants = self.get_all_flags( distinct_id, groups=(groups or {}), disable_geoip=disable_geoip, only_evaluate_locally=True ) + elif send_feature_flags: + feature_variants = self.get_all_flags( + distinct_id, groups=(groups or {}), disable_geoip=disable_geoip, only_evaluate_locally=False + ) for feature, variant in (feature_variants or {}).items(): extra_properties[f"$feature/{feature}"] = variant diff --git a/posthog/test/test_client.py b/posthog/test/test_client.py index 8f6c311..18a9bcc 100644 --- a/posthog/test/test_client.py +++ b/posthog/test/test_client.py @@ -271,12 +271,56 @@ def test_capture_exception_logs_when_enabled(self): self.assertEqual(logs.output[0], "ERROR:posthog:test exception\nNoneType: None") self.assertEqual(getattr(logs.records[0], "path"), "one/two/three") + @mock.patch("posthog.client.get") @mock.patch("posthog.client.flags") - def test_basic_capture_with_feature_flags(self, patch_flags): + def test_basic_capture_with_feature_flags(self, patch_flags, patch_get): patch_flags.return_value = {"featureFlags": {"beta-feature": "random-variant"}} + patch_get.return_value = { + "flags": [ + { + "id": 2, + "team_id": 1, + "name": "Beta Feature", + "key": "beta-feature", + "filters": { + "groups": [ + { + "properties": [], + "rollout_percentage": None + } + ], + "payloads": {}, + "multivariate": { + "variants": [ + { + "key": "random-variant", + "rollout_percentage": 100 + }, + { + "key": "test", + "rollout_percentage": 0 + } + ] + } + }, + "deleted": False, + "active": True, + "ensure_experience_continuity": False, + "has_encrypted_payloads": False, + "version": 2 + } + ], + "group_type_mapping": { + "0": "account", + "1": "organization", + "2": "instance", + "3": "project" + }, + "cohorts": {} + } client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, personal_api_key=FAKE_TEST_API_KEY) - success, msg = client.capture("distinct_id", "python test event", send_feature_flags=True) + success, msg = client.capture("distinct_id", "python test event") client.flush() self.assertTrue(success) self.assertFalse(self.failed) @@ -287,14 +331,19 @@ def test_basic_capture_with_feature_flags(self, patch_flags): self.assertEqual(msg["distinct_id"], "distinct_id") self.assertEqual(msg["properties"]["$lib"], "posthog-python") self.assertEqual(msg["properties"]["$lib_version"], VERSION) - self.assertEqual(msg["properties"]["$feature/beta-feature"], "random-variant") - self.assertEqual(msg["properties"]["$active_feature_flags"], ["beta-feature"]) - self.assertEqual(patch_flags.call_count, 1) + # Feature flag is not sent, because client.feature_flags is unset and send_feature_flags=False + assert "$feature/beta-feature" not in msg["properties"] + assert "$active_feature_flags" not in msg["properties"] + self.assertEqual(patch_flags.call_count, 0) + self.assertEqual(patch_get.call_count, 0) + @mock.patch("posthog.client.get") @mock.patch("posthog.client.flags") - def test_basic_capture_with_locally_evaluated_feature_flags(self, patch_flags): + def test_basic_capture_with_locally_evaluated_feature_flags(self, patch_flags, patch_get): patch_flags.return_value = {"featureFlags": {"beta-feature": "random-variant"}} + patch_get.return_value = {"flags": []} + client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, personal_api_key=FAKE_TEST_API_KEY) multivariate_flag = { @@ -375,23 +424,205 @@ def test_basic_capture_with_locally_evaluated_feature_flags(self, patch_flags): self.assertEqual(msg["distinct_id"], "distinct_id") self.assertEqual(msg["properties"]["$lib"], "posthog-python") self.assertEqual(msg["properties"]["$lib_version"], VERSION) + + # Feature flag is sent despite send_feature_flags=False + # because client.feature_flags is set self.assertEqual(msg["properties"]["$feature/beta-feature-local"], "third-variant") self.assertEqual(msg["properties"]["$feature/false-flag"], False) self.assertEqual(msg["properties"]["$active_feature_flags"], ["beta-feature-local"]) assert "$feature/beta-feature" not in msg["properties"] + # There are no calls to (/flags, /decide) or /api/feature_flag/local_evaluation self.assertEqual(patch_flags.call_count, 0) + self.assertEqual(patch_get.call_count, 0) - # test that flags are not evaluated without local evaluation - client.feature_flags = [] - success, msg = client.capture("distinct_id", "python test event") + @mock.patch("posthog.client.get") + @mock.patch("posthog.client.flags") + def test_basic_capture_with_local_evaluation_and_send_feature_flags(self, patch_flags, patch_get): + patch_flags.return_value = {"featureFlags": {"beta-feature": "random-variant"}} + patch_get.return_value = { + "flags": [ + { + "id": 2, + "team_id": 1, + "name": "Beta Feature", + "key": "beta-feature", + "filters": { + "groups": [ + { + "properties": [], + "rollout_percentage": None + } + ], + "payloads": {}, + "multivariate": { + "variants": [ + { + "key": "random-variant", + "rollout_percentage": 100 + }, + { + "key": "test", + "rollout_percentage": 0 + } + ] + } + }, + "deleted": False, + "active": True, + "ensure_experience_continuity": False, + "has_encrypted_payloads": False, + "version": 2 + } + ], + "group_type_mapping": { + "0": "account", + "1": "organization", + "2": "instance", + "3": "project" + }, + "cohorts": {} + } + + client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, personal_api_key=FAKE_TEST_API_KEY) + + multivariate_flag = { + "id": 1, + "name": "Beta Feature", + "key": "beta-feature-local", + "active": True, + "rollout_percentage": 100, + "filters": { + "groups": [ + { + "properties": [ + {"key": "email", "type": "person", "value": "test@posthog.com", "operator": "exact"} + ], + "rollout_percentage": 100, + }, + { + "rollout_percentage": 50, + }, + ], + "multivariate": { + "variants": [ + {"key": "first-variant", "name": "First Variant", "rollout_percentage": 50}, + {"key": "second-variant", "name": "Second Variant", "rollout_percentage": 25}, + {"key": "third-variant", "name": "Third Variant", "rollout_percentage": 25}, + ] + }, + "payloads": {"first-variant": "some-payload", "third-variant": {"a": "json"}}, + }, + } + basic_flag = { + "id": 1, + "name": "Beta Feature", + "key": "person-flag", + "active": True, + "filters": { + "groups": [ + { + "properties": [ + { + "key": "region", + "operator": "exact", + "value": ["USA"], + "type": "person", + } + ], + "rollout_percentage": 100, + } + ], + "payloads": {"true": 300}, + }, + } + false_flag = { + "id": 1, + "name": "Beta Feature", + "key": "false-flag", + "active": True, + "filters": { + "groups": [ + { + "properties": [], + "rollout_percentage": 0, + } + ], + "payloads": {"true": 300}, + }, + } + client.feature_flags = [multivariate_flag, basic_flag, false_flag] + + # test that flags are evaluated locally without calling /flags or /decide + success, msg = client.capture("distinct_id", "python test event", send_feature_flags=True) client.flush() self.assertTrue(success) self.assertFalse(self.failed) + + self.assertEqual(msg["event"], "python test event") + self.assertTrue(isinstance(msg["timestamp"], str)) + self.assertIsNone(msg.get("uuid")) + self.assertEqual(msg["distinct_id"], "distinct_id") + self.assertEqual(msg["properties"]["$lib"], "posthog-python") + self.assertEqual(msg["properties"]["$lib_version"], VERSION) + self.assertEqual(msg["properties"]["$feature/beta-feature-local"], "third-variant") + self.assertEqual(msg["properties"]["$feature/false-flag"], False) + self.assertEqual(msg["properties"]["$active_feature_flags"], ["beta-feature-local"]) assert "$feature/beta-feature" not in msg["properties"] - assert "$feature/beta-feature-local" not in msg["properties"] - assert "$feature/false-flag" not in msg["properties"] - assert "$active_feature_flags" not in msg["properties"] + + # There are no calls to (/flags, /decide) or /api/feature_flag/local_evaluation + # because client.feature_flags is set. + self.assertEqual(patch_flags.call_count, 0) + self.assertEqual(patch_get.call_count, 0) + + # Test that flags are evaluated with only call to local evaluation (/api/feature_flag/local_evaluation) + # Create a new client, so that client.feature_flags is None + client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, personal_api_key=FAKE_TEST_API_KEY) + success, msg = client.capture("distinct_id", "python test event", send_feature_flags=True) + client.flush() + self.assertTrue(success) + self.assertFalse(self.failed) + self.assertEqual(msg["properties"]["$feature/beta-feature"], "random-variant") + self.assertEqual(msg["properties"]["$active_feature_flags"], ["beta-feature"]) + self.assertEqual(patch_get.call_count, 1) + self.assertEqual(patch_flags.call_count, 0) + + @mock.patch("posthog.client.get") + @mock.patch("posthog.client.flags") + def test_basic_capture_with_local_evaluation_and_fallback_to_decide(self, patch_flags, patch_get): + patch_flags.return_value = {"featureFlags": {"beta-feature": "random-variant"}} + patch_get.return_value = {"flags": []} + + client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, personal_api_key=FAKE_TEST_API_KEY) + success, msg = client.capture("distinct_id", "python test event", send_feature_flags=True) + client.flush() + self.assertTrue(success) + self.assertFalse(self.failed) + self.assertEqual(msg["properties"]["$feature/beta-feature"], "random-variant") + self.assertEqual(msg["properties"]["$active_feature_flags"], ["beta-feature"]) + + # Both (/flags, /decide) and /api/feature_flag/local_evaluation are called + self.assertEqual(patch_get.call_count, 1) + self.assertEqual(patch_flags.call_count, 1) + + @mock.patch("posthog.client.get") + @mock.patch("posthog.client.flags") + def test_basic_capture_with_local_evaluation_disabled(self, patch_flags, patch_get): + patch_flags.return_value = {"featureFlags": {"beta-feature": "random-variant"}} + patch_get.return_value = {"flags": []} + + # Creating client without personal_api_key disables local evaluation + client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail) + success, msg = client.capture("distinct_id", "python test event", send_feature_flags=True) + client.flush() + self.assertTrue(success) + self.assertFalse(self.failed) + self.assertEqual(msg["properties"]["$feature/beta-feature"], "random-variant") + self.assertEqual(msg["properties"]["$active_feature_flags"], ["beta-feature"]) + + # Only (/flags, /decide) is called. /api/feature_flag/local_evaluation is not called + self.assertEqual(patch_flags.call_count, 1) + self.assertEqual(patch_get.call_count, 0) @mock.patch("posthog.client.get") def test_load_feature_flags_quota_limited(self, patch_get): @@ -519,8 +750,8 @@ def test_basic_capture_with_feature_flags_returns_active_only(self, patch_flags) timeout=3, distinct_id="distinct_id", groups={}, - person_properties=None, - group_properties=None, + person_properties={"distinct_id": "distinct_id"}, + group_properties={}, disable_geoip=True, ) @@ -561,8 +792,8 @@ def test_basic_capture_with_feature_flags_and_disable_geoip_returns_correctly(se timeout=12, distinct_id="distinct_id", groups={}, - person_properties=None, - group_properties=None, + person_properties={"distinct_id": "distinct_id"}, + group_properties={}, disable_geoip=False, )