-
Notifications
You must be signed in to change notification settings - Fork 168
feat: Add in-memory session persistence option #4124
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
base: main
Are you sure you want to change the base?
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
Hi @andrewatwood, thanks for your contribution! Having in-memory session storage would be a great addition, two concerns though: 1. Fallback strategy: From my understanding of your issue, you'd want to track users that have both cookies and local storage disabled and you'd want to use in memory session in that case. 2. Session sharing between rum and logs: To correlate RUM and Logs data, the two SDKs share the same storage entry and we want to have the same behavior with session in memory. We don't anticipate the changes needed to address those concerns to be too tricky, so if you want to have a go at it, we could provide additional support. |
|
Thanks for taking a look!
I'll take a timeboxed go at these changes just to see if it can get to resolution a little quicker. But it's really keeping the patch updated between major versions that may have changes that's the biggest effort for us. So, if those changes (including a new in-memory option 🤞) end up being rolled into changes for the next major version anyway, it's still solved on effectively the same timeline from our perspective. Appreciate your time, thanks again @bcaudan! |
|
Updated! Left the persistence array as optional to keep the change non-breaking. @bcaudan let me know if this is what you had in mind when you get some time, thanks! |
Motivation
A not-insignificant cohort (>5%) of users for the application I work on have their cookies and local storage disabled. Their sessions are short (~15s happy path) and self-contained, and the relevant context for any given session is nearly entirely limited to that individual single-page session, so long term persistence or multi-tab/window support is wholly unnecessary. However, tracking these sessions and any issues that my arise is still a requirement.
In-memory session storage for the RUM SDK allows us to bypass the requirement for enabled cookies or local storage, and have full access to the sessions originating from these users.
This has been tested extensively, and deployed successfully in production, with over 4M sessions using this persistence strategy in the last two weeks alone.
Changes
SessionPersistence.IN_MEMORY) that stores session data in memoryallowFallbackToLocalStorageflag for clarityTest instructions
On-device site datain Chrome)sessionPersistence: 'in-memory'I do likely need some help fleshing out any further e2e test coverage deemed necessary, as some of the test scripts seem to require more setup information than I am privy to.
Checklist