Skip to content

Commit bd28d05

Browse files
MariefayTrigger.dev RepoOps
authored andcommitted
fix(webapp): harden progressive trace loading
Clicking a span in the run trace view no longer reloads the page data. Traces past the view limit now show their first part with a "partially displayed" notice instead of an empty view, and the "Errors only" filter, live updates and the emergency span cap put less load on the server. Progressive loading is now controlled by its feature flag alone. Mono-RevId: 8d876197c351f77777bfd2871933212f359ff8d7
1 parent 3f8ba0a commit bd28d05

23 files changed

Lines changed: 645 additions & 166 deletions

‎.env.schema‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,6 @@ RUN_REPLICATION_CLICKHOUSE_URL=${CLICKHOUSE_URL}
4141
RUN_REPLICATION_ENABLED=1
4242
# LOGS_SEARCH_PROJECTOR_ENABLED=1
4343
# LOGS_SEARCH_PROJECTOR_PREVIEW_ENABLED=1
44-
# Live tail for progressive trace loading; "1" turns it on, anything else turns it off.
45-
# INCREMENTAL_LIVE_TAIL_ENABLED=0
4644
# Store task run spans/traces in ClickHouse so the dashboard trace view is
4745
# populated in local dev. The local stack is ClickHouse-backed (see above), so
4846
# leaving this unset falls back to the "postgres" store and dev run traces show

‎apps/webapp/app/env.server.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,8 +1116,6 @@ const EnvironmentSchema = z
11161116
EVENT_LOOP_UTILIZATION_MONITOR_ENABLED: z.string().default("1"),
11171117
MAXIMUM_LIVE_RELOADING_EVENTS: z.coerce.number().int().default(1000),
11181118
MAXIMUM_TRACE_SUMMARY_VIEW_COUNT: z.coerce.number().int().default(25_000),
1119-
// Live tail for progressive trace loading; "1" on, anything else off.
1120-
INCREMENTAL_LIVE_TAIL_ENABLED: z.string().default("0"),
11211119
MAXIMUM_TRACE_DETAILED_SUMMARY_VIEW_COUNT: z.coerce.number().int().default(10_000),
11221120
// Emergency circuit breaker: when set, clamps the trace summary and detailed
11231121
// summary span limits on both event store paths to this value. Unset = disabled.

