Show session recording durations in sessions list when available #4853

mariusandra · 2021-06-22T22:02:14Z

Changes

Show the "recording duration" as calculated by the time diff for $snapshot events, not just regular events like before.
More context in Inaccurate/unexpected session duration length (0 seconds) posthog-js#248

Testing locally with some random sessions it does bump a few "1sec" sessions to "~10sec" territory and add a few other minutes here and there:

Before:

After:

Checklist

All querysets/queries filter by Organization, by Team, and by User
Django backend tests
Jest frontend tests
Cypress end-to-end tests
Migrations are safe to run at scale (e.g. PostHog Cloud) – present proof if not obvious
Frontend/CSS is usable at 320px (iPhone SE) and decent at 360px (most phones)
Breaking changes are backwards-compatible. Ensure old/new frontend requests work with new/old backends, and vice versa.

timgl · 2021-06-23T11:12:00Z

This is a good solution but the original issue raised was about "Distribution of Session Lengths" in the sessions insight (internal link for example). I'd be harder to solve though as you'd need to do a query across both the events table and session recordings table.

Maybe the actual solution is trying to figure out why $pageleave doesn't get sent? I suspect it may have something to do with the fact that we're using that beacon to also send session recording data which is really big

mariusandra · 2021-06-23T11:30:21Z

There were several issues raised in the thread, including this one about incorrect durations in the sessions list. That's what the "more context in issue" bit above is referring to as well.

As for "Distribution of Session Lengths", there's no easy solution and what I replied in the thread still holds true: the best bet is to wait until we start showing a distribution of times between funnel steps.

Copying from the thread (with slight cleanup):

This will be soon possible with a funnel from pageview to pageleave for a specific URL. It’s already there for self-hosted users with postgres.

The “distribution of session lengths” table with a filter for a specific URL will probably be wrong. I think it’ll also consider it as a session if the user visited this URL twice, but then visited many other pages in between

It’s easy to get the full time spent in a session from a recording, but not so much for individual pages visited. Session recording events aren't tagged with page URLs and we'd have to parse a lot of json. Implementing it this way will require quite some dev time, has many unknowns (where do we store this? precalculation? …) and might not be worth it in the end.

Thus the best solution is still to wait a bit until we support this in funnels :)

paolodamico

A few minor comments, but code-wise looks good.

Not sure if displaying the session duration just relying on the recording size is the best approach here though. What if we didn't have a recording for the entirety of the session? I know for instance some users were conditionally turning session recording in certain pages only.

paolodamico · 2021-06-24T09:58:08Z

posthog/queries/sessions/session_recording.py

-            yield {"id": recording["session_id"], "viewed": recording["session_id"] in viewed}
+            if isinstance(recording["duration"], datetime.timedelta):
+                # postgres
+                recording_duration = recording["duration"].seconds


Might be better to use recording["duration"].total_seconds() in case the timedelta is not set with the seconds property?

oh django, you continue to surprise me :)

frontend/src/types.ts

paolodamico · 2021-06-24T10:03:55Z

frontend/src/scenes/sessions/SessionsView.tsx

