posthog-ios icon indicating copy to clipboard operation
posthog-ios copied to clipboard

Respect session replay feature flag after /decide endpoint call

Open ioannisj opened this issue 1 year ago • 7 comments

Description

Ticket: https://posthoghelp.zendesk.com/agent/tickets/20809

Session recordings start as soon as the app launches, but feature flag evaluation is executed asynchronously. If the cached value is different that the value returned from /decide endpoint, then the recording may start (and not stop) when the flag evaluates to false

A possible fix is to mem-cache screenshots and skip processing until the /decide API has been successfully called - there are some memory footprint consideration here. We may need to use a different disk-backed queue, and drop or merge the queues once the flag evaluates?

  • [ ] iOS/Flutter
  • [ ] Android/Flutter
  • [ ] RN

ioannisj avatar Nov 25 '24 11:11 ioannisj

@marandaneto Is this relevant to Android/RN as well?

ioannisj avatar Nov 25 '24 11:11 ioannisj

Currently, in the absence of a cached value - which is common for fresh installs - session recordings start as soon as the app launches, but feature flag evaluation is executed asynchronously.

This is not the case, in the absence of a cached value, the recording won't start. sessionReplayFlagActive is only enabled if there was a cached value and it was enabled before.

Similarly, if the cached value is different that the value returned from /decide endpoint, this could lead to a similar effect

This is the current behaviour and this is the issue.

A possible fix is to cache screenshots and skip processing until the /decide API has been successfully called (with an option to stop recording and clear replayQueue if flag is evaluated to false)

Yes, but be aware of the memory footprint since queuing up lots of things might blow up people's apps. Also if preloadFeatureFlags is disabled, and people don't call reloadFeatureFlags manually or call it much later, everything will be delayed and the memory footprint will be even worsened. I don't think we should clear replayQueue but rather a memory queue, the replayQueue might have items from previous sessions that were supposed to be recorded anyway.

@marandaneto Is this relevant to Android/RN as well?

Android yes, RN will "obey" the Android and iOS implementation

marandaneto avatar Nov 25 '24 12:11 marandaneto

This is not the case, in the absence of a cached value, the recording won't start. sessionReplayFlagActive is only enabled if there was a cached value and it was enabled before.

Yeah, true! my bad. Updated the description

ioannisj avatar Nov 25 '24 12:11 ioannisj

Consider the new remote config endpoint before doing this, might affect implementation

marandaneto avatar Feb 10 '25 10:02 marandaneto

Consider the new remote config endpoint before doing this, might affect implementation

@ioannisj is that still an issue since we migrated to remote config?

marandaneto avatar Apr 25 '25 11:04 marandaneto

I believe so since the async nature of fetching flags is still there (and from /decide endpoint as well). I think we need to figure out how we want to handle this until we get a response from /decide with new flag value. We could potentially just stopSessionRecording once the flags are re-evaluated to false but session replay is active, accepting the fact that a short part of the session recording will go through

ioannisj avatar Apr 25 '25 11:04 ioannisj

We could potentially just stopSessionRecording once the flags are re-evaluated to false but session replay is active, accepting the fact that a short part of the session recording will go through

this is how Android works already, and it was like this on iOS as well, did that change? Once remote config/flags are executed/revalued, we either start or stop the recording depending on the result (start if it was disabled before, stop if it was enabled but should not).

A possible fix is to mem-cache screenshots and skip processing until the /decide API has been successfully called - there are some memory footprint consideration here

This is the issue why I prefer not to do the memory caching here.

marandaneto avatar Apr 25 '25 12:04 marandaneto