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

Fix web session playback when a duration is not provided #50262

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

zmb3
Copy link
Collaborator

@zmb3 zmb3 commented Dec 15, 2024

Prior to Teleport 15, the web UI would download the entire session recording into browser memory before playing it back (crashing the browser tab for long sessions).

Starting with Teleport 15, session playback is streamed to the browser and played back as it is received instead of waiting for the entire session to be available.

A side effect of this change is that the browser needs to know the length of the session in order to render the progress bar during playback. Since the browser starts playing the session before it has received all of it, we started providing the length of the session via a URL query parameter.

Some users have grown accustomed to being able to access session recordings at their original URLs (without the duration query parameter). If you attempt to play recordings from these URLs after upgrading to v15, you'll get an error that the duration is missing.

To fix this, the web UI needs to request the duration of the session before it can begin playing it (unless the duration is provided via the URL). There are two ways we could get this information:

  1. By querying Teleport's audit log
  2. By reading the recording twice: once to get to the end event and compute the duration, and a second time to actually play it back.

Since we only have a session ID, an audit log query would be inefficient - we have no idea when the session occurred, so we'd have to search from the beginning of time. (This could be resolved by using a UUIDv7 for session IDs, but Teleport uses UUIDv4 today).

For this reason, we elect option 2. This commit creates a new web API endpoint that will fetch a session recording file and scan through it in the same way that streaming is done, but instaed of streaming the data through a websocket it simply reads through to the end to compute the session length. The benefit of this approach is that it will generally be faster than option 1 (unless the session is very long), and it effectively pre-downloads the recording file on the

Note: option 2 is not without its drawbacks - since the web UI is making two requests that both read the session recording, the audit log will show two separate session_recording.access events. This isn't ideal but it is good enough to get playback working again for customers who don't access playbacks by clicking the "Play" button in the UI.

Changelog: Restore the ability to play session recordings in the web UI without specifying the session duration in the URL.

@zmb3 zmb3 force-pushed the zmb3/playback-url branch 3 times, most recently from dcc3a91 to 9277557 Compare December 16, 2024 01:09
Copy link
Contributor

@gzdunek gzdunek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some minor comments on the JS code.

web/packages/teleport/src/Player/Player.tsx Outdated Show resolved Hide resolved
web/packages/teleport/src/Player/Player.tsx Outdated Show resolved Hide resolved
web/packages/teleport/src/Player/Player.tsx Outdated Show resolved Hide resolved
web/packages/teleport/src/config.ts Outdated Show resolved Hide resolved
@zmb3 zmb3 force-pushed the zmb3/playback-url branch 2 times, most recently from ab190f5 to 4d0c90d Compare December 17, 2024 17:15
@zmb3 zmb3 marked this pull request as ready for review December 17, 2024 17:22
@github-actions github-actions bot added audit-log Issues related to Teleports Audit Log size/sm ui labels Dec 17, 2024
@github-actions github-actions bot requested review from avatus and rudream December 17, 2024 17:22
@zmb3 zmb3 requested a review from rosstimothy December 17, 2024 19:24
@zmb3 zmb3 force-pushed the zmb3/playback-url branch 2 times, most recently from 7c3e151 to 1b290e1 Compare December 18, 2024 20:47
lib/web/tty_playback.go Show resolved Hide resolved
web/packages/teleport/src/Player/Player.tsx Outdated Show resolved Hide resolved
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from avatus December 18, 2024 21:54
Prior to Teleport 15, the web UI would download the entire session
recording into browser memory before playing it back (crashing the
browser tab for long sessions).

Starting with Teleport 15, session playback is streamed to the
browser and played back as it is received instead of waiting for
the entire session to be available.

A side effect of this change is that the browser needs to know
the length of the session in order to render the progress bar
during playback. Since the browser starts playing the session
before it has received all of it, we started providing the length
of the session via a URL query parameter.

Some users have grown accustomed to being able to access session
recordings at their original URLs (without the duration query
parameter). If you attempt to play recordings from these URLs after
upgrading to v15, you'll get an error that the duration is missing.

To fix this, the web UI needs to request the duration of the session
before it can begin playing it (unless the duration is provided via
the URL). There are two ways we could get this information:

1. By querying Teleport's audit log
2. By reading the recording twice: once to get to the end event
   and compute the duration, and a second time to actually play
   it back.

Since we only have a session ID, an audit log query would be
inefficient - we have no idea when the session occurred, so we'd
have to search from the beginning of time. (This could be resolved
by using a UUIDv7 for session IDs, but Teleport uses UUIDv4 today).

For this reason, we elect option 2. This commit creates a new web
API endpoint that will fetch a session recording file and scan through
it in the same way that streaming is done, but instaed of streaming
the data through a websocket it simply reads through to the end to
compute the session length. The benefit of this approach is that it
will generally be faster than option 1 (unless the session is very
long), and it effectively pre-downloads the recording file on the

Note: option 2 is not without its drawbacks - since the web UI is making
two requests that both read the session recording, the audit log will
show two separate session_recording.access events. This isn't ideal
but it is good enough to get playback working again for customers
who don't access playbacks by clicking the "Play" button in the UI.
@zmb3 zmb3 force-pushed the zmb3/playback-url branch from 1b290e1 to c4bdb8c Compare December 19, 2024 01:00
@zmb3 zmb3 added this pull request to the merge queue Dec 19, 2024
Merged via the queue into master with commit a9d27e1 Dec 19, 2024
45 checks passed
@zmb3 zmb3 deleted the zmb3/playback-url branch December 19, 2024 18:26
@public-teleport-github-review-bot

@zmb3 See the table below for backport results.

Branch Result
branch/v15 Failed
branch/v16 Failed
branch/v17 Failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants