Skip to content

feat!: implement experimentation tracking - #37

Merged
Zaimwa9 merged 16 commits into
mainfrom
feat/experimentation
Aug 7, 2026
Merged

feat!: implement experimentation tracking#37
Zaimwa9 merged 16 commits into
mainfrom
feat/experimentation

Conversation

@Zaimwa9

@Zaimwa9 Zaimwa9 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Changes

Ports the experimentation surface of the JS OpenFeature provider (open-feature/js-sdk-contrib#1591) to the Python provider.

  • Resolutions now carry reason, variant and flag_metadata (enabled, featureId, and experiment.arm / experiment.active / experiment.unit for multivariate assignments). Multivariate percentage-split assignments resolve with reason SPLIT, aligned with the engine's reason taxonomy.
  • Implements the OpenFeature Tracking API: custom events are forwarded to the Flagsmith client, and the reserved feature_flag.exposure event records experiment exposures, with an explicit variant, or variant-less (the provider resolves the flag for the context's targeting key and applies the same guards as the SDK's get_experiment_flag).
  • Adds FlagsmithExposureHook, an opt-in after-hook that records an exposure when an evaluation resolves with a variant and reason SPLIT, deduped per identity/flag/variant in a bounded thread-safe LRU. Attaching the hook to a call site is the experiment declaration; nothing auto-exposes.
  • Supports transient identities via the "transient": True context attribute.
  • Fixes track() crashing with an uncaught TypeError on flagsmith >=5.4 (the SDK renamed track_event(identity_identifier=) to identifier=).
  • Documents tracking and experimentation in the README and fixes the previous tracking example, which imported an API removed from the SDK in 5.4.0.

How did you test this code?

  • 68 unit tests: autospec'd Flagsmith client (signature drift fails loudly), an end-to-end test through the real OpenFeature SDK, and cross-SDK wire-contract tests pinning the event name, attribute keys and metadata keys shared with the JS provider.
  • A local parity harness against a real Flagsmith environment (remote and local evaluation), running the same scenarios through the raw SDK and through the provider and diffing the wire: evaluation details match, exposure and custom events are byte-identical, and a 2000-identity sweep confirmed the configured rollout (60% in experiment split evenly across four variants, 40% excluded never firing exposures).

BREAKING CHANGE

The minimum supported flagsmith version is now 5.5. track() previously crashed with a TypeError on flagsmith >=5.4 due to the SDK's renamed track_event signature; it now calls the current signature and sends TrackingEventDetails.value as the first-class Flagsmith event value instead of metadata["value"].

Zaimwa9 added 10 commits August 3, 2026 17:42
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
@Zaimwa9
Zaimwa9 requested a review from a team as a code owner August 4, 2026 14:24
@Zaimwa9
Zaimwa9 requested review from khvn26 and removed request for a team August 4, 2026 14:24
@Zaimwa9
Zaimwa9 marked this pull request as draft August 4, 2026 14:28
@Zaimwa9

Zaimwa9 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis review: ✅ Ship it

Solid, well-guarded experimentation surface. The tracking/exposure logic is correct against the SDK's track_event, track_exposure_event, and get_identity_flags signatures (verified against flagsmith==6.1.0). The variant-less exposure path is intentionally stricter than the SDK's get_experiment_flag — it requires flag.variant is not None, where the SDK would fall back to flag.value — which is the right call for OpenFeature-layer experiment semantics. Error containment is correct: the outer track() catch-all prevents violations of the OpenFeature spec's "never raises" contract (section 6), and the hook's own try/except prevents an after-hook error from flipping the evaluation to ERROR. The _event_processor private-attribute check is acknowledged and pragmatic given the SDK's lack of a public events-enabled signal. The bounded LRU dedupe is thread-safe and collision-resistant (json.dumps of a list, not a delimiter join). CI passes across Python 3.10–3.14.

Area Score
🎯 Correctness 5/5
🧪 Test coverage 5/5
📐 Code quality 5/5
🚀 Product impact 4/5
📝 Walkthrough
  • Tracking router (provider.py:_track) — routes feature_flag.exposure to the exposure path, blocks $-prefixed names, and sends everything else as a plain Flagsmith event with value promoted to first-class.
  • Exposure tracking (provider.py:_track_exposure) — explicit-variant path sends as rendered; variant-less path mirrors the SDK's get_experiment_flag guards (enabled, isinstance(Flag), variant present) with transient support.
  • Reason/metadata enrichment (provider.py:_build_details, _parse_reason, _build_flag_metadata) — all resolution paths now return reason, variant, and flag_metadata (keys aligned with the JS provider: featureId, experiment.arm, experiment.active, experiment.unit).
  • Exposure hook (hooks.py) — opt-in after-hook that fires on SPLIT reason (including engine-annotated "SPLIT; weight=30"), deduped in a bounded thread-safe LRU, fully error-contained.
  • Transient identity support"transient": True in flat context attributes maps to transient=True on get_identity_flags; stripped from traits, preserved in nested traits dict.
  • Tests & contract — 68 tests including cross-SDK wire-contract assertions (event name, attribute keys, metadata keys), an end-to-end test through the real OpenFeature SDK, thread-safety, and LRU eviction/collision scenarios.
🧪 How to verify
  1. Run pytest tests/ — all 68 tests should pass.
  2. Verify the cross-SDK contract: pytest tests/test_cross_sdk_contract.py -v — literal assertions on event names, attribute keys, and metadata keys must match the JS provider.
  3. Verify the end-to-end hook flow: pytest tests/test_hooks.py::test_hook_end_to_end_records_exposure_through_openfeature -v — exercises the full OpenFeature SDK → provider → Flagsmith client chain.
  4. Verify thread safety: pytest tests/test_hooks.py::test_hook_is_thread_safe -v — 100 threads, 10 unique identities, exactly 10 track calls.
  5. Verify events-disabled no-op: pytest tests/test_provider.py::test_exposure_is_noop_when_events_disabled -v — ensures no get_identity_flags or track_exposure_event calls fire when events are off.

Automate: all covered by pytest tests/.

Product take: This closes the experimentation gap between the JS and Python OpenFeature providers. Users can now record experiment exposures via the hook (recommended), explicit track(), or the native SDK — all sharing the same event pipeline. The SPLIT-only guard and bounded dedupe make it safe to attach the hook broadly without over-reporting. A meaningful capability addition for teams running Flagsmith experiments from Python services.

🧭 Assumptions & unverified claims
  • The track_exposure_event and enable_events APIs are assumed to exist from flagsmith>=5.5.0 (the new minimum); only 6.1.0 was verified.
  • The _event_processor private attribute is assumed to remain the canonical signal for events-enabled in future flagsmith versions; the SDK has no public equivalent.
  • Cross-SDK wire compatibility (event names, attribute keys, metadata keys) is asserted via literals in test_cross_sdk_contract.py; the JS provider's actual values were not verified independently in this review.

Clean experimentation surface, correct guards, thorough tests — the rare PR where the reviewer's job is to confirm rather than correct. · reviewed at 2682781

Signed-off-by: wadii <wadii.zaim@flagsmith.com>
@Zaimwa9
Zaimwa9 force-pushed the feat/experimentation branch from 4012ab6 to d070cb4 Compare August 4, 2026 14:53
Comment thread openfeature_flagsmith/hooks.py Outdated
Zaimwa9 added 2 commits August 4, 2026 17:38
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
@Zaimwa9
Zaimwa9 marked this pull request as ready for review August 5, 2026 08:10
Comment thread openfeature_flagsmith/hooks.py Outdated
Comment thread .github/workflows/pytest.yml
Zaimwa9 added 2 commits August 6, 2026 14:54
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
@Zaimwa9
Zaimwa9 force-pushed the feat/experimentation branch from 39feaf6 to b8aa931 Compare August 7, 2026 14:22
khvn26
khvn26 previously approved these changes Aug 7, 2026

@khvn26 khvn26 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM bar 1 nit.

Comment thread openfeature_flagsmith/provider.py
Signed-off-by: wadii <wadii.zaim@flagsmith.com>
@Zaimwa9
Zaimwa9 force-pushed the feat/experimentation branch from b8aa931 to b09d685 Compare August 7, 2026 16:03
@Zaimwa9
Zaimwa9 requested a review from khvn26 August 7, 2026 16:07

@khvn26 khvn26 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Zaimwa9
Zaimwa9 merged commit 897350a into main Aug 7, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants