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

Releasing session recording EPIC #1846

Closed
macobo opened this issue Oct 13, 2020 · 7 comments
Closed

Releasing session recording EPIC #1846

macobo opened this issue Oct 13, 2020 · 7 comments
Assignees
Labels

Comments

@macobo
Copy link
Contributor

macobo commented Oct 13, 2020

This issue aggregates work to be done for releasing #149 (session recording) to our users or to be done after the release.

Current plan

Note the order may change.

  • Implement a session recording retention cronjob (running only on cloud)
  • Make sure we don't send unneccessary data in posthog.js when users have opted out: $pageleave events are sent when capture_pageview: false posthog-js#92 https://github.com/PostHog/posthog/issues/1885
  • (Done) Implement $session_id logic for $snapshot events
  • Save sessions outside of session recording retention period to S3 inside cronjob. Allow users to see sessions stored on S3
  • (In review) Enable persistent URLs
  • Convert retention cronjob to plugin (? - depends on our progress with plugins)
  • Update settings page & other pages with the appropriate toggles - will work with Paolo on the exact details

Older notes

To do before release

1. Data capture enabling logic

Users:

  1. Might not all want to turn this feature on (e.g. privacy concerns)
  2. Not all instances might have enough storage to support the feature
  3. Might only care about a subset of users (e.g. not marketing site, but only live app)

Other products have solved this problem by:

  • Forcing users to specify who to record (cohort)
  • Setting limits on how much to record (e.g. next 5000 sessions)

@jamesefhawkins I believe you had input here?

Own thoughts:

  • Release it disabled, put it into release notes, add info toggle next to "Play session" informing users to enable it in settings
  • Measure, reach out to customers who enabled it to get feedback on their thoughts re toggles this needs.

2. Data storage, retention logic

Currently we're storing the events as $snapshot events in posthog_event table, piggybacking a lot of the logic of posthog.js/api.

I did a bit of measurement on the data. The average $snapshot event weighs (for app.posthog.com) 1.7kb. Regular posthog.js web events on our domain are 1.07kb. Raw data available on demand ;)

Compared to normal events, we'll be capturing ~10x the data for session recordings on app.posthog.com than normally.

Due to this, it seems sensible to set up data retention limits.

Proposal: Create a autoinstalled posthog plugin which in a cronjob removes sessions recording older than $N days in pg/clickhouse. Default $N to 7.

Depends on plugin support getting merged.

cc @fuziontech - Does the above approach seem reasonable? I'd prefer to continue using events table due to being able to piggy-back a lot of our infra.

3. Storing session data on S3

The above-mentioned plugin could also have an optional config value S3_URL, where it will save "old" recordings.

Proposed naming: session_recording/v1/{distinct_id}/{timestamp}.json

We'd also need to store links to these inside posthog db - @fuziontech / @mariusandra can plugins use ph models?

4. Creating persistent/sharable URLs

To make session recording useful, recordings should be linkable - this way they can be shared within development or product teams as something interesting is discovered.

Proposal: /sessions/{distinct_id}/{timestamp}/recording shows the player

5. posthog.js session recording improvements

Status: Done

Currently posthog.js:

  1. Calls decide endpoint
  2. Iff decide endpoint says to record session, it loads session recording js asynchronously
  3. As recording code is loaded, only then is the session recorded.

This can lead to missing several seconds every pageload.

Proposed approach:

  1. If session recording has been enabled in the past for this domain, load (cached) session recording js.
  2. Start recording events into memory/localstorage as recording.js loads
  3. If decide endpoint says to record session, send saved events to posthog

6. Sessions page performance, events page readability

With session recording turned on, the sessions page has become slow due to the amount of data being loaded.

This can be fixed by not loading/showing $snapshot events by default on that page.

