Skip to content

Commit 9d57aff

Browse files
authored
fix(webapp): make the Queues hero charts environment-wide (#4486)
## Summary The four charts above the queues table aggregated over **at most the 25 queues on the current page**. They reused the loader's already-paginated queue array as a ClickHouse `queue IN (...)` filter, so paging or re-sorting changed the values, and a name search matching nothing blanked the whole chart row. The stat tiles above them were already environment-wide, so the two rows disagreed. They now read `env_metrics`, the environment-level rollup that already exists for exactly this (the built-in Queues dashboard and the health report read it). That is both correct and queue-count-independent: no `GROUP BY queue` across an entire environment, and no client-side summing. Note this is not only a paging artifact: page 1 under-reported too. On the seeded environment below, page 1 read 82% saturation against a true 87%, because the environment's running total is not the sum of one page of per-queue gauges. Three related fixes ride along. **Scheduling delay and throttling sawed to zero.** Both are event-driven, so at the 10-second bucket a short range picks, most buckets hold no samples at all and were drawn as `0ms`. Measured over a 1-hour window: **232 of 349 buckets had no scheduling-delay samples**. A bucket where nothing started is not a bucket where nothing waited, so the line was both ugly and wrong. TRQL grows a `minBucketSeconds` floor, plumbed through the metric resource route, and the hero tiles set 60s. Buckets that still have no samples render as a gap instead of a dive to zero. **The floor must not feed a width-dependent headline.** Two of the four headlines are not peaks, so widening the plotted buckets moved them: - **Throttled** is a share of buckets that saw any throttling, so a single brief throttle came to mark a whole minute instead of ten seconds: the same seeded events read 17% at 10s and 85% at 60s. - **Scheduling delay p95** is a percentile, and merging quantile states over a wider bucket yields a p95 between the sub-buckets' own. Two 240s samples among twenty in one 10-second sub-bucket give a worst-of-six p95 of 240,000ms against a merged 60-second p95 of 5,000ms — a 48x understatement of a headline whose tooltip claims it is the worst in the window. Both charts keep the floor, since a readable line was the point of it. Their headlines now come from a second query at the range's natural bucket width, via an optional `readout` on the tile, so each means what its tooltip says regardless of how the plotted buckets are sized. Saturation and backlog are genuinely width-invariant (a max of maxes is the same at any width), so they are unchanged and issue no extra query. Both caught by Devin in review; I had wrongly lumped p95 in with the peaks. **Charts reported a hydration mismatch on every render.** Recharts resolved victory-vendor's CJS entry on the server and its ESM entry in the browser. Those bundle different d3-shape builds, and the CJS one predates d3-path's digit rounding, so every server-rendered curve carried full-precision coordinates while the client rounded to 3 decimals: ``` Server: M0,3C0.9305555555555555,3,1.8611111111111112,3,... Client: M0,3C0.931,3,1.861,3,... ``` Bundling recharts for SSR makes both sides resolve the same ESM build. Verified: 45 of 45 server-rendered chart curves now match the client, and the page loads with an empty console. ## Verification An isolated stack with 40 seeded queues (20 heavily loaded, 20 idle) and 90 minutes of 10-second buckets written into `queue_metrics_raw_v1`, so the real materialized views built `queue_metrics_v1`, `env_metrics_v1` and the 5m rollup. Ground truth for the environment: 260 running against a limit of 300 (**87% saturation**), 800 queued. | | before | after | | -- | -- | -- | | Saturation, page 1 | 82% peak | **87% peak** | | Saturation, page 2 | 5% peak | **87% peak** | | Backlog / delay, page 2 | "No activity" | **800 peak / 59.5s** | | Name search matching nothing | all four charts blank | charts stay environment-wide | | Metric refetches on a page change | 4, each painting a skeleton | **0, no skeleton** | | Buckets drawn as 0ms with no samples | 232 of 349 | **0** | | Throttled readout | 17% | **17%**, unchanged by the wider buckets | | Worst-p95 readout source | plotted buckets | **natural width**, so a sub-minute spike is not averaged away | | Crosshair reach, hovering one detail-page chart | 2 of 4 others | **4 of 4** | | SSR chart curves mismatching the client | 45 | **0** | The bucket floor was measured across ranges: it widens 10s to 60s at 30m and 1h, and is correctly a no-op at 12h (300s) and 7d (3600s). One extra request per page load, for the throttled readout. The built-in Queues dashboard, which reads `env_metrics` independently, agrees at 86.7% and 260 of 300. `internal-packages/tsql` suite green (612 tests), including 5 new ones for the floor that fail without it. Webapp typecheck, oxfmt and oxlint clean. Spot-checked the Run metrics dashboard and the per-queue detail page for SSR regressions from bundling recharts: both render, console clean. The queue detail page carries the same event-driven series, so its scheduling delay, throttling and per-key mean delay take the same treatment. ## Screenshots <img width="2540" height="580" alt="after-page1-charts" src="https://github.com/user-attachments/assets/6cd23f9c-e7fd-4918-bcfa-b1d3340b16d1" /> ## Rollout Already behind the per-organization `queueMetricsUiEnabled` flag, so only gated orgs see any of it. Blast radius is chart values on one page plus the SSR bundling of recharts; rollback is a revert with no data migration. ## Stated limitations - `wait_ms_count` and the quantile state both only count `wait_ms > 0`, so "nothing started in this bucket" and "everything started instantly" are indistinguishable in storage. Both render as a gap. Distinguishing them needs a schema change, which is not in this PR. - The queue name search deliberately no longer narrows the charts. It only did so incidentally and incorrectly before (first 25 matches, and blanked on zero matches). Search-scoped charts would need the full unpaginated matching set and a server-side aggregate; worth its own ticket if we want it. - Bundling recharts for SSR grows the server bundle slightly. That is the cost of both sides resolving one d3-shape build. - The plotted delay line is a smoothed 60-second view, so a sub-minute spike above the one-minute warning threshold can fail to colour the line even though the headline reports it and colours itself. - Every chart inside one synced group shares the floor, because the hover crosshair is a reference line on a category x-axis and only draws where the hovered bucket exists in the other chart's own data. That costs the queue detail page's gauges some resolution (1 minute instead of 10 seconds) in exchange for the crosshair working across the row. Separately, while taking the screenshots I found a pre-existing rendering bug unrelated to this change: a **perfectly flat** saturation series draws no line at all (the readout still shows the right percentage), which looks like the threshold gradient's offset degenerating when the series min equals its max. It reproduces on `main`, so it is not a regression here and I have left it alone; filed as its own issue. Refs TRI-12784
1 parent 75df940 commit 9d57aff

14 files changed

Lines changed: 354 additions & 135 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
The four charts at the top of the Queues page now always cover the whole environment, so paging through or re-sorting your queues no longer changes them. The scheduling delay chart also leaves a gap where no runs started, instead of dropping to zero.

apps/webapp/app/components/queues/QueueMetricCards.tsx

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ export function useQueueMetric(
5454
defaultPeriod?: string;
5555
/** Poll ClickHouse on this cadence (ms). Omit to use the query's default interval. */
5656
refreshIntervalMs?: number;
57+
/** Floor for the bucket width, for series too sparse to read at the range's natural width. */
58+
minBucketSeconds?: number;
5759
}
5860
) {
5961
return useMetricResourceQuery(query, {
@@ -62,6 +64,7 @@ export function useQueueMetric(
6264
defaultPeriod: opts.defaultPeriod ?? QUEUE_METRICS_DEFAULT_PERIOD,
6365
queues: [opts.queueName],
6466
fillGaps: opts.fillGaps,
67+
minBucketSeconds: opts.minBucketSeconds,
6568
refreshIntervalMs: opts.refreshIntervalMs,
6669
});
6770
}
@@ -120,6 +123,14 @@ type QueueMetricChartProps = {
120123
/** Reports whether the chart has data to plot (false once it settles on the "no activity" state),
121124
* so a wrapping card can hide the legend to match. */
122125
onHasDataChange?: (hasData: boolean) => void;
126+
/** Floor for the bucket width, for series too sparse to read at the range's natural width. */
127+
minBucketSeconds?: number;
128+
/**
129+
* Column whose value counts the samples behind the plotted series. Where it is zero the metric
130+
* has nothing to report, so every series breaks there instead of reading as a real zero. Keep it
131+
* out of `series` — it is read for this test only, never drawn.
132+
*/
133+
sampleCountColumn?: string;
123134
};
124135

125136
// Bare chart (no card chrome) so it can live inside a shared card, e.g. a tabbed panel.
@@ -136,22 +147,26 @@ export function QueueMetricChart({
136147
carryBackfill,
137148
thresholdStroke,
138149
onHasDataChange,
150+
minBucketSeconds,
151+
sampleCountColumn,
139152
}: QueueMetricChartProps) {
140153
const { rows, showLoading, failed } = useQueueMetric(query, {
141154
ids,
142155
timeRange,
143156
queueName,
144157
fillGaps,
145158
defaultPeriod,
159+
minBucketSeconds,
146160
});
147161

148162
const data = useMemo(() => {
149163
const points = rows
150164
.map((r) => {
151-
const point: { bucket: number } & Record<string, number> = {
165+
const point: { bucket: number } & Record<string, number | null> = {
152166
bucket: clickhouseTimeToMs(r.t),
153167
};
154-
for (const s of series) point[s.key] = toNumber(r[s.key]);
168+
const hasSamples = sampleCountColumn ? toNumber(r[sampleCountColumn]) > 0 : true;
169+
for (const s of series) point[s.key] = hasSamples ? toNumber(r[s.key]) : null;
155170
return point;
156171
})
157172
.filter((p) => Number.isFinite(p.bucket));
@@ -160,15 +175,15 @@ export function QueueMetricChart({
160175
// value and carry it back over the earlier buckets so the line doesn't start at a false 0.
161176
if (carryBackfill?.length) {
162177
for (const key of carryBackfill) {
163-
const first = points.findIndex((p) => p[key] > 0);
178+
const first = points.findIndex((p) => toNumber(p[key]) > 0);
164179
if (first > 0) {
165180
const value = points[first]![key]!;
166181
for (let i = 0; i < first; i++) points[i]![key] = value;
167182
}
168183
}
169184
}
170185
return points;
171-
}, [rows, series, carryBackfill]);
186+
}, [rows, series, carryBackfill, sampleCountColumn]);
172187

173188
const chartConfig = useMemo(() => {
174189
const cfg: ChartConfig = {};
@@ -205,9 +220,14 @@ export function QueueMetricChart({
205220

206221
// Report data presence so a wrapping card can hide its legend when the chart settles on the
207222
// "no activity" state. Only report once loaded, so the legend stays put while loading.
223+
const hasPlottedData = useMemo(
224+
() => data.some((point) => series.some((s) => point[s.key] != null)),
225+
[data, series]
226+
);
227+
208228
useEffect(() => {
209-
if (!showLoading) onHasDataChange?.(!failed && data.length > 0);
210-
}, [showLoading, failed, data.length, onHasDataChange]);
229+
if (!showLoading) onHasDataChange?.(!failed && hasPlottedData);
230+
}, [showLoading, failed, hasPlottedData, onHasDataChange]);
211231

212232
return (
213233
<Chart.Root

apps/webapp/app/hooks/useMetricResourceQuery.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ export type MetricResourceQueryOptions = {
2121
defaultPeriod: string;
2222
queues?: string[];
2323
fillGaps?: boolean;
24+
/** Floor for the query's bucket width, for series too sparse to read at the range's width. */
25+
minBucketSeconds?: number;
2426
refreshIntervalMs?: number;
2527
};
2628

@@ -49,13 +51,19 @@ function cacheSet(key: string, rows: MetricResourceRow[]) {
4951
* back-navigation to the queues list) shows its last data immediately and revalidates in the
5052
* background rather than flashing a loading skeleton.
5153
*/
54+
/**
55+
* An empty query means the caller has nothing to ask for, so no request is made and any rows or
56+
* failure left by a previous query are dropped — a caller that stops asking must not keep reading
57+
* the last answer, or a stale failure would outlive the query that caused it.
58+
*/
5259
export function useMetricResourceQuery(query: string, opts: MetricResourceQueryOptions) {
5360
const {
5461
organizationId,
5562
projectId,
5663
environmentId,
5764
defaultPeriod,
5865
fillGaps,
66+
minBucketSeconds,
5967
refreshIntervalMs = 60_000,
6068
} = opts;
6169
const { period, from, to } = opts.timeRange;
@@ -71,10 +79,22 @@ export function useMetricResourceQuery(query: string, opts: MetricResourceQueryO
7179
from ?? "",
7280
to ?? "",
7381
fillGaps ? 1 : 0,
82+
minBucketSeconds ?? "",
7483
queuesKey ?? "",
7584
query,
7685
].join("|"),
77-
[organizationId, projectId, environmentId, resolvedPeriod, from, to, fillGaps, queuesKey, query]
86+
[
87+
organizationId,
88+
projectId,
89+
environmentId,
90+
resolvedPeriod,
91+
from,
92+
to,
93+
fillGaps,
94+
minBucketSeconds,
95+
queuesKey,
96+
query,
97+
]
7898
);
7999

80100
const [rows, setRows] = useState<MetricResourceRow[] | null>(
@@ -86,6 +106,14 @@ export function useMetricResourceQuery(query: string, opts: MetricResourceQueryO
86106
const loadedKeyRef = useRef<string | null>(null);
87107

88108
const load = useCallback(() => {
109+
if (!query) {
110+
abortRef.current?.abort();
111+
loadedKeyRef.current = cacheKey;
112+
setRows(null);
113+
setFailed(false);
114+
setIsLoading(false);
115+
return;
116+
}
89117
abortRef.current?.abort();
90118
const controller = new AbortController();
91119
abortRef.current = controller;
@@ -112,6 +140,7 @@ export function useMetricResourceQuery(query: string, opts: MetricResourceQueryO
112140
organizationId,
113141
projectId,
114142
environmentId,
143+
...(minBucketSeconds !== undefined ? { minBucketSeconds } : {}),
115144
...(queuesKey !== undefined ? { queues: queuesKey.split(",") } : {}),
116145
}),
117146
signal: controller.signal,
@@ -142,6 +171,7 @@ export function useMetricResourceQuery(query: string, opts: MetricResourceQueryO
142171
from,
143172
to,
144173
fillGaps,
174+
minBucketSeconds,
145175
organizationId,
146176
projectId,
147177
environmentId,

0 commit comments

Comments
 (0)