feat: surface experiment metadata on flags and add event tracking - #94
Conversation
|
@themis-blindfold review |
⚖️ Themis review: 🟠 Fix before mergeThe experiment metadata and event batching largely align with the established SDK behaviour, but the explicit-user convenience path can reuse a prior identity's stored flag before recording an exposure. That would misattribute an experiment assignment when an application switches users. CI is still running; only the title check has completed.
🟠 Majors
📝 Walkthrough
🧪 How to verify
Product take: Experiment tracking is a solid capability improvement, but incorrect assignment attribution would undermine experiment results for apps that switch users. 🧭 Assumptions & unverified claimsNo unverified assumptions or claims. Experiments need the right person in the right bucket · reviewed at 56cd56c |
56cd56c to
a1cdd64
Compare
|
@themis-blindfold review |
⚖️ Themis review: 🧹 Ship it, nits insideThe experiment metadata and opt-in event pipeline match the intended behaviour, including identity-specific reloads and experiment-aware exposure deduplication. One README snippet needs a null check before it can be copied into a null-safe Dart app. CI is still running; the local Flutter SDK was unavailable for the targeted checks.
🧹 Nits
📝 Walkthrough
🧪 How to verify
Product take: A solid experimentation capability: clients can report enrolment-aware exposures and conversions without changing ordinary flag reads. 🧭 Assumptions & unverified claimsNo unverified assumptions or claims. A small null check, and the experiment instructions are ready for their close-up · reviewed at a1cdd64 |
|
@themis-blindfold review |
⚖️ Themis review: 🟠 Fix before mergeThe experiment metadata and event pipeline are broadly covered, and the completed publish, analysis, and test checks passed. One lifecycle race breaks the documented guarantee that awaiting
🟠 Majors
⚖️ Acknowledged
📝 Walkthrough
🧪 How to verify
Product take: This is a solid experimentation capability, but teardown-sensitive apps could lose attribution by trusting the documented flush completion signal. That makes the lifecycle guarantee worth fixing before release. 🧭 Assumptions & unverified claimsNo unverified assumptions or claims. Events are patient; app teardown generally is not. · reviewed at a1cdd64 |
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Brings the Flutter SDK to parity with JS (Flagsmith/flagsmith-js-client#420) and Python (Flagsmith/flagsmith-python-client#249) for experimentation. Core sends
metadata.experiment { id, name, in_experiment }on/identitiesflags while the feature's experiment is running (Flagsmith/flagsmith#8532).Flag.variant,Flag.reasonandFlag.experiment { id, name, inExperiment }, lifted frommetadata.experiment.nullon older servers and on/flags. Round-trips through persistent storage.FlagsmithConfig(enableEvents: true):EventProcessorbatches to{eventsURI}v1/events, flushes everyeventsFlushInterval(10 s) or ateventsMaxBuffer(1000), dedupes exposures per flush window, retries a failed POST once then drops. Never throws.getExperimentFlagresolves the flag and records one$flag_exposurewith the variant as value andmetadata.experiment_id, only whenflag.experiment.inExperimentis true. Skips are logged.trackEvent,trackExposureEvent,flushEvents;close()flushes best-effort. Event names starting with$are rejected.Release after Core (Flagsmith/flagsmith#8532) and Edge (Flagsmith/edge-api#719) are live. Against older servers no flag carries
experiment, so no exposure is ever recorded.How did you test this code?
flutter analyzeclean,flutter test162 passing: model parsing and round-trip,getExperimentFlaggating (enrolled, not enrolled, no metadata, disabled, missing, no identity, events off), dedupe, flush on interval / max buffer /flushEvents/close, retry then drop.