Skip to content

Commit 80e6e43

Browse files
authored
fix: Respect send_feature_flags setting and deprecate send_feature_flag_events in get_feature_flag_payload (#391)
* fix: Respect `send_feature_flags` when local eval is enabled * fix: Deprecate `send_feature_flag_events` in `get_feature_flag_payload`
1 parent 5d0bae1 commit 80e6e43

File tree

3 files changed

+106
-67
lines changed

3 files changed

+106
-67
lines changed

posthog/client.py

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import logging
33
import os
44
import sys
5+
import warnings
56
from datetime import datetime, timedelta
67
from typing import Any, Dict, Optional, Union
78
from typing_extensions import Unpack
@@ -679,15 +680,6 @@ def capture(
679680
f"[FEATURE FLAGS] Unable to get feature variants: {e}"
680681
)
681682

682-
elif self.feature_flags and event != "$feature_flag_called":
683-
# Local evaluation is enabled, flags are loaded, so try and get all flags we can without going to the server
684-
feature_variants = self.get_all_flags(
685-
distinct_id,
686-
groups=(groups or {}),
687-
disable_geoip=disable_geoip,
688-
only_evaluate_locally=True,
689-
)
690-
691683
for feature, variant in (feature_variants or {}).items():
692684
extra_properties[f"$feature/{feature}"] = variant
693685

@@ -1797,7 +1789,7 @@ def get_feature_flag_payload(
17971789
person_properties=None,
17981790
group_properties=None,
17991791
only_evaluate_locally=False,
1800-
send_feature_flag_events=True,
1792+
send_feature_flag_events=False,
18011793
disable_geoip=None,
18021794
):
18031795
"""
@@ -1811,7 +1803,7 @@ def get_feature_flag_payload(
18111803
person_properties: A dictionary of person properties.
18121804
group_properties: A dictionary of group properties.
18131805
only_evaluate_locally: Whether to only evaluate locally.
1814-
send_feature_flag_events: Whether to send feature flag events.
1806+
send_feature_flag_events: Deprecated. Use get_feature_flag() instead if you need events.
18151807
disable_geoip: Whether to disable GeoIP for this request.
18161808
18171809
Examples:
@@ -1827,6 +1819,14 @@ def get_feature_flag_payload(
18271819
Category:
18281820
Feature flags
18291821
"""
1822+
if send_feature_flag_events:
1823+
warnings.warn(
1824+
"send_feature_flag_events is deprecated in get_feature_flag_payload() and will be removed "
1825+
"in a future version. Use get_feature_flag() if you want to send $feature_flag_called events.",
1826+
DeprecationWarning,
1827+
stacklevel=2,
1828+
)
1829+
18301830
feature_flag_result = self._get_feature_flag_result(
18311831
key,
18321832
distinct_id,

posthog/test/test_client.py

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,9 @@ def test_basic_capture_with_locally_evaluated_feature_flags(self, patch_flags):
409409
)
410410
client.feature_flags = [multivariate_flag, basic_flag, false_flag]
411411

412-
msg_uuid = client.capture("python test event", distinct_id="distinct_id")
412+
msg_uuid = client.capture(
413+
"python test event", distinct_id="distinct_id", send_feature_flags=True
414+
)
413415
self.assertIsNotNone(msg_uuid)
414416
self.assertFalse(self.failed)
415417

@@ -565,6 +567,7 @@ def test_dont_override_capture_with_local_flags(self, patch_flags):
565567
"python test event",
566568
distinct_id="distinct_id",
567569
properties={"$feature/beta-feature-local": "my-custom-variant"},
570+
send_feature_flags=True,
568571
)
569572
self.assertIsNotNone(msg_uuid)
570573
self.assertFalse(self.failed)
@@ -746,6 +749,88 @@ def test_basic_capture_with_feature_flags_switched_off_doesnt_send_them(
746749

747750
self.assertEqual(patch_flags.call_count, 0)
748751

752+
@mock.patch("posthog.client.flags")
753+
def test_capture_with_send_feature_flags_false_and_local_evaluation_doesnt_send_flags(
754+
self, patch_flags
755+
):
756+
"""Test that send_feature_flags=False with local evaluation enabled does NOT send flags"""
757+
patch_flags.return_value = {"featureFlags": {"beta-feature": "remote-variant"}}
758+
759+
multivariate_flag = {
760+
"id": 1,
761+
"name": "Beta Feature",
762+
"key": "beta-feature-local",
763+
"active": True,
764+
"rollout_percentage": 100,
765+
"filters": {
766+
"groups": [
767+
{
768+
"rollout_percentage": 100,
769+
},
770+
],
771+
"multivariate": {
772+
"variants": [
773+
{
774+
"key": "first-variant",
775+
"name": "First Variant",
776+
"rollout_percentage": 50,
777+
},
778+
{
779+
"key": "second-variant",
780+
"name": "Second Variant",
781+
"rollout_percentage": 50,
782+
},
783+
]
784+
},
785+
},
786+
}
787+
simple_flag = {
788+
"id": 2,
789+
"name": "Simple Flag",
790+
"key": "simple-flag",
791+
"active": True,
792+
"filters": {
793+
"groups": [
794+
{
795+
"rollout_percentage": 100,
796+
}
797+
],
798+
},
799+
}
800+
801+
with mock.patch("posthog.client.batch_post") as mock_post:
802+
client = Client(
803+
FAKE_TEST_API_KEY,
804+
on_error=self.set_fail,
805+
personal_api_key=FAKE_TEST_API_KEY,
806+
sync_mode=True,
807+
)
808+
client.feature_flags = [multivariate_flag, simple_flag]
809+
810+
msg_uuid = client.capture(
811+
"python test event",
812+
distinct_id="distinct_id",
813+
send_feature_flags=False,
814+
)
815+
self.assertIsNotNone(msg_uuid)
816+
self.assertFalse(self.failed)
817+
818+
# Get the enqueued message from the mock
819+
mock_post.assert_called_once()
820+
batch_data = mock_post.call_args[1]["batch"]
821+
msg = batch_data[0]
822+
823+
self.assertEqual(msg["event"], "python test event")
824+
self.assertEqual(msg["distinct_id"], "distinct_id")
825+
826+
# CRITICAL: Verify local flags are NOT included in the event
827+
self.assertNotIn("$feature/beta-feature-local", msg["properties"])
828+
self.assertNotIn("$feature/simple-flag", msg["properties"])
829+
self.assertNotIn("$active_feature_flags", msg["properties"])
830+
831+
# CRITICAL: Verify the /flags API was NOT called
832+
self.assertEqual(patch_flags.call_count, 0)
833+
749834
@mock.patch("posthog.client.flags")
750835
def test_capture_with_send_feature_flags_true_and_local_evaluation_uses_local_flags(
751836
self, patch_flags

posthog/test/test_feature_flags.py

Lines changed: 9 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -3062,6 +3062,7 @@ def test_boolean_feature_flag_payload_decide(self, patch_flags, patch_capture):
30623062
"some-distinct-id",
30633063
match_value=True,
30643064
person_properties={"region": "USA"},
3065+
send_feature_flag_events=True,
30653066
),
30663067
300,
30673068
)
@@ -4051,7 +4052,9 @@ def test_capture_is_called_with_flag_details_and_payload(
40514052

40524053
self.assertEqual(
40534054
client.get_feature_flag_payload(
4054-
"decide-flag-with-payload", "some-distinct-id"
4055+
"decide-flag-with-payload",
4056+
"some-distinct-id",
4057+
send_feature_flag_events=True,
40554058
),
40564059
{"foo": "bar"},
40574060
)
@@ -4127,9 +4130,10 @@ def test_capture_is_called_but_does_not_add_all_flags(self, patch_flags):
41274130

41284131
@mock.patch.object(Client, "capture")
41294132
@mock.patch("posthog.client.flags")
4130-
def test_capture_is_called_in_get_feature_flag_payload(
4133+
def test_get_feature_flag_payload_does_not_send_feature_flag_called_events(
41314134
self, patch_flags, patch_capture
41324135
):
4136+
"""Test that get_feature_flag_payload does NOT send $feature_flag_called events"""
41334137
patch_flags.return_value = {
41344138
"featureFlags": {"person-flag": True},
41354139
"featureFlagPayloads": {"person-flag": 300},
@@ -4151,68 +4155,18 @@ def test_capture_is_called_in_get_feature_flag_payload(
41514155
"rollout_percentage": 100,
41524156
}
41534157
],
4158+
"payloads": {"true": '"payload"'},
41544159
},
41554160
}
41564161
]
41574162

