-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Changes from 6 commits
f4e59f0
922381a
0b15784
eef4c55
447aaf6
35f31db
a33aa9d
2008d71
7c9059f
396635b
65087da
8c2f730
e8186c9
572f423
1a2d79d
278f656
c23abb4
e1231ff
f66e139
87b9196
8bc9f36
f470b61
3260f78
4874f20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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' | ||
|
@@ -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 }) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
---|---|---|
|
@@ -138,12 +138,7 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>( | |
playerSettingsLogic, | ||
['setSpeed', 'setSkipInactivitySetting'], | ||
eventUsageLogic, | ||
[ | ||
'reportNextRecordingTriggered', | ||
'reportRecordingPlayerSkipInactivityToggled', | ||
'reportRecordingPlayerSpeedChanged', | ||
'reportRecordingExportedToFile', | ||
], | ||
['reportNextRecordingTriggered', 'reportRecordingExportedToFile'], | ||
], | ||
})), | ||
actions({ | ||
|
@@ -672,7 +667,6 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>( | |
} | ||
}, | ||
setSkipInactivitySetting: ({ skipInactivitySetting }) => { | ||
actions.reportRecordingPlayerSkipInactivityToggled(skipInactivitySetting) | ||
if (!values.currentSegment?.isActive && skipInactivitySetting) { | ||
actions.setSkippingInactivity(true) | ||
} else { | ||
|
@@ -848,9 +842,10 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>( | |
startScrub: () => { | ||
actions.stopAnimation() | ||
}, | ||
setSpeed: ({ speed }) => { | ||
actions.reportRecordingPlayerSpeedChanged(speed) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there are always three instances of the player logic, all three are listening to the player settings logic, so all three were reporting this making usage look higher than it is... luckily not an important metric |
||
actions.syncPlayerSpeed() | ||
setSpeed: () => { | ||
if (props.mode !== SessionRecordingPlayerMode.Preview) { | ||
actions.syncPlayerSpeed() | ||
} | ||
}, | ||
seekToTimestamp: ({ timestamp, forcePlay }, breakpoint) => { | ||
actions.stopAnimation() | ||
|
@@ -1018,7 +1013,7 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>( | |
cache.pausedMediaElements = values.endReached ? [] : playingElements | ||
}, | ||
restartIframePlayback: () => { | ||
cache.pausedMediaElements.forEach((el: HTMLMediaElement) => el.play()) | ||
cache.pausedMediaElements?.forEach((el: HTMLMediaElement) => el.play()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. saw an error from trying to access this when it was undefined while debugging the rest of this |
||
cache.pausedMediaElements = [] | ||
}, | ||
|
||
|
@@ -1143,6 +1138,16 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>( | |
actions.reportMessageTooLargeWarningSeen(values.sessionRecordingId) | ||
} | ||
}, | ||
currentPlayerState: (prev, next) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pretty sure we're setting this more than we need to, but this at least can report on that too high a number |
||
if (next === SessionPlayerState.ERROR && prev !== SessionPlayerState.ERROR) { | ||
posthog.capture('recording player error', { | ||
watchedSessionId: values.sessionRecordingId, | ||
currentTimestamp: values.currentTimestamp, | ||
currentSegment: values.currentSegment, | ||
currentPlayerTime: values.currentPlayerTime, | ||
}) | ||
} | ||
}, | ||
})), | ||
|
||
beforeUnmount(({ values, actions, cache, props }) => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import { BaseIcon, IconCheck, IconEye, IconLogomark, IconSearch, IconVideoCamera } from '@posthog/icons' | ||
import { BaseIcon, IconCheck, IconEye, IconHide, IconLogomark, IconSearch, IconVideoCamera } from '@posthog/icons' | ||
import { useActions, useValues } from 'kea' | ||
import { AnimatedCollapsible } from 'lib/components/AnimatedCollapsible' | ||
import { PropertyKeyInfo } from 'lib/components/PropertyKeyInfo' | ||
|
@@ -15,10 +15,10 @@ import { EventType } from '~/types' | |
|
||
import { ToolbarMenu } from '../bar/ToolbarMenu' | ||
|
||
function showEventMenuItem( | ||
function checkableMenuItem( | ||
label: string, | ||
count: number, | ||
icon: JSX.Element, | ||
count: number | null, | ||
icon: JSX.Element | null, | ||
isActive: boolean, | ||
onClick: () => void | ||
): LemonMenuItem { | ||
|
@@ -30,13 +30,15 @@ function showEventMenuItem( | |
{icon} | ||
{label} | ||
</div> | ||
<span | ||
// without setting fontVariant to none a single digit number between brackets gets rendered as a ligature 🤷 | ||
// eslint-disable-next-line react/forbid-dom-props | ||
style={{ fontVariant: 'none' }} | ||
> | ||
({count}) | ||
</span> | ||
{count !== null && ( | ||
<span | ||
// without setting fontVariant to none a single digit number between brackets gets rendered as a ligature 🤷 | ||
// eslint-disable-next-line react/forbid-dom-props | ||
style={{ fontVariant: 'none' }} | ||
> | ||
({count}) | ||
</span> | ||
)} | ||
</div> | ||
), | ||
active: isActive, | ||
|
@@ -70,39 +72,59 @@ export const EventDebugMenu = (): JSX.Element => { | |
searchFilteredEventsCount, | ||
expandedEvent, | ||
selectedEventTypes, | ||
hidePostHogProperties, | ||
hidePostHogFlags, | ||
expandedProperties, | ||
} = useValues(eventDebugMenuLogic) | ||
const { markExpanded, setSelectedEventType, setSearchText, setSearchVisible } = useActions(eventDebugMenuLogic) | ||
const { | ||
markExpanded, | ||
setSelectedEventType, | ||
setSearchText, | ||
setSearchVisible, | ||
setHidePostHogProperties, | ||
setHidePostHogFlags, | ||
} = useActions(eventDebugMenuLogic) | ||
|
||
const showEventsMenuItems = [ | ||
showEventMenuItem( | ||
checkableMenuItem( | ||
'PostHog Events', | ||
searchFilteredEventsCount['posthog'], | ||
<IconLogomark />, | ||
selectedEventTypes.includes('posthog'), | ||
() => setSelectedEventType('posthog', !selectedEventTypes.includes('posthog')) | ||
), | ||
showEventMenuItem( | ||
checkableMenuItem( | ||
'Custom Events', | ||
searchFilteredEventsCount['custom'], | ||
<IconVideoCamera />, | ||
selectedEventTypes.includes('custom'), | ||
() => setSelectedEventType('custom', !selectedEventTypes.includes('custom')) | ||
), | ||
showEventMenuItem( | ||
checkableMenuItem( | ||
'Replay Events', | ||
searchFilteredEventsCount['snapshot'], | ||
<IconUnverifiedEvent />, | ||
selectedEventTypes.includes('snapshot'), | ||
() => setSelectedEventType('snapshot', !selectedEventTypes.includes('snapshot')) | ||
), | ||
] | ||
|
||
const hideThingsMenuItems = [ | ||
checkableMenuItem('Hide PostHog properties', null, null, hidePostHogProperties, () => | ||
setHidePostHogProperties(!hidePostHogProperties) | ||
), | ||
checkableMenuItem('Hide PostHog flags', null, null, hidePostHogFlags, () => | ||
setHidePostHogFlags(!hidePostHogFlags) | ||
), | ||
] | ||
|
||
return ( | ||
<ToolbarMenu> | ||
<ToolbarMenu.Header noPadding> | ||
<div className="flex flex-col pb-2 space-y-1"> | ||
<div className="flex justify-center flex-col"> | ||
<SettingsBar border="bottom" className="justify-end"> | ||
<div className="flex-1 text-sm"> | ||
<div className="flex-1 text-sm pl-1"> | ||
View events from this page as they are sent to PostHog. | ||
</div> | ||
<SettingsToggle | ||
|
@@ -149,7 +171,7 @@ export const EventDebugMenu = (): JSX.Element => { | |
> | ||
<div className="my-1 ml-1 pl-2 border-l-2"> | ||
<SimpleKeyValueList | ||
item={e.properties} | ||
item={expandedProperties} | ||
emptyMessage={searchText ? 'No matching properties' : 'No properties'} | ||
/> | ||
</div> | ||
|
@@ -167,7 +189,13 @@ export const EventDebugMenu = (): JSX.Element => { | |
</div> | ||
</ToolbarMenu.Body> | ||
<ToolbarMenu.Footer noPadding> | ||
<SettingsBar border="top" className="justify-end"> | ||
<SettingsBar border="top" className="justify-between"> | ||
<SettingsMenu | ||
items={hideThingsMenuItems} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. found myself needing this while figuring out the rest of this |
||
highlightWhenActive={false} | ||
icon={<IconHide />} | ||
label="Hide properties" | ||
/> | ||
<SettingsMenu | ||
items={showEventsMenuItems} | ||
highlightWhenActive={false} | ||
|
There was a problem hiding this comment.
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