Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 58 additions & 10 deletions src/app/(main)/websites/[websiteId]/WebsiteChart.tsx
Original file line number Diff line number Diff line change
@@ -1,26 +1,37 @@
import { useMemo } from 'react';
import { LoadingPanel } from '@/components/common/LoadingPanel';
import { useDateRange, useTimezone } from '@/components/hooks';
import { useDateRange, useMessages, useNavigation, useTimezone } from '@/components/hooks';
import { useWebsitePageviewsQuery } from '@/components/hooks/queries/useWebsitePageviewsQuery';
import { MetricSeriesChart, type MetricSeriesKind } from '@/components/metrics/MetricSeriesChart';
import { PageviewsChart } from '@/components/metrics/PageviewsChart';

export type WebsiteChartMetric = 'pageviews' | MetricSeriesKind;

export const DEFAULT_WEBSITE_CHART_METRIC: WebsiteChartMetric = 'pageviews';

export function WebsiteChart({
websiteId,
compareMode,
}: {
websiteId: string;
compareMode?: boolean;
}) {
const { query } = useNavigation();
const metric = (query.metric as WebsiteChartMetric) ?? DEFAULT_WEBSITE_CHART_METRIC;
const { timezone } = useTimezone();
const { dateRange, dateCompare } = useDateRange({ timezone: timezone });
const { startDate, endDate, unit, value } = dateRange;
const { t, labels } = useMessages();
const { data, isLoading, isFetching, error } = useWebsitePageviewsQuery({
websiteId,
compare: compareMode ? dateCompare?.compare : undefined,
// Only ask the server for the session-series metrics when they are
// actually selected; otherwise the server skips the third DB query.
metric: metric === 'pageviews' ? undefined : metric,
});
const { pageviews, sessions, compare } = (data || {}) as any;
const { pageviews, sessions, bouncerate, visitduration, compare } = (data || {}) as any;

const chartData = useMemo(() => {
const pageviewsChartData = useMemo(() => {
if (!data) {
return { pageviews: [], sessions: [] };
}
Expand All @@ -45,15 +56,52 @@ export function WebsiteChart({
};
}, [data, startDate, endDate, unit]);

const metricChartData = useMemo(() => {
if (!data || metric === 'pageviews') return null;

const series = metric === 'bouncerate' ? bouncerate : visitduration;
const compareSeries = compare
? metric === 'bouncerate'
? compare.bouncerate
: compare.visitduration
: null;

return {
series: series ?? [],
...(compareSeries && {
compare: series.map(({ x }, i) => ({
x,
y: compareSeries[i]?.y ?? 0,
d: compareSeries[i]?.x,
})),
}),
};
}, [data, metric, bouncerate, visitduration, compare]);

return (
<LoadingPanel data={data} isFetching={isFetching} isLoading={isLoading} error={error}>
<PageviewsChart
key={value}
data={chartData}
minDate={startDate}
maxDate={endDate}
unit={unit}
/>
{metric === 'pageviews' ? (
<PageviewsChart
key={`${value}-pageviews`}
data={pageviewsChartData}
minDate={startDate}
maxDate={endDate}
unit={unit}
/>
) : (
<MetricSeriesChart
key={`${value}-${metric}`}
data={metricChartData}
Comment thread
yancat160 marked this conversation as resolved.
minDate={startDate}
maxDate={endDate}
unit={unit}
kind={metric}
label={metric === 'bouncerate' ? t(labels.bounceRate) : t(labels.visitDuration)}
comparePreviousLabel={`${
metric === 'bouncerate' ? t(labels.bounceRate) : t(labels.visitDuration)
} (${t(labels.previous)})`}
/>
)}
</LoadingPanel>
);
}
38 changes: 38 additions & 0 deletions src/app/(main)/websites/[websiteId]/WebsiteChartMetricFilter.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { ListItem, Row, Select } from '@umami/react-zen';
import { useMessages, useNavigation } from '@/components/hooks';
import type { WebsiteChartMetric } from './WebsiteChart';
import { DEFAULT_WEBSITE_CHART_METRIC } from './WebsiteChart';

export function WebsiteChartMetricFilter() {
const { t, labels } = useMessages();
const { router, query, updateParams } = useNavigation();

const options: { id: WebsiteChartMetric; label: string }[] = [
{ id: 'pageviews', label: `${t(labels.visitors)} / ${t(labels.views)}` },
{ id: 'bouncerate', label: t(labels.bounceRate) },
{ id: 'visitduration', label: t(labels.visitDuration) },
];

const selected = (query.metric as WebsiteChartMetric) ?? DEFAULT_WEBSITE_CHART_METRIC;

const handleChange = (value: string) => {
router.push(updateParams({ metric: value }));
};

return (
<Row>
<Select
value={selected}
onChange={handleChange}
popoverProps={{ placement: 'bottom right' }}
style={{ width: 180 }}
>
{options.map(({ id, label }) => (
<ListItem key={id} id={id}>
{label}
</ListItem>
))}
</Select>
</Row>
);
}
4 changes: 3 additions & 1 deletion src/app/(main)/websites/[websiteId]/WebsitePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ExpandedViewModal } from '@/app/(main)/websites/[websiteId]/ExpandedVie
import { Panel } from '@/components/common/Panel';
import { UnitFilter } from '@/components/input/UnitFilter';
import { WebsiteChart } from './WebsiteChart';
import { WebsiteChartMetricFilter } from './WebsiteChartMetricFilter';
import { WebsiteControls } from './WebsiteControls';
import { WebsiteMetricsBar } from './WebsiteMetricsBar';
import { WebsitePanels } from './WebsitePanels';
Expand All @@ -14,7 +15,8 @@ export function WebsitePage({ websiteId }: { websiteId: string }) {
<WebsiteControls websiteId={websiteId} allowBounceFilter={true} />
<WebsiteMetricsBar websiteId={websiteId} showChange={true} />
<Panel minHeight="520px">
<Row justifyContent="end">
<Row justifyContent="end" gap>
<WebsiteChartMetricFilter />
<UnitFilter />
</Row>
<WebsiteChart websiteId={websiteId} />
Expand Down
61 changes: 57 additions & 4 deletions src/app/api/websites/[websiteId]/pageviews/route.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
import { z } from 'zod';
import { getCompareDate } from '@/lib/date';
import { getQueryFilters, parseRequest } from '@/lib/request';
import { json, unauthorized } from '@/lib/response';
import { filterParams, withDateRange } from '@/lib/schema';
import { canViewWebsite } from '@/permissions';
import { getPageviewStats, getSessionStats } from '@/queries/sql';
import { getPageviewStats, getSessionStats, getSessionStatsSeries } from '@/queries/sql';

const seriesMetric = z.enum(['bouncerate', 'visitduration']);

export async function GET(
request: Request,
{ params }: { params: Promise<{ websiteId: string }> },
) {
const schema = withDateRange({
...filterParams,
metric: seriesMetric.optional(),
});

const { auth, query, error } = await parseRequest(request, schema);
Expand All @@ -26,20 +30,39 @@ export async function GET(
}

const filters = await getQueryFilters(query, websiteId);
// Only the bouncerate / visitduration metrics need the per-bucket session
// series. Skip the extra DB roundtrip for the default pageviews metric so a
// standard website-page load stays at two queries instead of three.
const wantsSessionSeries = query.metric === 'bouncerate' || query.metric === 'visitduration';

const [pageviews, sessions] = await Promise.all([
const [pageviews, sessions, sessionSeries] = await Promise.all([
getPageviewStats(websiteId, filters),
getSessionStats(websiteId, filters),
wantsSessionSeries ? getSessionStatsSeries(websiteId, filters) : Promise.resolve(null),
]);
Comment on lines +38 to 42

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Unconditional third DB query on every pageviews request

getSessionStatsSeries now runs in every call to this endpoint, even when the front-end is showing the default pageviews metric and will never read bouncerate or visitduration. The existing endpoint already performed 2 parallel queries; this PR silently adds a third for 100% of website-page loads, not just the subset of users who switch the metric selector. Under load the extra query will increase DB fan-out and latency for all visitors of all websites. The fix is to pass the chosen metric as a query parameter from the front-end and run getSessionStatsSeries only when the requested metric requires it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, applied in f9f0145d. The frontend now sends ?metric=bouncerate or ?metric=visitduration only when one of those metrics is selected, validated server-side via zod. getSessionStatsSeries (and the matching compare-period call) only run when the metric is set, so the default pageviews request is back to two parallel DB queries instead of three. Verified end-to-end: the response for /pageviews (no metric) now omits bouncerate and visitduration entirely, and the bouncerate / visitduration paths still render as before.


const bouncerate = sessionSeries
? sessionSeries.map(({ x, visits, bounces }) => ({
x,
y: Number(visits) > 0 ? (Number(bounces) / Number(visits)) * 100 : 0,
}))
: undefined;

const visitduration = sessionSeries
? sessionSeries.map(({ x, visits, totaltime }) => ({
x,
y: Number(visits) > 0 ? Number(totaltime) / Number(visits) : 0,
}))
: undefined;

if (filters.compare) {
const { startDate: compareStartDate, endDate: compareEndDate } = getCompareDate(
filters.compare,
filters.startDate,
filters.endDate,
);

const [comparePageviews, compareSessions] = await Promise.all([
const [comparePageviews, compareSessions, compareSeries] = await Promise.all([
getPageviewStats(websiteId, {
...filters,
startDate: compareStartDate,
Expand All @@ -50,21 +73,51 @@ export async function GET(
startDate: compareStartDate,
endDate: compareEndDate,
}),
wantsSessionSeries
? getSessionStatsSeries(websiteId, {
...filters,
startDate: compareStartDate,
endDate: compareEndDate,
})
: Promise.resolve(null),
]);

const compareBouncerate = compareSeries
? compareSeries.map(({ x, visits, bounces }) => ({
x,
y: Number(visits) > 0 ? (Number(bounces) / Number(visits)) * 100 : 0,
}))
: undefined;

const compareVisitduration = compareSeries
? compareSeries.map(({ x, visits, totaltime }) => ({
x,
y: Number(visits) > 0 ? Number(totaltime) / Number(visits) : 0,
}))
: undefined;

return json({
pageviews,
sessions,
...(bouncerate && { bouncerate }),
...(visitduration && { visitduration }),
startDate: filters.startDate,
endDate: filters.endDate,
compare: {
pageviews: comparePageviews,
sessions: compareSessions,
...(compareBouncerate && { bouncerate: compareBouncerate }),
...(compareVisitduration && { visitduration: compareVisitduration }),
startDate: compareStartDate,
endDate: compareEndDate,
},
});
}

return json({ pageviews, sessions });
return json({
pageviews,
sessions,
...(bouncerate && { bouncerate }),
...(visitduration && { visitduration }),
});
}
20 changes: 16 additions & 4 deletions src/components/charts/BarChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@ export interface BarChartProps extends ChartProps {
currency?: string;
renderXLabel?: (label: string, index: number, values: any[]) => string;
renderYLabel?: (label: string, index: number, values: any[]) => string;
renderTooltipValue?: (raw: { x: any; y: number }, datasetLabel: string) => string;
XAxisType?: string;
YAxisType?: string;
yMax?: number;
ySuggestedMax?: number;
minDate?: Date;
maxDate?: Date;
}
Expand All @@ -44,9 +47,12 @@ function BarChartComponent({
chartData,
renderXLabel,
renderYLabel,
renderTooltipValue,
unit,
XAxisType = 'timeseries',
YAxisType = 'linear',
yMax,
ySuggestedMax,
stacked = false,
minDate,
maxDate,
Expand Down Expand Up @@ -88,6 +94,8 @@ function BarChartComponent({
type: YAxisType,
min: 0,
beginAtZero: true,
...(yMax !== undefined && { max: yMax }),
...(ySuggestedMax !== undefined && { suggestedMax: ySuggestedMax }),
stacked: !!stacked,
grid: {
color: colors.chart.line,
Expand All @@ -113,6 +121,8 @@ function BarChartComponent({
locale,
XAxisType,
YAxisType,
yMax,
ySuggestedMax,
]);

const handleTooltip = useCallback(
Expand All @@ -126,9 +136,11 @@ function BarChartComponent({
locale,
),
color: labelColors?.[0]?.borderColor || labelColors?.[0]?.backgroundColor,
value: currency
? formatLongCurrency(dataPoints[0].raw.y, currency)
: `${formatLongNumber(dataPoints[0].raw.y)} ${dataPoints[0].dataset.label}`,
value: renderTooltipValue
? renderTooltipValue(dataPoints[0].raw, dataPoints[0].dataset.label)
: currency
? formatLongCurrency(dataPoints[0].raw.y, currency)
: `${formatLongNumber(dataPoints[0].raw.y)} ${dataPoints[0].dataset.label}`,
}
: null;

Expand All @@ -144,7 +156,7 @@ function BarChartComponent({
return nextTooltip;
});
},
[currency, locale, unit],
[currency, locale, unit, renderTooltipValue],
);

return (
Expand Down
16 changes: 14 additions & 2 deletions src/components/hooks/queries/useWebsitePageviewsQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,24 @@ import { useApi } from '../useApi';
import { useDateParameters } from '../useDateParameters';
import { useFilterParameters } from '../useFilterParameters';

export type WebsitePageviewsMetric = 'bouncerate' | 'visitduration';

export interface WebsitePageviewsData {
pageviews: { x: string; y: number }[];
sessions: { x: string; y: number }[];
// Only present when the request asked for the matching metric. Skipping
// them on the default pageviews request keeps the server from running an
// extra session-series query on every website-page load.
bouncerate?: { x: string; y: number }[];
visitduration?: { x: string; y: number }[];
}

export function useWebsitePageviewsQuery(
{ websiteId, compare }: { websiteId: string; compare?: string },
{
websiteId,
compare,
metric,
}: { websiteId: string; compare?: string; metric?: WebsitePageviewsMetric },
options?: ReactQueryOptions<WebsitePageviewsData>,
) {
const { get, useQuery } = useApi();
Expand All @@ -19,11 +30,12 @@ export function useWebsitePageviewsQuery(
return useQuery<WebsitePageviewsData>({
queryKey: [
'websites:pageviews',
{ websiteId, compare, startAt, endAt, unit, timezone, ...queryParams },
{ websiteId, compare, metric, startAt, endAt, unit, timezone, ...queryParams },
],
queryFn: () =>
get(`/websites/${websiteId}/pageviews`, {
compare,
...(metric && { metric }),
startAt,
endAt,
unit,
Expand Down
Loading