From 3292203049f232da2570db3c29da4e64d968fc82 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 3 Aug 2026 17:42:48 +0200 Subject: [PATCH 01/16] build: require flagsmith >=5.5 for the experimentation API Signed-off-by: wadii --- poetry.lock | 18 +++++++++--------- pyproject.toml | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/poetry.lock b/poetry.lock index 7c276dd..753b30c 100644 --- a/poetry.lock +++ b/poetry.lock @@ -184,18 +184,18 @@ typing = ["typing-extensions (>=4.8) ; python_version < \"3.11\""] [[package]] name = "flagsmith" -version = "5.2.0" +version = "6.1.0" description = "Flagsmith Python SDK" optional = false -python-versions = "<4,>=3.9" +python-versions = "<4,>=3.10" groups = ["main"] files = [ - {file = "flagsmith-5.2.0-py3-none-any.whl", hash = "sha256:07114d9ccaa1206d13a995bcd99a96ea4c4b7bda8c731b1d023ca233189879cc"}, - {file = "flagsmith-5.2.0.tar.gz", hash = "sha256:734d6ea733586fed2d96714203f8fb4f997e0039f3a4966f02c51574a7786d68"}, + {file = "flagsmith-6.1.0-py3-none-any.whl", hash = "sha256:c1adf56cd5cabcaf3fdd1b0dad8d3fc6e621a96bc2703b88c98ed9da91ead792"}, + {file = "flagsmith-6.1.0.tar.gz", hash = "sha256:f62e74f3aa2220702a60edc76e1eb7ebca46a2967da938455efa4d1642364f46"}, ] [package.dependencies] -flagsmith-flag-engine = ">=10.0.3,<11.0.0" +flagsmith-flag-engine = ">=10.2.0,<11.0.0" iso8601 = {version = ">=2.1.0,<3.0.0", markers = "python_version < \"3.11\""} requests = ">=2.32.3,<3.0.0" requests-futures = ">=1.0.1,<2.0.0" @@ -204,14 +204,14 @@ typing-extensions = ">=4.15.0,<5.0.0" [[package]] name = "flagsmith-flag-engine" -version = "10.0.3" +version = "10.2.0" description = "Flag engine for the Flagsmith API." optional = false python-versions = "*" groups = ["main"] files = [ - {file = "flagsmith_flag_engine-10.0.3-py3-none-any.whl", hash = "sha256:aed9009377fc1a6322483277f971f06d542668a69d93cbe4a3efd4baae78dfc1"}, - {file = "flagsmith_flag_engine-10.0.3.tar.gz", hash = "sha256:0aa449bb87bee54fc67b5c7ca25eca78246a7bbb5a6cc229260c3f262d58ac54"}, + {file = "flagsmith_flag_engine-10.2.0-py3-none-any.whl", hash = "sha256:c9bed3ee15487057dc61144d34d101d98db255f17d2c739f02794841a5c98502"}, + {file = "flagsmith_flag_engine-10.2.0.tar.gz", hash = "sha256:d935c9fb639e8acc5b9ff4599ec570e1b2f3f7b7874fc789a6eca3db5665a31b"}, ] [package.dependencies] @@ -802,4 +802,4 @@ test = ["covdefaults (>=2.3)", "coverage (>=7.2.7)", "coverage-enable-subprocess [metadata] lock-version = "2.1" python-versions = ">=3.10,<4.0" -content-hash = "59e0a22ab4299d665eebfd14d8a178a90c158c61a3b019194661a9f19d071942" +content-hash = "85101a6d38fa1b56f9a451a8340cb6cbc1e988f136545b472aef72fdae1ea359" diff --git a/pyproject.toml b/pyproject.toml index b68a085..a2c6355 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ authors = [ readme = "README.md" requires-python = ">=3.10,<4.0" dependencies = [ - "flagsmith (>=5.2.0,<7.0.0)", + "flagsmith (>=5.5.0,<7.0.0)", "openfeature-sdk (>=0.9.0,<0.10.0)", ] From dc991ae55a2ff19930a467e1bd748601490bb526 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 3 Aug 2026 17:44:40 +0200 Subject: [PATCH 02/16] feat: populate reason, variant and flag_metadata on resolutions Signed-off-by: wadii --- openfeature_flagsmith/provider.py | 56 ++++++++++- tests/test_provider.py | 152 ++++++++++++++++++++++++++++-- 2 files changed, 198 insertions(+), 10 deletions(-) diff --git a/openfeature_flagsmith/provider.py b/openfeature_flagsmith/provider.py index 9e075ae..f64c470 100644 --- a/openfeature_flagsmith/provider.py +++ b/openfeature_flagsmith/provider.py @@ -4,6 +4,7 @@ from flagsmith.exceptions import FlagsmithClientError from flagsmith.flagsmith import Flagsmith +from flagsmith.models import Flag from openfeature.evaluation_context import EvaluationContext from openfeature.exception import ( ErrorCode, @@ -11,7 +12,7 @@ ParseError, TypeMismatchError, ) -from openfeature.flag_evaluation import FlagResolutionDetails, FlagType +from openfeature.flag_evaluation import FlagResolutionDetails, FlagType, Reason from openfeature.provider import AbstractProvider, Metadata from openfeature.track import TrackingEventDetails @@ -164,7 +165,7 @@ def _resolve( raise FlagNotFoundError(error_message="Flag '%s' was not found." % flag_key) if flag_type == FlagType.BOOLEAN and not self.use_boolean_config_value: - return FlagResolutionDetails(value=flag.enabled) + return self._build_details(flag, flag.enabled, evaluation_context) if not (self.return_value_for_disabled_flags or flag.enabled): raise FlagsmithProviderError( @@ -174,10 +175,12 @@ def _resolve( required_type = _BASIC_FLAG_TYPE_MAPPINGS.get(flag_type) if required_type and isinstance(flag.value, required_type): - return FlagResolutionDetails(value=flag.value) + return self._build_details(flag, flag.value, evaluation_context) elif flag_type is FlagType.OBJECT and isinstance(flag.value, str): try: - return FlagResolutionDetails(value=json.loads(flag.value)) + return self._build_details( + flag, json.loads(flag.value), evaluation_context + ) except JSONDecodeError as e: msg = "Unable to parse object from value for flag '%s'" % flag_key raise ParseError(error_message=msg) from e @@ -187,6 +190,51 @@ def _resolve( % (flag_key, flag_type.value) ) + def _build_details( + self, + flag: typing.Any, + value: typing.Any, + evaluation_context: EvaluationContext, + ) -> FlagResolutionDetails: + return FlagResolutionDetails( + value=value, + reason=self._parse_reason(flag, evaluation_context), + # DefaultFlag has no `variant` attribute; never use bare access. + variant=getattr(flag, "variant", None), + flag_metadata=self._build_flag_metadata(flag), + ) + + def _parse_reason( + self, flag: typing.Any, evaluation_context: EvaluationContext + ) -> Reason: + if flag.is_default: + return Reason.DEFAULT + if not flag.enabled: + return Reason.DISABLED + # Offline documents may be arbitrarily old; the exposure hook treats + # anything but TARGETING_MATCH as not fresh enough to record. + if getattr(self._client, "offline_mode", False): + return Reason.STALE + if evaluation_context.targeting_key: + return Reason.TARGETING_MATCH + return Reason.STATIC + + def _build_flag_metadata( + self, flag: typing.Any + ) -> typing.Dict[str, typing.Union[bool, int, str]]: + # Keys are byte-identical with the JS provider (vendor-council aligned). + metadata: typing.Dict[str, typing.Union[bool, int, str]] = { + "enabled": flag.enabled + } + if isinstance(flag, Flag): + metadata["featureId"] = flag.feature_id + variant = getattr(flag, "variant", None) + if variant is not None: + metadata["experiment.arm"] = variant + metadata["experiment.active"] = flag.enabled + metadata["experiment.unit"] = "user" + return metadata + @staticmethod def _extract_traits( evaluation_context: typing.Optional[EvaluationContext], diff --git a/tests/test_provider.py b/tests/test_provider.py index 1da313a..030e1ca 100644 --- a/tests/test_provider.py +++ b/tests/test_provider.py @@ -11,6 +11,7 @@ ParseError, FlagNotFoundError, ) +from openfeature.flag_evaluation import Reason from openfeature.track import TrackingEventDetails from openfeature_flagsmith.exceptions import FlagsmithProviderError @@ -210,7 +211,7 @@ def test_resolve_string_details_when_not_enabled_and_return_value_for_disabled_f # Then assert result.value == value - assert result.reason is None + assert result.reason == Reason.DISABLED assert result.error_code is None @@ -258,7 +259,7 @@ def test_resolve_string_details_for_flagsmith_default_flag_when_use_flagsmith_de # Then assert result.value == value - assert result.reason is None + assert result.reason == Reason.DEFAULT assert result.error_code is None @@ -313,7 +314,7 @@ def test_identity_flags_are_used_if_targeting_key_provided( # Then assert result.value == value assert result.error_code is None - assert result.reason is None + assert result.reason == Reason.TARGETING_MATCH mock_flagsmith_client.get_identity_flags.assert_called_once_with( identifier=targeting_key, traits=traits @@ -349,7 +350,7 @@ def test_identity_flags_are_used_with_flat_attributes( # Then assert result.value == value assert result.error_code is None - assert result.reason is None + assert result.reason == Reason.TARGETING_MATCH mock_flagsmith_client.get_identity_flags.assert_called_once_with( identifier=targeting_key, traits=traits @@ -388,7 +389,7 @@ def test_identity_flags_flat_attributes_and_nested_traits_are_merged( # Then assert result.value == value assert result.error_code is None - assert result.reason is None + assert result.reason == Reason.TARGETING_MATCH mock_flagsmith_client.get_identity_flags.assert_called_once_with( identifier=targeting_key, @@ -450,7 +451,7 @@ def test_resolve_boolean_details_uses_enabled_when_use_boolean_config_value_is_f # Then assert result.value is True assert result.error_code is None - assert result.reason is None + assert result.reason == Reason.STATIC # --------------------------------------------------------------------------- @@ -591,3 +592,142 @@ def test_track_extracts_traits_from_context(mock_flagsmith_client: MagicMock) -> traits={"shared_key": "nested_value", "other": "kept"}, metadata=None, ) + + +# --------------------------------------------------------------------------- +# Reasons / variant / flag_metadata +# --------------------------------------------------------------------------- + + +def test_resolve_environment_flag_has_static_reason_and_metadata( + mock_flagsmith_client: MagicMock, +) -> None: + # Given + key = "my_feature" + mock_flagsmith_client.get_environment_flags.return_value = Flags( + {key: Flag(feature_id=42, feature_name=key, enabled=True, value="foo")} + ) + provider = FlagsmithProvider(mock_flagsmith_client) + + # When + result = provider.resolve_string_details(key, default_value="default") + + # Then + assert result.reason == Reason.STATIC + assert result.variant is None + assert result.flag_metadata == {"enabled": True, "featureId": 42} + + +def test_resolve_identity_flag_with_variant_has_experiment_metadata( + mock_flagsmith_client: MagicMock, +) -> None: + # Given + key = "my_experiment" + mock_flagsmith_client.get_identity_flags.return_value = Flags( + { + key: Flag( + feature_id=7, + feature_name=key, + enabled=True, + value="treatment-value", + variant="treatment", + ) + } + ) + provider = FlagsmithProvider(mock_flagsmith_client) + + # When + result = provider.resolve_string_details( + key, + default_value="control", + evaluation_context=EvaluationContext(targeting_key="user-1"), + ) + + # Then + assert result.reason == Reason.TARGETING_MATCH + assert result.variant == "treatment" + assert result.flag_metadata == { + "enabled": True, + "featureId": 7, + "experiment.arm": "treatment", + "experiment.active": True, + "experiment.unit": "user", + } + + +def test_resolve_boolean_details_disabled_flag_has_disabled_reason( + mock_flagsmith_client: MagicMock, +) -> None: + # Given - the boolean-as-enabled path resolves disabled flags today + key = "my_feature" + mock_flagsmith_client.get_environment_flags.return_value = Flags( + {key: Flag(feature_id=1, feature_name=key, enabled=False, value=None)} + ) + provider = FlagsmithProvider(mock_flagsmith_client) + + # When + result = provider.resolve_boolean_details(key, default_value=True) + + # Then + assert result.value is False + assert result.reason == Reason.DISABLED + + +def test_resolve_flagsmith_default_flag_metadata_has_no_feature_id( + mock_flagsmith_client: MagicMock, +) -> None: + # Given - DefaultFlag has no feature_id and no variant attribute + key = "my_feature" + mock_flagsmith_client.get_environment_flags.return_value = Flags( + {key: DefaultFlag(enabled=True, value="foo")} + ) + provider = FlagsmithProvider(mock_flagsmith_client, use_flagsmith_defaults=True) + + # When + result = provider.resolve_string_details(key, default_value="default") + + # Then + assert result.reason == Reason.DEFAULT + assert result.variant is None + assert result.flag_metadata == {"enabled": True} + + +def test_resolve_in_offline_mode_has_stale_reason( + mock_flagsmith_client: MagicMock, +) -> None: + # Given + key = "my_feature" + mock_flagsmith_client.offline_mode = True + mock_flagsmith_client.get_identity_flags.return_value = Flags( + {key: Flag(feature_id=1, feature_name=key, enabled=True, value="foo")} + ) + provider = FlagsmithProvider(mock_flagsmith_client) + + # When + result = provider.resolve_string_details( + key, + default_value="default", + evaluation_context=EvaluationContext(targeting_key="user-1"), + ) + + # Then - offline data must not read as a fresh targeting match + assert result.reason == Reason.STALE + + +def test_resolve_object_details_parsed_json_carries_reason_and_metadata( + mock_flagsmith_client: MagicMock, +) -> None: + # Given - the JSON-parse branch is the third resolving branch + key = "my_feature" + mock_flagsmith_client.get_environment_flags.return_value = Flags( + {key: Flag(feature_id=3, feature_name=key, enabled=True, value='{"a": 1}')} + ) + provider = FlagsmithProvider(mock_flagsmith_client) + + # When + result = provider.resolve_object_details(key, default_value={}) + + # Then + assert result.value == {"a": 1} + assert result.reason == Reason.STATIC + assert result.flag_metadata == {"enabled": True, "featureId": 3} From 521d98ccc18f2b5c78cd740b7ff47ff757c1a6d2 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 3 Aug 2026 17:45:42 +0200 Subject: [PATCH 03/16] feat: support transient identities via the evaluation context Signed-off-by: wadii --- openfeature_flagsmith/provider.py | 19 +++++++++- tests/test_provider.py | 63 ++++++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/openfeature_flagsmith/provider.py b/openfeature_flagsmith/provider.py index f64c470..27e8604 100644 --- a/openfeature_flagsmith/provider.py +++ b/openfeature_flagsmith/provider.py @@ -242,14 +242,31 @@ def _extract_traits( if not evaluation_context or not evaluation_context.attributes: return None nested = evaluation_context.attributes.get("traits", {}) - flat = {k: v for k, v in evaluation_context.attributes.items() if k != "traits"} + # `traits` is unpacked below; the flat `transient` key is an + # evaluation directive (see _is_transient), not a trait. + flat = { + k: v + for k, v in evaluation_context.attributes.items() + if k not in ("traits", "transient") + } merged = {**flat, **nested} return merged or None + @staticmethod + def _is_transient( + evaluation_context: typing.Optional[EvaluationContext], + ) -> bool: + return bool( + evaluation_context + and evaluation_context.attributes + and evaluation_context.attributes.get("transient") is True + ) + def _get_flags(self, evaluation_context: EvaluationContext = EvaluationContext()): if targeting_key := evaluation_context.targeting_key: return self._client.get_identity_flags( identifier=targeting_key, traits=self._extract_traits(evaluation_context) or {}, + transient=self._is_transient(evaluation_context), ) return self._client.get_environment_flags() diff --git a/tests/test_provider.py b/tests/test_provider.py index 030e1ca..155681f 100644 --- a/tests/test_provider.py +++ b/tests/test_provider.py @@ -317,7 +317,7 @@ def test_identity_flags_are_used_if_targeting_key_provided( assert result.reason == Reason.TARGETING_MATCH mock_flagsmith_client.get_identity_flags.assert_called_once_with( - identifier=targeting_key, traits=traits + identifier=targeting_key, traits=traits, transient=False ) @@ -353,7 +353,7 @@ def test_identity_flags_are_used_with_flat_attributes( assert result.reason == Reason.TARGETING_MATCH mock_flagsmith_client.get_identity_flags.assert_called_once_with( - identifier=targeting_key, traits=traits + identifier=targeting_key, traits=traits, transient=False ) @@ -394,6 +394,7 @@ def test_identity_flags_flat_attributes_and_nested_traits_are_merged( mock_flagsmith_client.get_identity_flags.assert_called_once_with( identifier=targeting_key, traits={"flat_trait": "flat_value", "nested_trait": "nested_value"}, + transient=False, ) @@ -430,6 +431,7 @@ def test_identity_flags_nested_traits_take_precedence_over_flat_attributes( mock_flagsmith_client.get_identity_flags.assert_called_once_with( identifier=targeting_key, traits={"shared_key": "nested_value"}, + transient=False, ) @@ -731,3 +733,60 @@ def test_resolve_object_details_parsed_json_carries_reason_and_metadata( assert result.value == {"a": 1} assert result.reason == Reason.STATIC assert result.flag_metadata == {"enabled": True, "featureId": 3} + + +# --------------------------------------------------------------------------- +# Transient identities +# --------------------------------------------------------------------------- + + +def test_transient_attribute_maps_to_transient_identity( + mock_flagsmith_client: MagicMock, +) -> None: + # Given + key = "key" + mock_flagsmith_client.get_identity_flags.return_value = Flags( + {key: Flag(feature_id=1, feature_name=key, enabled=True, value="foo")} + ) + provider = FlagsmithProvider(mock_flagsmith_client) + + # When + provider.resolve_string_details( + flag_key=key, + default_value="default", + evaluation_context=EvaluationContext( + targeting_key="user-1", + attributes={"transient": True, "plan": "pro"}, + ), + ) + + # Then - transient is a directive, not a trait + mock_flagsmith_client.get_identity_flags.assert_called_once_with( + identifier="user-1", traits={"plan": "pro"}, transient=True + ) + + +def test_nested_trait_named_transient_is_kept( + mock_flagsmith_client: MagicMock, +) -> None: + # Given - only the flat `transient` key is a directive + key = "key" + mock_flagsmith_client.get_identity_flags.return_value = Flags( + {key: Flag(feature_id=1, feature_name=key, enabled=True, value="foo")} + ) + provider = FlagsmithProvider(mock_flagsmith_client) + + # When + provider.resolve_string_details( + flag_key=key, + default_value="default", + evaluation_context=EvaluationContext( + targeting_key="user-1", + attributes={"traits": {"transient": "a-real-trait"}}, + ), + ) + + # Then + mock_flagsmith_client.get_identity_flags.assert_called_once_with( + identifier="user-1", traits={"transient": "a-real-trait"}, transient=False + ) From b39a66ceb7189afd58cb93f162ebf69df034a63b Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 3 Aug 2026 17:47:50 +0200 Subject: [PATCH 04/16] fix!: track() crashed on flagsmith >=5.4 and hid value in metadata Signed-off-by: wadii --- openfeature_flagsmith/provider.py | 107 ++++++++++++++------- openfeature_flagsmith/tracking.py | 13 +++ tests/test_provider.py | 148 +++++++++++++++++++----------- 3 files changed, 185 insertions(+), 83 deletions(-) create mode 100644 openfeature_flagsmith/tracking.py diff --git a/openfeature_flagsmith/provider.py b/openfeature_flagsmith/provider.py index 27e8604..963c4ac 100644 --- a/openfeature_flagsmith/provider.py +++ b/openfeature_flagsmith/provider.py @@ -1,4 +1,5 @@ import json +import logging import typing from json import JSONDecodeError @@ -17,6 +18,9 @@ from openfeature.track import TrackingEventDetails from openfeature_flagsmith.exceptions import FlagsmithProviderError +from openfeature_flagsmith.tracking import EXPOSURE_TRACKING_EVENT + +logger = logging.getLogger(__name__) _BASIC_FLAG_TYPE_MAPPINGS = { FlagType.BOOLEAN: bool, @@ -26,17 +30,6 @@ } -class TrackingMetadata(typing.TypedDict, total=False): - """ - Shape of the metadata dict forwarded to ``Flagsmith.track_event``. - - ``value`` holds the numeric value from ``TrackingEventDetails.value`` when - set. All other keys pass through from ``TrackingEventDetails.attributes``. - """ - - value: float - - class FlagsmithProvider(AbstractProvider): def __init__( self, @@ -57,41 +50,93 @@ def track( tracking_event_details: typing.Optional[TrackingEventDetails] = None, ) -> None: """ - Records a custom event via the Flagsmith client's pipeline analytics. + Route OpenFeature tracking events to Flagsmith. - No-ops if the client lacks pipeline analytics support or configuration. - An explicit ``tracking_event_details.value`` overrides any same-named - key in ``attributes``. + ``EXPOSURE_TRACKING_EVENT`` records a flag/variant exposure; any other + name becomes a plain Flagsmith event with ``details.value`` first-class. + No-ops unless the client was initialized with ``enable_events``. Never + raises (OpenFeature spec section 6): unexpected errors are logged. """ - # Guard against older flagsmith versions or duck-typed clients - # that don't have track_event. - if not hasattr(self._client, "track_event"): + try: + self._track(tracking_event_name, evaluation_context, tracking_event_details) + except Exception: + logger.warning( + 'Failed to process tracking event "%s".', + tracking_event_name, + exc_info=True, + ) + + def _track( + self, + tracking_event_name: str, + evaluation_context: typing.Optional[EvaluationContext], + tracking_event_details: typing.Optional[TrackingEventDetails], + ) -> None: + # Private-attribute pragmatism: the SDK has no public events-enabled + # signal yet. Checked up front so disabled events never trigger + # network side effects (identity persistence, flag fetches). + if getattr(self._client, "_event_processor", None) is None: + logger.debug( + 'Flagsmith events are disabled; dropping tracking event "%s".', + tracking_event_name, + ) return identifier = evaluation_context.targeting_key if evaluation_context else None traits = self._extract_traits(evaluation_context) - metadata: typing.Optional[TrackingMetadata] = None - if tracking_event_details is not None: - metadata = typing.cast( - TrackingMetadata, dict(tracking_event_details.attributes) + if tracking_event_name == EXPOSURE_TRACKING_EVENT: + self._track_exposure( + identifier, traits, evaluation_context, tracking_event_details ) - if tracking_event_details.value is not None: - metadata["value"] = tracking_event_details.value - if not metadata: - metadata = None + return + + if tracking_event_name.startswith("$"): + logger.warning( + '"%s" is a reserved Flagsmith event name; use "%s" to record' + " exposures.", + tracking_event_name, + EXPOSURE_TRACKING_EVENT, + ) + return + + value = tracking_event_details.value if tracking_event_details else None + attributes = ( + dict(tracking_event_details.attributes) if tracking_event_details else {} + ) + if value is not None and not isinstance(value, (int, float)): + logger.warning( + 'Tracking event "%s" details.value must be numeric;' + " sending without it.", + tracking_event_name, + ) + value = None try: self._client.track_event( tracking_event_name, - identity_identifier=identifier, + identifier=identifier, + value=value, traits=traits, - metadata=metadata, + metadata=attributes or None, ) except ValueError: - # Flagsmith raises ValueError when pipeline analytics is not - # configured; OpenFeature spec requires track() to no-op. - return + # Raised when events are disabled (racing the check above) or the + # SDK rejects the event name. + logger.debug( + 'Flagsmith rejected tracking event "%s"; dropping it.', + tracking_event_name, + exc_info=True, + ) + + def _track_exposure( + self, + identifier: typing.Optional[str], + traits: typing.Optional[typing.Dict[str, typing.Any]], + evaluation_context: typing.Optional[EvaluationContext], + tracking_event_details: typing.Optional[TrackingEventDetails], + ) -> None: + raise NotImplementedError # implemented in the exposure-routing task def get_metadata(self) -> Metadata: return Metadata(name="FlagsmithProvider") diff --git a/openfeature_flagsmith/tracking.py b/openfeature_flagsmith/tracking.py new file mode 100644 index 0000000..717784c --- /dev/null +++ b/openfeature_flagsmith/tracking.py @@ -0,0 +1,13 @@ +import typing + +EXPOSURE_TRACKING_EVENT: typing.Final[str] = "feature_flag.exposure" +""" +Reserved tracking-event name for recording flag/variant exposures. + +``client.track(EXPOSURE_TRACKING_EVENT, context, details)`` routes to +Flagsmith's exposure tracking instead of a plain analytics event. This is the +OpenFeature-facing name (identical across Flagsmith OpenFeature providers); on +the wire the Flagsmith SDK emits the ``$flag_exposure`` system event. + +Tracking is an experimental OpenFeature capability (spec section 6). +""" diff --git a/tests/test_provider.py b/tests/test_provider.py index 155681f..a5ff5ea 100644 --- a/tests/test_provider.py +++ b/tests/test_provider.py @@ -1,4 +1,4 @@ -from unittest.mock import MagicMock +from unittest.mock import MagicMock, create_autospec import pytest from flagsmith import Flagsmith @@ -19,8 +19,17 @@ @pytest.fixture() -def mock_flagsmith_client() -> MagicMock(): - return MagicMock(spec=Flagsmith) +def mock_flagsmith_client() -> MagicMock: + # create_autospec validates call signatures; the loose MagicMock(spec=...) + # it replaces let track_event(identity_identifier=...) pass silently. + return create_autospec(Flagsmith, instance=True) + + +@pytest.fixture() +def tracking_flagsmith_client(mock_flagsmith_client: MagicMock) -> MagicMock: + # The provider treats a client without _event_processor as events-disabled. + mock_flagsmith_client._event_processor = MagicMock() + return mock_flagsmith_client def test_get_metadata(mock_flagsmith_client: MagicMock) -> None: @@ -457,36 +466,48 @@ def test_resolve_boolean_details_uses_enabled_when_use_boolean_config_value_is_f # --------------------------------------------------------------------------- -# Tracking +# Tracking: custom events # --------------------------------------------------------------------------- -def test_track_is_noop_without_track_event_on_client() -> None: - # Given - client without track_event (e.g. older flagsmith version) - client = MagicMock(spec=[]) - provider = FlagsmithProvider(client) +def test_track_is_noop_when_events_disabled( + mock_flagsmith_client: MagicMock, +) -> None: + # Given - no _event_processor on the client (events not enabled) + provider = FlagsmithProvider(mock_flagsmith_client) + + # When + provider.track("purchase") + + # Then - dropped before any SDK call + mock_flagsmith_client.track_event.assert_not_called() + + +def test_track_swallows_value_error_from_sdk( + tracking_flagsmith_client: MagicMock, +) -> None: + # Given + tracking_flagsmith_client.track_event.side_effect = ValueError("events disabled") + provider = FlagsmithProvider(tracking_flagsmith_client) # When / Then - no error raised provider.track("purchase") -def test_track_is_noop_when_pipeline_analytics_not_configured( - mock_flagsmith_client: MagicMock, +def test_track_swallows_unexpected_exceptions( + tracking_flagsmith_client: MagicMock, ) -> None: - # Given - client has track_event but raises ValueError (no analytics config) - mock_flagsmith_client.track_event = MagicMock( - side_effect=ValueError("Pipeline analytics is not configured") - ) - provider = FlagsmithProvider(mock_flagsmith_client) + # Given - OF spec section 6: track() must never raise into the caller + tracking_flagsmith_client.track_event.side_effect = RuntimeError("boom") + provider = FlagsmithProvider(tracking_flagsmith_client) - # When / Then - no error raised, ValueError caught silently + # When / Then - no error raised provider.track("purchase") -def test_track_delegates_to_client(mock_flagsmith_client: MagicMock) -> None: +def test_track_delegates_to_client(tracking_flagsmith_client: MagicMock) -> None: # Given - mock_flagsmith_client.track_event = MagicMock() - provider = FlagsmithProvider(mock_flagsmith_client) + provider = FlagsmithProvider(tracking_flagsmith_client) # When provider.track( @@ -501,78 +522,85 @@ def test_track_delegates_to_client(mock_flagsmith_client: MagicMock) -> None: ), ) - # Then - mock_flagsmith_client.track_event.assert_called_once_with( + # Then - value is first-class, attributes become metadata + tracking_flagsmith_client.track_event.assert_called_once_with( "purchase", - identity_identifier="user-123", + identifier="user-123", + value=99.77, traits={"plan": "premium"}, - metadata={"value": 99.77, "currency": "USD"}, + metadata={"currency": "USD"}, ) -def test_track_with_minimal_args(mock_flagsmith_client: MagicMock) -> None: +def test_track_with_minimal_args(tracking_flagsmith_client: MagicMock) -> None: # Given - mock_flagsmith_client.track_event = MagicMock() - provider = FlagsmithProvider(mock_flagsmith_client) + provider = FlagsmithProvider(tracking_flagsmith_client) # When provider.track("signup") # Then - mock_flagsmith_client.track_event.assert_called_once_with( + tracking_flagsmith_client.track_event.assert_called_once_with( "signup", - identity_identifier=None, + identifier=None, + value=None, traits=None, metadata=None, ) -def test_track_value_takes_precedence_over_attributes_value( - mock_flagsmith_client: MagicMock, +def test_track_attributes_pass_through_as_metadata( + tracking_flagsmith_client: MagicMock, ) -> None: - # Given - attributes also has a "value" key - mock_flagsmith_client.track_event = MagicMock() - provider = FlagsmithProvider(mock_flagsmith_client) + # Given - attributes are metadata verbatim; details.value is first-class + provider = FlagsmithProvider(tracking_flagsmith_client) # When provider.track( "checkout", tracking_event_details=TrackingEventDetails( value=99.77, - attributes={"value": "should_be_overwritten", "other": "kept"}, + attributes={"value": "a-metadata-key", "other": "kept"}, ), ) - # Then - explicit .value wins over attributes["value"] - mock_flagsmith_client.track_event.assert_called_once_with( + # Then + tracking_flagsmith_client.track_event.assert_called_once_with( "checkout", - identity_identifier=None, + identifier=None, + value=99.77, traits=None, - metadata={"value": 99.77, "other": "kept"}, + metadata={"value": "a-metadata-key", "other": "kept"}, ) -def test_track_with_details_value_only(mock_flagsmith_client: MagicMock) -> None: +def test_track_non_numeric_value_is_dropped_with_warning( + tracking_flagsmith_client: MagicMock, +) -> None: # Given - mock_flagsmith_client.track_event = MagicMock() - provider = FlagsmithProvider(mock_flagsmith_client) + provider = FlagsmithProvider(tracking_flagsmith_client) - # When - provider.track("checkout", tracking_event_details=TrackingEventDetails(value=99.77)) + # When - value is typed float|None but nothing enforces it at runtime + provider.track( + "checkout", + tracking_event_details=TrackingEventDetails(value="99.77"), # type: ignore[arg-type] + ) - # Then - mock_flagsmith_client.track_event.assert_called_once_with( + # Then - sent without the value + tracking_flagsmith_client.track_event.assert_called_once_with( "checkout", - identity_identifier=None, + identifier=None, + value=None, traits=None, - metadata={"value": 99.77}, + metadata=None, ) -def test_track_extracts_traits_from_context(mock_flagsmith_client: MagicMock) -> None: +def test_track_extracts_traits_from_context( + tracking_flagsmith_client: MagicMock, +) -> None: # Given - nested traits take precedence over flat attributes (same rule as _get_flags) - mock_flagsmith_client.track_event = MagicMock() - provider = FlagsmithProvider(mock_flagsmith_client) + provider = FlagsmithProvider(tracking_flagsmith_client) # When provider.track( @@ -588,14 +616,30 @@ def test_track_extracts_traits_from_context(mock_flagsmith_client: MagicMock) -> ) # Then - mock_flagsmith_client.track_event.assert_called_once_with( + tracking_flagsmith_client.track_event.assert_called_once_with( "page_view", - identity_identifier="user-123", + identifier="user-123", traits={"shared_key": "nested_value", "other": "kept"}, + value=None, metadata=None, ) +def test_track_drops_reserved_dollar_names( + tracking_flagsmith_client: MagicMock, +) -> None: + # Given + provider = FlagsmithProvider(tracking_flagsmith_client) + + # When + provider.track("$flag_exposure") + provider.track("$anything") + + # Then - warned and dropped, never sent to the SDK + tracking_flagsmith_client.track_event.assert_not_called() + tracking_flagsmith_client.track_exposure_event.assert_not_called() + + # --------------------------------------------------------------------------- # Reasons / variant / flag_metadata # --------------------------------------------------------------------------- From c70cedd8fd189f1eea139205e45aed4e98c5a692 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 3 Aug 2026 17:48:58 +0200 Subject: [PATCH 05/16] feat: route feature_flag.exposure tracking events to Flagsmith Signed-off-by: wadii --- openfeature_flagsmith/provider.py | 80 +++++++++++- tests/test_provider.py | 196 ++++++++++++++++++++++++++++++ 2 files changed, 274 insertions(+), 2 deletions(-) diff --git a/openfeature_flagsmith/provider.py b/openfeature_flagsmith/provider.py index 963c4ac..c2a148c 100644 --- a/openfeature_flagsmith/provider.py +++ b/openfeature_flagsmith/provider.py @@ -3,7 +3,10 @@ import typing from json import JSONDecodeError -from flagsmith.exceptions import FlagsmithClientError +from flagsmith.exceptions import ( + FlagsmithClientError, + FlagsmithFeatureDoesNotExistError, +) from flagsmith.flagsmith import Flagsmith from flagsmith.models import Flag from openfeature.evaluation_context import EvaluationContext @@ -136,7 +139,80 @@ def _track_exposure( evaluation_context: typing.Optional[EvaluationContext], tracking_event_details: typing.Optional[TrackingEventDetails], ) -> None: - raise NotImplementedError # implemented in the exposure-routing task + attributes = ( + dict(tracking_event_details.attributes) if tracking_event_details else {} + ) + flag_key = attributes.pop("flag_key", None) + variant = attributes.pop("variant", None) + metadata = attributes or None + + if not isinstance(flag_key, str): + logger.warning( + '"%s" requires a string "flag_key" attribute; dropping exposure' + " event.", + EXPOSURE_TRACKING_EVENT, + ) + return + if not identifier: + logger.info( + 'Exposure for "%s" skipped: no targeting_key in the evaluation' + " context.", + flag_key, + ) + return + + if isinstance(variant, str): + self._client.track_exposure_event( + feature_name=flag_key, + identifier=identifier, + value=variant, + traits=traits, + metadata=metadata, + ) + return + + # Mirrors the SDK's get_experiment_flag guards, with the exposure + # attributed to the OF context's targeting key rather than any + # ambient identity. This resolution counts as a flag evaluation, + # exactly like get_experiment_flag itself. + try: + flag = self._client.get_identity_flags( + identifier=identifier, + traits=traits or {}, + transient=self._is_transient(evaluation_context), + ).get_flag(flag_key) + except FlagsmithFeatureDoesNotExistError: + logger.info('Exposure for "%s" skipped: flag does not exist.', flag_key) + return + except FlagsmithClientError: + logger.warning( + 'Exposure for "%s" skipped: failed to resolve the flag.', + flag_key, + exc_info=True, + ) + return + + if not isinstance(flag, Flag): + logger.info('Exposure for "%s" skipped: flag does not exist.', flag_key) + return + if not flag.enabled: + logger.info('Exposure for "%s" skipped: flag is disabled.', flag_key) + return + if flag.variant is None: + logger.info( + 'Exposure for "%s" skipped: experiments require an enabled' + " multivariate flag.", + flag_key, + ) + return + + self._client.track_exposure_event( + feature_name=flag_key, + identifier=identifier, + value=flag.variant, + traits=traits, + metadata=metadata, + ) def get_metadata(self) -> Metadata: return Metadata(name="FlagsmithProvider") diff --git a/tests/test_provider.py b/tests/test_provider.py index a5ff5ea..48a36d3 100644 --- a/tests/test_provider.py +++ b/tests/test_provider.py @@ -16,6 +16,7 @@ from openfeature_flagsmith.exceptions import FlagsmithProviderError from openfeature_flagsmith.provider import FlagsmithProvider +from openfeature_flagsmith.tracking import EXPOSURE_TRACKING_EVENT @pytest.fixture() @@ -834,3 +835,198 @@ def test_nested_trait_named_transient_is_kept( mock_flagsmith_client.get_identity_flags.assert_called_once_with( identifier="user-1", traits={"transient": "a-real-trait"}, transient=False ) + + +# --------------------------------------------------------------------------- +# Tracking: exposures +# --------------------------------------------------------------------------- + + +def test_exposure_with_explicit_variant_sends_as_rendered( + tracking_flagsmith_client: MagicMock, +) -> None: + # Given + provider = FlagsmithProvider(tracking_flagsmith_client) + + # When + provider.track( + EXPOSURE_TRACKING_EVENT, + evaluation_context=EvaluationContext( + targeting_key="user-1", attributes={"plan": "pro"} + ), + tracking_event_details=TrackingEventDetails( + attributes={"flag_key": "my_exp", "variant": "treatment", "page": "home"} + ), + ) + + # Then - no flag resolution; remaining attributes become metadata + tracking_flagsmith_client.track_exposure_event.assert_called_once_with( + feature_name="my_exp", + identifier="user-1", + value="treatment", + traits={"plan": "pro"}, + metadata={"page": "home"}, + ) + tracking_flagsmith_client.get_identity_flags.assert_not_called() + + +def test_exposure_without_flag_key_is_dropped( + tracking_flagsmith_client: MagicMock, +) -> None: + # Given + provider = FlagsmithProvider(tracking_flagsmith_client) + + # When + provider.track( + EXPOSURE_TRACKING_EVENT, + evaluation_context=EvaluationContext(targeting_key="user-1"), + tracking_event_details=TrackingEventDetails(attributes={"variant": "t"}), + ) + + # Then + tracking_flagsmith_client.track_exposure_event.assert_not_called() + + +def test_exposure_without_targeting_key_is_skipped( + tracking_flagsmith_client: MagicMock, +) -> None: + # Given - exposures attribute to the OF context, never ambient state + provider = FlagsmithProvider(tracking_flagsmith_client) + + # When + provider.track( + EXPOSURE_TRACKING_EVENT, + tracking_event_details=TrackingEventDetails( + attributes={"flag_key": "my_exp", "variant": "t"} + ), + ) + + # Then + tracking_flagsmith_client.track_exposure_event.assert_not_called() + + +def test_variantless_exposure_resolves_flag_and_sends_variant( + tracking_flagsmith_client: MagicMock, +) -> None: + # Given + tracking_flagsmith_client.get_identity_flags.return_value = Flags( + { + "my_exp": Flag( + feature_id=1, + feature_name="my_exp", + enabled=True, + value="v", + variant="treatment", + ) + } + ) + provider = FlagsmithProvider(tracking_flagsmith_client) + + # When + provider.track( + EXPOSURE_TRACKING_EVENT, + evaluation_context=EvaluationContext( + targeting_key="user-1", attributes={"transient": True} + ), + tracking_event_details=TrackingEventDetails(attributes={"flag_key": "my_exp"}), + ) + + # Then - resolution honors the transient directive + tracking_flagsmith_client.get_identity_flags.assert_called_once_with( + identifier="user-1", traits={}, transient=True + ) + tracking_flagsmith_client.track_exposure_event.assert_called_once_with( + feature_name="my_exp", + identifier="user-1", + value="treatment", + traits=None, + metadata=None, + ) + + +@pytest.mark.parametrize( + "flag", + [ + pytest.param(DefaultFlag(enabled=True, value="v"), id="default-flag"), + pytest.param( + Flag(feature_id=1, feature_name="my_exp", enabled=False, value="v"), + id="disabled", + ), + pytest.param( + Flag( + feature_id=1, + feature_name="my_exp", + enabled=True, + value="v", + variant=None, + ), + id="no-variant", + ), + ], +) +def test_variantless_exposure_guard_chain_skips( + tracking_flagsmith_client: MagicMock, flag +) -> None: + # Given - JS guard chain: real Flag, enabled, has variant + tracking_flagsmith_client.get_identity_flags.return_value = Flags({"my_exp": flag}) + provider = FlagsmithProvider(tracking_flagsmith_client) + + # When + provider.track( + EXPOSURE_TRACKING_EVENT, + evaluation_context=EvaluationContext(targeting_key="user-1"), + tracking_event_details=TrackingEventDetails(attributes={"flag_key": "my_exp"}), + ) + + # Then + tracking_flagsmith_client.track_exposure_event.assert_not_called() + + +def test_variantless_exposure_missing_flag_is_skipped( + tracking_flagsmith_client: MagicMock, +) -> None: + # Given - no default_flag_handler: get_flag raises + tracking_flagsmith_client.get_identity_flags.return_value = Flags({}) + provider = FlagsmithProvider(tracking_flagsmith_client) + + # When / Then - no error raised, no exposure recorded + provider.track( + EXPOSURE_TRACKING_EVENT, + evaluation_context=EvaluationContext(targeting_key="user-1"), + tracking_event_details=TrackingEventDetails(attributes={"flag_key": "nope"}), + ) + tracking_flagsmith_client.track_exposure_event.assert_not_called() + + +def test_variantless_exposure_client_error_is_swallowed( + tracking_flagsmith_client: MagicMock, +) -> None: + # Given + tracking_flagsmith_client.get_identity_flags.side_effect = FlagsmithClientError("") + provider = FlagsmithProvider(tracking_flagsmith_client) + + # When / Then - no error raised + provider.track( + EXPOSURE_TRACKING_EVENT, + evaluation_context=EvaluationContext(targeting_key="user-1"), + tracking_event_details=TrackingEventDetails(attributes={"flag_key": "my_exp"}), + ) + tracking_flagsmith_client.track_exposure_event.assert_not_called() + + +def test_exposure_is_noop_when_events_disabled( + mock_flagsmith_client: MagicMock, +) -> None: + # Given - no _event_processor: must not fetch flags or persist identities + provider = FlagsmithProvider(mock_flagsmith_client) + + # When + provider.track( + EXPOSURE_TRACKING_EVENT, + evaluation_context=EvaluationContext(targeting_key="user-1"), + tracking_event_details=TrackingEventDetails(attributes={"flag_key": "my_exp"}), + ) + + # Then + mock_flagsmith_client.get_identity_flags.assert_not_called() + mock_flagsmith_client.track_exposure_event.assert_not_called() From f0682e03610627564076140ddb208f6b1a165628 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 3 Aug 2026 17:50:14 +0200 Subject: [PATCH 06/16] feat: add FlagsmithExposureHook for evaluation-coupled exposures Signed-off-by: wadii --- openfeature_flagsmith/hooks.py | 96 +++++++++++++++ tests/test_hooks.py | 213 +++++++++++++++++++++++++++++++++ 2 files changed, 309 insertions(+) create mode 100644 openfeature_flagsmith/hooks.py create mode 100644 tests/test_hooks.py diff --git a/openfeature_flagsmith/hooks.py b/openfeature_flagsmith/hooks.py new file mode 100644 index 0000000..6820c44 --- /dev/null +++ b/openfeature_flagsmith/hooks.py @@ -0,0 +1,96 @@ +import json +import logging +import threading +import typing +from collections import OrderedDict + +from openfeature.flag_evaluation import FlagEvaluationDetails, Reason +from openfeature.hook import Hook, HookContext, HookHints +from openfeature.track import TrackingEventDetails + +from openfeature_flagsmith.tracking import EXPOSURE_TRACKING_EVENT + +if typing.TYPE_CHECKING: + from openfeature_flagsmith.provider import FlagsmithProvider + +logger = logging.getLogger(__name__) + +DEFAULT_MAX_DEDUPE_ENTRIES = 10_000 + + +class FlagsmithExposureHook(Hook): + """ + Records a Flagsmith exposure as a side effect of a flag evaluation, so one + call both resolves the flag and marks the identity as exposed to its + variant — the OpenFeature equivalent of Flagsmith's ``get_experiment_flag``:: + + hook = FlagsmithExposureHook(provider) + client.get_string_details( + "my_experiment_flag", + "control", + context, + FlagEvaluationOptions(hooks=[hook]), + ) + + Attaching the hook at a call site is the experiment declaration: + evaluations without it never record exposures. Exposures only fire for + multivariate flags resolved with reason ``TARGETING_MATCH`` (enabled, + identified, not offline), and are deduped per identity/flag/variant in a + bounded, thread-safe LRU for the hook instance's lifetime. + + Tracking is an experimental OpenFeature capability (spec section 6). + """ + + def __init__( + self, + provider: "FlagsmithProvider", + max_dedupe_entries: int = DEFAULT_MAX_DEDUPE_ENTRIES, + ) -> None: + self._provider = provider + self._max_dedupe_entries = max_dedupe_entries + self._seen: "OrderedDict[str, None]" = OrderedDict() + self._lock = threading.Lock() + + def after( + self, + hook_context: HookContext, + details: FlagEvaluationDetails, + hints: HookHints, + ) -> None: + # Fully error-contained: an uncaught after-hook error flips the + # evaluation itself to ERROR in the OpenFeature SDK. + try: + variant = details.variant + if not isinstance(variant, str): + return + if details.reason != Reason.TARGETING_MATCH: + logger.debug( + 'Exposure for "%s" skipped: resolution reason is %s, not' + " TARGETING_MATCH.", + details.flag_key, + details.reason, + ) + return + targeting_key = hook_context.evaluation_context.targeting_key + # json.dumps of the list avoids delimiter-collision false dedupes. + dedupe_key = json.dumps([targeting_key, details.flag_key, variant]) + with self._lock: + if dedupe_key in self._seen: + self._seen.move_to_end(dedupe_key) + return + self._seen[dedupe_key] = None + while len(self._seen) > self._max_dedupe_entries: + self._seen.popitem(last=False) + self._provider.track( + EXPOSURE_TRACKING_EVENT, + hook_context.evaluation_context, + TrackingEventDetails( + attributes={"flag_key": details.flag_key, "variant": variant} + ), + ) + except Exception: + logger.warning( + 'Failed to record the exposure for "%s".', + details.flag_key, + exc_info=True, + ) diff --git a/tests/test_hooks.py b/tests/test_hooks.py new file mode 100644 index 0000000..5831253 --- /dev/null +++ b/tests/test_hooks.py @@ -0,0 +1,213 @@ +import threading +from unittest.mock import MagicMock, create_autospec + +import pytest +from flagsmith import Flagsmith +from flagsmith.models import Flag, Flags +from openfeature import api +from openfeature.evaluation_context import EvaluationContext +from openfeature.flag_evaluation import ( + FlagEvaluationDetails, + FlagEvaluationOptions, + FlagType, + Reason, +) +from openfeature.hook import HookContext + +from openfeature_flagsmith.hooks import FlagsmithExposureHook +from openfeature_flagsmith.provider import FlagsmithProvider +from openfeature_flagsmith.tracking import EXPOSURE_TRACKING_EVENT + + +@pytest.fixture() +def mock_provider() -> MagicMock: + return create_autospec(FlagsmithProvider, instance=True) + + +def _hook_context(targeting_key="user-1") -> HookContext: + return HookContext( + flag_key="my_exp", + flag_type=FlagType.STRING, + default_value="control", + evaluation_context=EvaluationContext(targeting_key=targeting_key), + ) + + +def _details( + flag_key="my_exp", variant="treatment", reason=Reason.TARGETING_MATCH +) -> FlagEvaluationDetails: + return FlagEvaluationDetails( + flag_key=flag_key, value="v", variant=variant, reason=reason + ) + + +def test_hook_records_exposure_on_targeting_match(mock_provider: MagicMock) -> None: + # Given + hook = FlagsmithExposureHook(mock_provider) + context = _hook_context() + + # When + hook.after(hook_context=context, details=_details(), hints={}) + + # Then + mock_provider.track.assert_called_once() + name, of_context, details = mock_provider.track.call_args.args + assert name == EXPOSURE_TRACKING_EVENT + assert of_context is context.evaluation_context + assert details.attributes == {"flag_key": "my_exp", "variant": "treatment"} + + +def test_hook_skips_without_variant(mock_provider: MagicMock) -> None: + # Given + hook = FlagsmithExposureHook(mock_provider) + + # When + hook.after(hook_context=_hook_context(), details=_details(variant=None), hints={}) + + # Then + mock_provider.track.assert_not_called() + + +@pytest.mark.parametrize( + "reason", + [Reason.STATIC, Reason.DEFAULT, Reason.DISABLED, Reason.STALE, Reason.CACHED], +) +def test_hook_skips_on_non_targeting_match_reason( + mock_provider: MagicMock, reason: Reason +) -> None: + # Given + hook = FlagsmithExposureHook(mock_provider) + + # When + hook.after(hook_context=_hook_context(), details=_details(reason=reason), hints={}) + + # Then + mock_provider.track.assert_not_called() + + +def test_hook_dedupes_per_identity_flag_variant(mock_provider: MagicMock) -> None: + # Given + hook = FlagsmithExposureHook(mock_provider) + + # When - same triple twice, then each dimension varied + hook.after(hook_context=_hook_context(), details=_details(), hints={}) + hook.after(hook_context=_hook_context(), details=_details(), hints={}) + hook.after( + hook_context=_hook_context(targeting_key="user-2"), + details=_details(), + hints={}, + ) + hook.after( + hook_context=_hook_context(), details=_details(variant="control"), hints={} + ) + + # Then - 3 distinct exposures, 1 dedupe hit + assert mock_provider.track.call_count == 3 + + +def test_hook_dedupe_is_bounded_lru(mock_provider: MagicMock) -> None: + # Given a tiny bound + hook = FlagsmithExposureHook(mock_provider, max_dedupe_entries=2) + + # When - third key evicts the first, which then fires again + hook.after(hook_context=_hook_context("u1"), details=_details(), hints={}) + hook.after(hook_context=_hook_context("u2"), details=_details(), hints={}) + hook.after(hook_context=_hook_context("u3"), details=_details(), hints={}) + hook.after(hook_context=_hook_context("u1"), details=_details(), hints={}) + + # Then + assert mock_provider.track.call_count == 4 + + +def test_hook_dedupe_key_is_collision_safe(mock_provider: MagicMock) -> None: + # Given - a naive join would collide these two identity/flag pairs + hook = FlagsmithExposureHook(mock_provider) + + # When + hook.after( + hook_context=_hook_context('user"1'), + details=_details(flag_key="exp"), + hints={}, + ) + hook.after( + hook_context=_hook_context("user"), + details=_details(flag_key='1", "exp'), + hints={}, + ) + + # Then - two distinct exposures + assert mock_provider.track.call_count == 2 + + +def test_hook_swallows_provider_errors(mock_provider: MagicMock) -> None: + # Given - an uncaught after-hook error flips the evaluation to ERROR + mock_provider.track.side_effect = RuntimeError("boom") + hook = FlagsmithExposureHook(mock_provider) + + # When / Then - no error raised + hook.after(hook_context=_hook_context(), details=_details(), hints={}) + + +def test_hook_is_thread_safe(mock_provider: MagicMock) -> None: + # Given + hook = FlagsmithExposureHook(mock_provider) + + def fire(i: int) -> None: + hook.after( + hook_context=_hook_context(f"user-{i % 10}"), + details=_details(), + hints={}, + ) + + # When - 100 concurrent evaluations over 10 identities + threads = [threading.Thread(target=fire, args=(i,)) for i in range(100)] + for t in threads: + t.start() + for t in threads: + t.join() + + # Then - exactly one exposure per identity + assert mock_provider.track.call_count == 10 + + +def test_hook_end_to_end_records_exposure_through_openfeature() -> None: + # Given - real OF SDK wiring: provider + per-invocation hook + client = create_autospec(Flagsmith, instance=True) + client._event_processor = MagicMock() + client.get_identity_flags.return_value = Flags( + { + "my_exp": Flag( + feature_id=1, + feature_name="my_exp", + enabled=True, + value="treatment-value", + variant="treatment", + ) + } + ) + provider = FlagsmithProvider(client) + api.set_provider(provider) + try: + of_client = api.get_client() + hook = FlagsmithExposureHook(provider) + + # When + details = of_client.get_string_details( + "my_exp", + "control", + EvaluationContext(targeting_key="user-1"), + FlagEvaluationOptions(hooks=[hook]), + ) + + # Then - evaluation resolved AND the exposure reached the SDK + assert details.value == "treatment-value" + assert details.variant == "treatment" + client.track_exposure_event.assert_called_once_with( + feature_name="my_exp", + identifier="user-1", + value="treatment", + traits=None, + metadata=None, + ) + finally: + api.clear_providers() From 1c3ab22f15ddb864d757055b2d817e6a58c17460 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 3 Aug 2026 17:51:06 +0200 Subject: [PATCH 07/16] feat: re-export provider, hook and exposure constant from package root Signed-off-by: wadii --- openfeature_flagsmith/__init__.py | 9 +++++++++ tests/test_provider.py | 8 ++++++++ 2 files changed, 17 insertions(+) diff --git a/openfeature_flagsmith/__init__.py b/openfeature_flagsmith/__init__.py index e69de29..fcf5e72 100644 --- a/openfeature_flagsmith/__init__.py +++ b/openfeature_flagsmith/__init__.py @@ -0,0 +1,9 @@ +from openfeature_flagsmith.hooks import FlagsmithExposureHook +from openfeature_flagsmith.provider import FlagsmithProvider +from openfeature_flagsmith.tracking import EXPOSURE_TRACKING_EVENT + +__all__ = [ + "EXPOSURE_TRACKING_EVENT", + "FlagsmithExposureHook", + "FlagsmithProvider", +] diff --git a/tests/test_provider.py b/tests/test_provider.py index 48a36d3..13f0cec 100644 --- a/tests/test_provider.py +++ b/tests/test_provider.py @@ -1030,3 +1030,11 @@ def test_exposure_is_noop_when_events_disabled( # Then mock_flagsmith_client.get_identity_flags.assert_not_called() mock_flagsmith_client.track_exposure_event.assert_not_called() + + +def test_package_root_reexports() -> None: + import openfeature_flagsmith + + assert openfeature_flagsmith.FlagsmithProvider is FlagsmithProvider + assert openfeature_flagsmith.EXPOSURE_TRACKING_EVENT == "feature_flag.exposure" + assert openfeature_flagsmith.FlagsmithExposureHook is not None From f40e0d5ae63201bc533e632d2bbe8b0ea114c856 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 3 Aug 2026 17:51:42 +0200 Subject: [PATCH 08/16] docs: document exposure tiers and fix broken tracking example Signed-off-by: wadii --- README.md | 87 +++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 68 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index a1c6e08..d6daa4f 100644 --- a/README.md +++ b/README.md @@ -60,40 +60,85 @@ provider = FlagsmithProvider( The provider can then be used with the OpenFeature client as per [the documentation](https://openfeature.dev/docs/reference/concepts/evaluation-api#setting-a-provider). -### Tracking +### Tracking and experimentation -The provider supports the [OpenFeature tracking API](https://openfeature.dev/specification/sections/tracking/), which lets you associate user actions with feature flag evaluations for experimentation. +The provider supports the [OpenFeature tracking API](https://openfeature.dev/specification/sections/tracking/) (an experimental OpenFeature capability), which lets you record custom events and flag **exposures** for experimentation. -Tracking requires pipeline analytics to be enabled on the **Flagsmith client** (available from `flagsmith` version 5.2.0). The provider acts as a thin delegate — all buffering and flushing is managed by the client. +Tracking requires events to be enabled on the **Flagsmith client** (`flagsmith` ≥5.5). The provider acts as a thin delegate — all buffering and flushing is managed by the client. ```python -from flagsmith import Flagsmith, PipelineAnalyticsConfig +from flagsmith import Flagsmith from openfeature import api -from openfeature.evaluation_context import EvaluationContext -from openfeature.track import TrackingEventDetails -from openfeature_flagsmith.provider import FlagsmithProvider +from openfeature_flagsmith import FlagsmithProvider -# Enable pipeline analytics on the Flagsmith client client = Flagsmith( environment_key="your-environment-key", - pipeline_analytics_config=PipelineAnalyticsConfig( - analytics_server_url="https://analytics-collector.flagsmith.com/", - max_buffer_items=1000, # optional, default 1000 - flush_interval_seconds=10, # optional, default 10s - ), + enable_events=True, ) -api.set_provider(FlagsmithProvider(client=client)) +provider = FlagsmithProvider(client=client) +api.set_provider(provider) of_client = api.get_client() +``` + +If events are not enabled on the Flagsmith client, all tracking calls are silently dropped. + +#### Recording exposures + +An **exposure** marks an identity as having experienced an experiment variant. Exposures are never recorded automatically: evaluating a flag does not expose anyone. There are three ways to record them, from most to least recommended. + +**1. The exposure hook (recommended).** Attach `FlagsmithExposureHook` to the evaluations that *are* your experiment — attaching the hook is the experiment declaration: -# Flag evaluations are tracked automatically — no extra code needed -variant = of_client.get_string_value( - "checkout-variant", +```python +from openfeature.evaluation_context import EvaluationContext +from openfeature.flag_evaluation import FlagEvaluationOptions +from openfeature_flagsmith import FlagsmithExposureHook + +hook = FlagsmithExposureHook(provider) + +details = of_client.get_string_details( + "my_experiment_flag", "control", + EvaluationContext(targeting_key="user-123"), + FlagEvaluationOptions(hooks=[hook]), +) +``` + +The hook records an exposure only when the flag resolved with a variant and reason `TARGETING_MATCH` (enabled, identified, not offline), and dedupes per identity/flag/variant for the hook instance's lifetime (bounded, thread-safe). + +**2. Explicit `track()`.** Use the reserved `feature_flag.exposure` event name when you need to record an exposure decoupled from evaluation: + +```python +from openfeature.track import TrackingEventDetails +from openfeature_flagsmith import EXPOSURE_TRACKING_EVENT + +# With an explicit variant: sent as rendered. +of_client.track( + EXPOSURE_TRACKING_EVENT, + evaluation_context=EvaluationContext(targeting_key="user-123"), + tracking_event_details=TrackingEventDetails( + attributes={"flag_key": "my_experiment_flag", "variant": "treatment"} + ), +) + +# Without a variant: the provider resolves the flag for the targeting key and +# records the exposure only if the flag exists, is enabled and has a variant. +of_client.track( + EXPOSURE_TRACKING_EVENT, evaluation_context=EvaluationContext(targeting_key="user-123"), + tracking_event_details=TrackingEventDetails( + attributes={"flag_key": "my_experiment_flag"} + ), ) +``` + +**3. The native Flagsmith client.** `client.get_experiment_flag(...)` / `client.track_exposure_event(...)` work as documented in the [Flagsmith docs](https://docs.flagsmith.com/) and share the same event pipeline. -# Track a custom event explicitly +#### Custom events + +Any other event name is forwarded as a plain Flagsmith event. `TrackingEventDetails.value` must be numeric and is sent as the event value; `attributes` become event metadata; context traits are attached to the event. + +```python of_client.track( "purchase", evaluation_context=EvaluationContext( @@ -107,7 +152,11 @@ of_client.track( ) ``` -If `pipeline_analytics_config` is not set on the Flagsmith client, calls to `track()` are silently ignored. +#### Caveats + +- **Anonymous contexts**: exposures require a `targeting_key`; without one they are skipped (logged at info). +- **Reserved names**: event names starting with `$` are reserved for Flagsmith system events and are dropped with a warning — use `EXPOSURE_TRACKING_EVENT` to record exposures. +- **Transient identities** (Python provider only, remote evaluation only): set the context attribute `"transient": True` to evaluate an identity without persisting it. The variant-less exposure path honors it too. ### Evaluation Context From 7f7a9e44fe497a0679845251f8fbe0a07ed6ae76 Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 4 Aug 2026 14:37:54 +0200 Subject: [PATCH 09/16] feat: emit SPLIT for multivariate assignments and gate exposures on it Signed-off-by: wadii --- README.md | 2 +- openfeature_flagsmith/hooks.py | 22 ++++++++---- openfeature_flagsmith/provider.py | 8 ++++- tests/test_hooks.py | 57 +++++++++++++++++++++---------- tests/test_provider.py | 55 ++++++++++++++--------------- 5 files changed, 89 insertions(+), 55 deletions(-) diff --git a/README.md b/README.md index d6daa4f..fd551fb 100644 --- a/README.md +++ b/README.md @@ -104,7 +104,7 @@ details = of_client.get_string_details( ) ``` -The hook records an exposure only when the flag resolved with a variant and reason `TARGETING_MATCH` (enabled, identified, not offline), and dedupes per identity/flag/variant for the hook instance's lifetime (bounded, thread-safe). +The hook records an exposure only when the flag resolved with a variant and reason `SPLIT` — a multivariate percentage-split assignment (enabled, identified, not offline) — and dedupes per identity/flag/variant for the hook instance's lifetime (bounded, thread-safe). **2. Explicit `track()`.** Use the reserved `feature_flag.exposure` event name when you need to record an exposure decoupled from evaluation: diff --git a/openfeature_flagsmith/hooks.py b/openfeature_flagsmith/hooks.py index 6820c44..1ace98b 100644 --- a/openfeature_flagsmith/hooks.py +++ b/openfeature_flagsmith/hooks.py @@ -18,6 +18,15 @@ DEFAULT_MAX_DEDUPE_ENTRIES = 10_000 +def _is_split_reason(reason: typing.Union[str, Reason, None]) -> bool: + # The Flagsmith engine annotates reasons with k=v metadata + # ("SPLIT; weight=30"); compare the leading token so the gate keeps + # working once the API exposes annotated reasons. + if reason is None: + return False + return str(reason).split(";", 1)[0].strip() == Reason.SPLIT.value + + class FlagsmithExposureHook(Hook): """ Records a Flagsmith exposure as a side effect of a flag evaluation, so one @@ -34,9 +43,11 @@ class FlagsmithExposureHook(Hook): Attaching the hook at a call site is the experiment declaration: evaluations without it never record exposures. Exposures only fire for - multivariate flags resolved with reason ``TARGETING_MATCH`` (enabled, - identified, not offline), and are deduped per identity/flag/variant in a - bounded, thread-safe LRU for the hook instance's lifetime. + multivariate flags resolved with reason ``SPLIT`` (a percentage-split + assignment: enabled, identified, not offline; engine-annotated reason + strings like ``"SPLIT; weight=30"`` also match), and are deduped per + identity/flag/variant in a bounded, thread-safe LRU for the hook + instance's lifetime. Tracking is an experimental OpenFeature capability (spec section 6). """ @@ -63,10 +74,9 @@ def after( variant = details.variant if not isinstance(variant, str): return - if details.reason != Reason.TARGETING_MATCH: + if not _is_split_reason(details.reason): logger.debug( - 'Exposure for "%s" skipped: resolution reason is %s, not' - " TARGETING_MATCH.", + 'Exposure for "%s" skipped: resolution reason is %s, not' " SPLIT.", details.flag_key, details.reason, ) diff --git a/openfeature_flagsmith/provider.py b/openfeature_flagsmith/provider.py index c2a148c..c2347e2 100644 --- a/openfeature_flagsmith/provider.py +++ b/openfeature_flagsmith/provider.py @@ -333,10 +333,16 @@ def _parse_reason( if not flag.enabled: return Reason.DISABLED # Offline documents may be arbitrarily old; the exposure hook treats - # anything but TARGETING_MATCH as not fresh enough to record. + # anything but SPLIT as not fresh enough to record. if getattr(self._client, "offline_mode", False): return Reason.STALE if evaluation_context.targeting_key: + # Engine taxonomy: a variant means a multivariate percentage-split + # assignment (SPLIT); TARGETING_MATCH is reserved for segment + # matches, which the API can't distinguish from environment + # defaults yet, so no-variant identity evaluations stay coarse. + if getattr(flag, "variant", None) is not None: + return Reason.SPLIT return Reason.TARGETING_MATCH return Reason.STATIC diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 5831253..d46269e 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -34,14 +34,14 @@ def _hook_context(targeting_key="user-1") -> HookContext: def _details( - flag_key="my_exp", variant="treatment", reason=Reason.TARGETING_MATCH + flag_key="my_exp", variant="treatment", reason=Reason.SPLIT ) -> FlagEvaluationDetails: return FlagEvaluationDetails( flag_key=flag_key, value="v", variant=variant, reason=reason ) -def test_hook_records_exposure_on_targeting_match(mock_provider: MagicMock) -> None: +def test_hook_records_exposure_on_split(mock_provider: MagicMock) -> None: # Given hook = FlagsmithExposureHook(mock_provider) context = _hook_context() @@ -70,11 +70,18 @@ def test_hook_skips_without_variant(mock_provider: MagicMock) -> None: @pytest.mark.parametrize( "reason", - [Reason.STATIC, Reason.DEFAULT, Reason.DISABLED, Reason.STALE, Reason.CACHED], + [ + Reason.STATIC, + Reason.DEFAULT, + Reason.DISABLED, + Reason.STALE, + Reason.CACHED, + Reason.TARGETING_MATCH, + None, + "SPLITTER; weight=30", + ], ) -def test_hook_skips_on_non_targeting_match_reason( - mock_provider: MagicMock, reason: Reason -) -> None: +def test_hook_skips_on_non_split_reason(mock_provider: MagicMock, reason) -> None: # Given hook = FlagsmithExposureHook(mock_provider) @@ -85,11 +92,25 @@ def test_hook_skips_on_non_targeting_match_reason( mock_provider.track.assert_not_called() +@pytest.mark.parametrize("reason", ["SPLIT", "SPLIT; weight=30", "SPLIT ; seed=abc"]) +def test_hook_accepts_engine_annotated_split_reasons( + mock_provider: MagicMock, reason: str +) -> None: + # Given + hook = FlagsmithExposureHook(mock_provider) + + # When + hook.after(hook_context=_hook_context(), details=_details(reason=reason), hints={}) + + # Then + mock_provider.track.assert_called_once() + + def test_hook_dedupes_per_identity_flag_variant(mock_provider: MagicMock) -> None: # Given hook = FlagsmithExposureHook(mock_provider) - # When - same triple twice, then each dimension varied + # When hook.after(hook_context=_hook_context(), details=_details(), hints={}) hook.after(hook_context=_hook_context(), details=_details(), hints={}) hook.after( @@ -101,15 +122,15 @@ def test_hook_dedupes_per_identity_flag_variant(mock_provider: MagicMock) -> Non hook_context=_hook_context(), details=_details(variant="control"), hints={} ) - # Then - 3 distinct exposures, 1 dedupe hit + # Then assert mock_provider.track.call_count == 3 def test_hook_dedupe_is_bounded_lru(mock_provider: MagicMock) -> None: - # Given a tiny bound + # Given hook = FlagsmithExposureHook(mock_provider, max_dedupe_entries=2) - # When - third key evicts the first, which then fires again + # When hook.after(hook_context=_hook_context("u1"), details=_details(), hints={}) hook.after(hook_context=_hook_context("u2"), details=_details(), hints={}) hook.after(hook_context=_hook_context("u3"), details=_details(), hints={}) @@ -120,7 +141,7 @@ def test_hook_dedupe_is_bounded_lru(mock_provider: MagicMock) -> None: def test_hook_dedupe_key_is_collision_safe(mock_provider: MagicMock) -> None: - # Given - a naive join would collide these two identity/flag pairs + # Given hook = FlagsmithExposureHook(mock_provider) # When @@ -135,16 +156,16 @@ def test_hook_dedupe_key_is_collision_safe(mock_provider: MagicMock) -> None: hints={}, ) - # Then - two distinct exposures + # Then assert mock_provider.track.call_count == 2 def test_hook_swallows_provider_errors(mock_provider: MagicMock) -> None: - # Given - an uncaught after-hook error flips the evaluation to ERROR + # Given mock_provider.track.side_effect = RuntimeError("boom") hook = FlagsmithExposureHook(mock_provider) - # When / Then - no error raised + # When / Then hook.after(hook_context=_hook_context(), details=_details(), hints={}) @@ -159,19 +180,19 @@ def fire(i: int) -> None: hints={}, ) - # When - 100 concurrent evaluations over 10 identities + # When threads = [threading.Thread(target=fire, args=(i,)) for i in range(100)] for t in threads: t.start() for t in threads: t.join() - # Then - exactly one exposure per identity + # Then assert mock_provider.track.call_count == 10 def test_hook_end_to_end_records_exposure_through_openfeature() -> None: - # Given - real OF SDK wiring: provider + per-invocation hook + # Given client = create_autospec(Flagsmith, instance=True) client._event_processor = MagicMock() client.get_identity_flags.return_value = Flags( @@ -199,7 +220,7 @@ def test_hook_end_to_end_records_exposure_through_openfeature() -> None: FlagEvaluationOptions(hooks=[hook]), ) - # Then - evaluation resolved AND the exposure reached the SDK + # Then assert details.value == "treatment-value" assert details.variant == "treatment" client.track_exposure_event.assert_called_once_with( diff --git a/tests/test_provider.py b/tests/test_provider.py index 13f0cec..a72299c 100644 --- a/tests/test_provider.py +++ b/tests/test_provider.py @@ -21,14 +21,11 @@ @pytest.fixture() def mock_flagsmith_client() -> MagicMock: - # create_autospec validates call signatures; the loose MagicMock(spec=...) - # it replaces let track_event(identity_identifier=...) pass silently. return create_autospec(Flagsmith, instance=True) @pytest.fixture() def tracking_flagsmith_client(mock_flagsmith_client: MagicMock) -> MagicMock: - # The provider treats a client without _event_processor as events-disabled. mock_flagsmith_client._event_processor = MagicMock() return mock_flagsmith_client @@ -474,13 +471,13 @@ def test_resolve_boolean_details_uses_enabled_when_use_boolean_config_value_is_f def test_track_is_noop_when_events_disabled( mock_flagsmith_client: MagicMock, ) -> None: - # Given - no _event_processor on the client (events not enabled) + # Given provider = FlagsmithProvider(mock_flagsmith_client) # When provider.track("purchase") - # Then - dropped before any SDK call + # Then mock_flagsmith_client.track_event.assert_not_called() @@ -491,18 +488,18 @@ def test_track_swallows_value_error_from_sdk( tracking_flagsmith_client.track_event.side_effect = ValueError("events disabled") provider = FlagsmithProvider(tracking_flagsmith_client) - # When / Then - no error raised + # When / Then provider.track("purchase") def test_track_swallows_unexpected_exceptions( tracking_flagsmith_client: MagicMock, ) -> None: - # Given - OF spec section 6: track() must never raise into the caller + # Given tracking_flagsmith_client.track_event.side_effect = RuntimeError("boom") provider = FlagsmithProvider(tracking_flagsmith_client) - # When / Then - no error raised + # When / Then provider.track("purchase") @@ -523,7 +520,7 @@ def test_track_delegates_to_client(tracking_flagsmith_client: MagicMock) -> None ), ) - # Then - value is first-class, attributes become metadata + # Then tracking_flagsmith_client.track_event.assert_called_once_with( "purchase", identifier="user-123", @@ -553,7 +550,7 @@ def test_track_with_minimal_args(tracking_flagsmith_client: MagicMock) -> None: def test_track_attributes_pass_through_as_metadata( tracking_flagsmith_client: MagicMock, ) -> None: - # Given - attributes are metadata verbatim; details.value is first-class + # Given provider = FlagsmithProvider(tracking_flagsmith_client) # When @@ -581,13 +578,13 @@ def test_track_non_numeric_value_is_dropped_with_warning( # Given provider = FlagsmithProvider(tracking_flagsmith_client) - # When - value is typed float|None but nothing enforces it at runtime + # When provider.track( "checkout", tracking_event_details=TrackingEventDetails(value="99.77"), # type: ignore[arg-type] ) - # Then - sent without the value + # Then tracking_flagsmith_client.track_event.assert_called_once_with( "checkout", identifier=None, @@ -600,7 +597,7 @@ def test_track_non_numeric_value_is_dropped_with_warning( def test_track_extracts_traits_from_context( tracking_flagsmith_client: MagicMock, ) -> None: - # Given - nested traits take precedence over flat attributes (same rule as _get_flags) + # Given provider = FlagsmithProvider(tracking_flagsmith_client) # When @@ -636,7 +633,7 @@ def test_track_drops_reserved_dollar_names( provider.track("$flag_exposure") provider.track("$anything") - # Then - warned and dropped, never sent to the SDK + # Then tracking_flagsmith_client.track_event.assert_not_called() tracking_flagsmith_client.track_exposure_event.assert_not_called() @@ -691,7 +688,7 @@ def test_resolve_identity_flag_with_variant_has_experiment_metadata( ) # Then - assert result.reason == Reason.TARGETING_MATCH + assert result.reason == Reason.SPLIT assert result.variant == "treatment" assert result.flag_metadata == { "enabled": True, @@ -705,7 +702,7 @@ def test_resolve_identity_flag_with_variant_has_experiment_metadata( def test_resolve_boolean_details_disabled_flag_has_disabled_reason( mock_flagsmith_client: MagicMock, ) -> None: - # Given - the boolean-as-enabled path resolves disabled flags today + # Given key = "my_feature" mock_flagsmith_client.get_environment_flags.return_value = Flags( {key: Flag(feature_id=1, feature_name=key, enabled=False, value=None)} @@ -723,7 +720,7 @@ def test_resolve_boolean_details_disabled_flag_has_disabled_reason( def test_resolve_flagsmith_default_flag_metadata_has_no_feature_id( mock_flagsmith_client: MagicMock, ) -> None: - # Given - DefaultFlag has no feature_id and no variant attribute + # Given key = "my_feature" mock_flagsmith_client.get_environment_flags.return_value = Flags( {key: DefaultFlag(enabled=True, value="foo")} @@ -757,14 +754,14 @@ def test_resolve_in_offline_mode_has_stale_reason( evaluation_context=EvaluationContext(targeting_key="user-1"), ) - # Then - offline data must not read as a fresh targeting match + # Then assert result.reason == Reason.STALE def test_resolve_object_details_parsed_json_carries_reason_and_metadata( mock_flagsmith_client: MagicMock, ) -> None: - # Given - the JSON-parse branch is the third resolving branch + # Given key = "my_feature" mock_flagsmith_client.get_environment_flags.return_value = Flags( {key: Flag(feature_id=3, feature_name=key, enabled=True, value='{"a": 1}')} @@ -805,7 +802,7 @@ def test_transient_attribute_maps_to_transient_identity( ), ) - # Then - transient is a directive, not a trait + # Then mock_flagsmith_client.get_identity_flags.assert_called_once_with( identifier="user-1", traits={"plan": "pro"}, transient=True ) @@ -814,7 +811,7 @@ def test_transient_attribute_maps_to_transient_identity( def test_nested_trait_named_transient_is_kept( mock_flagsmith_client: MagicMock, ) -> None: - # Given - only the flat `transient` key is a directive + # Given key = "key" mock_flagsmith_client.get_identity_flags.return_value = Flags( {key: Flag(feature_id=1, feature_name=key, enabled=True, value="foo")} @@ -859,7 +856,7 @@ def test_exposure_with_explicit_variant_sends_as_rendered( ), ) - # Then - no flag resolution; remaining attributes become metadata + # Then tracking_flagsmith_client.track_exposure_event.assert_called_once_with( feature_name="my_exp", identifier="user-1", @@ -890,7 +887,7 @@ def test_exposure_without_flag_key_is_dropped( def test_exposure_without_targeting_key_is_skipped( tracking_flagsmith_client: MagicMock, ) -> None: - # Given - exposures attribute to the OF context, never ambient state + # Given provider = FlagsmithProvider(tracking_flagsmith_client) # When @@ -931,7 +928,7 @@ def test_variantless_exposure_resolves_flag_and_sends_variant( tracking_event_details=TrackingEventDetails(attributes={"flag_key": "my_exp"}), ) - # Then - resolution honors the transient directive + # Then tracking_flagsmith_client.get_identity_flags.assert_called_once_with( identifier="user-1", traits={}, transient=True ) @@ -967,7 +964,7 @@ def test_variantless_exposure_resolves_flag_and_sends_variant( def test_variantless_exposure_guard_chain_skips( tracking_flagsmith_client: MagicMock, flag ) -> None: - # Given - JS guard chain: real Flag, enabled, has variant + # Given tracking_flagsmith_client.get_identity_flags.return_value = Flags({"my_exp": flag}) provider = FlagsmithProvider(tracking_flagsmith_client) @@ -985,11 +982,11 @@ def test_variantless_exposure_guard_chain_skips( def test_variantless_exposure_missing_flag_is_skipped( tracking_flagsmith_client: MagicMock, ) -> None: - # Given - no default_flag_handler: get_flag raises + # Given tracking_flagsmith_client.get_identity_flags.return_value = Flags({}) provider = FlagsmithProvider(tracking_flagsmith_client) - # When / Then - no error raised, no exposure recorded + # When / Then provider.track( EXPOSURE_TRACKING_EVENT, evaluation_context=EvaluationContext(targeting_key="user-1"), @@ -1005,7 +1002,7 @@ def test_variantless_exposure_client_error_is_swallowed( tracking_flagsmith_client.get_identity_flags.side_effect = FlagsmithClientError("") provider = FlagsmithProvider(tracking_flagsmith_client) - # When / Then - no error raised + # When / Then provider.track( EXPOSURE_TRACKING_EVENT, evaluation_context=EvaluationContext(targeting_key="user-1"), @@ -1017,7 +1014,7 @@ def test_variantless_exposure_client_error_is_swallowed( def test_exposure_is_noop_when_events_disabled( mock_flagsmith_client: MagicMock, ) -> None: - # Given - no _event_processor: must not fetch flags or persist identities + # Given provider = FlagsmithProvider(mock_flagsmith_client) # When From 2682781b7903c8c45750630bd11f023353815992 Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 4 Aug 2026 15:37:18 +0200 Subject: [PATCH 10/16] test: pin the cross-SDK wire contract with literal assertions Signed-off-by: wadii --- tests/test_cross_sdk_contract.py | 100 +++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 tests/test_cross_sdk_contract.py diff --git a/tests/test_cross_sdk_contract.py b/tests/test_cross_sdk_contract.py new file mode 100644 index 0000000..8e39eda --- /dev/null +++ b/tests/test_cross_sdk_contract.py @@ -0,0 +1,100 @@ +"""Cross-SDK wire contract: these literals are shared with the JS provider +and the analytics pipeline — changing them here means changing them there.""" + +from unittest.mock import MagicMock, create_autospec + +from flagsmith import Flagsmith +from flagsmith.models import Flag, Flags +from openfeature.evaluation_context import EvaluationContext +from openfeature.flag_evaluation import FlagEvaluationDetails, FlagType, Reason +from openfeature.hook import HookContext +from openfeature.track import TrackingEventDetails + +from openfeature_flagsmith.hooks import FlagsmithExposureHook +from openfeature_flagsmith.provider import FlagsmithProvider +from openfeature_flagsmith.tracking import EXPOSURE_TRACKING_EVENT + + +def test_exposure_tracking_event_name() -> None: + assert EXPOSURE_TRACKING_EVENT == "feature_flag.exposure" + + +def test_track_reads_snake_case_exposure_attribute_keys() -> None: + # Given + client = create_autospec(Flagsmith, instance=True) + client._event_processor = MagicMock() + provider = FlagsmithProvider(client) + + # When + provider.track( + "feature_flag.exposure", + evaluation_context=EvaluationContext(targeting_key="user-1"), + tracking_event_details=TrackingEventDetails( + attributes={"flag_key": "exp", "variant": "arm-a"} + ), + ) + + # Then + client.track_exposure_event.assert_called_once_with( + feature_name="exp", + identifier="user-1", + value="arm-a", + traits=None, + metadata=None, + ) + + +def test_hook_emits_contract_attribute_keys() -> None: + # Given + provider = create_autospec(FlagsmithProvider, instance=True) + hook = FlagsmithExposureHook(provider) + + # When + hook.after( + hook_context=HookContext( + flag_key="exp", + flag_type=FlagType.STRING, + default_value="control", + evaluation_context=EvaluationContext(targeting_key="user-1"), + ), + details=FlagEvaluationDetails( + flag_key="exp", value="v", variant="arm-a", reason=Reason.SPLIT + ), + hints={}, + ) + + # Then + event_name, _, details = provider.track.call_args.args + assert event_name == "feature_flag.exposure" + assert details.attributes == {"flag_key": "exp", "variant": "arm-a"} + + +def test_experiment_flag_metadata_keys() -> None: + # Given + client = create_autospec(Flagsmith, instance=True) + client.get_identity_flags.return_value = Flags( + { + "exp": Flag( + feature_id=1, + feature_name="exp", + enabled=True, + value="v", + variant="arm-a", + ) + } + ) + provider = FlagsmithProvider(client) + + # When + result = provider.resolve_string_details( + "exp", "control", EvaluationContext(targeting_key="user-1") + ) + + # Then + assert result.flag_metadata == { + "enabled": True, + "featureId": 1, + "experiment.arm": "arm-a", + "experiment.active": True, + "experiment.unit": "user", + } From d070cb49b0b23f9cad406aae34c7ff6cd15a111d Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 4 Aug 2026 16:50:23 +0200 Subject: [PATCH 11/16] feat: remove ia slop Signed-off-by: wadii --- openfeature_flagsmith/hooks.py | 23 +++++++------------- openfeature_flagsmith/provider.py | 35 ++++++++++++------------------- openfeature_flagsmith/tracking.py | 8 ++----- 3 files changed, 22 insertions(+), 44 deletions(-) diff --git a/openfeature_flagsmith/hooks.py b/openfeature_flagsmith/hooks.py index 1ace98b..967d76d 100644 --- a/openfeature_flagsmith/hooks.py +++ b/openfeature_flagsmith/hooks.py @@ -19,9 +19,8 @@ def _is_split_reason(reason: typing.Union[str, Reason, None]) -> bool: - # The Flagsmith engine annotates reasons with k=v metadata - # ("SPLIT; weight=30"); compare the leading token so the gate keeps - # working once the API exposes annotated reasons. + # The engine annotates reasons ("SPLIT; weight=30"); compare the + # leading token. if reason is None: return False return str(reason).split(";", 1)[0].strip() == Reason.SPLIT.value @@ -29,9 +28,7 @@ def _is_split_reason(reason: typing.Union[str, Reason, None]) -> bool: class FlagsmithExposureHook(Hook): """ - Records a Flagsmith exposure as a side effect of a flag evaluation, so one - call both resolves the flag and marks the identity as exposed to its - variant — the OpenFeature equivalent of Flagsmith's ``get_experiment_flag``:: + Records a Flagsmith exposure as a side effect of a flag evaluation:: hook = FlagsmithExposureHook(provider) client.get_string_details( @@ -43,13 +40,8 @@ class FlagsmithExposureHook(Hook): Attaching the hook at a call site is the experiment declaration: evaluations without it never record exposures. Exposures only fire for - multivariate flags resolved with reason ``SPLIT`` (a percentage-split - assignment: enabled, identified, not offline; engine-annotated reason - strings like ``"SPLIT; weight=30"`` also match), and are deduped per - identity/flag/variant in a bounded, thread-safe LRU for the hook - instance's lifetime. - - Tracking is an experimental OpenFeature capability (spec section 6). + flags resolved with a variant and reason ``SPLIT``, deduped per + identity/flag/variant in a bounded, thread-safe LRU. """ def __init__( @@ -68,8 +60,7 @@ def after( details: FlagEvaluationDetails, hints: HookHints, ) -> None: - # Fully error-contained: an uncaught after-hook error flips the - # evaluation itself to ERROR in the OpenFeature SDK. + # An uncaught after-hook error would flip the evaluation to ERROR. try: variant = details.variant if not isinstance(variant, str): @@ -82,7 +73,7 @@ def after( ) return targeting_key = hook_context.evaluation_context.targeting_key - # json.dumps of the list avoids delimiter-collision false dedupes. + # json.dumps avoids delimiter collisions in the key. dedupe_key = json.dumps([targeting_key, details.flag_key, variant]) with self._lock: if dedupe_key in self._seen: diff --git a/openfeature_flagsmith/provider.py b/openfeature_flagsmith/provider.py index c2347e2..ade499b 100644 --- a/openfeature_flagsmith/provider.py +++ b/openfeature_flagsmith/provider.py @@ -56,9 +56,8 @@ def track( Route OpenFeature tracking events to Flagsmith. ``EXPOSURE_TRACKING_EVENT`` records a flag/variant exposure; any other - name becomes a plain Flagsmith event with ``details.value`` first-class. - No-ops unless the client was initialized with ``enable_events``. Never - raises (OpenFeature spec section 6): unexpected errors are logged. + name becomes a plain Flagsmith event. No-ops unless the client was + initialized with ``enable_events``. Never raises: errors are logged. """ try: self._track(tracking_event_name, evaluation_context, tracking_event_details) @@ -75,9 +74,8 @@ def _track( evaluation_context: typing.Optional[EvaluationContext], tracking_event_details: typing.Optional[TrackingEventDetails], ) -> None: - # Private-attribute pragmatism: the SDK has no public events-enabled - # signal yet. Checked up front so disabled events never trigger - # network side effects (identity persistence, flag fetches). + # The SDK has no public events-enabled signal; check first so + # disabled events cause no network side effects. if getattr(self._client, "_event_processor", None) is None: logger.debug( 'Flagsmith events are disabled; dropping tracking event "%s".', @@ -124,8 +122,7 @@ def _track( metadata=attributes or None, ) except ValueError: - # Raised when events are disabled (racing the check above) or the - # SDK rejects the event name. + # Events disabled (racing the check above) or name rejected. logger.debug( 'Flagsmith rejected tracking event "%s"; dropping it.', tracking_event_name, @@ -171,10 +168,8 @@ def _track_exposure( ) return - # Mirrors the SDK's get_experiment_flag guards, with the exposure - # attributed to the OF context's targeting key rather than any - # ambient identity. This resolution counts as a flag evaluation, - # exactly like get_experiment_flag itself. + # Mirrors the SDK's get_experiment_flag guards, attributed to the + # context's targeting key rather than any ambient identity. try: flag = self._client.get_identity_flags( identifier=identifier, @@ -320,7 +315,7 @@ def _build_details( return FlagResolutionDetails( value=value, reason=self._parse_reason(flag, evaluation_context), - # DefaultFlag has no `variant` attribute; never use bare access. + # DefaultFlag has no `variant` attribute. variant=getattr(flag, "variant", None), flag_metadata=self._build_flag_metadata(flag), ) @@ -332,15 +327,12 @@ def _parse_reason( return Reason.DEFAULT if not flag.enabled: return Reason.DISABLED - # Offline documents may be arbitrarily old; the exposure hook treats - # anything but SPLIT as not fresh enough to record. + # Offline documents may be arbitrarily old. if getattr(self._client, "offline_mode", False): return Reason.STALE if evaluation_context.targeting_key: - # Engine taxonomy: a variant means a multivariate percentage-split - # assignment (SPLIT); TARGETING_MATCH is reserved for segment - # matches, which the API can't distinguish from environment - # defaults yet, so no-variant identity evaluations stay coarse. + # A variant means a percentage-split assignment (SPLIT); + # TARGETING_MATCH is reserved for segment matches. if getattr(flag, "variant", None) is not None: return Reason.SPLIT return Reason.TARGETING_MATCH @@ -349,7 +341,7 @@ def _parse_reason( def _build_flag_metadata( self, flag: typing.Any ) -> typing.Dict[str, typing.Union[bool, int, str]]: - # Keys are byte-identical with the JS provider (vendor-council aligned). + # Keys are byte-identical with the JS provider. metadata: typing.Dict[str, typing.Union[bool, int, str]] = { "enabled": flag.enabled } @@ -369,8 +361,7 @@ def _extract_traits( if not evaluation_context or not evaluation_context.attributes: return None nested = evaluation_context.attributes.get("traits", {}) - # `traits` is unpacked below; the flat `transient` key is an - # evaluation directive (see _is_transient), not a trait. + # The flat `transient` key is an evaluation directive, not a trait. flat = { k: v for k, v in evaluation_context.attributes.items() diff --git a/openfeature_flagsmith/tracking.py b/openfeature_flagsmith/tracking.py index 717784c..9821ae0 100644 --- a/openfeature_flagsmith/tracking.py +++ b/openfeature_flagsmith/tracking.py @@ -4,10 +4,6 @@ """ Reserved tracking-event name for recording flag/variant exposures. -``client.track(EXPOSURE_TRACKING_EVENT, context, details)`` routes to -Flagsmith's exposure tracking instead of a plain analytics event. This is the -OpenFeature-facing name (identical across Flagsmith OpenFeature providers); on -the wire the Flagsmith SDK emits the ``$flag_exposure`` system event. - -Tracking is an experimental OpenFeature capability (spec section 6). +OpenFeature-facing name, identical across Flagsmith providers; on the wire +the SDK emits the ``$flag_exposure`` system event. """ From d4f42f6b31a20f1461923283e9c67c1380c9d9fe Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 4 Aug 2026 17:38:53 +0200 Subject: [PATCH 12/16] fix: warn and drop exposures carrying a non-string variant Signed-off-by: wadii --- openfeature_flagsmith/provider.py | 7 +++++++ tests/test_provider.py | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/openfeature_flagsmith/provider.py b/openfeature_flagsmith/provider.py index ade499b..3782432 100644 --- a/openfeature_flagsmith/provider.py +++ b/openfeature_flagsmith/provider.py @@ -150,6 +150,13 @@ def _track_exposure( EXPOSURE_TRACKING_EVENT, ) return + if variant is not None and not isinstance(variant, str): + logger.warning( + '"%s" requires a string "variant" attribute when provided;' + " dropping exposure event.", + EXPOSURE_TRACKING_EVENT, + ) + return if not identifier: logger.info( 'Exposure for "%s" skipped: no targeting_key in the evaluation' diff --git a/tests/test_provider.py b/tests/test_provider.py index a72299c..fc11da1 100644 --- a/tests/test_provider.py +++ b/tests/test_provider.py @@ -884,6 +884,26 @@ def test_exposure_without_flag_key_is_dropped( tracking_flagsmith_client.track_exposure_event.assert_not_called() +def test_exposure_with_non_string_variant_is_dropped( + tracking_flagsmith_client: MagicMock, +) -> None: + # Given + provider = FlagsmithProvider(tracking_flagsmith_client) + + # When + provider.track( + EXPOSURE_TRACKING_EVENT, + evaluation_context=EvaluationContext(targeting_key="user-1"), + tracking_event_details=TrackingEventDetails( + attributes={"flag_key": "my_exp", "variant": 123} + ), + ) + + # Then + tracking_flagsmith_client.track_exposure_event.assert_not_called() + tracking_flagsmith_client.get_identity_flags.assert_not_called() + + def test_exposure_without_targeting_key_is_skipped( tracking_flagsmith_client: MagicMock, ) -> None: From 8e81177520318dff6b22a846abd6989b02eef747 Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 4 Aug 2026 17:38:53 +0200 Subject: [PATCH 13/16] ci: run the test suite against the minimum supported flagsmith Signed-off-by: wadii --- .github/workflows/pytest.yml | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 687ec41..dfc7e44 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -37,3 +37,28 @@ jobs: - name: Run Tests run: poetry run pytest + + test-sdk-floor: + runs-on: ubuntu-latest + name: Pytest against the minimum supported flagsmith version + + steps: + - name: Cloning repo + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + + - name: Install Dependencies + run: | + python -m pip install --upgrade pip + pip install poetry + poetry install --no-root + poetry run pip install flagsmith==5.5.0 + + - name: Run Tests + run: poetry run pytest From 59ec7b66a0467da56014ebde85ec6fba07eec2fc Mon Sep 17 00:00:00 2001 From: wadii Date: Thu, 6 Aug 2026 14:54:45 +0200 Subject: [PATCH 14/16] refactor: drop the hook dedupe LRU in favor of SDK-side deduplication Signed-off-by: wadii --- README.md | 2 +- openfeature_flagsmith/hooks.py | 30 ++----------- tests/test_hooks.py | 77 ---------------------------------- 3 files changed, 5 insertions(+), 104 deletions(-) diff --git a/README.md b/README.md index fd551fb..daa82a4 100644 --- a/README.md +++ b/README.md @@ -104,7 +104,7 @@ details = of_client.get_string_details( ) ``` -The hook records an exposure only when the flag resolved with a variant and reason `SPLIT` — a multivariate percentage-split assignment (enabled, identified, not offline) — and dedupes per identity/flag/variant for the hook instance's lifetime (bounded, thread-safe). +The hook records an exposure only when the flag resolved with a variant and reason `SPLIT` — a multivariate percentage-split assignment (enabled, identified, not offline). Repeated evaluations are safe: duplicate exposures are deduplicated downstream. **2. Explicit `track()`.** Use the reserved `feature_flag.exposure` event name when you need to record an exposure decoupled from evaluation: diff --git a/openfeature_flagsmith/hooks.py b/openfeature_flagsmith/hooks.py index 967d76d..6f64b15 100644 --- a/openfeature_flagsmith/hooks.py +++ b/openfeature_flagsmith/hooks.py @@ -1,8 +1,5 @@ -import json import logging -import threading import typing -from collections import OrderedDict from openfeature.flag_evaluation import FlagEvaluationDetails, Reason from openfeature.hook import Hook, HookContext, HookHints @@ -15,8 +12,6 @@ logger = logging.getLogger(__name__) -DEFAULT_MAX_DEDUPE_ENTRIES = 10_000 - def _is_split_reason(reason: typing.Union[str, Reason, None]) -> bool: # The engine annotates reasons ("SPLIT; weight=30"); compare the @@ -40,19 +35,12 @@ class FlagsmithExposureHook(Hook): Attaching the hook at a call site is the experiment declaration: evaluations without it never record exposures. Exposures only fire for - flags resolved with a variant and reason ``SPLIT``, deduped per - identity/flag/variant in a bounded, thread-safe LRU. + flags resolved with a variant and reason ``SPLIT``; duplicate exposures + are deduplicated downstream. """ - def __init__( - self, - provider: "FlagsmithProvider", - max_dedupe_entries: int = DEFAULT_MAX_DEDUPE_ENTRIES, - ) -> None: + def __init__(self, provider: "FlagsmithProvider") -> None: self._provider = provider - self._max_dedupe_entries = max_dedupe_entries - self._seen: "OrderedDict[str, None]" = OrderedDict() - self._lock = threading.Lock() def after( self, @@ -67,21 +55,11 @@ def after( return if not _is_split_reason(details.reason): logger.debug( - 'Exposure for "%s" skipped: resolution reason is %s, not' " SPLIT.", + 'Exposure for "%s" skipped: resolution reason is %s, not SPLIT.', details.flag_key, details.reason, ) return - targeting_key = hook_context.evaluation_context.targeting_key - # json.dumps avoids delimiter collisions in the key. - dedupe_key = json.dumps([targeting_key, details.flag_key, variant]) - with self._lock: - if dedupe_key in self._seen: - self._seen.move_to_end(dedupe_key) - return - self._seen[dedupe_key] = None - while len(self._seen) > self._max_dedupe_entries: - self._seen.popitem(last=False) self._provider.track( EXPOSURE_TRACKING_EVENT, hook_context.evaluation_context, diff --git a/tests/test_hooks.py b/tests/test_hooks.py index d46269e..09969bf 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -1,4 +1,3 @@ -import threading from unittest.mock import MagicMock, create_autospec import pytest @@ -106,60 +105,6 @@ def test_hook_accepts_engine_annotated_split_reasons( mock_provider.track.assert_called_once() -def test_hook_dedupes_per_identity_flag_variant(mock_provider: MagicMock) -> None: - # Given - hook = FlagsmithExposureHook(mock_provider) - - # When - hook.after(hook_context=_hook_context(), details=_details(), hints={}) - hook.after(hook_context=_hook_context(), details=_details(), hints={}) - hook.after( - hook_context=_hook_context(targeting_key="user-2"), - details=_details(), - hints={}, - ) - hook.after( - hook_context=_hook_context(), details=_details(variant="control"), hints={} - ) - - # Then - assert mock_provider.track.call_count == 3 - - -def test_hook_dedupe_is_bounded_lru(mock_provider: MagicMock) -> None: - # Given - hook = FlagsmithExposureHook(mock_provider, max_dedupe_entries=2) - - # When - hook.after(hook_context=_hook_context("u1"), details=_details(), hints={}) - hook.after(hook_context=_hook_context("u2"), details=_details(), hints={}) - hook.after(hook_context=_hook_context("u3"), details=_details(), hints={}) - hook.after(hook_context=_hook_context("u1"), details=_details(), hints={}) - - # Then - assert mock_provider.track.call_count == 4 - - -def test_hook_dedupe_key_is_collision_safe(mock_provider: MagicMock) -> None: - # Given - hook = FlagsmithExposureHook(mock_provider) - - # When - hook.after( - hook_context=_hook_context('user"1'), - details=_details(flag_key="exp"), - hints={}, - ) - hook.after( - hook_context=_hook_context("user"), - details=_details(flag_key='1", "exp'), - hints={}, - ) - - # Then - assert mock_provider.track.call_count == 2 - - def test_hook_swallows_provider_errors(mock_provider: MagicMock) -> None: # Given mock_provider.track.side_effect = RuntimeError("boom") @@ -169,28 +114,6 @@ def test_hook_swallows_provider_errors(mock_provider: MagicMock) -> None: hook.after(hook_context=_hook_context(), details=_details(), hints={}) -def test_hook_is_thread_safe(mock_provider: MagicMock) -> None: - # Given - hook = FlagsmithExposureHook(mock_provider) - - def fire(i: int) -> None: - hook.after( - hook_context=_hook_context(f"user-{i % 10}"), - details=_details(), - hints={}, - ) - - # When - threads = [threading.Thread(target=fire, args=(i,)) for i in range(100)] - for t in threads: - t.start() - for t in threads: - t.join() - - # Then - assert mock_provider.track.call_count == 10 - - def test_hook_end_to_end_records_exposure_through_openfeature() -> None: # Given client = create_autospec(Flagsmith, instance=True) From 940dda7d646ca224b86872248161d9a5c0e6cac4 Mon Sep 17 00:00:00 2001 From: wadii Date: Fri, 7 Aug 2026 14:05:29 +0200 Subject: [PATCH 15/16] feat: forward engine evaluation reasons from flagsmith >=6.2 Signed-off-by: wadii --- README.md | 2 +- openfeature_flagsmith/provider.py | 6 +- poetry.lock | 6 +- tests/test_hooks.py | 48 +++++++++++++++ tests/test_provider.py | 98 +++++++++++++++++++++++++++++++ 5 files changed, 155 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index daa82a4..814d20a 100644 --- a/README.md +++ b/README.md @@ -104,7 +104,7 @@ details = of_client.get_string_details( ) ``` -The hook records an exposure only when the flag resolved with a variant and reason `SPLIT` — a multivariate percentage-split assignment (enabled, identified, not offline). Repeated evaluations are safe: duplicate exposures are deduplicated downstream. +The hook records an exposure only when the flag resolved with a variant and reason `SPLIT` — a multivariate percentage-split assignment (enabled, identified, not offline). With `flagsmith` ≥6.2 resolution reasons come from the Flagsmith engine verbatim (e.g. `SPLIT; weight=30`); on older SDKs or APIs the provider infers them. Repeated evaluations are safe: duplicate exposures are deduplicated downstream. **2. Explicit `track()`.** Use the reserved `feature_flag.exposure` event name when you need to record an exposure decoupled from evaluation: diff --git a/openfeature_flagsmith/provider.py b/openfeature_flagsmith/provider.py index 3782432..d1a0f88 100644 --- a/openfeature_flagsmith/provider.py +++ b/openfeature_flagsmith/provider.py @@ -329,7 +329,7 @@ def _build_details( def _parse_reason( self, flag: typing.Any, evaluation_context: EvaluationContext - ) -> Reason: + ) -> typing.Union[str, Reason]: if flag.is_default: return Reason.DEFAULT if not flag.enabled: @@ -337,6 +337,10 @@ def _parse_reason( # Offline documents may be arbitrarily old. if getattr(self._client, "offline_mode", False): return Reason.STALE + # Engine reason, verbatim ("SPLIT; weight=30"), on flagsmith >=6.2. + engine_reason = getattr(flag, "reason", None) + if engine_reason is not None: + return engine_reason if evaluation_context.targeting_key: # A variant means a percentage-split assignment (SPLIT); # TARGETING_MATCH is reserved for segment matches. diff --git a/poetry.lock b/poetry.lock index 753b30c..16bae76 100644 --- a/poetry.lock +++ b/poetry.lock @@ -184,14 +184,14 @@ typing = ["typing-extensions (>=4.8) ; python_version < \"3.11\""] [[package]] name = "flagsmith" -version = "6.1.0" +version = "6.2.0" description = "Flagsmith Python SDK" optional = false python-versions = "<4,>=3.10" groups = ["main"] files = [ - {file = "flagsmith-6.1.0-py3-none-any.whl", hash = "sha256:c1adf56cd5cabcaf3fdd1b0dad8d3fc6e621a96bc2703b88c98ed9da91ead792"}, - {file = "flagsmith-6.1.0.tar.gz", hash = "sha256:f62e74f3aa2220702a60edc76e1eb7ebca46a2967da938455efa4d1642364f46"}, + {file = "flagsmith-6.2.0-py3-none-any.whl", hash = "sha256:2147c751461e8a056ad2a792105b9012938ee299b9108f92e457d396473aca1d"}, + {file = "flagsmith-6.2.0.tar.gz", hash = "sha256:be0c144016c3cd5ea5c23de86df1d96b97c71a98594cf2b9a31a775f75ee8b11"}, ] [package.dependencies] diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 09969bf..c3628c7 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -3,6 +3,7 @@ import pytest from flagsmith import Flagsmith from flagsmith.models import Flag, Flags +from flagsmith.version import __version__ as flagsmith_version from openfeature import api from openfeature.evaluation_context import EvaluationContext from openfeature.flag_evaluation import ( @@ -114,6 +115,53 @@ def test_hook_swallows_provider_errors(mock_provider: MagicMock) -> None: hook.after(hook_context=_hook_context(), details=_details(), hints={}) +@pytest.mark.skipif( + tuple(int(p) for p in flagsmith_version.split(".")[:2]) < (6, 2), + reason="flagsmith >=6.2 surfaces engine reasons", +) +def test_hook_end_to_end_fires_on_engine_annotated_reason() -> None: + # Given + client = create_autospec(Flagsmith, instance=True) + client._event_processor = MagicMock() + client.get_identity_flags.return_value = Flags( + { + "my_exp": Flag( + feature_id=1, + feature_name="my_exp", + enabled=True, + value="treatment-value", + variant="treatment", + reason="SPLIT; weight=30", + ) + } + ) + provider = FlagsmithProvider(client) + api.set_provider(provider) + try: + of_client = api.get_client() + hook = FlagsmithExposureHook(provider) + + # When + details = of_client.get_string_details( + "my_exp", + "control", + EvaluationContext(targeting_key="user-1"), + FlagEvaluationOptions(hooks=[hook]), + ) + + # Then + assert details.reason == "SPLIT; weight=30" + client.track_exposure_event.assert_called_once_with( + feature_name="my_exp", + identifier="user-1", + value="treatment", + traits=None, + metadata=None, + ) + finally: + api.clear_providers() + + def test_hook_end_to_end_records_exposure_through_openfeature() -> None: # Given client = create_autospec(Flagsmith, instance=True) diff --git a/tests/test_provider.py b/tests/test_provider.py index fc11da1..eacdf6c 100644 --- a/tests/test_provider.py +++ b/tests/test_provider.py @@ -4,6 +4,7 @@ from flagsmith import Flagsmith from flagsmith.exceptions import FlagsmithClientError from flagsmith.models import DefaultFlag, Flag, Flags +from flagsmith.version import __version__ as flagsmith_version from openfeature.evaluation_context import EvaluationContext from openfeature.exception import ( ErrorCode, @@ -642,6 +643,103 @@ def test_track_drops_reserved_dollar_names( # Reasons / variant / flag_metadata # --------------------------------------------------------------------------- +requires_engine_reasons = pytest.mark.skipif( + tuple(int(p) for p in flagsmith_version.split(".")[:2]) < (6, 2), + reason="flagsmith >=6.2 surfaces engine reasons", +) + + +@requires_engine_reasons +@pytest.mark.parametrize( + "engine_reason", + ["SPLIT; weight=50.0", "TARGETING_MATCH; segment=premium", "DEFAULT"], +) +def test_engine_reason_is_forwarded_verbatim( + mock_flagsmith_client: MagicMock, engine_reason: str +) -> None: + # Given + key = "my_feature" + mock_flagsmith_client.get_identity_flags.return_value = Flags( + { + key: Flag( + feature_id=1, + feature_name=key, + enabled=True, + value="v", + variant="treatment", + reason=engine_reason, + ) + } + ) + provider = FlagsmithProvider(mock_flagsmith_client) + + # When + result = provider.resolve_string_details( + key, + default_value="default", + evaluation_context=EvaluationContext(targeting_key="user-1"), + ) + + # Then + assert result.reason == engine_reason + + +@requires_engine_reasons +def test_disabled_reason_wins_over_engine_reason( + mock_flagsmith_client: MagicMock, +) -> None: + # Given + key = "my_feature" + mock_flagsmith_client.get_environment_flags.return_value = Flags( + { + key: Flag( + feature_id=1, + feature_name=key, + enabled=False, + value="v", + reason="DEFAULT", + ) + } + ) + provider = FlagsmithProvider(mock_flagsmith_client) + + # When + result = provider.resolve_boolean_details(key, default_value=True) + + # Then + assert result.reason == Reason.DISABLED + + +@requires_engine_reasons +def test_stale_reason_wins_over_engine_reason( + mock_flagsmith_client: MagicMock, +) -> None: + # Given + key = "my_feature" + mock_flagsmith_client.offline_mode = True + mock_flagsmith_client.get_identity_flags.return_value = Flags( + { + key: Flag( + feature_id=1, + feature_name=key, + enabled=True, + value="v", + reason="SPLIT; weight=50.0", + ) + } + ) + provider = FlagsmithProvider(mock_flagsmith_client) + + # When + result = provider.resolve_string_details( + key, + default_value="default", + evaluation_context=EvaluationContext(targeting_key="user-1"), + ) + + # Then + assert result.reason == Reason.STALE + def test_resolve_environment_flag_has_static_reason_and_metadata( mock_flagsmith_client: MagicMock, From b09d68505452baaf12579bcd4b72c710a709ab68 Mon Sep 17 00:00:00 2001 From: wadii Date: Fri, 7 Aug 2026 14:55:43 +0200 Subject: [PATCH 16/16] feat: map the engine DEFAULT reason to STATIC Signed-off-by: wadii --- README.md | 2 +- openfeature_flagsmith/provider.py | 13 +++++++++---- tests/test_provider.py | 32 ++++++++++++++++++++++++++++++- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 814d20a..23956a6 100644 --- a/README.md +++ b/README.md @@ -104,7 +104,7 @@ details = of_client.get_string_details( ) ``` -The hook records an exposure only when the flag resolved with a variant and reason `SPLIT` — a multivariate percentage-split assignment (enabled, identified, not offline). With `flagsmith` ≥6.2 resolution reasons come from the Flagsmith engine verbatim (e.g. `SPLIT; weight=30`); on older SDKs or APIs the provider infers them. Repeated evaluations are safe: duplicate exposures are deduplicated downstream. +The hook records an exposure only when the flag resolved with a variant and reason `SPLIT` — a multivariate percentage-split assignment (enabled, identified, not offline). With `flagsmith` ≥6.2 resolution reasons come from the Flagsmith engine (e.g. `SPLIT; weight=30`; the engine's `DEFAULT` maps to `STATIC`); on older SDKs or APIs the provider infers them. Repeated evaluations are safe: duplicate exposures are deduplicated downstream. **2. Explicit `track()`.** Use the reserved `feature_flag.exposure` event name when you need to record an exposure decoupled from evaluation: diff --git a/openfeature_flagsmith/provider.py b/openfeature_flagsmith/provider.py index d1a0f88..6d2686e 100644 --- a/openfeature_flagsmith/provider.py +++ b/openfeature_flagsmith/provider.py @@ -337,10 +337,15 @@ def _parse_reason( # Offline documents may be arbitrarily old. if getattr(self._client, "offline_mode", False): return Reason.STALE - # Engine reason, verbatim ("SPLIT; weight=30"), on flagsmith >=6.2. - engine_reason = getattr(flag, "reason", None) - if engine_reason is not None: - return engine_reason + client_reason = getattr(flag, "reason", None) + if client_reason is not None: + # The Flagsmith client's DEFAULT means the environment default + # state was served; OpenFeature reserves DEFAULT for the code + # default. + # TODO remove when https://github.com/Flagsmith/flagsmith-engine/issues/341 is addressed + if str(client_reason).split(";", 1)[0].strip() == "DEFAULT": + return Reason.STATIC + return client_reason if evaluation_context.targeting_key: # A variant means a percentage-split assignment (SPLIT); # TARGETING_MATCH is reserved for segment matches. diff --git a/tests/test_provider.py b/tests/test_provider.py index eacdf6c..8220aee 100644 --- a/tests/test_provider.py +++ b/tests/test_provider.py @@ -652,7 +652,7 @@ def test_track_drops_reserved_dollar_names( @requires_engine_reasons @pytest.mark.parametrize( "engine_reason", - ["SPLIT; weight=50.0", "TARGETING_MATCH; segment=premium", "DEFAULT"], + ["SPLIT; weight=50.0", "TARGETING_MATCH; segment=premium"], ) def test_engine_reason_is_forwarded_verbatim( mock_flagsmith_client: MagicMock, engine_reason: str @@ -684,6 +684,36 @@ def test_engine_reason_is_forwarded_verbatim( assert result.reason == engine_reason +@requires_engine_reasons +def test_engine_default_reason_maps_to_static( + mock_flagsmith_client: MagicMock, +) -> None: + # Given + key = "my_feature" + mock_flagsmith_client.get_identity_flags.return_value = Flags( + { + key: Flag( + feature_id=1, + feature_name=key, + enabled=True, + value="v", + reason="DEFAULT", + ) + } + ) + provider = FlagsmithProvider(mock_flagsmith_client) + + # When + result = provider.resolve_string_details( + key, + default_value="default", + evaluation_context=EvaluationContext(targeting_key="user-1"), + ) + + # Then + assert result.reason == Reason.STATIC + + @requires_engine_reasons def test_disabled_reason_wins_over_engine_reason( mock_flagsmith_client: MagicMock,