From d088ff01dd85e94e21121280d257a68ac5df503c Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Mon, 10 Feb 2025 11:18:46 -0800 Subject: [PATCH 1/3] Add failing test to demonstrate problem --- posthog/test/test_feature_flags.py | 52 ++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/posthog/test/test_feature_flags.py b/posthog/test/test_feature_flags.py index b46d4cfd..4761d9f1 100644 --- a/posthog/test/test_feature_flags.py +++ b/posthog/test/test_feature_flags.py @@ -2339,6 +2339,58 @@ def test_capture_is_called(self, patch_decide, patch_capture): disable_geoip=None, ) + @mock.patch("posthog.client.decide") + def test_capture_is_called_but_does_not_add_all_flags(self, patch_decide): + patch_decide.return_value = {"featureFlags": {"decide-flag": "decide-value"}} + client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY) + client.feature_flags = [ + { + "id": 1, + "name": "Beta Feature", + "key": "complex-flag", + "active": True, + "filters": { + "groups": [ + { + "properties": [{"key": "region", "value": "USA"}], + "rollout_percentage": 100, + }, + ], + }, + }, + { + "id": 2, + "name": "Gamma Feature", + "key": "simple-flag", + "active": True, + "filters": { + "groups": [ + { + "properties": [], + "rollout_percentage": 100, + }, + ], + }, + } + ] + + self.assertTrue( + client.get_feature_flag( + "complex-flag", "some-distinct-id", person_properties={"region": "USA"} + ) + ) + + # Grab the capture message that was just added to the queue + msg = client.queue.get(block=False) + assert msg["event"] == "$feature_flag_called" + assert msg["properties"]["$feature_flag"] == "complex-flag" + assert msg["properties"]["$feature_flag_response"] is True + assert msg["properties"]["locally_evaluated"] is True + assert msg["properties"]["$feature/complex-flag"] is True + assert "$feature/simple-flag" not in msg["properties"] + assert "$active_feature_flags" not in msg["properties"] + + @mock.patch.object(Client, "capture") @mock.patch("posthog.client.decide") def test_capture_is_called_in_get_feature_flag_payload(self, patch_decide, patch_capture): From e7ce22f7b8c15a44a3f90300ffaa3d6f91320556 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Mon, 10 Feb 2025 11:21:10 -0800 Subject: [PATCH 2/3] Do not enrich the $feature_flag_called event with more feature flag data --- posthog/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/client.py b/posthog/client.py index 8f258a1c..d38253b7 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -245,7 +245,7 @@ def capture( except Exception as e: self.log.exception(f"[FEATURE FLAGS] Unable to get feature variants: {e}") - elif self.feature_flags: + elif 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 From b40eb80e35186396d1fdc810fcc894b504153c61 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Mon, 10 Feb 2025 11:34:54 -0800 Subject: [PATCH 3/3] Reformat --- posthog/test/test_feature_flags.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/posthog/test/test_feature_flags.py b/posthog/test/test_feature_flags.py index 4761d9f1..62de7300 100644 --- a/posthog/test/test_feature_flags.py +++ b/posthog/test/test_feature_flags.py @@ -2371,13 +2371,11 @@ def test_capture_is_called_but_does_not_add_all_flags(self, patch_decide): }, ], }, - } + }, ] self.assertTrue( - client.get_feature_flag( - "complex-flag", "some-distinct-id", person_properties={"region": "USA"} - ) + client.get_feature_flag("complex-flag", "some-distinct-id", person_properties={"region": "USA"}) ) # Grab the capture message that was just added to the queue @@ -2390,7 +2388,6 @@ def test_capture_is_called_but_does_not_add_all_flags(self, patch_decide): assert "$feature/simple-flag" not in msg["properties"] assert "$active_feature_flags" not in msg["properties"] - @mock.patch.object(Client, "capture") @mock.patch("posthog.client.decide") def test_capture_is_called_in_get_feature_flag_payload(self, patch_decide, patch_capture):