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.

Sign up for GitHub

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

Jump to bottom

chore: instrument time insights are marked as loading #11222

Merged
merged 8 commits into from Aug 12, 2022
Merged

chore: instrument time insights are marked as loading #11222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ca04f42
chore: instrument time insights are marked as loading
pauldambra Aug 10, 2022
6440a13
instrument manual dashboard card refresh
pauldambra Aug 10, 2022
5995883
report on dashboard loading time
pauldambra Aug 10, 2022
0c03b3e
just dashboards
pauldambra Aug 10, 2022
1aa0f11
just dashboards
pauldambra Aug 10, 2022
ebf2bc8
pass args down to satisfy kea signature
pauldambra Aug 11, 2022
71a7cc6
use date not dayjs
pauldambra Aug 11, 2022
f195fd3
fix test
pauldambra Aug 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions frontend/src/lib/utils/eventUsageLogic.ts
View file
Open in desktop
Expand Up @@ -23,6 +23,7 @@ import {
PropertyGroupFilter,
FilterLogicalOperator,
PropertyFilterValue,
InsightShortId,
} from '~/types'
import type { Dayjs } from 'lib/dayjs'
import { preflightLogic } from 'scenes/PreflightCheck/preflightLogic'
Expand Down Expand Up @@ -439,6 +440,10 @@ export const eventUsageLogic = kea<eventUsageLogicType>({
loadTime,
error,
}),
reportInsightRefreshTime: (loadingMilliseconds: number, insightShortId: InsightShortId) => ({
loadingMilliseconds,
insightShortId,
}),
reportInsightOpenedFromRecentInsightList: true,
reportRecordingOpenedFromRecentRecordingList: true,
reportPersonOpenedFromNewlySeenPersonsList: true,
Expand All @@ -454,8 +459,18 @@ export const eventUsageLogic = kea<eventUsageLogicType>({
reportFailedToCreateFeatureFlagWithCohort: (code: string, detail: string) => ({ code, detail }),
reportInviteMembersButtonClicked: true,
reportIngestionSidebarButtonClicked: (name: string) => ({ name }),
reportDashboardLoadingTime: (loadingMilliseconds: number, dashboardId: number) => ({
loadingMilliseconds,
dashboardId,
}),
},
listeners: ({ values }) => ({
reportDashboardLoadingTime: async ({ loadingMilliseconds, dashboardId }) => {
posthog.capture('dashboard loading time', { loadingMilliseconds, dashboardId })
},
reportInsightRefreshTime: async ({ loadingMilliseconds, insightShortId }) => {
posthog.capture('insight refresh time', { loadingMilliseconds, insightShortId })
},
reportEventsTablePollingReactedToPageVisibility: async ({ pageIsVisible }) => {
posthog.capture(`events table polling ${pageIsVisible ? 'resumed' : 'paused'}`, { pageIsVisible })
},
Expand Down
15 changes: 8 additions & 7 deletions frontend/src/scenes/dashboard/dashboardLogic.test.ts
View file
Open in desktop
Expand Up @@ -10,6 +10,7 @@ import { resumeKeaLoadersErrors, silenceKeaLoadersErrors } from '~/initKea'
import { useMocks } from '~/mocks/jest'
import { dayjs, now } from 'lib/dayjs'
import { teamLogic } from 'scenes/teamLogic'
import anything = jasmine.anything

const dashboardJson = _dashboardJson as any as DashboardType

Expand Down Expand Up @@ -229,7 +230,7 @@ describe('dashboardLogic', () => {
})
.toFinishAllListeners()
.toMatchValues({
refreshStatus: { 1001: { error: true } },
refreshStatus: { 1001: { error: true, timer: anything() } },
})
})
})
Expand Down Expand Up @@ -279,8 +280,8 @@ describe('dashboardLogic', () => {
])
.toMatchValues({
refreshStatus: {
[dashboards['5'].items[0].short_id]: { loading: true },
[dashboards['5'].items[1].short_id]: { loading: true },
[dashboards['5'].items[0].short_id]: { loading: true, timer: anything() },
[dashboards['5'].items[1].short_id]: { loading: true, timer: anything() },
},
refreshMetrics: {
completed: 0,
Expand All @@ -301,8 +302,8 @@ describe('dashboardLogic', () => {
])
.toMatchValues({
refreshStatus: {
[dashboards['5'].items[0].short_id]: { refreshed: true },
[dashboards['5'].items[1].short_id]: { refreshed: true },
[dashboards['5'].items[0].short_id]: { refreshed: true, timer: anything() },
[dashboards['5'].items[1].short_id]: { refreshed: true, timer: anything() },
},
refreshMetrics: {
completed: 2,
Expand All @@ -322,7 +323,7 @@ describe('dashboardLogic', () => {
])
.toMatchValues({
refreshStatus: {
[dashboards['5'].items[0].short_id]: { loading: true },
[dashboards['5'].items[0].short_id]: { loading: true, timer: anything() },
},
refreshMetrics: {
completed: 0,
Expand All @@ -337,7 +338,7 @@ describe('dashboardLogic', () => {
])
.toMatchValues({
refreshStatus: {
[dashboards['5'].items[0].short_id]: { refreshed: true },
[dashboards['5'].items[0].short_id]: { refreshed: true, timer: anything() },
},
refreshMetrics: {
completed: 1,
Expand Down
73 changes: 50 additions & 23 deletions frontend/src/scenes/dashboard/dashboardLogic.ts
View file
Open in desktop
Expand Up @@ -2,7 +2,6 @@ import { isBreakpoint, kea } from 'kea'
import api from 'lib/api'
import { dashboardsModel } from '~/models/dashboardsModel'
import { router } from 'kea-router'
import { dayjs, now } from 'lib/dayjs'
import { clearDOMTextSelection, isUserLoggedIn, toParams } from 'lib/utils'
import { insightsModel } from '~/models/insightsModel'
import { DashboardPrivilegeLevel, OrganizationMembershipLevel } from 'lib/constants'
Expand All @@ -27,6 +26,7 @@ import { teamLogic } from '../teamLogic'
import { urls } from 'scenes/urls'
import { userLogic } from 'scenes/userLogic'
import { mergeWithDashboardTile } from 'scenes/insights/utils/dashboardTiles'
import { dayjs, now } from 'lib/dayjs'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these were forgotten to be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Interestingly. We use dayjs elsewhere in the file already but we never return the Dayjs type. So kea typegen never has to deal with it.

I thought I'd not rewrite existing functionality since there's a good chance we leave this in for a few days to get a snapshot of data and then remove it


export const BREAKPOINTS: Record<DashboardLayoutSize, number> = {
sm: 1024,
Expand All @@ -44,6 +44,13 @@ export interface DashboardLogicProps {
placement?: DashboardPlacement
}

export interface RefreshStatus {
loading?: boolean
refreshed?: boolean
error?: boolean
timer?: Date | null
}

export const AUTO_REFRESH_INITIAL_INTERVAL_SECONDS = 300

export const dashboardLogic = kea<dashboardLogicType>({
Expand Down Expand Up @@ -252,34 +259,28 @@ export const dashboardLogic = kea<dashboardLogicType>({
},
},
],
loadTimer: [null as Date | null, { loadDashboardItems: () => new Date() }],
refreshStatus: [
{} as Record<
string,
{
loading?: boolean
refreshed?: boolean
error?: boolean
}
>,
{} as Record<string, RefreshStatus>,
{
setRefreshStatus: (state, { shortId, loading }) => ({
...state,
[shortId]: loading ? { loading: true } : { refreshed: true },
[shortId]: loading
? { loading: true, timer: new Date() }
: { refreshed: true, timer: state[shortId]?.timer || null },
}),
setRefreshStatuses: (_, { shortIds, loading }) =>
setRefreshStatuses: (state, { shortIds, loading }) =>
Object.fromEntries(
shortIds.map((shortId) => [shortId, loading ? { loading: true } : { refreshed: true }])
) as Record<
string,
{
loading?: boolean
refreshed?: boolean
error?: boolean
}
>,
shortIds.map((shortId) => [
shortId,
loading
? { loading: true, timer: new Date() }
: { refreshed: true, timer: state[shortId]?.timer || null },
])
) as Record<string, RefreshStatus>,
setRefreshError: (state, { shortId }) => ({
...state,
[shortId]: { error: true },
[shortId]: { error: true, timer: state[shortId]?.timer || null },
}),
refreshAllDashboardItems: () => ({}),
},
Expand Down Expand Up @@ -562,7 +563,31 @@ export const dashboardLogic = kea<dashboardLogicType>({
}
},
}),
listeners: ({ actions, values, cache, props }) => ({
sharedListeners: ({ values, props }) => ({
reportRefreshTiming: ({ shortId }) => {
const refreshStatus = values.refreshStatus[shortId]

if (refreshStatus?.timer) {
const loadingMilliseconds = new Date().getTime() - refreshStatus.timer.getTime()
eventUsageLogic.actions.reportInsightRefreshTime(loadingMilliseconds, shortId)
}
},
reportLoadTiming: () => {
if (!props.id) {
// what even is loading?!
return
}
if (values.loadTimer) {
const loadingMilliseconds = new Date().getTime() - values.loadTimer.getTime()
eventUsageLogic.actions.reportDashboardLoadingTime(loadingMilliseconds, props.id)
}
},
}),
listeners: ({ actions, values, cache, props, sharedListeners }) => ({
setRefreshError: sharedListeners.reportRefreshTiming,
setRefreshStatuses: sharedListeners.reportRefreshTiming,
setRefreshStatus: sharedListeners.reportRefreshTiming,
loadDashboardItemsFailure: sharedListeners.reportLoadTiming,
triggerDashboardUpdate: ({ payload }) => {
if (values.dashboard) {
dashboardsModel.actions.updateDashboard({ id: values.dashboard.id, ...payload })
Expand Down Expand Up @@ -729,7 +754,9 @@ export const dashboardLogic = kea<dashboardLogicType>({
}, values.autoRefresh.interval * 1000)
}
},
loadDashboardItemsSuccess: () => {
loadDashboardItemsSuccess: function (...args) {
sharedListeners.reportLoadTiming(...args)

// Initial load of actual data for dashboard items after general dashboard is fetched
if (values.lastRefreshed && values.lastRefreshed.isBefore(now().subtract(3, 'hours'))) {
actions.refreshAllDashboardItems()
Expand Down