@@ -211,7 +217,9 @@ export function SessionsView({ personIds, isPersonPage = false }: SessionsTableP
            </div>

            <ResizableTable
-                locale={{ emptyText: 'No Sessions on ' + dayjs(selectedDate).format('YYYY-MM-DD') }}
+                locale={{
+                    emptyText: selectedDate ? 'No Sessions on ' + selectedDate.format('YYYY-MM-DD') : 'No Sessions',


Nit: Date format would look better like this (and in line with the new tooltips & legends)

selectedDate.year() == dayjs().year() ? 'MMM D' : 'MMM D, YYYY'

cool! we should probably util this some day

paolodamico · 2021-06-24T10:05:07Z

frontend/src/scenes/sessions/SessionsView.tsx

@@ -104,6 +104,12 @@ export function SessionsView({ personIds, isPersonPage = false }: SessionsTableP
        {
            title: 'Session duration',
            render: function RenderDuration(session: SessionType) {
+                if (session.session_recordings.length > 0) {
+                    const sum = session.session_recordings
+                        .map(({ recording_duration }) => recording_duration)


Not sure if we need to do recording_duration || 0 in case it's null or undefined from the backend?

It should always have some value, but to be sure, I added a or 0 in the backend.

mariusandra · 2021-06-24T15:41:42Z

I added one fix to also update the session end time according to the recording duration, when it makes sense to do so.

Not sure if displaying the session duration just relying on the recording size is the best approach here though. What if we didn't have a recording for the entirety of the session? I know for instance some users were conditionally turning session recording in certain pages only.

Well, we do Math.max(duration_from_recordings, duration_from_events), so this should be fine. Worst case, nothing changes. Best case, the length will be a bit longer.

paolodamico

🚢 !

paolodamico · 2021-06-25T08:30:10Z

Unrelated failing tests

mariusandra added 2 commits June 22, 2021 23:59

show session recording durations in sessions list

a05e592

always take the bigger unit

3f98139

mariusandra requested review from macobo and timgl June 22, 2021 22:02

mariusandra mentioned this pull request Jun 22, 2021

Inaccurate/unexpected session duration length (0 seconds) PostHog/posthog-js#248

Closed

mariusandra marked this pull request as draft June 22, 2021 22:08

mariusandra added 3 commits June 23, 2021 09:19

fix tests

75fb22f

fix sessions frontend old type errors

9d9c1f9

support both postgres and clickhouse time ranges

0d87130

mariusandra marked this pull request as ready for review June 23, 2021 08:10

mariusandra requested a review from paolodamico June 23, 2021 08:27

paolodamico approved these changes Jun 24, 2021

View reviewed changes

mariusandra added 3 commits June 24, 2021 17:05

implement feedback

258f3d8

Merge branch 'master' into session-recording-duration

08d1d8f

use the session length to calculate the end time

d61674d

mariusandra requested a review from paolodamico June 24, 2021 15:41

paolodamico approved these changes Jun 25, 2021

View reviewed changes

Merge branch 'master' into session-recording-duration

70c44ee

paolodamico merged commit 11e18c2 into master Jun 25, 2021

paolodamico deleted the session-recording-duration branch June 25, 2021 08:30

mariusandra mentioned this pull request Jun 28, 2021

What is sessions page? #4884

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show session recording durations in sessions list when available #4853

Show session recording durations in sessions list when available #4853

mariusandra commented Jun 22, 2021

timgl commented Jun 23, 2021 •

edited

Loading

mariusandra commented Jun 23, 2021

paolodamico left a comment

paolodamico Jun 24, 2021

mariusandra Jun 24, 2021

paolodamico Jun 24, 2021

mariusandra Jun 24, 2021

paolodamico Jun 24, 2021

mariusandra Jun 24, 2021

mariusandra commented Jun 24, 2021

paolodamico left a comment

paolodamico commented Jun 25, 2021

Show session recording durations in sessions list when available #4853

Show session recording durations in sessions list when available #4853

Conversation

mariusandra commented Jun 22, 2021

Changes

Checklist

timgl commented Jun 23, 2021 • edited Loading

mariusandra commented Jun 23, 2021

paolodamico left a comment

Choose a reason for hiding this comment

paolodamico Jun 24, 2021

Choose a reason for hiding this comment

mariusandra Jun 24, 2021

Choose a reason for hiding this comment

paolodamico Jun 24, 2021

Choose a reason for hiding this comment

mariusandra Jun 24, 2021

Choose a reason for hiding this comment

paolodamico Jun 24, 2021

Choose a reason for hiding this comment

mariusandra Jun 24, 2021

Choose a reason for hiding this comment

mariusandra commented Jun 24, 2021

paolodamico left a comment

Choose a reason for hiding this comment

paolodamico commented Jun 25, 2021

timgl commented Jun 23, 2021 •

edited

Loading