Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
65 changes: 55 additions & 10 deletions src/app/(main)/websites/[websiteId]/WebsiteChart.tsx
Original file line number Diff line number Diff line change
@@ -1,26 +1,34 @@
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,
});
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 +53,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
37 changes: 33 additions & 4 deletions src/app/api/websites/[websiteId]/pageviews/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ 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';

export async function GET(
request: Request,
Expand All @@ -27,19 +27,30 @@ export async function GET(

const filters = await getQueryFilters(query, websiteId);

const [pageviews, sessions] = await Promise.all([
const [pageviews, sessions, sessionSeries] = await Promise.all([
getPageviewStats(websiteId, filters),
getSessionStats(websiteId, filters),
getSessionStatsSeries(websiteId, filters),
]);
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.map(({ x, visits, bounces }) => ({
x,
y: Number(visits) > 0 ? (Number(bounces) / Number(visits)) * 100 : 0,
}));

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

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 +61,39 @@ export async function GET(
startDate: compareStartDate,
endDate: compareEndDate,
}),
getSessionStatsSeries(websiteId, {
...filters,
startDate: compareStartDate,
endDate: compareEndDate,
}),
]);

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

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

return json({ pageviews, sessions });
return json({ pageviews, sessions, bouncerate, 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]?.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
2 changes: 2 additions & 0 deletions src/components/hooks/queries/useWebsitePageviewsQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { useFilterParameters } from '../useFilterParameters';
export interface WebsitePageviewsData {
pageviews: { x: string; y: number }[];
sessions: { x: string; y: number }[];
bouncerate: { x: string; y: number }[];
visitduration: { x: string; y: number }[];
}

export function useWebsitePageviewsQuery(
Expand Down
Loading