Skip to content

Commit 1ce9e99

Browse files
committed
fix(webapp): stop rejecting long custom queue metric durations
The period pattern only accepted a count of up to four digits, so a custom duration the picker allows (10000 minutes, a little under 7 days) was treated as unusable and quietly replaced with the default. The count is now unbounded and the retention bound is what rules a window out. Resolution also clamps the period from the URL, not just the remembered default, so a period wider than the plan's query period shows the window the data actually covers instead of the one that was asked for. The plan lookup reads through the limit cache and is skipped entirely on the classic Queues page, which has no time filter: the page revalidates on an interval, so an uncached platform call would repeat for the life of the tab.
1 parent 4a55f03 commit 1ce9e99

4 files changed

Lines changed: 28 additions & 10 deletions

File tree

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,20 @@
1-
import { getLimit } from "~/services/platform.v3.server";
1+
import { getCachedLimit } from "~/services/platform.v3.server";
22
import { QUEUE_METRICS_RETENTION_DAYS } from "./queueMetricsPeriod";
33

44
/**
55
* The furthest back this org can query queue metrics: their plan's query period, capped at the
66
* 30 day retention. Same limit `executeQuery` enforces, so the queue-metric queries that bypass it
77
* and go straight to ClickHouse stay in step with the ones that don't.
8+
*
9+
* Read through the limit cache: the queues page revalidates on an interval, so this runs far more
10+
* often than a one-off page load. Whole days keep the derived period string well formed.
811
*/
912
export async function queueMetricsMaxPeriodDays(organizationId: string): Promise<number> {
10-
const planPeriodDays = await getLimit(
13+
const cached = await getCachedLimit(
1114
organizationId,
1215
"queryPeriodDays",
1316
QUEUE_METRICS_RETENTION_DAYS
1417
);
15-
return Math.min(planPeriodDays, QUEUE_METRICS_RETENTION_DAYS);
18+
const planPeriodDays = cached.val ?? QUEUE_METRICS_RETENTION_DAYS;
19+
return Math.max(1, Math.min(Math.floor(planPeriodDays), QUEUE_METRICS_RETENTION_DAYS));
1620
}

apps/webapp/app/components/queues/queueMetricsPeriod.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,12 @@ export const QUEUE_METRICS_DEFAULT_PERIOD = "1h";
1515
const COOKIE_NAME = "queueMetricsPeriod";
1616
const COOKIE_MAX_AGE_SECONDS = 60 * 60 * 24 * 365;
1717

18-
/** The shape TimeFilter writes: a count plus a minute/hour/day unit (presets and custom durations). */
19-
const PERIOD_PATTERN = /^\d{1,4}[mhd]$/;
18+
/**
19+
* The shape TimeFilter writes: a count plus a minute/hour/day unit. The count is unbounded here
20+
* because the picker accepts any positive integer for a custom duration (`10000m` is a little under
21+
* 7 days); the retention bound below is what rules a window out.
22+
*/
23+
const PERIOD_PATTERN = /^\d+[mhd]$/;
2024

2125
/** Queue metrics are retained for 30 days, so a longer window can only ever render empty. */
2226
export const QUEUE_METRICS_RETENTION_DAYS = 30;
@@ -61,7 +65,8 @@ export function useRememberQueueMetricsPeriod(period: string | undefined) {
6165
/**
6266
* The window the page should show: a usable period in the URL wins, an absolute range means "no
6367
* period", and everything else (including a period the picker could never produce, e.g. a
64-
* hand-edited `?period=garbage`) falls back to the remembered default the loader resolved.
68+
* hand-edited `?period=garbage`) falls back to the remembered default the loader resolved. The
69+
* result is held inside the org's plan query period, since that is the window the data will cover.
6570
*
6671
* Both the loaders and the client-side chart queries resolve through here, so they can't disagree
6772
* about the window.
@@ -71,15 +76,17 @@ export function resolveQueueMetricsPeriod({
7176
from,
7277
to,
7378
defaultPeriod,
79+
maxPeriodDays,
7480
}: {
7581
period: string | undefined;
7682
from: string | undefined;
7783
to: string | undefined;
7884
defaultPeriod: string;
85+
maxPeriodDays: number;
7986
}): string | null {
80-
if (isPeriod(period)) return period;
87+
if (isPeriod(period)) return clampQueueMetricsPeriod(period, maxPeriodDays);
8188
if (from || to) return null;
82-
return defaultPeriod;
89+
return clampQueueMetricsPeriod(defaultPeriod, maxPeriodDays);
8390
}
8491

8592
/**

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ import { canAccessQueueMetricsUi } from "~/v3/canAccessQueueMetricsUi.server";
104104
import { QueueAllocationPresenter } from "~/presenters/v3/QueueAllocationPresenter.server";
105105
import {
106106
QUEUE_METRICS_DEFAULT_PERIOD,
107+
QUEUE_METRICS_RETENTION_DAYS,
107108
clampQueueMetricsPeriod,
108109
clipQueueMetricsWindow,
109110
queueMetricsPeriodFromRequest,
@@ -170,7 +171,9 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
170171
// no metrics query fires.
171172
const queueMetricsUiEnabled = await canAccessQueueMetricsUi({ userId, organizationSlug });
172173

173-
const maxPeriodDays = await queueMetricsMaxPeriodDays(environment.organizationId);
174+
const maxPeriodDays = queueMetricsUiEnabled
175+
? await queueMetricsMaxPeriodDays(environment.organizationId)
176+
: QUEUE_METRICS_RETENTION_DAYS;
174177
const defaultPeriod = clampQueueMetricsPeriod(
175178
queueMetricsPeriodFromRequest(request),
176179
maxPeriodDays
@@ -209,7 +212,9 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
209212
);
210213
const timeRange = clipQueueMetricsWindow(
211214
timeFilterFromTo({
212-
period: resolveQueueMetricsPeriod({ period, from, to, defaultPeriod }) ?? undefined,
215+
period:
216+
resolveQueueMetricsPeriod({ period, from, to, defaultPeriod, maxPeriodDays }) ??
217+
undefined,
213218
from: parseFiniteInt(from),
214219
to: parseFiniteInt(to),
215220
defaultPeriod,
@@ -406,6 +411,7 @@ function QueuesWithMetricsView() {
406411
from: value("from"),
407412
to: value("to"),
408413
defaultPeriod,
414+
maxPeriodDays,
409415
}),
410416
from: value("from") ?? null,
411417
to: value("to") ?? null,

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues_.$queueParam/route.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@ export default function Page() {
230230
from: value("from"),
231231
to: value("to"),
232232
defaultPeriod,
233+
maxPeriodDays,
233234
}),
234235
from: value("from") ?? null,
235236
to: value("to") ?? null,

0 commit comments

Comments
 (0)