From 2b91e56cf2c18e3f2ac46e53a3d93db59e88ae15 Mon Sep 17 00:00:00 2001 From: Sergio Marcelino Date: Thu, 16 Jul 2026 12:08:05 -0300 Subject: [PATCH] fix(console): keep live traces listed and drop empty payload cards in traces view --- .../src/pages/TracesV2/hooks/useAllSpans.ts | 22 ++++- .../hooks/useSpanFilteredTraceRows.test.ts | 90 ++++++++++++++++++ .../hooks/useSpanFilteredTraceRows.ts | 94 ++++++++++++++++--- .../src/pages/TracesV2/hooks/useTraceData.ts | 40 ++++++-- .../TracesV2/lib/functionCallFromSpan.test.ts | 66 +++++++++++++ .../TracesV2/lib/functionCallFromSpan.ts | 19 +++- 6 files changed, 302 insertions(+), 29 deletions(-) diff --git a/console/web/src/pages/TracesV2/hooks/useAllSpans.ts b/console/web/src/pages/TracesV2/hooks/useAllSpans.ts index af0f4dd18..2e5adf2df 100644 --- a/console/web/src/pages/TracesV2/hooks/useAllSpans.ts +++ b/console/web/src/pages/TracesV2/hooks/useAllSpans.ts @@ -10,9 +10,10 @@ * are pruned as frames arrive, and a hard cap keeps a busy engine from * growing the map without limit (oldest effective-end evicted first). * Paused / hidden-tab frames are dropped, matching the rows stream; a - * reconnect or unpause re-seeds once, REPLACING the map — the engine store - * is the source of truth, and merging would immortalize a stale pending - * span whose close frame was lost across an engine restart. + * reconnect, unpause, or tab-visible re-seeds once, REPLACING the map — + * the engine store is the source of truth, and merging would immortalize + * a stale pending span whose close frame was lost across an engine + * restart. * * Engines without the all-spans stream simply never deliver a frame: the * strip then shows the seed's spans and refreshes on reconnects only. @@ -144,9 +145,24 @@ export function useAllSpans(isPaused: boolean): readonly StoredSpan[] { void seedRef.current() } }) + // Hidden-tab frames are dropped above, so the map has a hole after a + // tab switch — re-seed on return, mirroring the list's recovery in + // `useTraceData`. (REPLACE semantics, see the module docstring.) + let offVisibility: (() => void) | undefined + if (typeof document !== 'undefined') { + const onVisible = () => { + if (document.visibilityState === 'visible' && !isPausedRef.current) { + void seedRef.current() + } + } + document.addEventListener('visibilitychange', onVisible) + offVisibility = () => + document.removeEventListener('visibilitychange', onVisible) + } stop = () => { offFeed() offConn() + offVisibility?.() } })() diff --git a/console/web/src/pages/TracesV2/hooks/useSpanFilteredTraceRows.test.ts b/console/web/src/pages/TracesV2/hooks/useSpanFilteredTraceRows.test.ts index 67c572d90..25c2b1caa 100644 --- a/console/web/src/pages/TracesV2/hooks/useSpanFilteredTraceRows.test.ts +++ b/console/web/src/pages/TracesV2/hooks/useSpanFilteredTraceRows.test.ts @@ -3,6 +3,7 @@ import type { StoredSpan } from '../api/traces' import type { TimelineSpan } from '../components/timeline/layout' import type { SpanFilterSelection } from '../lib/spanFilters' import { + liveTraceIds, mergeFetchedVerdicts, reconcileTraceVisibility, rowRootFilterKeys, @@ -208,6 +209,50 @@ describe('reconcileTraceVisibility', () => { ).toEqual(['t-1']) }) + it('keeps a visible verdict when the visible bars prune out but hidden ones linger', () => { + // The mid-run vanish: the feed retains ~2min, so during a long quiet + // stretch a turn's visible bars prune away while its hidden + // bookkeeping keeps the trace in the feed. Visibility is monotone — + // the row must not flip back to hidden. + const verdicts = new Map() + const sel = selection({ hiddenGroups: new Set(['harness::turn']) }) + const rows = [row('t-1', 'harness::turn')] + const dispatch = bar({ id: 'root', groupKey: 'harness::turn' }) + const step = bar({ id: 'step', groupKey: 'harness::turn step' }) + expect( + ids(reconcileTraceVisibility(verdicts, [dispatch, step], rows, sel)), + ).toEqual(['t-1']) + // `step` pruned from the feed; only the hidden dispatch bar remains. + expect( + ids(reconcileTraceVisibility(verdicts, [dispatch], rows, sel)), + ).toEqual(['t-1']) + }) + + it('keeps a live trace visible even while every bar in the feed is hidden', () => { + // A running turn: worker spans only reach the feed when they CLOSE, so + // early on the feed holds nothing but hidden dispatch plumbing. The + // row must not hide while the trace is live. + const verdicts = new Map() + const sel = selection({ hiddenGroups: new Set(['harness::turn']) }) + const rows = [row('t-1', 'harness::turn')] + const dispatch = bar({ id: 'root', groupKey: 'harness::turn' }) + expect( + ids( + reconcileTraceVisibility( + verdicts, + [dispatch], + rows, + sel, + new Set(['t-1']), + ), + ), + ).toEqual(['t-1']) + // Settled (no longer live) with a still-hidden composition → hides. + expect( + ids(reconcileTraceVisibility(verdicts, [dispatch], rows, sel)), + ).toEqual([]) + }) + it('drops cached verdicts once the trace leaves the list too', () => { const verdicts = new Map() const sel = selection({ hiddenGroups: new Set(['fn']) }) @@ -306,4 +351,49 @@ describe('mergeFetchedVerdicts', () => { expect(verdicts.get('t-visible')).toBe(true) expect(verdicts.has('t-hidden')).toBe(false) }) + + it('never downgrades a visible verdict — the read raced a newer feed frame', () => { + const verdicts = new Map([['t-1', true]]) + mergeFetchedVerdicts( + verdicts, + ['t-1'], + [ + stored({ + span_id: 'h1', + attributes: [['function_id', 'fn']], + }), + ], + sel, + ) + expect(verdicts.get('t-1')).toBe(true) + }) +}) + +describe('liveTraceIds', () => { + // Realistic epoch ms — `toMs` sniffs nano vs ms by magnitude. + const now = 1_700_000_000_000 + + it('counts a pending snapshot as live', () => { + const live = liveTraceIds( + [stored({ span_id: 's1', trace_id: 't-1', pending: true })], + now, + ) + expect(live.has('t-1')).toBe(true) + }) + + it('counts a just-ended span as live, an old one as settled', () => { + const justEnded = stored({ + span_id: 's1', + trace_id: 't-recent', + end_time_unix_nano: (now - 2_000) * 1e6, + }) + const longSettled = stored({ + span_id: 's2', + trace_id: 't-old', + end_time_unix_nano: (now - 60_000) * 1e6, + }) + const live = liveTraceIds([justEnded, longSettled], now) + expect(live.has('t-recent')).toBe(true) + expect(live.has('t-old')).toBe(false) + }) }) diff --git a/console/web/src/pages/TracesV2/hooks/useSpanFilteredTraceRows.ts b/console/web/src/pages/TracesV2/hooks/useSpanFilteredTraceRows.ts index d886a11a4..5c25e931f 100644 --- a/console/web/src/pages/TracesV2/hooks/useSpanFilteredTraceRows.ts +++ b/console/web/src/pages/TracesV2/hooks/useSpanFilteredTraceRows.ts @@ -27,13 +27,26 @@ // read) never keep a row alive, so a kept row always opens to a non-empty // detail. // -// Feed verdicts recompute on every frame (a live trace must flip visible -// the moment a surviving span arrives — the first frames of a turn are all -// dispatch plumbing) and STICK once the trace prunes out of the feed: a -// settled trace's composition never changes. Fetched verdicts land in the -// same cache. The cache resets when the selection changes; a row stays -// visible while its verdict is unknown or its read failed — better to -// show a hideable row than to hide real work on a guess. +// Under a FIXED selection visibility is MONOTONE: spans only accumulate +// in the store, so a trace with one visible span has it forever. A `true` +// verdict therefore sticks; only unknown traces start at `false` and flip +// the moment a surviving span arrives (the first frames of a turn are all +// dispatch plumbing). Without the stick, a long run would vanish mid-way: +// the feed retains ~2min, so a quiet stretch (one long LLM/tool call) +// prunes the visible bars while hidden bookkeeping keeps the trace in the +// feed — and a feed-only recompute would flip the row back to hidden. +// Fetched verdicts land in the same cache. The cache resets when the +// selection changes; a row stays visible while its verdict is unknown or +// its read failed — better to show a hideable row than to hide real work +// on a guess. +// +// LIVE traces get one more guard: worker spans reach the store only when +// they CLOSE, so a running turn's composition is structurally incomplete — +// its first visible span may be an LLM call that takes minutes to close. +// A negative verdict is a guess there, so a row whose trace is still live +// (pending span in the feed, or a span end within the last few seconds) +// stays visible regardless. Bookkeeping traces settle in well under a +// second, so the exemption never resurfaces them. import { useEffect, useMemo, useRef, useState } from 'react' import { fetchTraces, type StoredSpan } from '../api/traces' @@ -48,6 +61,7 @@ import { spanFilterGroupKey, storedSpansToTimelineSpans, } from '../lib/timelineSpans' +import { isPendingSpan, toMs } from '../lib/traceTransform' import { getWorkerName } from '../lib/traceUtils' import type { TraceListItem } from './useTraceData' @@ -57,6 +71,34 @@ import type { TraceListItem } from './useTraceData' const FETCH_TRACE_CHUNK = 20 const FETCH_SPAN_LIMIT = 10_000 +/** How recently a span of the trace must have ENDED for the trace to still + * count as live when no pending snapshot is in the feed (engines with + * `live_spans` off). Generous on purpose — liveness only defers hiding. */ +const LIVE_END_SLACK_MS = 10_000 + +/** + * Traces with work still in flight, judged from the raw feed: a pending + * snapshot (the engine's queue wrapper stays pending for a turn's whole + * lifetime — see `followTurn.ts`), or a span that ended moments ago. + * Exported for tests. + */ +export function liveTraceIds( + spans: readonly StoredSpan[], + now: number, +): ReadonlySet { + const live = new Set() + for (const span of spans) { + if (live.has(span.trace_id)) continue + if ( + isPendingSpan(span) || + now - toMs(span.end_time_unix_nano) <= LIVE_END_SLACK_MS + ) { + live.add(span.trace_id) + } + } + return live +} + /** * The row's ROOT span rendered as filter keys, mirroring how * `storedSpansToTimelineSpans` keys the same span in the feed (a listed @@ -82,14 +124,18 @@ export function rowRootFilterKeys(row: TraceListItem): SpanFilterKeys { * One reconcile pass: refresh `verdicts` from the bars currently in the * feed, seed visible-root verdicts from the rows themselves, drop entries * for traces gone from both the feed and the list, and return the rows - * that survive (unknown verdicts stay visible). Mutates `verdicts` — the - * hook owns the map across renders. Exported for tests. + * that survive (unknown verdicts stay visible, and so do rows in + * `liveTraces` — a running trace's negative verdict is a guess, its + * visible spans may simply not have closed yet). Verdicts are monotone + * within a selection: `true` sticks, `false` re-evaluates. Mutates + * `verdicts` — the hook owns the map across renders. Exported for tests. */ export function reconcileTraceVisibility( verdicts: Map, bars: readonly TimelineSpan[], rows: readonly TraceListItem[], selection: SpanFilterSelection, + liveTraces: ReadonlySet = new Set(), ): readonly TraceListItem[] { const inFeed = new Set() for (const bar of bars) { @@ -97,7 +143,10 @@ export function reconcileTraceVisibility( if (!traceId) continue if (!inFeed.has(traceId)) { inFeed.add(traceId) - verdicts.set(traceId, false) + // Monotone: a known-visible trace stays visible (spans only + // accumulate); only unknown traces start hidden pending a + // surviving bar. + if (!verdicts.has(traceId)) verdicts.set(traceId, false) } if (!verdicts.get(traceId) && !isSpanBarHidden(bar, selection)) { verdicts.set(traceId, true) @@ -118,7 +167,9 @@ export function reconcileTraceVisibility( verdicts.delete(traceId) } } - const kept = rows.filter((r) => verdicts.get(r.traceId) !== false) + const kept = rows.filter( + (r) => verdicts.get(r.traceId) !== false || liveTraces.has(r.traceId), + ) // Identity-stable when nothing is hidden, so downstream memos hold. return kept.length === rows.length ? rows : kept } @@ -159,7 +210,9 @@ function verdictBars(spans: readonly StoredSpan[]): TimelineSpan[] { * longer has spans for counts as hidden (its detail would be empty). A * truncated response (span count at the limit) only trusts POSITIVE * verdicts: a visible span proves visibility, but "all hidden" might just - * mean the visible spans were cut off. Exported for tests. + * mean the visible spans were cut off. A `true` verdict already in the + * cache is never downgraded — the read raced a feed frame that proved + * visibility after the snapshot was taken. Exported for tests. */ export function mergeFetchedVerdicts( verdicts: Map, @@ -177,7 +230,9 @@ export function mergeFetchedVerdicts( const truncated = spans.length >= spanLimit for (const traceId of requested) { if (visible.has(traceId)) verdicts.set(traceId, true) - else if (!truncated) verdicts.set(traceId, false) + else if (!truncated && !verdicts.get(traceId)) { + verdicts.set(traceId, false) + } } } @@ -207,6 +262,9 @@ export function useSpanFilteredTraceRows( const [fetchTick, setFetchTick] = useState(0) // fetchTick isn't read in the body — it signals `cache.verdicts` grew. + // The liveness set is derived from the feed at reconcile time (feed + // frames arrive continuously while anything is live, so it stays fresh + // without its own clock). // biome-ignore lint/correctness/useExhaustiveDependencies: see above const visibleRows = useMemo(() => { let cache = cacheRef.current @@ -214,8 +272,14 @@ export function useSpanFilteredTraceRows( cache = { selection, verdicts: new Map(), requested: new Set() } cacheRef.current = cache } - return reconcileTraceVisibility(cache.verdicts, bars, rows, selection) - }, [rows, bars, selection, fetchTick]) + return reconcileTraceVisibility( + cache.verdicts, + bars, + rows, + selection, + liveTraceIds(feedSpans, Date.now()), + ) + }, [rows, bars, feedSpans, selection, fetchTick]) // Composition reads for the rows neither source could judge: hidden // root, no feed coverage. Runs after the memo above, so feed verdicts diff --git a/console/web/src/pages/TracesV2/hooks/useTraceData.ts b/console/web/src/pages/TracesV2/hooks/useTraceData.ts index 18de86088..52cad61c1 100644 --- a/console/web/src/pages/TracesV2/hooks/useTraceData.ts +++ b/console/web/src/pages/TracesV2/hooks/useTraceData.ts @@ -202,13 +202,18 @@ export function useTraceData({ qc.invalidateQueries({ queryKey: ['traceGroupMembers'] }) } - // ── Trace-tags refresh ──────────────────────────────────────────────── + // ── Trace-tags refresh / row backfill ───────────────────────────────── // Streamed rows arrive WITHOUT `trace_tags` (only `traces::list` merges // them), and a row's tags can change after it exists — the tag-bearing // span (e.g. the harness turn step carrying `iii.session.name`) closes // well after the root row was pushed. Both funnel into one debounced // `trace_ids` read whose `trace_tags` are merged back into the cache; a // filtered list can't be patched row-wise, so it refetches instead. + // The same read doubles as the list's self-heal: a trace whose + // `trace-rows` frame was dropped (broadcast lag, reconnect gap) keeps + // firing span activity, and if its fetched ROOT isn't in the cache yet + // it is inserted — without this, a missed row frame leaves the trace + // off the list until the next full reseed. const pendingTagIds = new Set() let tagFlushTimer: ReturnType | undefined @@ -242,22 +247,36 @@ export function useTraceData({ limit: ids.length, }) if (disposed) return + if (res.spans.length === 0) return const tagsByTrace = new Map( res.spans .filter((s) => s.trace_tags) .map((s) => [s.trace_id, s.trace_tags]), ) - if (tagsByTrace.size === 0) return qc.setQueryData(key, (old) => { if (!old) return old let changed = false + const known = new Set(old.spans.map((s) => s.trace_id)) const spans = old.spans.map((s) => { const tags = tagsByTrace.get(s.trace_id) if (!tags || tagsEqual(tags, s.trace_tags)) return s changed = true return { ...s, trace_tags: tags } }) - return changed ? { ...old, spans } : old + // Backfill: roots the rows stream never delivered (dropped + // frame) join the list here instead of waiting for a reseed. + const missing = res.spans.filter( + (s) => !s.parent_span_id && !known.has(s.trace_id), + ) + if (missing.length === 0) { + return changed ? { ...old, spans } : old + } + const merged = mergeTraceListSpans( + spans, + missing, + DEFAULT_TRACE_LIMIT, + ) + return { ...old, spans: merged, total: merged.length } }) } catch { // Transient read failure — the next activity window retries. @@ -306,16 +325,17 @@ export function useTraceData({ qc.invalidateQueries({ queryKey: ['traceGroupMembers'] }) }) - // Span activity on traces we already list = their merged tags may have - // just changed (late tag-bearing span, renamed session). Only known - // rows schedule — activity for traces outside the list is noise here. + // Span activity = a listed trace's merged tags may have just changed + // (late tag-bearing span, renamed session), OR a trace the rows + // stream never delivered is doing work. Both route through the same + // debounced read: known rows get their tags patched, unknown roots + // get backfilled (see `flushTagRefresh`). Gated on the seed having + // landed — before that the seed read itself covers everything. const offActivity = startTraceActivityFeed(client, (traceIds) => { if (isPausedRef.current || isHidden()) return const cached = qc.getQueryData(mergeKeyRef.current.key) - if (!cached || cached.spans.length === 0) return - const known = new Set(cached.spans.map((s) => s.trace_id)) - const relevant = traceIds.filter((id) => known.has(id)) - if (relevant.length > 0) scheduleTagRefresh(relevant) + if (!cached) return + scheduleTagRefresh(traceIds) }) const offConn = client.addConnectionStateListener((state) => { diff --git a/console/web/src/pages/TracesV2/lib/functionCallFromSpan.test.ts b/console/web/src/pages/TracesV2/lib/functionCallFromSpan.test.ts index 3418139d6..b43641379 100644 --- a/console/web/src/pages/TracesV2/lib/functionCallFromSpan.test.ts +++ b/console/web/src/pages/TracesV2/lib/functionCallFromSpan.test.ts @@ -146,4 +146,70 @@ describe('functionCallFromSpan', () => { expect(call?.running).toBeUndefined() expect(call?.durationMs).toBe(120) }) + + it('renders no card for a span that only inherits its identity and has no payloads', () => { + // A scope/plumbing span inside an invocation: identity comes from the + // ancestor walk (or baggage), but the SDK's payload events live on the + // invocation span — a card here would just show request/response "empty". + const execute = vis({ + span_id: 'execute', + name: 'execute worker::list', + }) + const inner = vis({ + span_id: 'inner', + parent_span_id: 'execute', + name: 'HTTP GET', + attributes: { 'iii.function.id': 'worker::list' }, + }) + expect(functionCallFromSpan(inner, byId(execute, inner))).toBeNull() + }) + + it('keeps the card for an inherited-identity span that captured payload data', () => { + const execute = vis({ + span_id: 'execute', + name: 'execute worker::list', + }) + const inner = vis({ + span_id: 'inner', + parent_span_id: 'execute', + name: 'tool call', + attributes: { 'tool.arguments': '{"query":"x"}' }, + }) + const call = functionCallFromSpan(inner, byId(execute, inner)) + expect(call?.functionId).toBe('worker::list') + expect(call?.input).toEqual({ query: 'x' }) + }) + + it('keeps the card for an inherited-identity span that errored', () => { + const execute = vis({ + span_id: 'execute', + name: 'execute worker::list', + }) + const inner = vis({ + span_id: 'inner', + parent_span_id: 'execute', + name: 'HTTP GET', + status: 'error', + events: [ + { + name: 'exception', + timestamp_unix_nano: 1, + attributes: { 'exception.message': 'boom' }, + }, + ], + }) + const call = functionCallFromSpan(inner, byId(execute, inner)) + expect(call?.output).toEqual({ error: 'boom' }) + }) + + it('still renders an empty card for a true invocation span', () => { + // The invocation span is the one place "empty" is honest — payload + // capture may be disabled (III_DISABLE_TRACE_PAYLOADS) yet the call + // itself is real and worth a card. + const call = functionCallFromSpan( + vis({ name: 'execute worker::list', attributes: {} }), + ) + expect(call?.functionId).toBe('worker::list') + expect(call?.input).toBeUndefined() + }) }) diff --git a/console/web/src/pages/TracesV2/lib/functionCallFromSpan.ts b/console/web/src/pages/TracesV2/lib/functionCallFromSpan.ts index 900983bb6..aea84b42a 100644 --- a/console/web/src/pages/TracesV2/lib/functionCallFromSpan.ts +++ b/console/web/src/pages/TracesV2/lib/functionCallFromSpan.ts @@ -10,7 +10,10 @@ * in one of the OTel/iii attributes (`faas.invoked_name`, `function_id`, * `iii.function.id`) or is a worker-SDK handler span named `execute ` * (those spans carry the function id only in their NAME — see - * `explicitFunctionId`). + * `explicitFunctionId`). A span that only INHERITS an identity (ancestor + * invocation / baggage) renders a card only when it actually captured + * payload data — otherwise nested plumbing spans would all show a card + * with two "empty" panes. * * Input/output: best effort from the iii-sdk auto-capture events, which * store JSON payloads under the `iii.payload.json` event attribute. Event @@ -184,6 +187,20 @@ export function functionCallFromSpan( outputEvent?.value ?? (span.status === 'error' ? exceptionOutput(span) : undefined) + // Only the invocation span itself (own explicit identity) always gets a + // card. A span that merely INHERITS its function id — the ancestor walk + // or baggage above — is machinery inside that call (scope spans, queue + // wrappers, HTTP/DB clients); the SDK attaches `iii.payload.json` events + // to the invocation span only, so an inherited span without captured + // data would render a card whose panes both read "empty". + if ( + explicitFunctionId(span) === null && + input === undefined && + output === undefined + ) { + return null + } + if (span.pending) { // Still running: no duration or output yet — the card renders its // `running` pulse instead.