4158-
# Call get_feature_flag_payload with match_value=None to trigger get_feature_flag
4159-
client.get_feature_flag_payload(
4160-
key="person-flag",
4161-
distinct_id="some-distinct-id",
4162-
person_properties={"region": "USA", "name": "Aloha"},
4163-
)
4164-
4165-
# Assert that capture was called once, with the correct parameters
4166-
self.assertEqual(patch_capture.call_count, 1)
4167-
patch_capture.assert_called_with(
4168-
"$feature_flag_called",
4169-
distinct_id="some-distinct-id",
4170-
properties={
4171-
"$feature_flag": "person-flag",
4172-
"$feature_flag_response": True,
4173-
"locally_evaluated": True,
4174-
"$feature/person-flag": True,
4175-
},
4176-
groups={},
4177-
disable_geoip=None,
4178-
)
4179-
4180-
# Reset mocks for further tests
4181-
patch_capture.reset_mock()
4182-
patch_flags.reset_mock()
4183-
4184-
# Call get_feature_flag_payload again for the same user; capture should not be called again because we've already reported an event for this distinct_id + flag
4185-
client.get_feature_flag_payload(
4163+
payload = client.get_feature_flag_payload(
41864164
key="person-flag",
41874165
distinct_id="some-distinct-id",
41884166
person_properties={"region": "USA", "name": "Aloha"},
41894167
)
4190-
4168+
self.assertIsNotNone(payload)
41914169
self.assertEqual(patch_capture.call_count, 0)
4192-
patch_capture.reset_mock()
4193-
4194-
# Call get_feature_flag_payload for a different user; capture should be called
4195-
client.get_feature_flag_payload(
4196-
key="person-flag",
4197-
distinct_id="some-distinct-id2",
4198-
person_properties={"region": "USA", "name": "Aloha"},
4199-
)
4200-
4201-
self.assertEqual(patch_capture.call_count, 1)
4202-
patch_capture.assert_called_with(
4203-
"$feature_flag_called",
4204-
distinct_id="some-distinct-id2",
4205-
properties={
4206-
"$feature_flag": "person-flag",
4207-
"$feature_flag_response": True,
4208-
"locally_evaluated": True,
4209-
"$feature/person-flag": True,
4210-
},
4211-
groups={},
4212-
disable_geoip=None,
4213-
)
4214-
4215-
patch_capture.reset_mock()
42164170

42174171
@mock.patch("posthog.client.flags")
42184172
def test_fallback_to_api_in_get_feature_flag_payload_when_flag_has_static_cohort(

0 commit comments

Comments
 (0)