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: report when player state is error #26989

Merged
merged 24 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f4e59f0
feat: report when player state is error
pauldambra Dec 17, 2024
922381a
Update UI snapshots for `chromium` (1)
github-actions[bot] Dec 17, 2024
0b15784
Update UI snapshots for `chromium` (2)
github-actions[bot] Dec 17, 2024
eef4c55
Update UI snapshots for `chromium` (1)
github-actions[bot] Dec 17, 2024
447aaf6
stop overreporting player speed changed
pauldambra Dec 17, 2024
35f31db
don't over report skip inactivity
pauldambra Dec 17, 2024
a33aa9d
Update UI snapshots for `chromium` (1)
github-actions[bot] Dec 17, 2024
2008d71
more like this maybe
pauldambra Dec 17, 2024
7c9059f
don't report an error at the start of a recording
pauldambra Dec 17, 2024
396635b
fix
pauldambra Dec 17, 2024
65087da
fix
pauldambra Dec 17, 2024
8c2f730
Merge branch 'master' into feat/pd/report-when-player-state-is-error
pauldambra Dec 18, 2024
e8186c9
fix
pauldambra Dec 18, 2024
572f423
Update UI snapshots for `chromium` (2)
github-actions[bot] Dec 18, 2024
1a2d79d
Update UI snapshots for `chromium` (2)
github-actions[bot] Dec 18, 2024
278f656
Merge branch 'master' into feat/pd/report-when-player-state-is-error
pauldambra Dec 18, 2024
c23abb4
Add a couple of tooltips
pauldambra Dec 18, 2024
e1231ff
flyby fix and tooltips
pauldambra Dec 18, 2024
f66e139
flyby fix and tooltips
pauldambra Dec 18, 2024
87b9196
fix test
pauldambra Dec 18, 2024
8bc9f36
fix
pauldambra Dec 18, 2024
f470b61
Update UI snapshots for `chromium` (2)
github-actions[bot] Dec 18, 2024
3260f78
fix
pauldambra Dec 18, 2024
4874f20
Update UI snapshots for `chromium` (2)
github-actions[bot] Dec 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 0 additions & 8 deletions frontend/src/lib/utils/eventUsageLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,8 +441,6 @@ export const eventUsageLogic = kea<eventUsageLogicType>([
reportRecordingsListPropertiesFetched: (loadTime: number) => ({ loadTime }),
reportRecordingsListFilterAdded: (filterType: SessionRecordingFilterType) => ({ filterType }),
reportRecordingPlayerSeekbarEventHovered: true,
reportRecordingPlayerSpeedChanged: (newSpeed: number) => ({ newSpeed }),
Copy link
Member Author

Choose a reason for hiding this comment

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

i hate this gigantic logic that just hides calling posthog capture

reportRecordingPlayerSkipInactivityToggled: (skipInactivity: boolean) => ({ skipInactivity }),
reportRecordingInspectorItemExpanded: (tab: InspectorListItemType, index: number) => ({ tab, index }),
reportRecordingInspectorMiniFilterViewed: (minifilterKey: MiniFilterKey, enabled: boolean) => ({
minifilterKey,
Expand Down Expand Up @@ -948,12 +946,6 @@ export const eventUsageLogic = kea<eventUsageLogicType>([
reportRecordingPlayerSeekbarEventHovered: () => {
posthog.capture('recording player seekbar event hovered')
},
reportRecordingPlayerSpeedChanged: ({ newSpeed }) => {
posthog.capture('recording player speed changed', { new_speed: newSpeed })
},
reportRecordingPlayerSkipInactivityToggled: ({ skipInactivity }) => {
posthog.capture('recording player skip inactivity toggled', { skip_inactivity: skipInactivity })
},
reportRecordingInspectorItemExpanded: ({ tab, index }) => {
posthog.capture('recording inspector item expanded', { tab: 'replay-4000', type: tab, index })
},
Expand Down
23 changes: 1 addition & 22 deletions frontend/src/scenes/session-recordings/player/PlayerMeta.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import './PlayerMeta.scss'

import { LemonBanner, LemonSelect, LemonSelectOption, Link } from '@posthog/lemon-ui'
import { LemonSelect, LemonSelectOption, Link } from '@posthog/lemon-ui'
import clsx from 'clsx'
import { useActions, useValues } from 'kea'
import { CopyToClipboardInline } from 'lib/components/CopyToClipboard'
Expand Down Expand Up @@ -59,26 +59,6 @@ function URLOrScreen({ lastUrl }: { lastUrl: string | undefined }): JSX.Element
)
}

function PlayerWarningsRow(): JSX.Element | null {
Copy link
Member Author

Choose a reason for hiding this comment

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

we don't set these anywhere any more. so we don't need this code

const { messageTooLargeWarnings } = useValues(sessionRecordingPlayerLogic)

return messageTooLargeWarnings.length ? (
<div>
<LemonBanner
type="error"
action={{
children: 'Learn more',
to: 'https://posthog.com/docs/session-replay/troubleshooting#message-too-large-warning',
targetBlank: true,
}}
>
This session recording had recording data that was too large and could not be captured. This will mean
playback is not 100% accurate.{' '}
</LemonBanner>
</div>
) : null
}

export function PlayerMeta({ iconsOnly }: { iconsOnly: boolean }): JSX.Element {
const { logicProps, isFullScreen } = useValues(sessionRecordingPlayerLogic)

Expand Down Expand Up @@ -206,7 +186,6 @@ export function PlayerMeta({ iconsOnly }: { iconsOnly: boolean }): JSX.Element {
<PlayerMetaLinks iconsOnly={iconsOnly} />
{resolutionView}
</div>
<PlayerWarningsRow />
</div>
</DraggableToNotebook>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ function PinToPlaylistButton({
/>
) : (
<PlaylistPopoverButton
tooltip={tooltip}
setPinnedInCurrentPlaylist={logicProps.setPinned}
icon={logicProps.pinned ? <IconPinFilled /> : <IconPin />}
{...buttonProps}
Expand Down Expand Up @@ -135,7 +136,7 @@ export function PlayerMetaLinks({ iconsOnly }: { iconsOnly: boolean }): JSX.Elem
{buttonContent('Comment')}
</NotebookSelectButton>

<LemonButton icon={<IconShare />} onClick={onShare} {...commonProps}>
<LemonButton icon={<IconShare />} onClick={onShare} {...commonProps} tooltip="Share this recording">
{buttonContent('Share')}
</LemonButton>

Expand All @@ -149,6 +150,7 @@ export function PlayerMetaLinks({ iconsOnly }: { iconsOnly: boolean }): JSX.Elem
attrs: { id: sessionRecordingId },
})
}}
tooltip="Comment in a notebook"
/>
) : null}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { LemonButton } from '@posthog/lemon-ui'
import clsx from 'clsx'
import { BindLogic, useActions, useValues } from 'kea'
import { BuilderHog2 } from 'lib/components/hedgehogs'
import { dayjs } from 'lib/dayjs'
import { FloatingContainerContext } from 'lib/hooks/useFloatingContainerContext'
import { HotkeysInterface, useKeyboardHotkeys } from 'lib/hooks/useKeyboardHotkeys'
import { usePageVisibility } from 'lib/hooks/usePageVisibility'
Expand Down Expand Up @@ -87,11 +86,9 @@ export function SessionRecordingPlayer(props: SessionRecordingPlayerProps): JSX.
setSpeed,
closeExplorer,
} = useActions(sessionRecordingPlayerLogic(logicProps))
const { isNotFound, snapshotsInvalid, start } = useValues(sessionRecordingDataLogic(logicProps))
const { isNotFound, isRecentAndInvalid } = useValues(sessionRecordingDataLogic(logicProps))
const { loadSnapshots } = useActions(sessionRecordingDataLogic(logicProps))
const { isFullScreen, explorerMode, isBuffering, messageTooLargeWarnings } = useValues(
sessionRecordingPlayerLogic(logicProps)
)
const { isFullScreen, explorerMode, isBuffering } = useValues(sessionRecordingPlayerLogic(logicProps))
const { setPlayNextAnimationInterrupted } = useActions(sessionRecordingPlayerLogic(logicProps))
const speedHotkeys = useMemo(() => createPlaybackSpeedKey(setSpeed), [setSpeed])
const { isVerticallyStacked, sidebarOpen, playbackMode } = useValues(playerSettingsLogic)
Expand Down Expand Up @@ -158,9 +155,6 @@ export function SessionRecordingPlayer(props: SessionRecordingPlayerProps): JSX.
}
)

