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

When two tabs are open, the resulting session recording can merge the two tabs. #6522

Closed
rcmarron opened this issue Oct 18, 2021 · 14 comments
Closed
Labels
bug Something isn't working right

Comments

@rcmarron
Copy link
Contributor

Bug description

  • Open posthog up in two tabs
  • Switch back and forth between the two tabs
  • Watch the resulting recording and see a weird merged view

I think this is happening because we're sharing the session_id across tabs

image

Expected behavior

We'd have a separate session recording per tab

Thank you for your bug report – we love squashing them!

@rcmarron rcmarron added bug Something isn't working right session recording labels Oct 18, 2021
@rcmarron
Copy link
Contributor Author

After putting more thought in this this issue, I think there's a bit of complexity to this scenario. The simplest approach would be to create a new session_id for each new tab/window, and the recordings would be split by this ID and that would solve the playback issues.

However, it has a big problem: If we have multiple recordings happening for the same user at the same time (as we often would if recordings are split by tabs), then we would have no way to determine which recording shows X event happening. (the best that we could do is "check these X recordings to see that event")

I see two possible solutions of varying levels of work:

Option 1

I think a full/proper solution involves:

Having data in this form would allow us to definitively state which recording shows which events occurring. And we could build a UX where overlapping recordings makes sense (e.g. a button that says 'view this user's other windows' if there are overlapping recordings.)

From what I understand tackling #6005 and adding a session column to the events table is a significant undertaking. And it might be more work than it's worth right now.

Option 2 (easier, but has some issues)

We don't split sessions on new windows/tabs. Instead, we:

  • Get more aggressive about sending full snapshots (send them every-time we detect a new window/tab). This would help with the rendering issues in the screenshot above.
  • Send an event when we detect a new tab. This would allow us to display a clear indicator to the user that the recording has switched tabs/windows.

This solution seems less involved, but it doesn't handle a couple of cases:

  • If a user is on two separate devices or browsers at the same time. It would still create two recordings, and we could not tell you which one has a specific event. (this is the case today)
  • If changes are happening on two tabs at the same time, the experience is going to break. For example, if a user is working on one tab, and there is a clock on the other tab. Both tabs will be sending up session recording events to the same 'recording', and the renderer will fall apart. Also, if we're informing users about tab/window changes via some alert, we would constantly be doing that.

I'm sure there are other solutions here that I'm not thinking of too. Would love to hear any thoughts you might have @macobo @paolodamico @mariusandra @alexkim205

@mariusandra
Copy link
Collaborator

Can we send a tab_id with a recording? Just inside the snapshot metadata, not as a separate new database field. This would be a random uuid generated whenever a new tab loads, so similar to session_id, just not persisted to localStorage.

The player could then be smart enough to group recordings with a different tab_id and also play them in a different tab in the recorder. Perhaps we can even get fancy and have a split screen view a'la macOS mission control.

@paolodamico
Copy link
Contributor

I think the idea of the separate tab_id makes sense. Though I would see if we could send the ID in events too so we can identify in which tab the event occurred. Definitely out of my expertise but I would assume we can figure out materialisation into a separate column later, i.e. treat this as any other event property.

@rcmarron
Copy link
Contributor Author

Makes sense to me:

I'm thinking the work stages out as:

V1 (stop the CSS bugs)

  • Add tab_id (or probably window_id) to each session recording event
  • Ensure we take a full snapshot on each new window_id
  • The query for the recording list sends back the list of window_ids for each recording
  • If there are multiple windows in a recording, make it clear to the user in the list view (maybe using the same 'multi-recording button' as before)
  • When the player fetches snapshots, it queries based on both the session_id + window_id
  • Maybe add session_id + tab_id into the event properties (double check this approach with @macobo)

V2 (full support of multi-window recordings)

  • Some type of multi-view player for multiple windows
  • Clear indication of which window has which events.

@macobo
Copy link
Contributor

macobo commented Oct 20, 2021

My 2c: The original intent for session_id was to be unique per tab, but seems like something misfired. Perhaps there's a way to make that happen still?

@mariusandra
Copy link
Collaborator

That's a legitimately good complexity reduction 👍

Currently with a session_id + window_id combo we could anyway just capture stuff happening on the same device, not a session that was separately recorded on your phone... so we'd need to search for the user's other overlapping sessions anyway, if we want a good multi-tab playback experience. So splitting session_id and window_id doesn't get us far.

@rcmarron
Copy link
Contributor Author

My 2c: The original intent for session_id was to be unique per tab, but seems like something misfired. Perhaps there's a way to make that happen still?

Sticking with just session_id would definitely simplify things, and I love that. It looks like the change is pretty simple (store the session_id in session storage instead of local storage/cookies)

I still think it'd still be a big win to tie events to a session_id, so that we can definitively say 'X event happened in Y session recording'. As we move to using session recordings in diagnosing causes, this will be important. While the problem exists today (when a user is on 2 devices), when we move to recordings being split by tabs/windows, I think it will be exacerbated.

If it's as simple as simple as what @paolodamico mentioned above (add the session_id to the event properties and then we can extract that as a materialized column), then I'd say we at least start by tagging events with the info, and that sets us up to do the rest later. I think being backward compatible on the query is going to be a pain, so the more that we can get ahead of that issue the better.

@macobo do you have thoughts on the materialized column approach here? I don't know much about that.

@mariusandra
Copy link
Collaborator

Making session_id unique per tab, and adding it to event properties will let us show events that happened inside a specific recording, for the case when a user has multiple parallel sessions. We can also distinguish between frontend events and other backend events, and display them accordingly. So I'd give a go in adding the property.

This will require us to redefine a "session" as a recorded session, and not "all events by a user not more than 30min apart", like most competitors do, so that also LGTM.

@neilkakkar
Copy link
Collaborator

This is also very cool because then it aligns Paths to session recordings as well: We can get rid of the 30 min apart semantics there and defer to unique session_ids.

@rcmarron
Copy link
Contributor Author

Nice - @neilkakkar are paths calculated as being within a single session right now?

@paolodamico
Copy link
Contributor

Maybe missing something, and maybe this is just a naming thing, but I would advocate not to do the session_id for now. I think at some point we'll want to support session-based analytics and we may need to expose this to users for them to determine what's a session for them.

In terms of session-based analytics, if a user is operating on two separate tabs that is still pretty valid as a single session.

@mariusandra
Copy link
Collaborator

This issue will never end 😅.

It's a naming thing. We decided to rename "session" to not mean "any event anywhere for the user", but "events connected to this browsing session". Then in the player and other views we have a clear separation on what's part of the recording, and what are other events the user also performed at this time (other sessions in other tabs or devices, server events), which we'd query for and show with less emphasis.

You're worried calling this "browser tab session" a "session" will confuse users doing "session-based analytics" (that's just group by session_id, right?) and they'll get bad data, since different tabs/devices would normally be considered as part of one meta-session? Is this the case? Couple of our competitors do define "session" in the "browser tab session" fashion though.

@rcmarron
Copy link
Contributor Author

@paolodamico @alexkim205 and I just spent some time chatting about this one - a lot of nuance here!

The conclusion that we came to is that we're going to add window_id and session_id to both session recording events and normal events.

  • Session id: Spans windows/tabs and cycles after 30 minutes of inactivity. This is the same behavior that the session_id has today. We could eventually allow the customer to configure their definition of a session in posthog-js and other SDKs (how Amplitude does it)
  • Window id: Does not span tabs/windows (stored in SessionStorage) but persists on reloads. Also cycles with the session id.

A row in the user's "recording list" is based on the session_id, and the player experience will account for multiple overlapping windows designated by the window_id. A user can filter this list by the events with matching session_ids (e.g. show me all recordings where event X occurs), when opening a recording, we will use the window_id to make sure the events are shown on the appropriate playback experience (for the case of overlapping tabs)

Reasons we landed here:

  • Quickly closing a tab and re-opening it should not start a 'new recording' in the customer's eyes. So we need to track something at a higher level than the rrweb recording. Hence the session_id. At the same time, we need to clearly separate recording events, so that they don't overlap. Hence the window_id.
  • It allows us to definitively state that a specific event corresponds to a specific spot in a rrweb playback experience.
  • This approach should set us up well for session based analytics. We would be defining a session in the same way that amplitude does. It also does not stop us from deciding that we should define a session differently on the server if we want to go that route.

Alternative
The big alternative that we considered was to only have the window_id and push out any decisions regarding sessions. Drawbacks with that approach are:

  • For the user experience, would end up grouping based on window_ids that occurred close to each other to form a 'recording row' in the UI. These queries will be much faster if we can key off the session_id. Particularly queries that are looking for recordings with multiple events that potentially span window_ids (we'd be in a very similar spot to our previous sessions page).
  • The session recording event table already has the session_id column, so it would be a messy migration. And the idea of using the existing session_id column as the window_id sets us up for a world of confusion when we want to have proper session based analytics.

@clarkus I'd love to talk about what a playback experience that supports multiple tabs/windows could look like. I'm in favor of starting as simple as possible and moving from there.

@clarkus
Copy link
Contributor

clarkus commented Oct 22, 2021

@clarkus I'd love to talk about what a playback experience that supports multiple tabs/windows could look like. I'm in favor of starting as simple as possible and moving from there.

I'll have to think about that a bit. Given that a user can only focus on and trigger events in one tab at a time, it seems like we'd have to communicate tab switches or other context changes as promoted events. The question would be how do we do that in an obvious way. I'll think about it a bit. Happy to chat about it as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working right
Projects
None yet
Development

No branches or pull requests

6 participants