Skip to content

Commit

Permalink
fix: Performance issue with player logs (#17948)
Browse files Browse the repository at this point in the history
  • Loading branch information
benjackwhite authored Oct 12, 2023
1 parent 503e13b commit 36521eb
Show file tree
Hide file tree
Showing 5 changed files with 176 additions and 34 deletions.
65 changes: 61 additions & 4 deletions frontend/src/scenes/notebooks/Notebook/SlashCommands.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ import {
import { forwardRef, useCallback, useEffect, useImperativeHandle, useMemo, useState } from 'react'
import { EditorCommands, EditorRange } from './utils'
import { NotebookNodeType } from '~/types'
import { examples } from '~/queries/examples'
import { Popover } from 'lib/lemon-ui/Popover'
import { KeyboardShortcut } from '~/layout/navigation-3000/components/KeyboardShortcut'
import Fuse from 'fuse.js'
import { useValues } from 'kea'
import { notebookLogic } from './notebookLogic'
import { selectFile } from '../Nodes/utils'
import { NodeKind } from '~/queries/schema'
import { defaultDataTableColumns } from '~/queries/nodes/DataTable/utils'

type SlashCommandsProps = {
mode: 'slash' | 'add'
Expand Down Expand Up @@ -243,21 +244,77 @@ const SLASH_COMMANDS: SlashCommandsItem[] = [
search: 'sql',
icon: <InsightSQLIcon noBackground color="currentColor" />,
command: (chain) =>
chain.insertContent({ type: NotebookNodeType.Query, attrs: { query: examples['HogQLTable'] } }),
chain.insertContent({
type: NotebookNodeType.Query,
attrs: {
query: {
kind: NodeKind.DataTableNode,
full: true,
source: {
kind: NodeKind.HogQLQuery,
query: `select event,
person.properties.email,
properties.$browser,
count()
from events
where {filters} -- replaced with global date and property filters
and person.properties.email is not null
group by event,
properties.$browser,
person.properties.email
order by count() desc
limit 100`,
filters: {
dateRange: {
date_from: '-24h',
},
},
},
},
},
}),
},
{
title: 'Events',
search: 'data explore',
icon: <IconTableChart />,
command: (chain) =>
chain.insertContent({ type: NotebookNodeType.Query, attrs: { query: examples['EventsTableFull'] } }),
chain.insertContent({
type: NotebookNodeType.Query,
attrs: {
query: {
kind: NodeKind.DataTableNode,
full: true,
source: {
kind: NodeKind.EventsQuery,
select: defaultDataTableColumns(NodeKind.EventsQuery),
properties: [],
after: '-24h',
limit: 100,
},
},
},
}),
},
{
title: 'Persons',
search: 'people users',
icon: <IconCohort />,
command: (chain) =>
chain.insertContent({ type: NotebookNodeType.Query, attrs: { query: examples['PersonsTableFull'] } }),
chain.insertContent({
type: NotebookNodeType.Query,
attrs: {
query: {
kind: NodeKind.DataTableNode,
full: true,
columns: defaultDataTableColumns(NodeKind.PersonsNode),
source: {
kind: NodeKind.PersonsNode,
properties: [],
},
},
},
}),
},
{
title: 'Session Replays',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,27 @@ const allNotebooks = [
{
title: 'my amazing notebook',
short_id: 'abc',
created_by: {
first_name: 'Ben',
email: '[email protected]',
},
},
{
title: 'and another amazing notebook',
short_id: 'def',
created_by: {
first_name: 'Paul',
email: '[email protected]',
},
},
{
title: 'an empty notebook',
short_id: 'ghi',
created_by: {
first_name: 'David',
email: '[email protected]',
},
},
{ title: 'and another amazing notebook', short_id: 'def' },
{ title: 'an empty notebook', short_id: 'ghi' },
]

const Template: StoryFn<typeof NotebookSelectButton> = (props) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { notebookNodeLogicType } from '../Nodes/notebookNodeLogicType'
import { FlaggedFeature } from 'lib/components/FlaggedFeature'
import { FEATURE_FLAGS } from 'lib/constants'
import { ReactChild, useEffect } from 'react'
import { LemonDivider } from '@posthog/lemon-ui'
import { LemonDivider, ProfilePicture } from '@posthog/lemon-ui'

export type NotebookSelectProps = NotebookSelectButtonLogicProps & {
newNotebookTitle?: string
Expand Down Expand Up @@ -50,8 +50,22 @@ function NotebooksChoiceList(props: {
) : (
props.notebooks.map((notebook, i) => {
return (
<LemonButton key={i} fullWidth onClick={() => props.onClick(notebook.short_id)}>
{notebook.title || `Untitled (${notebook.short_id})`}
<LemonButton
key={i}
sideIcon={
notebook.created_by ? (
<ProfilePicture
name={notebook.created_by?.first_name}
email={notebook.created_by?.email}
size="md"
title={`Created by ${notebook.created_by?.first_name} <${notebook.created_by?.email}>`}
/>
) : null
}
fullWidth
onClick={() => props.onClick(notebook.short_id)}
>
<span className="truncate">{notebook.title || `Untitled (${notebook.short_id})`}</span>
</LemonButton>
)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ import { urls } from 'scenes/urls'

describe('sessionRecordingPlayerLogic', () => {
let logic: ReturnType<typeof sessionRecordingPlayerLogic.build>
const mockWarn = jest.fn()

beforeEach(() => {
console.warn = mockWarn
mockWarn.mockClear()
useMocks({
get: {
'/api/projects/:team/session_recordings/:id/snapshots/': (req, res, ctx) => {
Expand Down Expand Up @@ -324,5 +327,32 @@ describe('sessionRecordingPlayerLogic', () => {
}),
})
})

it('captures replayer warnings', async () => {
jest.useFakeTimers()
logic = sessionRecordingPlayerLogic({
sessionRecordingId: '4',
playerKey: 'test',
matchingEventsMatchType: {
matchType: 'uuid',
eventUUIDs: listOfMatchingEvents.map((event) => event.uuid),
},
})
logic.mount()

console.warn('[replayer]', 'test')
console.warn('[replayer]', 'test2')

expect(mockWarn).not.toHaveBeenCalled()

expect((window as any).__posthog_player_warnings).toEqual([
['[replayer]', 'test'],
['[replayer]', 'test2'],
])
jest.runOnlyPendingTimers()
expect(mockWarn).toHaveBeenCalledWith(
'[PostHog Replayer] 2 warnings (window.__posthog_player_warnings to safely log them)'
)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -958,13 +958,13 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>(
delete (window as any).__debug_player

actions.stopAnimation()
cache.resetConsoleWarn?.()

cache.hasInitialized = false
clearTimeout(cache.consoleWarnDebounceTimer)
document.removeEventListener('fullscreenchange', cache.fullScreenListener)
cache.pausedMediaElements = []
values.player?.replayer?.pause()
actions.setPlayer(null)
cache.unmountConsoleWarns?.()

const playTimeMs = values.playingTimeTracking.watchTime || 0
const summaryAnalytics: RecordingViewedSummaryAnalytics = {
Expand Down Expand Up @@ -1017,29 +1017,7 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>(

cache.openTime = performance.now()

// NOTE: RRWeb can log _alot_ of warnings, so we debounce the count otherwise we just end up making the performance worse
let warningCount = 0
cache.consoleWarnDebounceTimer = null

const debouncedCounter = (): void => {
warningCount += 1

if (!cache.consoleWarnDebounceTimer) {
cache.consoleWarnDebounceTimer = setTimeout(() => {
cache.consoleWarnDebounceTimer = null
actions.incrementWarningCount(warningCount)
warningCount = 0
}, 1000)
}
}

cache.resetConsoleWarn = wrapConsole('warn', (args) => {
if (typeof args[0] === 'string' && args[0].includes('[replayer]')) {
debouncedCounter()
}

return true
})
cache.unmountConsoleWarns = manageConsoleWarns(cache, actions.incrementWarningCount)
}),
])

Expand All @@ -1048,3 +1026,48 @@ export const getCurrentPlayerTime = (logicProps: SessionRecordingPlayerLogicProp
const playerTime = sessionRecordingPlayerLogic.findMounted(logicProps)?.values.currentPlayerTime || 0
return Math.floor(playerTime / 1000)
}

export const manageConsoleWarns = (cache: any, onIncrement: (count: number) => void): (() => void) => {
// NOTE: RRWeb can log _alot_ of warnings, so we debounce the count otherwise we just end up making the performance worse
// We also don't log the warnings directly. Sometimes the sheer size of messages and warnings can cause the browser to crash deserializing it all
;(window as any).__posthog_player_warnings = []
const warnings: any[][] = (window as any).__posthog_player_warnings

let counter = 0

let consoleWarnDebounceTimer: NodeJS.Timeout | null = null

const actualConsoleWarn = console.warn

const debouncedCounter = (args: any[]): void => {
warnings.push(args)
counter += 1

if (!consoleWarnDebounceTimer) {
consoleWarnDebounceTimer = setTimeout(() => {
consoleWarnDebounceTimer = null
onIncrement(warnings.length)

actualConsoleWarn(
`[PostHog Replayer] ${counter} warnings (window.__posthog_player_warnings to safely log them)`
)
counter = 0
}, 1000)
}
}

const resetConsoleWarn = wrapConsole('warn', (args) => {
if (typeof args[0] === 'string' && args[0].includes('[replayer]')) {
debouncedCounter(args)
// WARNING: Logging these out can cause the browser to completely crash, so we want to delay it and
return false
}

return true
})

return () => {
resetConsoleWarn()
clearTimeout(cache.consoleWarnDebounceTimer)
}
}

0 comments on commit 36521eb

Please sign in to comment.