const lessThanFiveMinutesOld = dayjs().diff(start, 'minute') <= 5
const cannotPlayback = snapshotsInvalid && lessThanFiveMinutesOld && !messageTooLargeWarnings

Comment on lines -161 to -163
Copy link
Member Author

Choose a reason for hiding this comment

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

giving this a better name

const { draggable, elementProps } = useNotebookDrag({ href: urls.replaySingle(sessionRecordingId) })

if (isNotFound) {
Expand Down Expand Up @@ -198,7 +192,7 @@ export function SessionRecordingPlayer(props: SessionRecordingPlayerProps): JSX.
className="SessionRecordingPlayer__main flex flex-col h-full w-full"
ref={playerMainRef}
>
{cannotPlayback ? (
{isRecentAndInvalid ? (
<div className="flex flex-1 flex-col items-center justify-center">
<BuilderHog2 height={200} />
<h1>We're still working on it</h1>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function ShowMouseTail(): JSX.Element {

return (
<SettingsToggle
title="Show mouse tail"
title="Show a tail following the cursor to make it easier to see"
Copy link
Member Author

Choose a reason for hiding this comment

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

various changes to ensure there are tooltips on buttons

label="Show mouse tail"
active={showMouseTail}
data-attr="show-mouse-tail"
Expand All @@ -97,7 +97,7 @@ function SkipInactivity(): JSX.Element {

return (
<SettingsToggle
title="Skip inactivity"
title="Skip inactivite parts of the recording"
label="Skip inactivity"
active={skipInactivitySetting}
data-attr="skip-inactivity"
Expand All @@ -106,6 +106,36 @@ function SkipInactivity(): JSX.Element {
)
}

function SetTimeFormat(): JSX.Element {
const { timestampFormat } = useValues(playerSettingsLogic)
const { setTimestampFormat } = useActions(playerSettingsLogic)

return (
<SettingsMenu
highlightWhenActive={false}
items={[
{
label: 'UTC',
onClick: () => setTimestampFormat(TimestampFormat.UTC),
active: timestampFormat === TimestampFormat.UTC,
},
{
label: 'Device',
onClick: () => setTimestampFormat(TimestampFormat.Device),
active: timestampFormat === TimestampFormat.Device,
},
{
label: 'Relative',
onClick: () => setTimestampFormat(TimestampFormat.Relative),
active: timestampFormat === TimestampFormat.Relative,
},
]}
icon={<IconClock />}
label={TimestampFormatToLabel[timestampFormat]}
/>
)
}

function InspectDOM(): JSX.Element {
const { sessionPlayerMetaData } = useValues(sessionRecordingPlayerLogic)
const { openExplorer } = useActions(sessionRecordingPlayerLogic)
Expand All @@ -125,39 +155,15 @@ function InspectDOM(): JSX.Element {
}

function PlayerBottomSettings(): JSX.Element {
const { timestampFormat } = useValues(playerSettingsLogic)
const { setTimestampFormat } = useActions(playerSettingsLogic)

return (
<SettingsBar border="top" className="justify-between">
<div className="flex flex-row gap-0.5">
<SkipInactivity />
<ShowMouseTail />
<SetPlaybackSpeed />
<SettingsMenu
highlightWhenActive={false}
items={[
{
label: 'UTC',
onClick: () => setTimestampFormat(TimestampFormat.UTC),
active: timestampFormat === TimestampFormat.UTC,
},
{
label: 'Device',
onClick: () => setTimestampFormat(TimestampFormat.Device),
active: timestampFormat === TimestampFormat.Device,
},
{
label: 'Relative',
onClick: () => setTimestampFormat(TimestampFormat.Relative),
active: timestampFormat === TimestampFormat.Relative,
},
]}
icon={<IconClock />}
label={TimestampFormatToLabel[timestampFormat]}
/>
<InspectDOM />
<SetTimeFormat />
</div>
<InspectDOM />
Copy link
Member Author

Choose a reason for hiding this comment

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

and to separate the inspect dom button from the player buttons

</SettingsBar>
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { actions, connect, kea, path, reducers, selectors } from 'kea'
import { actions, connect, kea, listeners, path, reducers, selectors } from 'kea'
import posthog from 'posthog-js'
import { teamLogic } from 'scenes/teamLogic'

import { AutoplayDirection, SessionRecordingSidebarStacking } from '~/types'
Expand Down Expand Up @@ -122,4 +123,13 @@ export const playerSettingsLogic = kea<playerSettingsLogicType>([
(preferredSidebarStacking) => preferredSidebarStacking === SessionRecordingSidebarStacking.Vertical,
],
}),

listeners({
setSpeed: ({ speed }) => {
posthog.capture('recording player speed changed', { new_speed: speed })
Copy link
Member Author

Choose a reason for hiding this comment

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

moved these captures closer to the source

},
setSkipInactivitySetting: ({ skipInactivitySetting }) => {
posthog.capture('recording player skip inactivity toggled', { skip_inactivity: skipInactivitySetting })
},
}),
])
Original file line number Diff line number Diff line change
Expand Up @@ -1087,13 +1087,13 @@ export const sessionRecordingDataLogic = kea<sessionRecordingDataLogicType>([
if (everyWindowMissingFullSnapshot) {
// video is definitely unplayable
posthog.capture('recording_has_no_full_snapshot', {
sessionId: sessionRecordingId,
watchedSession: sessionRecordingId,
teamId: currentTeam?.id,
teamName: currentTeam?.name,
})
} else if (anyWindowMissingFullSnapshot) {
posthog.capture('recording_window_missing_full_snapshot', {
sessionId: sessionRecordingId,
watchedSession: sessionRecordingId,
teamID: currentTeam?.id,
teamName: currentTeam?.name,
})
Expand All @@ -1103,6 +1103,14 @@ export const sessionRecordingDataLogic = kea<sessionRecordingDataLogicType>([
},
],

isRecentAndInvalid: [
(s) => [s.start, s.snapshotsInvalid],
(start, snapshotsInvalid) => {
const lessThanFiveMinutesOld = dayjs().diff(start, 'minute') <= 5
return snapshotsInvalid && lessThanFiveMinutesOld
},
],

bufferedToTime: [
(s) => [s.segments],
(segments): number | null => {
Expand Down Expand Up @@ -1160,6 +1168,13 @@ export const sessionRecordingDataLogic = kea<sessionRecordingDataLogicType>([
actions.loadFullEventData(value)
}
},
isRecentAndInvalid: (prev: boolean, next: boolean) => {
if (!prev && next) {
posthog.capture('recording cannot playback yet', {
watchedSession: values.sessionPlayerData.sessionRecordingId,
})
}
},
})),
afterMount(({ cache }) => {
resetTimingsCache(cache)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,15 +141,15 @@ describe('sessionRecordingPlayerLogic', () => {
sessionRecordingDataLogic({ sessionRecordingId: '2' }).actionTypes.loadSnapshotSourcesFailure,
])
.toFinishAllListeners()
.toDispatchActions(['setErrorPlayerState'])
.toDispatchActions(['setPlayerError'])

expect(logic.values).toMatchObject({
sessionPlayerData: {
person: recordingMetaJson.person,
snapshotsByWindowId: {},
bufferedToTime: 0,
},
isErrored: true,
playerError: 'loadSnapshotSourcesFailure',
})
resumeKeaLoadersErrors()
})
Expand Down
Loading
Loading