feat: use the variant as the exposure event value (#222)

Author: gagantrivedi

flagsmith/flagsmith.py

@@ -362,11 +362,11 @@ def get_experiment_flag(
         if not self._event_processor:
             raise ValueError("Events must be enabled to use experiment flags.")
         flag = self.get_identity_flags(identifier, traits).get_flag(feature_name)
-        if not flag.is_default:
+        if isinstance(flag, Flag):
             self.track_exposure_event(
                 feature_name=feature_name,
                 identifier=identifier,
-                value=flag.value,
+                value=flag.variant if flag.variant is not None else flag.value,
                 traits=traits,
             )
         return flag

tests/conftest.py

@@ -23,6 +23,32 @@
 DATA_DIR = os.path.join(os.path.dirname(__file__), "data")
 
 
+@pytest.fixture(autouse=True)
+def stop_flagsmith_background_threads(
+    monkeypatch: pytest.MonkeyPatch,
+) -> Generator[None, None, None]:
+    # Flagsmith starts polling, streaming and event-processor background
+    # threads. Track every instance created during a test and stop its threads
+    # on teardown so they don't leak across the suite (mirroring __del__, but
+    # deterministically rather than at GC).
+    instances: typing.List[Flagsmith] = []
+    original_init = Flagsmith.__init__
+
+    def tracking_init(self: Flagsmith, *args: typing.Any, **kwargs: typing.Any) -> None:
+        instances.append(self)
+        original_init(self, *args, **kwargs)
+
+    monkeypatch.setattr(Flagsmith, "__init__", tracking_init)
+    yield
+    for flagsmith in instances:
+        if getattr(flagsmith, "environment_data_polling_manager_thread", None):
+            flagsmith.environment_data_polling_manager_thread.stop()
+        if getattr(flagsmith, "event_stream_thread", None):
+            flagsmith.event_stream_thread.stop()
+        if flagsmith._event_processor:
+            flagsmith._event_processor.stop()
+
+
 @pytest.fixture()
 def analytics_processor() -> AnalyticsProcessor:
     return AnalyticsProcessor(

tests/test_flagsmith.py

@@ -959,12 +959,8 @@ def test_track_event_raises_without_config(api_key: str) -> None:
 
 def test_track_event_rejects_reserved_prefix(api_key: str) -> None:
     flagsmith = Flagsmith(environment_key=api_key, enable_events=True)
-    try:
-        with pytest.raises(ValueError, match='reserved "\\$" prefix'):
-            flagsmith.track_event("$made_up")
-    finally:
-        if flagsmith._event_processor:
-            flagsmith._event_processor.stop()
+    with pytest.raises(ValueError, match='reserved "\\$" prefix'):
+        flagsmith.track_event("$made_up")
 
 
 def test_event_processor_config_without_enable_events_raises(api_key: str) -> None:
@@ -978,15 +974,11 @@ def test_event_processor_config_without_enable_events_raises(api_key: str) -> No
 
 def test_enable_events_without_config_uses_default(api_key: str) -> None:
     flagsmith = Flagsmith(environment_key=api_key, enable_events=True)
-    try:
-        assert flagsmith._event_processor is not None
-        assert (
-            flagsmith._event_processor._batch_endpoint
-            == "https://events.api.flagsmith.com/v1/events"
-        )
-    finally:
-        if flagsmith._event_processor:
-            flagsmith._event_processor.stop()
+    assert flagsmith._event_processor is not None
+    assert (
+        flagsmith._event_processor._batch_endpoint
+        == "https://events.api.flagsmith.com/v1/events"
+    )
 
 
 def test_track_event_delegates_to_event_processor(
@@ -1116,3 +1108,73 @@ def default_flag_handler(feature_name: str) -> DefaultFlag:
     assert result.is_default is True
     assert result.value == "default-variant"
     mock_track.assert_not_called()
+
+
+def test_get_experiment_flag_uses_variant_as_exposure_value(
+    mocker: MockerFixture, api_key: str
+) -> None:
+    # Given - a resolved flag carrying a variant
+    config = EventProcessorConfig(events_api_url="http://test/")
+    flagsmith = Flagsmith(
+        environment_key=api_key, enable_events=True, event_processor_config=config
+    )
+    flag = Flag(
+        enabled=True,
+        value="blue",
+        feature_name="checkout_v2",
+        feature_id=1,
+        variant="control",
+    )
+    mocker.patch.object(
+        flagsmith,
+        "get_identity_flags",
+        return_value=Flags(flags={"checkout_v2": flag}),
+    )
+    mock_track = mocker.patch.object(flagsmith._event_processor, "track_exposure_event")
+
+    # When
+    flagsmith.get_experiment_flag(feature_name="checkout_v2", identifier="user1")
+
+    # Then - the exposure value is the variant, not the flag value
+    mock_track.assert_called_once_with(
+        feature_name="checkout_v2",
+        identifier="user1",
+        value="control",
+        traits=None,
+        metadata=None,
+    )
+
+
+def test_get_experiment_flag_falls_back_to_value_without_variant(
+    mocker: MockerFixture, api_key: str
+) -> None:
+    # Given - a resolved flag with no variant
+    config = EventProcessorConfig(events_api_url="http://test/")
+    flagsmith = Flagsmith(
+        environment_key=api_key, enable_events=True, event_processor_config=config
+    )
+    flag = Flag(
+        enabled=True,
+        value="blue",
+        feature_name="checkout_v2",
+        feature_id=1,
+        variant=None,
+    )
+    mocker.patch.object(
+        flagsmith,
+        "get_identity_flags",
+        return_value=Flags(flags={"checkout_v2": flag}),
+    )
+    mock_track = mocker.patch.object(flagsmith._event_processor, "track_exposure_event")
+
+    # When
+    flagsmith.get_experiment_flag(feature_name="checkout_v2", identifier="user1")
+
+    # Then - the exposure value falls back to the flag value
+    mock_track.assert_called_once_with(
+        feature_name="checkout_v2",
+        identifier="user1",
+        value="blue",
+        traits=None,
+        metadata=None,
+    )