Skip to content

Commit 07277d3

Browse files
neilkakkarneilkakkar
authored
feat(flags): Enable local evaluation for all cohorts (#91)
1 parent 15d0716 commit 07277d3

File tree

5 files changed

+266
-9
lines changed

5 files changed

+266
-9
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 2.4.0 - 2023-03-14
2+
3+
1. Support evaluating all cohorts in feature flags for local evaluation
4+
15
## 2.3.1 - 2023-02-07
26

37
1. Log instead of raise error on posthog personal api key errors

‎posthog/client.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ def __init__(
6565
self.feature_flags = None
6666
self.feature_flags_by_key = None
6767
self.group_type_mapping = None
68+
self.cohorts = None
6869
self.poll_interval = poll_interval
6970
self.poller = None
7071
self.distinct_ids_feature_flags_reported = SizeLimitedDict(MAX_DICT_SIZE, set)
@@ -155,6 +156,8 @@ def get_decide(self, distinct_id, groups=None, person_properties=None, group_pro
155156
"group_properties": group_properties,
156157
}
157158
resp_data = decide(self.api_key, self.host, timeout=10, **request_data)
159+
160+
print(resp_data)
158161
return resp_data
159162

160163
def capture(
@@ -374,7 +377,7 @@ def _load_feature_flags(self):
374377
try:
375378
response = get(
376379
self.personal_api_key,
377-
f"/api/feature_flag/local_evaluation/?token={self.api_key}",
380+
f"/api/feature_flag/local_evaluation/?token={self.api_key}&send_cohorts",
378381
self.host,
379382
timeout=10,
380383
)
@@ -384,6 +387,7 @@ def _load_feature_flags(self):
384387
flag["key"]: flag for flag in self.feature_flags if flag.get("key") is not None
385388
}
386389
self.group_type_mapping = response["group_type_mapping"] or {}
390+
self.cohorts = response["cohorts"] or {}
387391

388392
except APIError as e:
389393
if e.status == 401:
@@ -449,7 +453,7 @@ def _compute_flag_locally(self, feature_flag, distinct_id, *, groups={}, person_
449453
focused_group_properties = group_properties[group_name]
450454
return match_feature_flag_properties(feature_flag, groups[group_name], focused_group_properties)
451455
else:
452-
return match_feature_flag_properties(feature_flag, distinct_id, person_properties)
456+
return match_feature_flag_properties(feature_flag, distinct_id, person_properties, self.cohorts)
453457

454458
def feature_enabled(
455459
self,

‎posthog/feature_flags.py‎

Lines changed: 102 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import datetime
22
import hashlib
3+
import logging
34
import re
45

56
from dateutil import parser
@@ -8,6 +9,8 @@
89

910
__LONG_SCALE__ = float(0xFFFFFFFFFFFFFFF)
1011

12+
log = logging.getLogger("posthog")
13+
1114

1215
class InconclusiveMatchError(Exception):
1316
pass
@@ -42,9 +45,10 @@ def variant_lookup_table(feature_flag):
4245
return lookup_table
4346

4447

45-
def match_feature_flag_properties(flag, distinct_id, properties):
48+
def match_feature_flag_properties(flag, distinct_id, properties, cohort_properties=None):
4649
flag_conditions = (flag.get("filters") or {}).get("groups") or []
4750
is_inconclusive = False
51+
cohort_properties = cohort_properties or {}
4852

4953
# Stable sort conditions with variant overrides to the top. This ensures that if overrides are present, they are
5054
# evaluated first, and the variant override is applied to the first matching condition.
@@ -57,7 +61,7 @@ def match_feature_flag_properties(flag, distinct_id, properties):
5761
try:
5862
# if any one condition resolves to True, we can shortcircuit and return
5963
# the matching variant
60-
if is_condition_match(flag, distinct_id, condition, properties):
64+
if is_condition_match(flag, distinct_id, condition, properties, cohort_properties):
6165
variant_override = condition.get("variant")
6266
# Some filters can be explicitly set to null, which require accessing variants like so
6367
flag_variants = ((flag.get("filters") or {}).get("multivariate") or {}).get("variants") or []
@@ -77,12 +81,19 @@ def match_feature_flag_properties(flag, distinct_id, properties):
7781
return False
7882

7983

80-
def is_condition_match(feature_flag, distinct_id, condition, properties):
84+
def is_condition_match(feature_flag, distinct_id, condition, properties, cohort_properties):
8185
rollout_percentage = condition.get("rollout_percentage")
8286
if len(condition.get("properties") or []) > 0:
83-
if not all(match_property(prop, properties) for prop in condition.get("properties")):
84-
return False
85-
elif rollout_percentage is None:
87+
for prop in condition.get("properties"):
88+
property_type = prop.get("type")
89+
if property_type == "cohort":
90+
matches = match_cohort(prop, properties, cohort_properties)
91+
else:
92+
matches = match_property(prop, properties)
93+
if not matches:
94+
return False
95+
96+
if rollout_percentage is None:
8697
return True
8798

8899
if rollout_percentage is not None and _hash(feature_flag["key"], distinct_id) > (rollout_percentage / 100):
@@ -175,3 +186,88 @@ def match_property(property, property_values) -> bool:
175186
raise InconclusiveMatchError("The date provided must be a string or date object")
176187

177188
return False
189+
190+
191+
def match_cohort(property, property_values, cohort_properties) -> bool:
192+
# Cohort properties are in the form of property groups like this:
193+
# {
194+
# "cohort_id": {
195+
# "type": "AND|OR",
196+
# "values": [{
197+
# "key": "property_name", "value": "property_value"
198+
# }]
199+
# }
200+
# }
201+
cohort_id = str(property.get("value"))
202+
if cohort_id not in cohort_properties:
203+
raise InconclusiveMatchError("can't match cohort without a given cohort property value")
204+
205+
property_group = cohort_properties[cohort_id]
206+
return match_property_group(property_group, property_values, cohort_properties)
207+
208+
209+
def match_property_group(property_group, property_values, cohort_properties) -> bool:
210+
if not property_group:
211+
return True
212+
213+
property_group_type = property_group.get("type")
214+
properties = property_group.get("values")
215+
216+
if not properties or len(properties) == 0:
217+
# empty groups are no-ops, always match
218+
return True
219+
220+
error_matching_locally = False
221+
222+
if "values" in properties[0]:
223+
# a nested property group
224+
for prop in properties:
225+
try:
226+
matches = match_property_group(prop, property_values, cohort_properties)
227+
if property_group_type == "AND":
228+
if not matches:
229+
return False
230+
else:
231+
# OR group
232+
if matches:
233+
return True
234+
except InconclusiveMatchError as e:
235+
log.debug(f"Failed to compute property {prop} locally: {e}")
236+
error_matching_locally = True
237+
238+
if error_matching_locally:
239+
raise InconclusiveMatchError("Can't match cohort without a given cohort property value")
240+
# if we get here, all matched in AND case, or none matched in OR case
241+
return property_group_type == "AND"
242+
243+
else:
244+
for prop in properties:
245+
try:
246+
if prop.get("type") == "cohort":
247+
matches = match_cohort(prop, property_values, cohort_properties)
248+
else:
249+
matches = match_property(prop, property_values)
250+
251+
negation = prop.get("negation", False)
252+
253+
if property_group_type == "AND":
254+
# if negated property, do the inverse
255+
if not matches and not negation:
256+
return False
257+
if matches and negation:
258+
return False
259+
else:
260+
# OR group
261+
if matches and not negation:
262+
return True
263+
if not matches and negation:
264+
return True
265+
except InconclusiveMatchError as e:
266+
log.debug(f"Failed to compute property {prop} locally: {e}")
267+
error_matching_locally = True
268+
269+
if error_matching_locally:
270+
raise InconclusiveMatchError("can't match cohort without a given cohort property value")
271+
272+
# if we get here, all matched in AND case, or none matched in OR case
273+
return property_group_type == "AND"

‎posthog/test/test_feature_flags.py‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -960,6 +960,159 @@ def test_compute_inactive_flags_locally(self, patch_decide, patch_capture):
960960
self.assertEqual(patch_decide.call_count, 0)
961961
self.assertEqual(patch_capture.call_count, 0)
962962

963+
@mock.patch("posthog.client.decide")
964+
@mock.patch("posthog.client.get")
965+
def test_feature_flags_local_evaluation_for_cohorts(self, patch_get, patch_decide):
966+
client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
967+
client.feature_flags = [
968+
{
969+
"id": 2,
970+
"name": "Beta Feature",
971+
"key": "beta-feature",
972+
"is_simple_flag": False,
973+
"active": True,
974+
"filters": {
975+
"groups": [
976+
{
977+
"properties": [
978+
{
979+
"key": "region",
980+
"operator": "exact",
981+
"value": ["USA"],
982+
"type": "person",
983+
},
984+
{"key": "id", "value": 98, "operator": None, "type": "cohort"},
985+
],
986+
"rollout_percentage": 100,
987+
}
988+
],
989+
},
990+
},
991+
]
992+
client.cohorts = {
993+
"98": {
994+
"type": "OR",
995+
"values": [
996+
{"key": "id", "value": 1, "type": "cohort"},
997+
{
998+
"key": "nation",
999+
"operator": "exact",
1000+
"value": ["UK"],
1001+
"type": "person",
1002+
},
1003+
],
1004+
},
1005+
"1": {
1006+
"type": "AND",
1007+
"values": [{"key": "other", "operator": "exact", "value": ["thing"], "type": "person"}],
1008+
},
1009+
}
1010+
1011+
feature_flag_match = client.get_feature_flag(
1012+
"beta-feature", "some-distinct-id", person_properties={"region": "UK"}
1013+
)
1014+
1015+
self.assertEqual(feature_flag_match, False)
1016+
self.assertEqual(patch_decide.call_count, 0)
1017+
self.assertEqual(patch_get.call_count, 0)
1018+
1019+
feature_flag_match = client.get_feature_flag(
1020+
"beta-feature", "some-distinct-id", person_properties={"region": "USA", "nation": "UK"}
1021+
)
1022+
# even though 'other' property is not present, the cohort should still match since it's an OR condition
1023+
self.assertEqual(feature_flag_match, True)
1024+
self.assertEqual(patch_decide.call_count, 0)
1025+
self.assertEqual(patch_get.call_count, 0)
1026+
1027+
feature_flag_match = client.get_feature_flag(
1028+
"beta-feature", "some-distinct-id", person_properties={"region": "USA", "other": "thing"}
1029+
)
1030+
self.assertEqual(feature_flag_match, True)
1031+
self.assertEqual(patch_decide.call_count, 0)
1032+
self.assertEqual(patch_get.call_count, 0)
1033+
1034+
@mock.patch("posthog.client.decide")
1035+
@mock.patch("posthog.client.get")
1036+
def test_feature_flags_local_evaluation_for_negated_cohorts(self, patch_get, patch_decide):
1037+
client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
1038+
client.feature_flags = [
1039+
{
1040+
"id": 2,
1041+
"name": "Beta Feature",
1042+
"key": "beta-feature",
1043+
"is_simple_flag": False,
1044+
"active": True,
1045+
"filters": {
1046+
"groups": [
1047+
{
1048+
"properties": [
1049+
{
1050+
"key": "region",
1051+
"operator": "exact",
1052+
"value": ["USA"],
1053+
"type": "person",
1054+
},
1055+
{"key": "id", "value": 98, "operator": None, "type": "cohort"},
1056+
],
1057+
"rollout_percentage": 100,
1058+
}
1059+
],
1060+
},
1061+
},
1062+
]
1063+
client.cohorts = {
1064+
"98": {
1065+
"type": "OR",
1066+
"values": [
1067+
{"key": "id", "value": 1, "type": "cohort"},
1068+
{
1069+
"key": "nation",
1070+
"operator": "exact",
1071+
"value": ["UK"],
1072+
"type": "person",
1073+
},
1074+
],
1075+
},
1076+
"1": {
1077+
"type": "AND",
1078+
"values": [
1079+
{"key": "other", "operator": "exact", "value": ["thing"], "type": "person", "negation": True}
1080+
],
1081+
},
1082+
}
1083+
1084+
feature_flag_match = client.get_feature_flag(
1085+
"beta-feature", "some-distinct-id", person_properties={"region": "UK"}
1086+
)
1087+
1088+
self.assertEqual(feature_flag_match, False)
1089+
self.assertEqual(patch_decide.call_count, 0)
1090+
self.assertEqual(patch_get.call_count, 0)
1091+
1092+
feature_flag_match = client.get_feature_flag(
1093+
"beta-feature", "some-distinct-id", person_properties={"region": "USA", "nation": "UK"}
1094+
)
1095+
# even though 'other' property is not present, the cohort should still match since it's an OR condition
1096+
self.assertEqual(feature_flag_match, True)
1097+
self.assertEqual(patch_decide.call_count, 0)
1098+
self.assertEqual(patch_get.call_count, 0)
1099+
1100+
feature_flag_match = client.get_feature_flag(
1101+
"beta-feature", "some-distinct-id", person_properties={"region": "USA", "other": "thing"}
1102+
)
1103+
# since 'other' is negated, we return False. Since 'nation' is not present, we can't tell whether the flag should be true or false, so go to decide
1104+
self.assertEqual(patch_decide.call_count, 1)
1105+
self.assertEqual(patch_get.call_count, 0)
1106+
1107+
patch_decide.reset_mock()
1108+
1109+
feature_flag_match = client.get_feature_flag(
1110+
"beta-feature", "some-distinct-id", person_properties={"region": "USA", "other": "thing2"}
1111+
)
1112+
self.assertEqual(feature_flag_match, True)
1113+
self.assertEqual(patch_decide.call_count, 0)
1114+
self.assertEqual(patch_get.call_count, 0)
1115+
9631116
@mock.patch("posthog.client.Poller")
9641117
@mock.patch("posthog.client.get")
9651118
def test_load_feature_flags(self, patch_get, patch_poll):

‎posthog/version.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
VERSION = "2.3.1"
1+
VERSION = "2.4.0"
22

33
if __name__ == "__main__":
44
print(VERSION, end="")

0 commit comments

Comments
 (0)