From 359c93a9b07cd9cd326afdd9225e20652c3c6de1 Mon Sep 17 00:00:00 2001 From: Anders Asheim Hennum Date: Thu, 29 Oct 2026 16:14:45 +0100 Subject: [PATCH] fix: send flag id and version on locally evaluated $feature_flag_called Locally evaluated flags in get_feature_flag(), feature_enabled() and get_feature_flag_result() sent $feature_flag_called without $feature_flag_id or $feature_flag_version, because both were read only from remote /flags metadata. Read them from the local flag definition that produced the value. evaluate_flags() already sent the id for local flags but dropped the definition's version; send it too. --- .../local-flag-called-id-version.md | 5 +++ posthog/client.py | 20 ++++++++-- posthog/test/test_evaluate_flags.py | 32 +++++++++++++++ .../test_feature_flag_called_minimization.py | 1 + posthog/test/test_feature_flag_result.py | 3 ++ posthog/test/test_feature_flags.py | 39 +++++++++++++++++++ 6 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 .sampo/changesets/local-flag-called-id-version.md diff --git a/.sampo/changesets/local-flag-called-id-version.md b/.sampo/changesets/local-flag-called-id-version.md new file mode 100644 index 000000000..18e818423 --- /dev/null +++ b/.sampo/changesets/local-flag-called-id-version.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +Add `$feature_flag_id` and `$feature_flag_version` to `$feature_flag_called` events for locally evaluated flags in `get_feature_flag()`, `feature_enabled()` and `get_feature_flag_result()`, and `$feature_flag_version` for locally evaluated flags in `evaluate_flags()`, taken from the flag definition. Remotely evaluated flags already sent both. diff --git a/posthog/client.py b/posthog/client.py index 3b918b0ba..079c51645 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -4149,12 +4149,18 @@ def _get_feature_flag_result( # Source the gate the same way as has_experiment above; see # _capture_feature_flag_called_if_needed for why. minimal_flag_called_events = self._minimal_flag_called_events + # Remotely-evaluated flags carry id and version in the response metadata; + # locally-evaluated flags read them from the definition that produced the value. + local_flag_id: Optional[int] = None + local_flag_version: Optional[int] = None if flag_was_locally_evaluated: - local_def = (self.feature_flags_by_key or {}).get(key) + local_def = snapshot["flags_by_key"].get(key) if isinstance(local_def, dict): has_experiment = _parse_has_experiment( local_def.get("has_experiment") ) + local_flag_id = local_def.get("id") + local_flag_version = local_def.get("version") elif isinstance(flag_details, FeatureFlag): has_experiment = _metadata_has_experiment(flag_details.metadata) minimal_flag_called_events = remote_minimal_flag_called_events @@ -4173,6 +4179,8 @@ def _get_feature_flag_result( feature_flag_error, has_experiment, minimal_flag_called_events, + local_flag_id=local_flag_id, + local_flag_version=local_flag_version, ) return flag_result @@ -4462,6 +4470,8 @@ def _capture_feature_flag_called( feature_flag_error: Optional[str] = None, has_experiment: Optional[bool] = None, minimal_flag_called_events: bool = False, + local_flag_id: Optional[int] = None, + local_flag_version: Optional[int] = None, ): properties: dict[str, Any] = { "$feature_flag": key, @@ -4486,6 +4496,10 @@ def _capture_feature_flag_called( properties["$feature_flag_version"] = flag_details.metadata.version if flag_details.metadata.id: properties["$feature_flag_id"] = flag_details.metadata.id + if local_flag_version: + properties["$feature_flag_version"] = local_flag_version + if local_flag_id: + properties["$feature_flag_id"] = local_flag_id if feature_flag_error: properties["$feature_flag_error"] = feature_flag_error @@ -4921,9 +4935,7 @@ def evaluate_flags( variant=value if isinstance(value, str) else None, payload=_parse_flag_payload(local_payloads.get(key)), id=flag_def.get("id"), - # The local-evaluation flag definition does not carry a version field; - # only the remote ``/flags`` response does via ``metadata.version``. - version=None, + version=flag_def.get("version"), reason="Evaluated locally", locally_evaluated=True, has_experiment=_parse_has_experiment(flag_def.get("has_experiment")), diff --git a/posthog/test/test_evaluate_flags.py b/posthog/test/test_evaluate_flags.py index 25ca18e51..ef12f0afd 100644 --- a/posthog/test/test_evaluate_flags.py +++ b/posthog/test/test_evaluate_flags.py @@ -431,6 +431,38 @@ def test_flag_called_event_carries_parsed_local_payload( self.assertEqual(properties["$feature_flag_payload"], {"copy": "new"}) +class TestEvaluateFlagsLocalFlagIdentity(unittest.TestCase): + """Locally-evaluated flags report the definition's id and version, the same + properties remote ``/flags`` metadata supplies.""" + + def setUp(self): + self.client = Client(FAKE_TEST_API_KEY, secret_key="test") + self.client.feature_flags = [ + _always_on_flag("local-flag", id=4211, version=7), + _always_on_flag("unversioned-flag", id=4212), + ] + + def tearDown(self): + self.client.shutdown() + + @mock.patch("posthog.client.flags") + @mock.patch.object(Client, "capture") + def test_flag_called_event_carries_definition_id_and_version( + self, patch_capture, patch_flags + ): + flags = self.client.evaluate_flags("user-1") + flags.is_enabled("local-flag") + flags.get_flag("unversioned-flag") + + patch_flags.assert_not_called() + properties = _flag_called_properties(patch_capture) + self.assertTrue(properties["local-flag"]["locally_evaluated"]) + self.assertEqual(properties["local-flag"]["$feature_flag_id"], 4211) + self.assertEqual(properties["local-flag"]["$feature_flag_version"], 7) + self.assertEqual(properties["unversioned-flag"]["$feature_flag_id"], 4212) + self.assertNotIn("$feature_flag_version", properties["unversioned-flag"]) + + class TestEvaluateFlagsMissingLocalDefinition(unittest.TestCase): def setUp(self): self.client = Client(FAKE_TEST_API_KEY, secret_key="test") diff --git a/posthog/test/test_feature_flag_called_minimization.py b/posthog/test/test_feature_flag_called_minimization.py index 32310beca..9a23e6b17 100644 --- a/posthog/test/test_feature_flag_called_minimization.py +++ b/posthog/test/test_feature_flag_called_minimization.py @@ -297,6 +297,7 @@ def test_gated_non_experiment_flag_sends_exactly_the_allowlist( { "$feature_flag", "$feature_flag_response", + "$feature_flag_id", "$feature_flag_has_experiment", "locally_evaluated", "$lib", diff --git a/posthog/test/test_feature_flag_result.py b/posthog/test/test_feature_flag_result.py index fcfdc968c..5b57ebbd4 100644 --- a/posthog/test/test_feature_flag_result.py +++ b/posthog/test/test_feature_flag_result.py @@ -242,6 +242,7 @@ def test_get_feature_flag_result_boolean_local_evaluation(self, patch_capture): "locally_evaluated": True, "$feature/person-flag": True, "$feature_flag_payload": 300, + "$feature_flag_id": 1, }, groups={}, disable_geoip=None, @@ -299,6 +300,7 @@ def test_get_feature_flag_result_variant_local_evaluation(self, patch_capture): "locally_evaluated": True, "$feature/person-flag": "variant-1", "$feature_flag_payload": {"some": "value"}, + "$feature_flag_id": 1, }, groups={}, disable_geoip=None, @@ -323,6 +325,7 @@ def test_get_feature_flag_result_variant_local_evaluation(self, patch_capture): "$feature_flag_response": "variant-2", "locally_evaluated": True, "$feature/person-flag": "variant-2", + "$feature_flag_id": 1, }, groups={}, disable_geoip=None, diff --git a/posthog/test/test_feature_flags.py b/posthog/test/test_feature_flags.py index 14a1ed18e..b52ec3bfa 100644 --- a/posthog/test/test_feature_flags.py +++ b/posthog/test/test_feature_flags.py @@ -7181,6 +7181,41 @@ def test_year_parsing(self): class TestCaptureCalls(unittest.TestCase): + @parameterized.expand( + [ + ("get_feature_flag", lambda c: c.get_feature_flag), + ("feature_enabled", lambda c: c.feature_enabled), + ("get_feature_flag_result", lambda c: c.get_feature_flag_result), + ] + ) + @mock.patch.object(Client, "capture") + @mock.patch("posthog.client.flags") + def test_locally_evaluated_flag_called_event_carries_definition_id_and_version( + self, _name, getter, patch_flags, patch_capture + ): + client = Client(FAKE_TEST_API_KEY, secret_key=FAKE_TEST_API_KEY) + client.feature_flags = [ + { + "id": 4211, + "version": 7, + "name": "Local Feature", + "key": "local-flag", + "active": True, + "filters": { + "groups": [{"properties": [], "rollout_percentage": 100}], + }, + } + ] + + self.assertTrue(getter(client)("local-flag", "some-distinct-id")) + + patch_flags.assert_not_called() + patch_capture.assert_called_once() + properties = patch_capture.call_args[1]["properties"] + self.assertTrue(properties["locally_evaluated"]) + self.assertEqual(properties["$feature_flag_id"], 4211) + self.assertEqual(properties["$feature_flag_version"], 7) + @mock.patch.object(Client, "capture") @mock.patch("posthog.client.flags") def test_capture_is_called(self, patch_flags, patch_capture): @@ -7219,6 +7254,7 @@ def test_capture_is_called(self, patch_flags, patch_capture): "$feature_flag_response": True, "locally_evaluated": True, "$feature/complex-flag": True, + "$feature_flag_id": 1, }, groups={}, disable_geoip=None, @@ -7253,6 +7289,7 @@ def test_capture_is_called(self, patch_flags, patch_capture): "$feature_flag_response": True, "locally_evaluated": True, "$feature/complex-flag": True, + "$feature_flag_id": 1, }, groups={}, disable_geoip=None, @@ -7591,6 +7628,7 @@ def test_disable_geoip_get_flag_capture_call(self, patch_flags, patch_capture): "$feature_flag_response": True, "locally_evaluated": True, "$feature/complex-flag": True, + "$feature_flag_id": 1, }, groups={}, disable_geoip=False, @@ -7713,6 +7751,7 @@ def test_capture_multiple_users_doesnt_out_of_memory( "$feature_flag_response": True, "locally_evaluated": True, "$feature/complex-flag": True, + "$feature_flag_id": 1, }, groups={}, disable_geoip=None,