Related note from @Twixes (https://posthog.slack.com/archives/C0113360FFV/p1602674096139100)

We're now getting huge numbers of $snapshot events which makes the Event list hardly readable, could we maybe hide them? They're only for session recordings, is that right?

7. Cypress specs, posthog.js specs, documentation

After release

X. Improving player UX

There is a lot of metadata we could expose about the session: e.g. urls, locations, window sizes and more.

In addition, we should persist user settings in the player (speed, skip inactive).

We could also make it start the next session once the current one finishes - combined with #1835 or being able to jump from funnel => people who converted/dropped out it would make for an interesting experience.

@mariusandra I believe you had input here?

X. Make sure SVG-s are loaded properly

We have seen that graphs on posthog.js do not (yet) load correctly - investigate.

X. Minimizing network traffic in posthog.js

Currently, $snapshot events are sent along with normal events every time 5 snapshots have been triggered. This causes a lot of unnecessary network requests and we could do additional batching within our recording code to minimize this.

@macobo macobo self-assigned this Oct 13, 2020
@yakkomajuri
Copy link
Contributor

A couple quick points from me:

  • I played around a bit and there's more work to be done regarding blocking element recording (extending Added rr-block class to sensitive elements #1834)
  • We could add optOutSessionRecording to the posthog-js config
  • We could definitely consider a local rrweb event store that only gets sent to the server 10% of the time + if there's an exception

@timgl
Copy link
Collaborator

timgl commented Oct 13, 2020

I'd be in favour of using a similar system that we use for feature flags to decide who gets recorded, ie either filtering or a percentage of users. However I don't think this is blocking a soft release.

On the data side, there's three quick things we can do

  1. By default we capture a bunch of properties for each snapshot (browser, path, os, active feature flags etc etc). To reduce size we could remove all of those just for $snapshot events, cutting down data quite a bit
  2. We should also combine multiple rrweb events into a single posthog event using a separate queue in posthog-js. This will make the data more palatable too.
  3. We could look at compressing the dom representation using Lz string.

I think the minimum todos for a soft release are

  1. Reduce data a bit
  2. Create the retention plugin
  3. Blocking element recording as per @yakkomajuri

@macobo
Copy link
Contributor Author

macobo commented Oct 13, 2020

@yakkomajuri

I played around a bit and there's more work to be done regarding blocking element recording (extending #1834)
Did you mean in-app? Outside of the app, already made one PR upstream, waiting for feedback on it. Until that's in though we can just document rr-ignore.

We could add optOutSessionRecording to the posthog-js config
Good point. This matters w/ e.g. wanting to blacklist session recording on marketing site.

We could definitely consider a local rrweb event store that only gets sent to the server 10% of the time + if there's an exception
Agreed in abstract. This was somewhat covered in point 1 - hoping to get some feedback on this to see what our user needs are. 👍

@timgl

Agreed re reducing the data by cutting properties - it will cut the data need by 30-50%, which is significant.

@mariusandra
Copy link
Collaborator

2 and 3 - currently we don't have a working plugin system with scheduled jobs. We'll have it by the end of the week, but not yet. I can take into account whatever is needed. For sure I want it to have some access to call PH API commands so we could have plugins that insert events. If that will be via the HTTP API or via from posthog.models import Bla, remains to be seen.

Regarding sessions, I've been thinking for a while that having a dedicated session_id field makes some sense. It could be just a hash as I wrote here.

This would make all session queries faster, provide a convenient way to group events and enable deterministic session recording URLs. This would solve issue #6 as well.

In fact, I could implement this as a plugin that stores this on a property, though official support as a field in the events table might be a better choice.

Point nr x:

If you browse a site as a logged in user from both your phone and your computer (different user agents), it should show up as two separate sessions.

macobo added a commit to PostHog/posthog-js that referenced this issue Oct 13, 2020
Related to issue PostHog/posthog#1846

This also introduces given2 as a testing util to simplify some of the
testing.
macobo added a commit to PostHog/posthog-js that referenced this issue Oct 14, 2020
@paolodamico
Copy link
Contributor

On the topic of UX, I was thinking something along these lines (feel free to challenge):

  • By default session recording would be turned off. We would have prompts to activate it from the cohorts page, and maybe sessions page too, as @macobo mentioned.
  • When enabled, we would only capture sessions after an exception occurs (we can test the impact of this, might make sense to give users the option to turn this off) but my guess is it'll be helpful for almost everyone, and we can simplify config here.
  • To capture regular sessions, users would configure this from cohorts, by enabling session recording with a sampling rate (see proposal below).

On self-hosted
Emphasis on space on disk being used.
image

On cloud
Retention period based on user's plan.
image

Cohorts
This is how recording for regular sessions would be enabled (cohort-based and with a sampling rate). Please note this is just a skeleton proposal, I do think we need to address #1881 before changing this.

image

@macobo macobo mentioned this issue Oct 16, 2020
@macobo macobo added the epic label Oct 16, 2020
@macobo
Copy link
Contributor Author

macobo commented Oct 16, 2020

Thanks @paolodamico, this is great input and I like the design!

Some considerations:

  • I think we want to turning on all session recordings on small instances - cohorts require a lot of setup to get to the sweet spot. Maybe link to a "All users" cohort from settings page or allow setting a global capture percentage?
  • We have no way of detecting when errors occur currently - there's future work there to integrate with more platforms like sentry or roll our own error detection algorithm. I propose we leave this out right now.
  • Perhaps we should expose the recordings size in stats page instead?
  • It's possible to store recordings in S3 - in which case after 7 days we would not be "deleting" them but moving them to a different medium (with dirt cheap storage). The exact details on how we'll allow users to configure this will depend on if/how this will become a plugin.

Here's my current plan (updated after plugins epic/experience from last week):

  • Implement a session recording retention cronjob (running only on cloud)
  • Make sure we don't send unneccessary data in posthog.js when users have opted out: $pageleave events are sent when capture_pageview: false posthog-js#92 https://github.com/PostHog/posthog/issues/1885
  • Implement $session_id logic for $snapshot events
  • Save sessions outside of session recording retention period to S3 inside cronjob. Allow users to see sessions stored on S3
  • Enable persistent URLs
  • Convert retention cronjob to plugin (? - depends on our progress with plugins)
  • Update settings page & other pages with the appropriate toggles - will work with Paolo on the exact details

Note the order may change.

@macobo
Copy link
Contributor Author

macobo commented Nov 5, 2020

Session recording is now out! Note that it requires updating any npm-based posthog.js installations to >= 1.5.2 or 1.6.0

There is a bunch of things to do - I've created a whole host of issues and tagged them with session recording. Feel free to share any and all feedback in issues.

@macobo macobo closed this as completed Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants