feat: v2 analytics and split testing #247
Conversation
# Conflicts: # flagsmith-core.ts
# Conflicts: # flagsmith-core.ts # lib/flagsmith-es/package.json # lib/flagsmith/package.json # lib/react-native-flagsmith/package.json
| @@ -523,6 +502,7 @@ const Flagsmith = class { | |||
| this.withTraits = {} | |||
| } | |||
| this.identity = userId; | |||
| this.events.map(this.trackEvent) | |||
There was a problem hiding this comment.
This would cause 1 API request per tracked event - is this intentional or batching not supported by the API?
Previously, identify would make at most 1 API call, whereas now it could make any number of calls
There was a problem hiding this comment.
It isn't batched no. These events are specific to conversions e.g. checkout so I don't imagine there'd be a lot
| * Only available for self hosted split testing analytics. | ||
| * Track a conversion event within your application, used for split testing analytics. | ||
| */ | ||
| trackEvent: (event: string) => Promise<void>; |
There was a problem hiding this comment.
I think it's safe to assume that events could be recorded at a much higher rate than flags are evaluated, so they should definitely be batched and flushed in the background. The current implementation is probably good enough to validate the idea in a PoC but I would exclude this method from this SDK's public/versioned API until we can implement event batching.
This method being async is also not ergonomic to use on event handlers for DOM events like clicking, hovering or navigating.
There was a problem hiding this comment.
I can't think of a case where we'd track events often, and even if they were it'd happen way less than flag evaluations since they'd occur potentially every render. The only usecase I can imagine is conversions e.g. checkout.
| evaluations: Object.keys(evaluations).map((feature_name)=>( | ||
| { | ||
| feature_name, | ||
| "identity_identifier": this.identity||null, |
There was a problem hiding this comment.
Unless I'm missing something, the current this.identity here could be different than the one when the event happened, for example if the user logged out/in before analytics were flushed.
The same applies to hasFeature below - haven't reviewed this in much detail so might be completely off
There was a problem hiding this comment.
Hmmm yeah I suppose they should be more of a snapshot, I'll have a think about this / look if there are any nasty race conditions
trackEventfunction, this will let applications record conversion events e.g. checkout and have them recorded in split testing analyticsTo opt in, this adds
splitTestingAnalyticsto flagsmith.init