‎apps/webapp/app/hooks/useProgressiveTrace.ts‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ type WireChunkResponse = {
4242
events: WireChunkEvent[];
4343
nextCursor: TraceChunkCursor | null;
4444
hasMore: boolean;
45+
// The server capped the result: errors-only matches, or paging under the emergency cap.
46+
isTruncated?: boolean;
4547
// Server time (ms) taken before the page was read.
4648
readAt?: number;
4749
};
@@ -53,7 +55,6 @@ type ProgressiveMeta = {
5355
hasMore: boolean;
5456
buildOptions: BuildTraceViewOptions;
5557
showDebug: boolean;
56-
totalSpans?: number;
5758
maxSpans?: number;
5859
liveTailEnabled?: boolean;
5960
// Server time (ms) before the first chunk was read; the first tail reads back to it.
@@ -79,6 +80,8 @@ export type ProgressiveTraceState = {
7980
linkedRunIdBySpanId: Record<string, string>;
8081
isComplete: boolean;
8182
isTruncated: boolean;
83+
// Errors-only matches were capped; separate from `isTruncated`, which stops live reload.
84+
errorsTruncated: boolean;
8285
loadFailed: boolean;
8386
};
8487

@@ -96,6 +99,7 @@ function initialState(trace: ProgressiveTraceInput): ProgressiveTraceState {
9699
linkedRunIdBySpanId: trace.linkedRunIdBySpanId ?? {},
97100
isComplete: !trace.progressive?.hasMore,
98101
isTruncated: false,
102+
errorsTruncated: false,
99103
loadFailed: false,
100104
};
101105
}
@@ -192,6 +196,8 @@ export function useProgressiveTrace(
192196
const errorsFetchedRef = useRef(false);
193197
// The deep-link payload already merged, so a kept tree only merges new ones.
194198
const mergedSupplementaryRef = useRef<WireChunkEvent[] | null>(null);
199+
// The first chunk already merged; a same-run revalidate merges only a new one.
200+
const mergedFirstEventsRef = useRef<WireChunkEvent[] | null>(null);
195201
// Assembler the in-flight tail targets; a stale tail never touches the current one.
196202
const tailingRef = useRef<TraceChunkAssembler | null>(null);
197203
// Timestamp of the last tail read, for the size-aware cadence governor.
@@ -220,6 +226,7 @@ export function useProgressiveTrace(
220226
linkedRunIdBySpanId: view.linkedRunIdBySpanId,
221227
isComplete: prev.isComplete,
222228
isTruncated: prev.isTruncated,
229+
errorsTruncated: prev.errorsTruncated,
223230
loadFailed: prev.loadFailed,
224231
}));
225232
}, []);
@@ -249,6 +256,7 @@ export function useProgressiveTrace(
249256

250257
const assembler = new TraceChunkAssembler();
251258
assembler.mergeChunk(meta.firstEvents.map(toChunkEvent));
259+
mergedFirstEventsRef.current = meta.firstEvents;
252260
if (meta.supplementaryFirstEvents?.length) {
253261
assembler.mergeChunk(meta.supplementaryFirstEvents.map(toChunkEvent), {
254262
source: "deeplink",
@@ -338,7 +346,7 @@ export function useProgressiveTrace(
338346
requestRebuild();
339347
cursor = data.hasMore ? data.nextCursor : null;
340348

341-
if (assembler.size > maxSpans) {
349+
if (assembler.size > maxSpans || data.isTruncated) {
342350
markComplete(true);
343351
return;
344352
}
@@ -360,9 +368,12 @@ export function useProgressiveTrace(
360368
};
361369
}, [identity, chunkPath, retryGeneration, rebuild]);
362370

371+
// Every chunk is loaded, so the client-side filter already sees every error.
372+
const fullyLoaded = state.isComplete && !state.loadFailed && !state.isTruncated;
373+
363374
useEffect(() => {
364375
const meta = latestTraceRef.current.progressive;
365-
if (!errorsOnly || !meta || errorsFetchedRef.current) {
376+
if (!errorsOnly || !meta || errorsFetchedRef.current || fullyLoaded) {
366377
return;
367378
}
368379

@@ -374,12 +385,15 @@ export function useProgressiveTrace(
374385
errorsFetchedRef.current = true;
375386
assembler.mergeChunk(data.events.map(toChunkEvent), { source: "errors" });
376387
rebuild(meta, assembler);
388+
if (data.isTruncated) {
389+
setState((prev) => (prev.errorsTruncated ? prev : { ...prev, errorsTruncated: true }));
390+
}
377391
})();
378392

379393
return () => {
380394
cancelled = true;
381395
};
382-
}, [errorsOnly, identity, chunkPath, retryGeneration, rebuild]);
396+
}, [errorsOnly, fullyLoaded, identity, chunkPath, retryGeneration, rebuild]);
383397

384398
const runTail = useCallback(() => {
385399
const meta = latestTraceRef.current.progressive;
@@ -477,6 +491,20 @@ export function useProgressiveTrace(
477491
rebuild(meta, assembler);
478492
}, [supplementary, rebuild]);
479493

494+
// A kept tree merges a revalidated first chunk, e.g. the root's final row after the
495+
// tail stopped at the view ceiling.
496+
const firstEvents = progressive?.firstEvents;
497+
useEffect(() => {
498+
const meta = latestTraceRef.current.progressive;
499+
const assembler = assemblerRef.current;
500+
if (!meta?.liveTailEnabled || !assembler || !firstEvents) return;
501+
if (firstEvents === mergedFirstEventsRef.current) return;
502+
mergedFirstEventsRef.current = firstEvents;
503+
assembler.mergeChunk(firstEvents.map(toChunkEvent), { source: "revalidate" });
504+
if (!assembler.changedSinceRender) return;
505+
rebuild(meta, assembler);
506+
}, [firstEvents, rebuild]);
507+
480508
// Stable trigger so the route's SSE effect doesn't re-fire on chunkPath changes.
481509
useEffect(() => {
482510
tailLiveRef.current = runTail;

‎apps/webapp/app/hooks/useRunStatusBackstop.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ import type { RunStatusData } from "~/routes/resources.orgs.$organizationSlug.pr
44

55
const DEFAULT_POLL_MS = 15_000;
66

7-
// Polls the run status until it finishes, then calls `onFinished` once. Failed polls
8-
// are ignored and retried on the next interval.
7+
// Polls the run status and calls `onFinished` on each poll that sees the run finished,
8+
// until the caller disables it. The reload `onFinished` triggers can read a lagging
9+
// replica or be interrupted, so polling continues until the page data shows the run
10+
// finished. Failed polls are ignored and retried on the next interval.
911
export function useRunStatusBackstop({
1012
enabled,
1113
statusPath,
@@ -37,8 +39,6 @@ export function useRunStatusBackstop({
3739
if (stopped || fetchError || !response.ok) return;
3840
const [parseError, data] = await tryCatch(response.json() as Promise<RunStatusData>);
3941
if (stopped || parseError || (!data.isFinished && data.completedAt === null)) return;
40-
stopped = true;
41-
window.clearInterval(id);
4242
onFinishedRef.current();
4343
};
4444
const id = window.setInterval(() => void poll(), pollMs);

‎apps/webapp/app/presenters/v3/RunPresenter.server.ts‎

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ export class RunPresenter {
224224
const firstChunkPromise = progressivePromise.then((progressiveEnabled) =>
225225
isRootRunView &&
226226
progressiveEnabled &&
227-
canLiveTail(env.INCREMENTAL_LIVE_TAIL_ENABLED, run.taskEventStore)
227+
canLiveTail(run.taskEventStore, env.TRACE_VIEW_EMERGENCY_SPAN_CAP)
228228
? repository.getTraceChunk(
229229
getTaskEventStoreTableForRun(run),
230230
environment.id,
@@ -252,39 +252,6 @@ export class RunPresenter {
252252
isAdmin: user?.admin ?? false,
253253
};
254254

255-
let totalSpans: number | undefined;
256-
if (isRootRunView && firstChunk?.hasMore) {
257-
const spanCount = await repository.getTraceSpanCount(
258-
getTaskEventStoreTableForRun(run),
259-
environment.id,
260-
run.traceId,
261-
traceTimeBounds.startCreatedAt,
262-
traceTimeBounds.endCreatedAt,
263-
{ includeDebugLogs: showDebug }
264-
);
265-
totalSpans = typeof spanCount === "number" ? spanCount : undefined;
266-
267-
if (typeof spanCount === "number" && spanCount > repository.maximumTraceViewCount) {
268-
return {
269-
run: runData,
270-
trace: {
271-
events: [],
272-
duration: 0,
273-
rootStartedAt: undefined,
274-
rootSpanStatus: "completed" as const,
275-
startedAt: run.startedAt,
276-
queuedDuration,
277-
overridesBySpanId: {},
278-
linkedRunIdBySpanId: {},
279-
isTruncated: true,
280-
missingAnchor: true,
281-
progressive: undefined,
282-
},
283-
maximumLiveReloadingSetting: repository.maximumLiveReloadingSetting,
284-
};
285-
}
286-
}
287-
288255
if (firstChunk && hasWriteTimes(firstChunk.events)) {
289256
const firstEvents = stripAdminOnlyEventRows(firstChunk.events, buildOptions.isAdmin);
290257

@@ -334,7 +301,6 @@ export class RunPresenter {
334301
hasMore: firstChunk.hasMore,
335302
buildOptions,
336303
showDebug,
337-
totalSpans,
338304
maxSpans: repository.maximumTraceViewCount,
339305
liveTailEnabled: true,
340306
firstChunkReadAt,

‎apps/webapp/app/presenters/v3/TraceChunkPresenter.server.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export class TraceChunkPresenter {
3838
filter?: "errors";
3939
/** Live tail: only return rows written at/after this write time (ms since epoch). */
4040
tailInsertedAtSinceMs?: number;
41-
}): Promise<TraceChunk | undefined> {
41+
}): Promise<(TraceChunk & { isTruncated?: boolean }) | undefined> {
4242
const run = await runStore.findRun(
4343
{ friendlyId: runFriendlyId },
4444
{
@@ -98,21 +98,22 @@ export class TraceChunkPresenter {
9898
const endCreatedAt = run.completedAt ?? undefined;
9999

100100
if (filter === "errors") {
101-
const events = await repository.getTraceErrorEvents(
101+
const errors = await repository.getTraceErrorEvents(
102102
storeTable,
103103
environment.id,
104104
run.traceId,
105105
startCreatedAt,
106106
endCreatedAt,
107107
{ includeDebugLogs: showDebug }
108108
);
109-
if (!events) {
109+
if (!errors) {
110110
return undefined;
111111
}
112112
return {
113-
events: stripAdminOnlyEventRows(events, isAdmin),
113+
events: stripAdminOnlyEventRows(errors.events, isAdmin),
114114
nextCursor: null,
115115
hasMore: false,
116+
isTruncated: errors.isTruncated,
116117
};
117118
}
118119

‎apps/webapp/app/presenters/v3/liveTailGate.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
// Progressive loading only runs with the live tail: without it every stream ping would
2-
// reset and re-download the tree. The env switch turns both off, and only the v2 store
3-
// has the write time the tail pages by.
4-
export function canLiveTail(envSwitch: string, taskEventStore: string): boolean {
5-
return envSwitch === "1" && taskEventStore === "clickhouse_v2";
1+
// Progressive loading only runs with the live tail (without it every stream ping would
2+
// reset and re-download the tree), and only the v2 store has the write time it pages by.
3+
// The emergency span cap turns both off, so new loads take the summary path that honours it.
4+
export function canLiveTail(taskEventStore: string, emergencySpanCap: number | undefined): boolean {
5+
return taskEventStore === "clickhouse_v2" && emergencySpanCap === undefined;
66
}
77

88
export function hasWriteTimes(events: { insertedAt?: string }[]): boolean {

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

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,9 @@ import { findRunByIdWithMollifierFallback } from "~/v3/mollifier/readFallback.se
104104
import { buildSyntheticRunHeader } from "~/v3/mollifier/syntheticRunHeader.server";
105105
import { buildSyntheticTraceForBufferedRun } from "~/v3/mollifier/syntheticTrace.server";
106106
import { clickhouseFactory } from "~/services/clickhouse/clickhouseFactoryInstance.server";
107-
import { getImpersonationId } from "~/services/impersonation.server";
108107
import { logger } from "~/services/logger.server";
109108
import { getResizableSnapshot } from "~/services/resizablePanel.server";
110-
import { requireUserId } from "~/services/session.server";
109+
import { requireUser } from "~/services/session.server";
111110
import { rbac } from "~/services/rbac.server";
112111
import { runAgentPageContext } from "~/components/dashboard-agent/suggested-prompts";
113112
import { WhenAgentUnavailable } from "~/components/dashboard-agent/WhenAgentUnavailable";
@@ -128,6 +127,7 @@ import {
128127
} from "~/utils/pathBuilder";
129128
import type { SpanOverride } from "~/v3/eventRepository/eventRepository.types";
130129
import { useCurrentPlan } from "../_app.orgs.$organizationSlug/route";
130+
import { shouldRevalidateRunPage } from "./shouldRevalidateRunPage";
131131
import { SpanView } from "../resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route";
132132
import { pageMeta } from "~/utils/pageTitle";
133133

@@ -278,24 +278,28 @@ async function runWritePermissions(request: Request, userId: string, organizatio
278278
return { canReplayRun: canWriteRun, canCancelRun: canWriteRun };
279279
}
280280

281+
export { shouldRevalidateRunPage as shouldRevalidate };
282+
281283
export const handle: Handle = {
282284
agentPageContext: (data) => runAgentPageContext(data),
283285
};
284286

285287
export const loader = async ({ request, params }: LoaderFunctionArgs) => {
286-
const userId = await requireUserId(request);
287-
const impersonationId = await getImpersonationId(request);
288+
const user = await requireUser(request);
289+
const userId = user.id;
288290
const { projectParam, organizationSlug, envParam, runParam } = v3RunParamsSchema.parse(params);
289291

290292
const url = new URL(request.url);
291-
const showDebug = url.searchParams.get("showDebug") === "true";
293+
// Same rule as the trace-chunk route, so the first chunk and later chunks agree.
294+
const showDebug =
295+
url.searchParams.get("showDebug") === "true" && (user.admin || user.isImpersonating);
292296
const selectedSpanId = url.searchParams.get("span") ?? undefined;
293297

294298
const presenter = new RunPresenter();
295299
const [error, result] = await tryCatch(
296300
presenter.call({
297301
userId,
298-
showDeletedLogs: !!impersonationId,
302+
showDeletedLogs: user.isImpersonating,
299303
projectSlug: projectParam,
300304
runFriendlyId: runParam,
301305
environmentSlug: envParam,
@@ -677,6 +681,7 @@ function TraceView({ run, trace, maximumLiveReloadingSetting, resizable }: Trace
677681
linkedRunIdBySpanId,
678682
isComplete,
679683
isTruncated: progressiveIsTruncated,
684+
errorsTruncated,
680685
loadFailed,
681686
tailLive,
682687
liveTailEnabled,
@@ -725,21 +730,26 @@ function TraceView({ run, trace, maximumLiveReloadingSetting, resizable }: Trace
725730
}
726731
}, [streamedEvents, liveTailEnabled, tailLive]);
727732

728-
// Tail path: refresh run-row-fed controls once the root span goes terminal.
733+
// The page data still shows the run unfinished, so a finish reload is useful.
734+
const runDataUnfinished = !run.isFinished && run.completedAt === null;
735+
736+
// Tail path: refresh run-row-fed controls once the root span goes terminal, unless a
737+
// reload already brought the finished run (e.g. the backstop's).
729738
const prevRootSpanRef = useRef({ runId: run.friendlyId, status: rootSpanStatus });
730739
useEffect(() => {
731740
const previous = prevRootSpanRef.current;
732741
prevRootSpanRef.current = { runId: run.friendlyId, status: rootSpanStatus };
733742
if (!liveTailEnabled || previous.runId !== run.friendlyId) return;
734-
if (previous.status === "executing" && rootSpanStatus !== "executing") {
743+
if (previous.status === "executing" && rootSpanStatus !== "executing" && runDataUnfinished) {
735744
revalidatorRef.current.revalidate();
736745
}
737-
}, [run.friendlyId, rootSpanStatus, liveTailEnabled]);
746+
}, [run.friendlyId, rootSpanStatus, liveTailEnabled, runDataUnfinished]);
738747

739-
// Tail path backstop: reconcile once the run finishes, even if the tail missed the
740-
// root's final row. Polls whatever the stream is doing.
748+
// Tail path backstop: reload once the run finishes, even if the tail missed the
749+
// root's final row. Polls whatever the stream is doing, and keeps reloading until the
750+
// page data shows the run finished (a reload can read a lagging replica).
741751
useRunStatusBackstop({
742-
enabled: liveTailEnabled && run.completedAt === null,
752+
enabled: liveTailEnabled && runDataUnfinished,
743753
statusPath,
744754
shouldSkip: () => document.visibilityState === "hidden",
745755
onFinished: () => {
@@ -812,6 +822,13 @@ function TraceView({ run, trace, maximumLiveReloadingSetting, resizable }: Trace
812822
</Callout>
813823
</div>
814824
)}
825+
{errorsOnly && errorsTruncated && !traceIsTruncated && (
826+
<div className="shrink-0 border-b border-grid-bright px-3 py-2">
827+
<Callout variant="info" className="text-sm">
828+
This trace is large, so Errors only may not show every error.
829+
</Callout>
830+
</div>
831+
)}
815832
<div className="min-h-0 flex-1">
816833
<TasksTreeView
817834
selectedId={selectedSpanId}

0 commit comments

Comments
 (0)