Skip to content

Commit 8f6d6c8

Browse files
fix: include group context in $feature_flag_called dedupe key (#625)
* fix: include group context in $feature_flag_called dedupe key In `_capture_feature_flag_called_if_needed`, the per-distinct_id dedupe key only included the flag key and response. For group-scoped flags, this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. Include the sorted `groups` map in the dedupe key so that the same (distinct_id, flag, response) combination fires once per distinct group context. Repeated calls under the same group context still dedupe, and calls that pass the same map in a different key order still dedupe (the groups are canonicalized via `sorted(groups.items())`). Mirrors the posthog-node fix in PostHog/posthog-js#3658. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * chore: add changeset Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * refactor: use tuple for feature_flag_reported_key and parameterize dedupe tests
1 parent bf63368 commit 8f6d6c8

3 files changed

Lines changed: 89 additions & 6 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
pypi/posthog: patch
3+
---
4+
5+
Include group context in the `$feature_flag_called` dedupe key so group-scoped flags fire a separate event for each group a user is evaluated under, instead of being dedup-ed against the first group context the same `(distinct_id, flag, response)` was seen under.

‎posthog/client.py‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2250,14 +2250,17 @@ def _capture_feature_flag_called_if_needed(
22502250
groups: Optional[Dict[str, str]] = None,
22512251
disable_geoip: Optional[bool] = None,
22522252
) -> None:
2253-
"""Fire a ``$feature_flag_called`` event if the (distinct_id, flag, response)
2254-
triple hasn't already been reported on this client. Shared by the single-flag
2255-
evaluation path and ``FeatureFlagEvaluations.is_enabled() / get_flag()`` so
2256-
both paths dedupe identically.
2253+
"""Fire a ``$feature_flag_called`` event if the (distinct_id, flag, response,
2254+
groups) tuple hasn't already been reported on this client. Group context is
2255+
included so that group-scoped flags fire a separate event for each group a
2256+
user is evaluated under. Shared by the single-flag evaluation path and
2257+
``FeatureFlagEvaluations.is_enabled() / get_flag()`` so both paths dedupe
2258+
identically.
22572259
"""
2258-
feature_flag_reported_key = (
2259-
f"{key}_{'::null::' if response is None else str(response)}"
2260+
groups_key = (
2261+
tuple(sorted((str(k), str(v)) for k, v in groups.items())) if groups else ()
22602262
)
2263+
feature_flag_reported_key = (key, response, groups_key)
22612264

22622265
reported_flags = self.distinct_ids_feature_flags_reported.get(distinct_id)
22632266
if reported_flags is None:

‎posthog/test/test_feature_flags.py‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5271,6 +5271,81 @@ def test_disable_geoip_get_flag_capture_call(self, patch_flags, patch_capture):
52715271
disable_geoip=False,
52725272
)
52735273

5274+
@mock.patch.object(Client, "capture")
5275+
@mock.patch("posthog.client.flags")
5276+
def test_capture_fires_per_group_context(self, patch_flags, patch_capture):
5277+
client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
5278+
client.feature_flags = [
5279+
{
5280+
"id": 1,
5281+
"name": "Group flag",
5282+
"key": "group-scoped-flag",
5283+
"active": True,
5284+
"filters": {
5285+
"groups": [{"properties": [], "rollout_percentage": 100}],
5286+
},
5287+
}
5288+
]
5289+
5290+
# Same user, same flag, same response — but two different group contexts.
5291+
client.get_feature_flag(
5292+
"group-scoped-flag", "user-1", groups={"company": "org-a"}
5293+
)
5294+
client.get_feature_flag(
5295+
"group-scoped-flag", "user-1", groups={"company": "org-b"}
5296+
)
5297+
5298+
flag_called_groups = [
5299+
call.kwargs.get("groups")
5300+
for call in patch_capture.call_args_list
5301+
if call.args and call.args[0] == "$feature_flag_called"
5302+
]
5303+
self.assertEqual(len(flag_called_groups), 2)
5304+
self.assertIn({"company": "org-a"}, flag_called_groups)
5305+
self.assertIn({"company": "org-b"}, flag_called_groups)
5306+
5307+
@parameterized.expand(
5308+
[
5309+
(
5310+
"same_context",
5311+
{"company": "org-a"},
5312+
{"company": "org-a"},
5313+
),
5314+
(
5315+
"different_key_order",
5316+
{"company": "org-a", "team": "red"},
5317+
{"team": "red", "company": "org-a"},
5318+
),
5319+
]
5320+
)
5321+
@mock.patch.object(Client, "capture")
5322+
@mock.patch("posthog.client.flags")
5323+
def test_capture_dedupes_repeated_calls_under_same_group_context(
5324+
self, _name, first_groups, second_groups, patch_flags, patch_capture
5325+
):
5326+
client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
5327+
client.feature_flags = [
5328+
{
5329+
"id": 1,
5330+
"name": "Group flag",
5331+
"key": "group-scoped-flag",
5332+
"active": True,
5333+
"filters": {
5334+
"groups": [{"properties": [], "rollout_percentage": 100}],
5335+
},
5336+
}
5337+
]
5338+
5339+
client.get_feature_flag("group-scoped-flag", "user-1", groups=first_groups)
5340+
client.get_feature_flag("group-scoped-flag", "user-1", groups=second_groups)
5341+
5342+
flag_called_count = sum(
5343+
1
5344+
for call in patch_capture.call_args_list
5345+
if call.args and call.args[0] == "$feature_flag_called"
5346+
)
5347+
self.assertEqual(flag_called_count, 1)
5348+
52745349
@mock.patch.object(Client, "capture")
52755350
@mock.patch("posthog.client.flags")
52765351
def test_capture_multiple_users_doesnt_out_of_memory(

0 commit comments

Comments
 (0)