recording: do not rotate the session id for hybrid SDKs

[marandaneto]

💡 Motivation and Context

Relates to https://github.com/PostHog/posthog-js-lite/pull/307/files

💚 How did you test it?

📝 Checklist

• I reviewed the submitted code.
• I added tests to verify the changes.
• I updated the docs if needed.
• No breaking change or entry added to the changelog.

[ioannisj]

Consider moving these checks directly in PostHogSessionManager? We can encapsulate there and just add a guard isEnabled to each of its methods. This way we centralize the logic and avoid to remember adding this check every time we interact with session manager in the future.

[marandaneto]

Consider moving these checks directly in PostHogSessionManager? We can encapsulate there and just add a guard isEnabled to each of its methods. This way we centralize the logic and avoid to remember adding this check every time we interact with session manager in the future.

good point, done

[ioannisj]

Left a minor comment. LGTM

[ioannisj]

Is it safe to compute this once? Depends on when postHogSdkName is assigned, but after that it probably won't be changing value

// postHogSdkName will be set to eg posthog-react-native if not
private let isiOSNativeSdk: Bool = postHogSdkName == postHogiOSSdkName

[marandaneto]

The session manager is a singleton that is created before the SDK is initiated so in this case the only "safe" way is every time.
Before moving the check from PostHogSDK to here, I computed only once because I knew the new value was already assigned.
So sadly not.

[ioannisj]

Could be a lazy var that's calculated the first time it's accessed. If that won't work either then it means that some sessions may be cleared or rotated as well unintentionally

[marandaneto]

Yep, and that's what I want to avoid since the bug is exactly about that, so it's ok to pay the performance cost which is minimal, and if the bug is solved, I can look into making this better.