Skip to content

Commit

Permalink
feat(web-analytics): Add warning when conversion goal is a custom act…
Browse files Browse the repository at this point in the history
…ion with no session id (#26441)
  • Loading branch information
robbie-c authored Nov 27, 2024
1 parent f250d52 commit 9597fe8
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 7 deletions.
1 change: 1 addition & 0 deletions frontend/src/lib/constants.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ export const FEATURE_FLAGS = {
SELF_SERVE_CREDIT_OVERRIDE: 'self-serve-credit-override', // owner: @zach
EXPERIMENTS_MIGRATION_DISABLE_UI: 'experiments-migration-disable-ui', // owner: @jurajmajerik #team-experiments
CUSTOM_CSS_THEMES: 'custom-css-themes', // owner: @daibhin
WEB_ANALYTICS_WARN_CUSTOM_EVENT_NO_SESSION: 'web-analytics-warn-custom-event-no-session', // owner: @robbie-c #team-web-analytics
} as const
export type FeatureFlagKey = (typeof FEATURE_FLAGS)[keyof typeof FEATURE_FLAGS]

Expand Down
28 changes: 26 additions & 2 deletions frontend/src/scenes/web-analytics/WebAnalyticsHealthCheck.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,34 @@
import { useValues } from 'kea'
import { useFeatureFlag } from 'lib/hooks/useFeatureFlag'
import { LemonBanner } from 'lib/lemon-ui/LemonBanner'
import { Link } from 'lib/lemon-ui/Link'
import { webAnalyticsLogic } from 'scenes/web-analytics/webAnalyticsLogic'
import { ConversionGoalWarning, webAnalyticsLogic } from 'scenes/web-analytics/webAnalyticsLogic'

export const WebAnalyticsHealthCheck = (): JSX.Element | null => {
const { statusCheck } = useValues(webAnalyticsLogic)
const { statusCheck, conversionGoalWarning } = useValues(webAnalyticsLogic)
const isFlagConversionGoalWarningsSet = useFeatureFlag('WEB_ANALYTICS_WARN_CUSTOM_EVENT_NO_SESSION')

if (conversionGoalWarning && isFlagConversionGoalWarningsSet) {
switch (conversionGoalWarning) {
case ConversionGoalWarning.CustomEventWithNoSessionId:
return (
<LemonBanner type="warning" className="mt-2">
<p>
A custom event has been set as a conversion goal, but it has been seen with no{' '}
<code>$session_id</code>. This means that some queries will not be able to include these
events.
</p>
<p>
To fix this, please see{' '}
<Link to="https://posthog.com/docs/data/sessions#custom-session-ids">
documentation for custom session IDs
</Link>
.
</p>
</LemonBanner>
)
}
}

// No need to show loading or error states for this warning
if (!statusCheck) {
Expand Down
72 changes: 67 additions & 5 deletions frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import { actions, afterMount, connect, kea, listeners, path, reducers, selectors } from 'kea'
import { actions, afterMount, BreakPointFunction, connect, kea, listeners, path, reducers, selectors } from 'kea'
import { loaders } from 'kea-loaders'
import { actionToUrl, urlToAction } from 'kea-router'
import { windowValues } from 'kea-window-values'
import api from 'lib/api'
import { FEATURE_FLAGS, RETENTION_FIRST_TIME, STALE_EVENT_SECONDS } from 'lib/constants'
import { dayjs } from 'lib/dayjs'
import { Link, PostHogComDocsURL } from 'lib/lemon-ui/Link/Link'
import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
import { featureFlagLogic, FeatureFlagsSet } from 'lib/logic/featureFlagLogic'
import { getDefaultInterval, isNotNil, objectsEqual, updateDatesWithInterval } from 'lib/utils'
import { errorTrackingQuery } from 'scenes/error-tracking/queries'
import { urls } from 'scenes/urls'

import { hogqlQuery } from '~/queries/query'
import {
ActionConversionGoal,
ActionsNode,
Expand Down Expand Up @@ -194,6 +195,10 @@ export interface WebAnalyticsStatusCheck {
isSendingPageLeavesScroll: boolean
}

export enum ConversionGoalWarning {
CustomEventWithNoSessionId = 'CustomEventWithNoSessionId',
}

export const GEOIP_PLUGIN_URLS = [
'https://github.com/PostHog/posthog-plugin-geoip',
'https://www.npmjs.com/package/@posthog/geoip-plugin',
Expand Down Expand Up @@ -269,6 +274,7 @@ export const webAnalyticsLogic = kea<webAnalyticsLogicType>([
openAsNewInsight: (tileId: TileId, tabId?: string) => {
return { tileId, tabId }
},
setConversionGoalWarning: (warning: ConversionGoalWarning | null) => ({ warning }),
}),
reducers({
webAnalyticsFilters: [
Expand Down Expand Up @@ -455,6 +461,12 @@ export const webAnalyticsLogic = kea<webAnalyticsLogicType>([
setConversionGoal: (_, { conversionGoal }) => conversionGoal,
},
],
conversionGoalWarning: [
null as ConversionGoalWarning | null,
{
setConversionGoalWarning: (_, { warning }) => warning,
},
],
}),
selectors(({ actions, values }) => ({
graphsTab: [(s) => [s._graphsTab], (graphsTab: string | null) => graphsTab || GraphsTab.UNIQUE_USERS],
Expand Down Expand Up @@ -1489,18 +1501,68 @@ export const webAnalyticsLogic = kea<webAnalyticsLogicType>([
}
}
}

return {
setGraphsTab: ({ tab }) => {
checkGraphsTabIsCompatibleWithConversionGoal(tab, values.conversionGoal)
},
setConversionGoal: ({ conversionGoal }) => {
checkGraphsTabIsCompatibleWithConversionGoal(values.graphsTab, conversionGoal)
},
setConversionGoal: [
({ conversionGoal }) => {
checkGraphsTabIsCompatibleWithConversionGoal(values.graphsTab, conversionGoal)
},
({ conversionGoal }, breakpoint) =>
checkCustomEventConversionGoalHasSessionIdsHelper(
conversionGoal,
breakpoint,
actions.setConversionGoalWarning,
values.featureFlags
),
],
}
}),
afterMount(({ actions, values }) => {
checkCustomEventConversionGoalHasSessionIdsHelper(
values.conversionGoal,
undefined,
actions.setConversionGoalWarning,
values.featureFlags
).catch(() => {
// ignore, this warning is just a nice-to-have, no point showing an error to the user
})
}),
])

const isDefinitionStale = (definition: EventDefinition | PropertyDefinition): boolean => {
const parsedLastSeen = definition.last_seen_at ? dayjs(definition.last_seen_at) : null
return !!parsedLastSeen && dayjs().diff(parsedLastSeen, 'seconds') > STALE_EVENT_SECONDS
}

const checkCustomEventConversionGoalHasSessionIdsHelper = async (
conversionGoal: WebAnalyticsConversionGoal | null,
breakpoint: BreakPointFunction | undefined,
setConversionGoalWarning: (warning: ConversionGoalWarning | null) => void,
featureFlags: FeatureFlagsSet
): Promise<void> => {
if (!featureFlags[FEATURE_FLAGS.WEB_ANALYTICS_WARN_CUSTOM_EVENT_NO_SESSION]) {
return
}

if (!conversionGoal || !('customEventName' in conversionGoal) || !conversionGoal.customEventName) {
setConversionGoalWarning(null)
return
}
const { customEventName } = conversionGoal
// check if we have any conversion events from the last week without sessions ids

const response = await hogqlQuery(
`select count() from events where timestamp >= (now() - toIntervalHour(24)) AND ($session_id IS NULL OR $session_id = '') AND event = {event}`,
{ event: customEventName }
)
breakpoint?.()
const row = response.results[0]
if (row[0]) {
setConversionGoalWarning(ConversionGoalWarning.CustomEventWithNoSessionId)
} else {
setConversionGoalWarning(null)
}
}

0 comments on commit 9597fe8

Please sign in to comment.