From ca3b23c9d3c3e25a7b18e44887fcf51c66f22a11 Mon Sep 17 00:00:00 2001 From: FindMalek Date: Thu, 27 Aug 2026 17:28:58 +0100 Subject: [PATCH 1/2] perf(rpc): batch goals bulkAnalytics ClickHouse queries for unfiltered goals bulkAnalytics fired 2 ClickHouse queries per goal (completions + denominator) inside a Promise.all, so a website with 20 goals issued ~40 concurrent round-trips on every dashboard load. Goals with no filters at all (request-level or goal-level) now get grouped by their effective start date and counted in one batched query per date bucket via processGoalsConversionCountsBatch, plus one shared getTotalWebsiteUsers call, instead of 2 queries per goal. Any goal with a filter keeps the original one-query-per-goal path unchanged: buildIdentifiedEventStream only threads filter conditions through the step at array index 0 (correct for its funnel-entry-filter use case), so batching filtered goals together would silently apply one goal's filter to another's count. Scoping the batch to the zero-filter case keeps that shared query builder untouched. Fixes #679 --- packages/rpc/package.json | 2 +- .../lib/analytics-utils-goals-batch.test.ts | 93 ++++++++++++++ packages/rpc/src/lib/analytics-utils.ts | 113 ++++++++++++----- packages/rpc/src/routers/goals.ts | 117 +++++++++++++++--- 4 files changed, 272 insertions(+), 53 deletions(-) create mode 100644 packages/rpc/src/lib/analytics-utils-goals-batch.test.ts diff --git a/packages/rpc/package.json b/packages/rpc/package.json index 0df6320c1..cf494a7d5 100644 --- a/packages/rpc/package.json +++ b/packages/rpc/package.json @@ -7,7 +7,7 @@ "types": "./src/index.ts", "scripts": { "check-types": "tsc --noEmit", - "test": "REDIS_URL=\"${REDIS_URL:-redis://localhost:6379}\" bun test --isolate src/routers src/lib/analytics-utils.integration.test.ts src/lib/funnels-cache.test.ts src/middleware/*.test.ts src/procedures/*.test.ts src/services/insight-schedule.test.ts src/services/uptime-lifecycle.test.ts src/services/uptime-scheduler.test.ts src/utils/*.test.ts", + "test": "REDIS_URL=\"${REDIS_URL:-redis://localhost:6379}\" bun test --isolate src/routers src/lib/analytics-utils.integration.test.ts src/lib/analytics-utils-goals-batch.test.ts src/lib/funnels-cache.test.ts src/middleware/*.test.ts src/procedures/*.test.ts src/services/insight-schedule.test.ts src/services/uptime-lifecycle.test.ts src/services/uptime-scheduler.test.ts src/utils/*.test.ts", "test:integration": "bun test src/services/uptime-scheduler.integration.test.ts" }, "exports": { diff --git a/packages/rpc/src/lib/analytics-utils-goals-batch.test.ts b/packages/rpc/src/lib/analytics-utils-goals-batch.test.ts new file mode 100644 index 000000000..3a18db3c1 --- /dev/null +++ b/packages/rpc/src/lib/analytics-utils-goals-batch.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, mock, test } from "bun:test"; + +const chQueryMock = mock((_query: string, _params?: Record) => + Promise.resolve([] as unknown[]) +); + +mock.module("@databuddy/db/clickhouse", () => ({ + chQuery: chQueryMock, + chCommand: mock(async () => undefined), +})); + +const { buildGoalAnalyticsResult, processGoalsConversionCountsBatch } = + await import("./analytics-utils"); + +describe("processGoalsConversionCountsBatch", () => { + test("returns an empty map and issues no query for an empty step list", async () => { + chQueryMock.mockClear(); + + const result = await processGoalsConversionCountsBatch([], { + websiteId: "site_1", + startDate: "2026-01-01", + endDate: "2026-01-07 23:59:59", + }); + + expect(result.size).toBe(0); + expect(chQueryMock).not.toHaveBeenCalled(); + }); + + test("counts every goal in a single ClickHouse round trip", async () => { + chQueryMock.mockClear(); + chQueryMock.mockImplementationOnce(() => + Promise.resolve([ + { step_num: 1, completions: 42 }, + { step_num: 2, completions: 7 }, + { step_num: 3, completions: 0 }, + ]) + ); + + const result = await processGoalsConversionCountsBatch( + [ + { step_number: 1, type: "PAGE_VIEW", target: "/pricing", name: "Pricing" }, + { step_number: 2, type: "EVENT", target: "signup", name: "Signup" }, + { step_number: 3, type: "EVENT", target: "purchase", name: "Purchase" }, + ], + { + websiteId: "site_1", + startDate: "2026-01-01", + endDate: "2026-01-07 23:59:59", + } + ); + + // One query counts all three goals, instead of one query per goal. + expect(chQueryMock).toHaveBeenCalledTimes(1); + expect(result.get(1)).toBe(42); + expect(result.get(2)).toBe(7); + expect(result.get(3)).toBe(0); + }); + + test("never receives a non-empty filter list, which would make batching unsafe", async () => { + chQueryMock.mockClear(); + chQueryMock.mockImplementationOnce(() => Promise.resolve([])); + + await processGoalsConversionCountsBatch( + [{ step_number: 1, type: "EVENT", target: "signup", name: "Signup" }], + { websiteId: "site_1", startDate: "2026-01-01", endDate: "2026-01-07" } + ); + + const [query] = chQueryMock.mock.calls.at(-1) as [string]; + // The generated step-match condition must not carry any per-index filter + // gating (see the correctness note on processGoalsConversionCountsBatch); + // there is no "filters" parameter for a caller to pass one through. + expect(query).not.toContain("browserFilter"); + expect(query).not.toContain("customFilter"); + }); +}); + +describe("buildGoalAnalyticsResult", () => { + test("computes conversion rate from completions and total entered users", () => { + const analytics = buildGoalAnalyticsResult("Signup", 25, 100); + + expect(analytics.total_users_completed).toBe(25); + expect(analytics.total_users_entered).toBe(100); + expect(analytics.overall_conversion_rate).toBe(25); + expect(analytics.steps_analytics).toHaveLength(1); + expect(analytics.steps_analytics[0]?.step_name).toBe("Signup"); + }); + + test("reports a zero conversion rate instead of dividing by zero", () => { + const analytics = buildGoalAnalyticsResult("Signup", 0, 0); + + expect(analytics.overall_conversion_rate).toBe(0); + }); +}); diff --git a/packages/rpc/src/lib/analytics-utils.ts b/packages/rpc/src/lib/analytics-utils.ts index aff8685c4..2750c4ffa 100644 --- a/packages/rpc/src/lib/analytics-utils.ts +++ b/packages/rpc/src/lib/analytics-utils.ts @@ -855,6 +855,44 @@ ORDER BY step_num, date`; }; }; +export const buildGoalAnalyticsResult = ( + stepName: string, + completions: number, + totalWebsiteUsers: number +): FunnelAnalytics => ({ + overall_conversion_rate: pct(completions, totalWebsiteUsers), + total_users_entered: totalWebsiteUsers, + total_users_completed: completions, + avg_completion_time: 0, + avg_completion_time_formatted: "—", + biggest_dropoff_step: 1, + biggest_dropoff_rate: 0, + duration_available: false, + steps_analytics: [ + { + step_number: 1, + step_name: stepName, + users: completions, + total_users: totalWebsiteUsers, + conversion_rate: pct(completions, totalWebsiteUsers), + dropoffs: 0, + dropoff_rate: 0, + avg_time_to_complete: 0, + error_context_available: false, + error_count: 0, + error_rate: 0, + top_errors: [], + }, + ], + error_insights: { + available: false, + total_errors: 0, + sessions_with_errors: 0, + dropoffs_with_errors: 0, + error_correlation_rate: 0, + }, +}); + export const processGoalAnalytics = async ( steps: AnalyticsStep[], filters: Filter[], @@ -873,39 +911,48 @@ export const processGoalAnalytics = async ( abortSignal ); - return { - overall_conversion_rate: pct(completions, totalWebsiteUsers), - total_users_entered: totalWebsiteUsers, - total_users_completed: completions, - avg_completion_time: 0, - avg_completion_time_formatted: "—", - biggest_dropoff_step: 1, - biggest_dropoff_rate: 0, - duration_available: false, - steps_analytics: [ - { - step_number: 1, - step_name: step.name, - users: completions, - total_users: totalWebsiteUsers, - conversion_rate: pct(completions, totalWebsiteUsers), - dropoffs: 0, - dropoff_rate: 0, - avg_time_to_complete: 0, - error_context_available: false, - error_count: 0, - error_rate: 0, - top_errors: [], - }, - ], - error_insights: { - available: false, - total_errors: 0, - sessions_with_errors: 0, - dropoffs_with_errors: 0, - error_correlation_rate: 0, - }, - }; + return buildGoalAnalyticsResult(step.name, completions, totalWebsiteUsers); +}; + +/** + * Counts completions for several independent (non-sequential) goals in one query. + * + * Safe only when no filters apply: `buildIdentifiedEventStream` only threads its + * `filters` argument into the match condition for the step at array index 0, since + * that's correct for its normal caller (funnel entry filters gating step 1). With + * an empty filter list that branch never fires, so every step's condition reduces + * to the same plain target/type match regardless of index — which is what makes it + * safe to treat each array entry as an independent goal instead of a funnel step. + * Never pass a non-empty `filters` array through this path. + */ +export const processGoalsConversionCountsBatch = async ( + steps: AnalyticsStep[], + params: ClickhouseQueryParams, + abortSignal?: AbortSignal +): Promise> => { + if (steps.length === 0) { + return new Map(); + } + + const query = `WITH ${visitorIdentityCtes}, +${buildIdentifiedEventStream(steps, [], params)} +SELECT toUInt8(step) AS step_num, uniqExact(vid) AS completions +FROM events +GROUP BY step_num`; + const rows = await chQuery<{ step_num: number; completions: number }>( + query, + params, + { abort_signal: abortSignal } + ); + + const result = new Map(); + for (const row of rows) { + result.set( + toFiniteNumber(row.step_num, 0), + toFiniteNumber(row.completions, 0) + ); + } + return result; }; // Referrer analytics — step matching in ClickHouse, referrer grouping in JS diff --git a/packages/rpc/src/routers/goals.ts b/packages/rpc/src/routers/goals.ts index e7b82f3b3..229242956 100644 --- a/packages/rpc/src/routers/goals.ts +++ b/packages/rpc/src/routers/goals.ts @@ -11,8 +11,10 @@ import { z } from "zod"; import { rpcError } from "../errors"; import { type AnalyticsStep, + buildGoalAnalyticsResult, getTotalWebsiteUsers, processGoalAnalytics, + processGoalsConversionCountsBatch, } from "../lib/analytics-utils"; import { logger } from "../lib/logger"; import { invalidateGoalsCache } from "../lib/goals-cache"; @@ -478,8 +480,95 @@ export const goalsRouter = { .orderBy(desc(goals.createdAt)); const requestFilters = input.filters ?? []; - const results = await Promise.all( - goalsList.map(async (goal): Promise<[string, GoalAnalyticsResult]> => { + type Goal = (typeof goalsList)[number]; + + // Goals with no filters at all (neither request-level nor goal-level) can be + // counted together in one ClickHouse query per shared date range, since their + // step-match conditions are identical in shape regardless of order — see + // processGoalsConversionCountsBatch. Any goal with a filter keeps the original + // one-query-per-goal path, since batching filtered goals is not safe. + const batchGroups = new Map(); + const individualGoals: { goal: Goal; combinedFilters: Filter[] }[] = []; + + for (const goal of goalsList) { + const filters = (goal.filters as Filter[]) || []; + const combinedFilters = [...requestFilters, ...filters]; + if (combinedFilters.length > 0) { + individualGoals.push({ goal, combinedFilters }); + continue; + } + + const effectiveStartDate = getEffectiveStartDate( + startDate, + goal.createdAt, + goal.ignoreHistoricData + ); + const group = batchGroups.get(effectiveStartDate) ?? []; + group.push(goal); + batchGroups.set(effectiveStartDate, group); + } + + const analyticsByGoal: Record = {}; + + await Promise.all([ + ...Array.from(batchGroups.entries()).map( + async ([effectiveStartDate, groupGoals]) => { + try { + const [totalUsers, completionsByStep] = await Promise.all([ + getTotalWebsiteUsers( + input.websiteId, + effectiveStartDate, + endDate, + [] + ), + processGoalsConversionCountsBatch( + groupGoals.map( + (goal, index): AnalyticsStep => ({ + step_number: index + 1, + type: getAnalyticsStepType(goal.type), + target: goal.target, + name: goal.name, + }) + ), + { + websiteId: input.websiteId, + startDate: effectiveStartDate, + endDate: `${endDate} 23:59:59`, + } + ), + ]); + + groupGoals.forEach((goal, index) => { + const completions = completionsByStep.get(index + 1) ?? 0; + analyticsByGoal[goal.id] = { + ok: true, + data: buildGoalAnalyticsResult( + goal.name, + completions, + totalUsers + ), + }; + }); + } catch (error) { + logger.error( + { + error, + websiteId: input.websiteId, + effectiveStartDate, + goalIds: groupGoals.map((goal) => goal.id), + }, + "Failed to process batched goal analytics" + ); + for (const goal of groupGoals) { + analyticsByGoal[goal.id] = { + ok: false, + error: "Failed to process goal analytics", + }; + } + } + } + ), + ...individualGoals.map(async ({ goal, combinedFilters }) => { const effectiveStartDate = getEffectiveStartDate( startDate, goal.createdAt, @@ -495,9 +584,6 @@ export const goalsRouter = { }, ]; - const filters = (goal.filters as Filter[]) || []; - const combinedFilters = [...requestFilters, ...filters]; - try { const totalUsers = await getTotalWebsiteUsers( input.websiteId, @@ -515,7 +601,7 @@ export const goalsRouter = { }, totalUsers ); - return [goal.id, { ok: true, data: analytics }]; + analyticsByGoal[goal.id] = { ok: true, data: analytics }; } catch (error) { logger.error( { @@ -525,21 +611,14 @@ export const goalsRouter = { }, "Failed to process goal analytics" ); - return [ - goal.id, - { - ok: false, - error: "Failed to process goal analytics", - }, - ]; + analyticsByGoal[goal.id] = { + ok: false, + error: "Failed to process goal analytics", + }; } - }) - ); + }), + ]); - const analyticsByGoal: Record = {}; - for (const [goalId, result] of results) { - analyticsByGoal[goalId] = result; - } return analyticsByGoal; }), }; From dcef465623a8ba6fb2086c303c10d8d66ad79c70 Mon Sep 17 00:00:00 2001 From: FindMalek Date: Thu, 27 Aug 2026 17:40:26 +0100 Subject: [PATCH 2/2] fix(rpc): cap goal batch size and isolate batched query failures Fixes two issues Greptile flagged on the batching PR: processGoalsConversionCountsBatch encoded each goal as a ClickHouse UInt8 step number, which wraps past 255 and silently merges unrelated goals' completion counts on unlimited-plan websites with large goal counts. Grouping and chunking is now capped at 255 goals per query and extracted into groupGoalsForBulkAnalytics, a pure function with its own tests, instead of inline router logic. A failed batched query previously marked every goal in that date bucket as failed. It now falls back to the original per-goal queries for just that bucket, so one bad query no longer takes down otherwise healthy sibling goals. --- packages/rpc/package.json | 2 +- .../lib/analytics-utils-goals-batch.test.ts | 4 - packages/rpc/src/lib/analytics-utils.ts | 11 -- .../lib/goals-bulk-analytics-grouping.test.ts | 103 +++++++++++ .../src/lib/goals-bulk-analytics-grouping.ts | 77 ++++++++ packages/rpc/src/routers/goals.ts | 164 +++++++----------- 6 files changed, 248 insertions(+), 113 deletions(-) create mode 100644 packages/rpc/src/lib/goals-bulk-analytics-grouping.test.ts create mode 100644 packages/rpc/src/lib/goals-bulk-analytics-grouping.ts diff --git a/packages/rpc/package.json b/packages/rpc/package.json index cf494a7d5..683d6fc8f 100644 --- a/packages/rpc/package.json +++ b/packages/rpc/package.json @@ -7,7 +7,7 @@ "types": "./src/index.ts", "scripts": { "check-types": "tsc --noEmit", - "test": "REDIS_URL=\"${REDIS_URL:-redis://localhost:6379}\" bun test --isolate src/routers src/lib/analytics-utils.integration.test.ts src/lib/analytics-utils-goals-batch.test.ts src/lib/funnels-cache.test.ts src/middleware/*.test.ts src/procedures/*.test.ts src/services/insight-schedule.test.ts src/services/uptime-lifecycle.test.ts src/services/uptime-scheduler.test.ts src/utils/*.test.ts", + "test": "REDIS_URL=\"${REDIS_URL:-redis://localhost:6379}\" bun test --isolate src/routers src/lib/analytics-utils.integration.test.ts src/lib/analytics-utils-goals-batch.test.ts src/lib/goals-bulk-analytics-grouping.test.ts src/lib/funnels-cache.test.ts src/middleware/*.test.ts src/procedures/*.test.ts src/services/insight-schedule.test.ts src/services/uptime-lifecycle.test.ts src/services/uptime-scheduler.test.ts src/utils/*.test.ts", "test:integration": "bun test src/services/uptime-scheduler.integration.test.ts" }, "exports": { diff --git a/packages/rpc/src/lib/analytics-utils-goals-batch.test.ts b/packages/rpc/src/lib/analytics-utils-goals-batch.test.ts index 3a18db3c1..872975812 100644 --- a/packages/rpc/src/lib/analytics-utils-goals-batch.test.ts +++ b/packages/rpc/src/lib/analytics-utils-goals-batch.test.ts @@ -49,7 +49,6 @@ describe("processGoalsConversionCountsBatch", () => { } ); - // One query counts all three goals, instead of one query per goal. expect(chQueryMock).toHaveBeenCalledTimes(1); expect(result.get(1)).toBe(42); expect(result.get(2)).toBe(7); @@ -66,9 +65,6 @@ describe("processGoalsConversionCountsBatch", () => { ); const [query] = chQueryMock.mock.calls.at(-1) as [string]; - // The generated step-match condition must not carry any per-index filter - // gating (see the correctness note on processGoalsConversionCountsBatch); - // there is no "filters" parameter for a caller to pass one through. expect(query).not.toContain("browserFilter"); expect(query).not.toContain("customFilter"); }); diff --git a/packages/rpc/src/lib/analytics-utils.ts b/packages/rpc/src/lib/analytics-utils.ts index 2750c4ffa..2c153f471 100644 --- a/packages/rpc/src/lib/analytics-utils.ts +++ b/packages/rpc/src/lib/analytics-utils.ts @@ -914,17 +914,6 @@ export const processGoalAnalytics = async ( return buildGoalAnalyticsResult(step.name, completions, totalWebsiteUsers); }; -/** - * Counts completions for several independent (non-sequential) goals in one query. - * - * Safe only when no filters apply: `buildIdentifiedEventStream` only threads its - * `filters` argument into the match condition for the step at array index 0, since - * that's correct for its normal caller (funnel entry filters gating step 1). With - * an empty filter list that branch never fires, so every step's condition reduces - * to the same plain target/type match regardless of index — which is what makes it - * safe to treat each array entry as an independent goal instead of a funnel step. - * Never pass a non-empty `filters` array through this path. - */ export const processGoalsConversionCountsBatch = async ( steps: AnalyticsStep[], params: ClickhouseQueryParams, diff --git a/packages/rpc/src/lib/goals-bulk-analytics-grouping.test.ts b/packages/rpc/src/lib/goals-bulk-analytics-grouping.test.ts new file mode 100644 index 000000000..b5733450f --- /dev/null +++ b/packages/rpc/src/lib/goals-bulk-analytics-grouping.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, test } from "bun:test"; +import { groupGoalsForBulkAnalytics } from "./goals-bulk-analytics-grouping"; + +interface TestFilter { + field: string; + operator: string; + value: string; +} + +interface TestGoal { + createdAt: Date | null; + filters: TestFilter[] | null; + id: string; + ignoreHistoricData: boolean; +} + +const goal = (id: string, filters: TestFilter[] | null = null): TestGoal => ({ + id, + createdAt: null, + ignoreHistoricData: false, + filters, +}); + +describe("groupGoalsForBulkAnalytics", () => { + test("splits a batch larger than chunkSize into multiple chunks", () => { + const goals = Array.from({ length: 5 }, (_, i) => goal(`g${i}`)); + + const { batchChunks, individualGoals } = groupGoalsForBulkAnalytics( + goals, + [], + "2026-01-01", + 2 + ); + + expect(individualGoals).toHaveLength(0); + expect(batchChunks).toHaveLength(3); + expect(batchChunks.map((chunk) => chunk.goals.length)).toEqual([2, 2, 1]); + expect(batchChunks.flatMap((chunk) => chunk.goals.map((g) => g.id))).toEqual( + ["g0", "g1", "g2", "g3", "g4"] + ); + }); + + test("routes goals with a goal-level filter to individualGoals", () => { + const filtered = goal("filtered", [ + { field: "path", operator: "equals", value: "/pricing" }, + ]); + const unfiltered = goal("unfiltered"); + + const { batchChunks, individualGoals } = groupGoalsForBulkAnalytics( + [filtered, unfiltered], + [], + "2026-01-01", + 255 + ); + + expect(individualGoals).toEqual([ + { goal: filtered, combinedFilters: filtered.filters }, + ]); + expect(batchChunks).toHaveLength(1); + expect(batchChunks[0]?.goals).toEqual([unfiltered]); + }); + + test("routes every goal to individualGoals when a request-level filter applies", () => { + const goals = [goal("a"), goal("b")]; + const requestFilters: TestFilter[] = [ + { field: "country", operator: "equals", value: "US" }, + ]; + + const { batchChunks, individualGoals } = groupGoalsForBulkAnalytics( + goals, + requestFilters, + "2026-01-01", + 255 + ); + + expect(batchChunks).toHaveLength(0); + expect(individualGoals).toHaveLength(2); + for (const entry of individualGoals) { + expect(entry.combinedFilters).toEqual(requestFilters); + } + }); + + test("groups filter-free goals by effective start date into separate chunks", () => { + const recent = goal("recent"); + const backfilled: TestGoal = { + id: "backfilled", + createdAt: new Date("2026-01-15"), + ignoreHistoricData: true, + filters: null, + }; + + const { batchChunks } = groupGoalsForBulkAnalytics( + [recent, backfilled], + [], + "2026-01-01", + 255 + ); + + expect(batchChunks).toHaveLength(2); + const dates = batchChunks.map((chunk) => chunk.effectiveStartDate).sort(); + expect(dates).toEqual(["2026-01-01", "2026-01-15"]); + }); +}); diff --git a/packages/rpc/src/lib/goals-bulk-analytics-grouping.ts b/packages/rpc/src/lib/goals-bulk-analytics-grouping.ts new file mode 100644 index 000000000..53d43339f --- /dev/null +++ b/packages/rpc/src/lib/goals-bulk-analytics-grouping.ts @@ -0,0 +1,77 @@ +export interface GoalForGrouping { + createdAt: Date | null; + filters: unknown; + id: string; + ignoreHistoricData: boolean; +} + +export const getEffectiveStartDate = ( + requestedStartDate: string, + createdAt: Date | null, + ignoreHistoricData: boolean +): string => { + if (!(ignoreHistoricData && createdAt)) { + return requestedStartDate; + } + + const createdDate = new Date(createdAt).toISOString().split("T")[0]; + return new Date(requestedStartDate) > new Date(createdDate) + ? requestedStartDate + : createdDate; +}; + +export interface BatchChunk { + effectiveStartDate: string; + goals: TGoal[]; +} + +export interface GroupedGoalsForBulkAnalytics< + TGoal extends GoalForGrouping, + TFilter, +> { + batchChunks: BatchChunk[]; + individualGoals: { combinedFilters: TFilter[]; goal: TGoal }[]; +} + +export function groupGoalsForBulkAnalytics< + TGoal extends GoalForGrouping, + TFilter, +>( + goalsList: TGoal[], + requestFilters: TFilter[], + startDate: string, + chunkSize: number +): GroupedGoalsForBulkAnalytics { + const batchGroups = new Map(); + const individualGoals: { combinedFilters: TFilter[]; goal: TGoal }[] = []; + + for (const goal of goalsList) { + const filters = (goal.filters as TFilter[]) || []; + const combinedFilters = [...requestFilters, ...filters]; + if (combinedFilters.length > 0) { + individualGoals.push({ goal, combinedFilters }); + continue; + } + + const effectiveStartDate = getEffectiveStartDate( + startDate, + goal.createdAt, + goal.ignoreHistoricData + ); + const group = batchGroups.get(effectiveStartDate) ?? []; + group.push(goal); + batchGroups.set(effectiveStartDate, group); + } + + const batchChunks: BatchChunk[] = []; + for (const [effectiveStartDate, groupGoals] of batchGroups) { + for (let i = 0; i < groupGoals.length; i += chunkSize) { + batchChunks.push({ + effectiveStartDate, + goals: groupGoals.slice(i, i + chunkSize), + }); + } + } + + return { batchChunks, individualGoals }; +} diff --git a/packages/rpc/src/routers/goals.ts b/packages/rpc/src/routers/goals.ts index 229242956..bd1df1de0 100644 --- a/packages/rpc/src/routers/goals.ts +++ b/packages/rpc/src/routers/goals.ts @@ -16,8 +16,12 @@ import { processGoalAnalytics, processGoalsConversionCountsBatch, } from "../lib/analytics-utils"; -import { logger } from "../lib/logger"; import { invalidateGoalsCache } from "../lib/goals-cache"; +import { + getEffectiveStartDate, + groupGoalsForBulkAnalytics, +} from "../lib/goals-bulk-analytics-grouping"; +import { logger } from "../lib/logger"; import { setTrackProperties } from "../middleware/track-mutation"; import { publicProcedure, trackedProcedure } from "../orpc"; import { @@ -29,6 +33,7 @@ import { requireFeatureWithLimit } from "../types/billing"; import { queueDefinitionChangeRechecks } from "./insights"; const ANALYTICS_CACHE_TTL = 180; +const BATCH_CHUNK_SIZE = 255; const cache = createDrizzleCache({ redis, namespace: "goals" }); const filterSchema = z.object({ @@ -134,21 +139,6 @@ const goalAnalyticsResultSchema = z.discriminatedUnion("ok", [ type GoalAnalyticsResult = z.infer; -const getEffectiveStartDate = ( - requestedStartDate: string, - createdAt: Date | null, - ignoreHistoricData: boolean -): string => { - if (!(ignoreHistoricData && createdAt)) { - return requestedStartDate; - } - - const createdDate = new Date(createdAt).toISOString().split("T")[0]; - return new Date(requestedStartDate) > new Date(createdDate) - ? requestedStartDate - : createdDate; -}; - const getAnalyticsStepType = (type: "CUSTOM" | "EVENT" | "PAGE_VIEW") => type === "PAGE_VIEW" ? "PAGE_VIEW" : "EVENT"; @@ -482,37 +472,66 @@ export const goalsRouter = { const requestFilters = input.filters ?? []; type Goal = (typeof goalsList)[number]; - // Goals with no filters at all (neither request-level nor goal-level) can be - // counted together in one ClickHouse query per shared date range, since their - // step-match conditions are identical in shape regardless of order — see - // processGoalsConversionCountsBatch. Any goal with a filter keeps the original - // one-query-per-goal path, since batching filtered goals is not safe. - const batchGroups = new Map(); - const individualGoals: { goal: Goal; combinedFilters: Filter[] }[] = []; - - for (const goal of goalsList) { - const filters = (goal.filters as Filter[]) || []; - const combinedFilters = [...requestFilters, ...filters]; - if (combinedFilters.length > 0) { - individualGoals.push({ goal, combinedFilters }); - continue; - } + const analyticsByGoal: Record = {}; + const runGoalIndividually = async ( + goal: Goal, + combinedFilters: Filter[] + ) => { const effectiveStartDate = getEffectiveStartDate( startDate, goal.createdAt, goal.ignoreHistoricData ); - const group = batchGroups.get(effectiveStartDate) ?? []; - group.push(goal); - batchGroups.set(effectiveStartDate, group); - } + const steps: AnalyticsStep[] = [ + { + step_number: 1, + type: getAnalyticsStepType(goal.type), + target: goal.target, + name: goal.name, + }, + ]; + + try { + const totalUsers = await getTotalWebsiteUsers( + input.websiteId, + effectiveStartDate, + endDate, + combinedFilters + ); + const analytics = await processGoalAnalytics( + steps, + combinedFilters, + { + websiteId: input.websiteId, + startDate: effectiveStartDate, + endDate: `${endDate} 23:59:59`, + }, + totalUsers + ); + analyticsByGoal[goal.id] = { ok: true, data: analytics }; + } catch (error) { + logger.error( + { error, goalId: goal.id, websiteId: input.websiteId }, + "Failed to process goal analytics" + ); + analyticsByGoal[goal.id] = { + ok: false, + error: "Failed to process goal analytics", + }; + } + }; - const analyticsByGoal: Record = {}; + const { batchChunks, individualGoals } = groupGoalsForBulkAnalytics( + goalsList, + requestFilters, + startDate, + BATCH_CHUNK_SIZE + ); await Promise.all([ - ...Array.from(batchGroups.entries()).map( - async ([effectiveStartDate, groupGoals]) => { + ...batchChunks.map( + async ({ effectiveStartDate, goals: chunkGoals }) => { try { const [totalUsers, completionsByStep] = await Promise.all([ getTotalWebsiteUsers( @@ -522,7 +541,7 @@ export const goalsRouter = { [] ), processGoalsConversionCountsBatch( - groupGoals.map( + chunkGoals.map( (goal, index): AnalyticsStep => ({ step_number: index + 1, type: getAnalyticsStepType(goal.type), @@ -538,7 +557,7 @@ export const goalsRouter = { ), ]); - groupGoals.forEach((goal, index) => { + chunkGoals.forEach((goal, index) => { const completions = completionsByStep.get(index + 1) ?? 0; analyticsByGoal[goal.id] = { ok: true, @@ -555,68 +574,19 @@ export const goalsRouter = { error, websiteId: input.websiteId, effectiveStartDate, - goalIds: groupGoals.map((goal) => goal.id), + goalIds: chunkGoals.map((goal) => goal.id), }, - "Failed to process batched goal analytics" + "Batched goal analytics query failed; falling back to per-goal queries" + ); + await Promise.all( + chunkGoals.map((goal) => runGoalIndividually(goal, [])) ); - for (const goal of groupGoals) { - analyticsByGoal[goal.id] = { - ok: false, - error: "Failed to process goal analytics", - }; - } } } ), - ...individualGoals.map(async ({ goal, combinedFilters }) => { - const effectiveStartDate = getEffectiveStartDate( - startDate, - goal.createdAt, - goal.ignoreHistoricData - ); - - const steps: AnalyticsStep[] = [ - { - step_number: 1, - type: getAnalyticsStepType(goal.type), - target: goal.target, - name: goal.name, - }, - ]; - - try { - const totalUsers = await getTotalWebsiteUsers( - input.websiteId, - effectiveStartDate, - endDate, - combinedFilters - ); - const analytics = await processGoalAnalytics( - steps, - combinedFilters, - { - websiteId: input.websiteId, - startDate: effectiveStartDate, - endDate: `${endDate} 23:59:59`, - }, - totalUsers - ); - analyticsByGoal[goal.id] = { ok: true, data: analytics }; - } catch (error) { - logger.error( - { - error, - goalId: goal.id, - websiteId: input.websiteId, - }, - "Failed to process goal analytics" - ); - analyticsByGoal[goal.id] = { - ok: false, - error: "Failed to process goal analytics", - }; - } - }), + ...individualGoals.map(({ goal, combinedFilters }) => + runGoalIndividually(goal, combinedFilters) + ), ]); return analyticsByGoal;