Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minimal duration support for Session replay #189

Open
4 tasks
marandaneto opened this issue Oct 7, 2024 · 8 comments
Open
4 tasks

Minimal duration support for Session replay #189

marandaneto opened this issue Oct 7, 2024 · 8 comments
Labels
enhancement New feature or request Session Replay

Comments

@marandaneto
Copy link
Member

marandaneto commented Oct 7, 2024

Description

https://posthog.com/docs/session-replay/how-to-control-which-sessions-you-record#minimum-duration

  • Android
  • iOS
  • Flutter
  • RN
@marandaneto marandaneto added enhancement New feature or request Session Replay labels Oct 7, 2024
@karntrehan
Copy link

@marandaneto are there any good first issues to contribute to? I guess I can pick this up as well but would need some discussions with the team on the implementation - i.e. = If this should be a separate layer, if this should use an existing layer, what should be the default duration length, etc..

Looking to start contributing to the Android / Flutter SDK / BE of PostHog as we have been using it extensively across products. :)

@marandaneto
Copy link
Member Author

Hello @karntrehan, I am happy to hear that and glad to guide you through the implementation as well :)
The configuration will be read from the API here

config.snapshotEndpoint = it["endpoint"] as? String
?: config.snapshotEndpoint
sessionReplayFlagActive = isRecordingActive(this.featureFlags ?: mapOf(), it)
config.cachePreferences?.setValue(SESSION_REPLAY, it)
// TODO:
// consoleLogRecordingEnabled -> Boolean or null
// networkPayloadCapture -> Boolean or null, can also be networkPayloadCapture={recordBody=true, recordHeaders=true}
// sampleRate, etc

Most likely we will add this config here too https://github.com/PostHog/posthog-android/blob/main/posthog-android/src/main/java/com/posthog/android/replay/PostHogSessionReplayConfig.kt
See the JS implementation https://github.com/PostHog/posthog-js/blob/8e7e1ea6bc95199d9cc031ea975847cbc9209f55/src/types.ts#L386
Look for minimumDurationMilliseconds, receivedMinimumDuration, and SESSION_RECORDING_MINIMUM_DURATION.
I think a draft PR with making this work is a good start, by checking the JS SDK, and then we see how to structure, and organize the code and so on, WDYT?

@karntrehan
Copy link

Thank you for the details.
Will pick this up sometime today / tomorrow and send across a draft PR.

@karntrehan
Copy link

Hie @marandaneto,
I can see that API key phc_QFbR1y41s5sxnNTZoyKG2NJo2RlsCIWkUfdpawgb40D is added to the sample code itself here. I hope that is not a concern. There are a bunch of other keys there as well. We could risk a DDoS? Not sure of the server side handling to prevent this.

Also, could you please configure a min time for this project and let me know? I have looked at the code and going to start making changes to the config first.

Thanks!

@marandaneto
Copy link
Member Author

@karntrehan the apiKey is considered public and not private, and the server side has DDoS protection but there's not much we can do about it since apps can be reverse-engineered and there's no way to protect such keys, people could send trash data to your project though but it's not something common.
It's easier if you create an organization and project for yourself (PostHog has a free tier so you don't need to pay anything until a certain limit and for sure you'll not hit such limits).

@marandaneto
Copy link
Member Author

I configured min. 1s for the project with API key phc_QFbR1y41s5sxnNTZoyKG2NJo2RlsCIWkUfdpawgb40D just in case you wanna test it quickly =)

@karntrehan
Copy link

@marandaneto Got it.

Are these DDoS protections also built in into the self-hosting offering? If not, we should talk about this in the comparision.

On the key being easy to reverse engineer, I somewhat agree. We can always use the secrets approach to make it slightly harder to guess. This would also promote people to do it at their end as well. Just an added security.

Also, the addition of the config helps. Thanks!

karntrehan added a commit to karntrehan/posthog-android that referenced this issue Oct 17, 2024
karntrehan added a commit to karntrehan/posthog-android that referenced this issue Oct 17, 2024
@karntrehan
Copy link

karntrehan commented Oct 17, 2024

@marandaneto I have for now created a commit for Adding minSessionDuration to config and update usage.

As a next steps I was adding a handling to the PostHogReplayIntegration but the uninstall function is not getting called when the app is pushed to the background or killed.

Do let me know if I am on the right track here: Deleting the recording if the time diff is less than min session time.

Or maybe add this check before the capture here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Session Replay
Projects
None yet
Development

No branches or pull requests

2 participants