From 9d689cf50dd3df6a921c7ea4ab295c44bd051118 Mon Sep 17 00:00:00 2001 From: Ytallo Layon Date: Sun, 31 May 2026 12:14:26 -0300 Subject: [PATCH 1/4] test(spanTree): enhance tests for flattenTree with new scenarios - Added multiple test cases to `spanTree.test.ts` to verify the behavior of `flattenTree` when `hideEngineRouting` is enabled. - Tests include scenarios for keeping worker calls visible while hiding engine dispatch wrappers, nesting worker rows per hop, and ensuring correct visibility when child calls share the same service. - Updated `spanTree.ts` with a new utility function `isHideableRoutingNode` to determine if a span should be hidden based on its name and parent context, improving the logic for rendering spans in the tree structure. --- .../web/src/pages/Traces/lib/spanTree.test.ts | 129 ++++++++++++++++++ console/web/src/pages/Traces/lib/spanTree.ts | 64 +++++++-- 2 files changed, 185 insertions(+), 8 deletions(-) diff --git a/console/web/src/pages/Traces/lib/spanTree.test.ts b/console/web/src/pages/Traces/lib/spanTree.test.ts index 76879b852..5ec04e967 100644 --- a/console/web/src/pages/Traces/lib/spanTree.test.ts +++ b/console/web/src/pages/Traces/lib/spanTree.test.ts @@ -295,6 +295,135 @@ describe('flattenTree — hideEngineRouting depth offset', () => { expect(flat.map((r) => r.span_id)).toEqual(['r', 'c']) expect(flat.map((r) => r.displayDepth)).toEqual([0, 1]) }) + + it('keeps the worker call X row and hides engine dispatch wrappers', () => { + const tree = buildSpanTree([ + makeSpan({ + span_id: 'hi', + service_name: 'iii-test', + name: 'handle_invocation harness::trigger', + depth: 0, + }), + makeSpan({ + span_id: 'ec', + parent_span_id: 'hi', + service_name: 'iii-test', + name: 'call harness::trigger', + depth: 1, + }), + makeSpan({ + span_id: 'wc', + parent_span_id: 'ec', + service_name: 'harness', + name: 'call harness::trigger', + kind: 'server', + depth: 2, + }), + makeSpan({ + span_id: 'user', + parent_span_id: 'wc', + service_name: 'harness', + name: 'fn_queue turn-step', + depth: 3, + }), + ]) + const flat = flattenTree(tree, { + expandedIds: expandAll(tree), + hideEngineRouting: true, + collapseEngineRoutingPairs: false, + }) + expect(flat.map((r) => r.span_id)).toEqual(['wc', 'user']) + expect(flat.map((r) => r.displayDepth)).toEqual([0, 1]) + }) + + it('nests one worker row per hop across a two-hop chain', () => { + const tree = buildSpanTree([ + makeSpan({ + span_id: 'hi-x', + service_name: 'iii-test', + name: 'handle_invocation fn-x', + depth: 0, + }), + makeSpan({ + span_id: 'ec-x', + parent_span_id: 'hi-x', + service_name: 'iii-test', + name: 'call fn-x', + depth: 1, + }), + makeSpan({ + span_id: 'wc-x', + parent_span_id: 'ec-x', + service_name: 'harness', + name: 'call fn-x', + kind: 'server', + depth: 2, + }), + makeSpan({ + span_id: 'hi-y', + parent_span_id: 'wc-x', + service_name: 'iii-test', + name: 'handle_invocation fn-y', + depth: 3, + }), + makeSpan({ + span_id: 'ec-y', + parent_span_id: 'hi-y', + service_name: 'iii-test', + name: 'call fn-y', + depth: 4, + }), + makeSpan({ + span_id: 'wc-y', + parent_span_id: 'ec-y', + service_name: 'harness', + name: 'call fn-y', + kind: 'server', + depth: 5, + }), + makeSpan({ + span_id: 'work', + parent_span_id: 'wc-y', + service_name: 'harness', + name: 'post /v1/messages', + depth: 6, + }), + ]) + const flat = flattenTree(tree, { + expandedIds: expandAll(tree), + hideEngineRouting: true, + collapseEngineRoutingPairs: false, + }) + expect(flat.map((r) => r.span_id)).toEqual(['wc-x', 'wc-y', 'work']) + expect(flat.map((r) => r.displayDepth)).toEqual([0, 1, 2]) + }) + + it('does not hide a worker call X when its child call X shares the same service', () => { + const tree = buildSpanTree([ + makeSpan({ + span_id: 'wc', + service_name: 'harness', + name: 'call fn-x', + kind: 'server', + depth: 0, + }), + makeSpan({ + span_id: 'nested', + parent_span_id: 'wc', + service_name: 'harness', + name: 'call fn-x', + kind: 'server', + depth: 1, + }), + ]) + const flat = flattenTree(tree, { + expandedIds: expandAll(tree), + hideEngineRouting: true, + collapseEngineRoutingPairs: false, + }) + expect(flat.map((r) => r.span_id)).toEqual(['wc', 'nested']) + expect(flat.map((r) => r.displayDepth)).toEqual([0, 1]) + }) }) describe('flattenTree — collapseEngineRoutingPairs', () => { diff --git a/console/web/src/pages/Traces/lib/spanTree.ts b/console/web/src/pages/Traces/lib/spanTree.ts index 9a06fa34a..d967d01da 100644 --- a/console/web/src/pages/Traces/lib/spanTree.ts +++ b/console/web/src/pages/Traces/lib/spanTree.ts @@ -6,15 +6,57 @@ // waterfall to a different render layer (canvas, native, SVG export) // can reuse these helpers directly. -import { isEngineRoutingPair, isEngineRoutingSpan } from './spanLabel' +import { isEngineRoutingPair } from './spanLabel' import type { VisualizationSpan } from './traceTransform' +const HANDLE_INVOCATION_PREFIX = 'handle_invocation ' as const +const CALL_PREFIX = 'call ' as const + +function functionIdFromCallName(name: string): string | null { + if (!name.startsWith(CALL_PREFIX)) return null + return name.slice(CALL_PREFIX.length) +} + +function functionIdFromHandleInvocationName(name: string): string | null { + if (!name.startsWith(HANDLE_INVOCATION_PREFIX)) return null + return name.slice(HANDLE_INVOCATION_PREFIX.length) +} + export interface SpanNode extends VisualizationSpan { children: SpanNode[] isExpanded: boolean isCriticalPath: boolean } +/** + * Whether a span should be hidden when `hideEngineRouting` is on. + * + * Hides engine dispatch wrappers but keeps the worker's `call X` SERVER + * span (the row that carries invocation events/logs). Uses tree structure + * and cross-service boundaries — not a hardcoded engine service name — + * so it survives `OTEL_SERVICE_NAME` overrides. + */ +export function isHideableRoutingNode( + node: Pick, + parent: Pick | undefined, +): boolean { + if (node.name.startsWith(HANDLE_INVOCATION_PREFIX)) return true + + const callFn = functionIdFromCallName(node.name) + if (callFn === null) return false + + const parentFn = parent + ? functionIdFromHandleInvocationName(parent.name) + : null + if (parentFn !== null && parentFn === callFn) return true + + return node.children.some((child) => { + const childFn = functionIdFromCallName(child.name) + if (childFn === null || childFn !== callFn) return false + return child.service_name !== node.service_name + }) +} + export interface FlatSpanRow extends SpanNode { /** * Visible indentation depth after applying `hideEngineRouting`. @@ -33,9 +75,11 @@ export interface FlatSpanRow extends SpanNode { export interface FlattenOptions { /** Span IDs the user has expanded. Collapsed nodes hide their subtree. */ expandedIds: Set - /** When true, engine routing spans (`handle_invocation X`, `call X` on the - * `iii` service) are skipped during render and their children render at - * the parent's depth instead. */ + /** When true, engine dispatch wrappers are skipped during render and their + * children render at the parent's depth instead: every `handle_invocation X`, + * the engine's own `call X` under that wrapper, and any `call X` that has a + * same-named `call X` child on a different service (engine→worker RPC). + * The worker's innermost `call X` row is kept. */ hideEngineRouting: boolean /** When true, a `handle_invocation X` parent with a single `call X` child * is rendered as ONE row, with the child's subtree promoted under the @@ -143,10 +187,14 @@ export function flattenTree( ): FlatSpanRow[] { const result: FlatSpanRow[] = [] - function traverse(node: SpanNode, depthOffset: number) { + function traverse( + node: SpanNode, + depthOffset: number, + parent: SpanNode | undefined, + ) { if (opts.onlyCriticalPath && !node.isCriticalPath) return - const hidden = opts.hideEngineRouting && isEngineRoutingSpan(node) + const hidden = opts.hideEngineRouting && isHideableRoutingNode(node, parent) let mergedRouting = false let descendants = node.children @@ -172,13 +220,13 @@ export function flattenTree( const childrenVisible = hidden || opts.expandedIds.has(node.span_id) if (childrenVisible) { for (const child of descendants) { - traverse(child, nextOffset) + traverse(child, nextOffset, node) } } } for (const node of nodes) { - traverse(node, 0) + traverse(node, 0, undefined) } return result } From c54f0ada2e10e2e3b35602d0f4917c2aa1db2213 Mon Sep 17 00:00:00 2001 From: Ytallo Layon Date: Sun, 31 May 2026 17:47:40 -0300 Subject: [PATCH 2/4] feat(harness): push-based traces live-refresh via ui::traces::changed Add a fanout pump that subscribes to the agent::turn_end stream and, with a ~400ms trailing coalesce, pushes an empty ui::traces::changed signal to every all-sessions subscriber, reusing the existing FanoutState + ui::subscribe and the function_not_found eviction path. Add the console-side subscriber (useTracesLiveRefresh + a framework-free startTracesSubscription/makeTracesChangedHandler core) that invalidates the ['traces'] and ['traceGroups'] React Query caches on the signal, re-subscribes and re-syncs on WS reconnect, and re-syncs on tab-visible. This lets the Traces view refresh on real activity instead of a 3s polling timer. The Traces page wiring (dropping refetchInterval, calling the hook) lives in the page files and is tracked with the broader Traces-page changes. --- console/web/src/lib/devtools-stream.test.ts | 228 ++++++++++++++++++ console/web/src/lib/devtools-stream.ts | 170 +++++++++++++ harness/src/harness/fanout/index.ts | 3 + harness/src/harness/fanout/traces-changed.ts | 93 +++++++ .../harness/fanout/traces-changed.test.ts | 173 +++++++++++++ 5 files changed, 667 insertions(+) create mode 100644 console/web/src/lib/devtools-stream.test.ts create mode 100644 console/web/src/lib/devtools-stream.ts create mode 100644 harness/src/harness/fanout/traces-changed.ts create mode 100644 harness/tests/harness/fanout/traces-changed.test.ts diff --git a/console/web/src/lib/devtools-stream.test.ts b/console/web/src/lib/devtools-stream.test.ts new file mode 100644 index 000000000..aad018756 --- /dev/null +++ b/console/web/src/lib/devtools-stream.test.ts @@ -0,0 +1,228 @@ +import type { QueryClient } from '@tanstack/react-query' +import { describe, expect, it, vi } from 'vitest' +import type { IIIConnectionState, IiiClient } from '@/lib/iii-client' +import { + makeTracesChangedHandler, + startTracesSubscription, +} from './devtools-stream' + +function fakeQueryClient() { + const invalidateQueries = vi.fn() + return { + client: { invalidateQueries } as unknown as QueryClient, + invalidateQueries, + } +} + +function fakeClient() { + const calls: Array<{ fn: string; payload: unknown }> = [] + let signalHandler: ((p: unknown) => void) | null = null + let connListener: ((s: IIIConnectionState) => void) | null = null + const offSignal = vi.fn() + const offConn = vi.fn() + const on = vi.fn((fn: string, handler: (p: unknown) => void) => { + if (fn === 'ui::traces::changed') signalHandler = handler + return offSignal + }) + const call = vi.fn(async (fn: string, payload?: Record) => { + calls.push({ fn, payload }) + return null + }) + const addConnectionStateListener = vi.fn( + (handler: (s: IIIConnectionState) => void) => { + connListener = handler + return offConn + }, + ) + const client = { + browserId: 'console-test', + on, + call, + addConnectionStateListener, + dispose: vi.fn(async () => {}), + } as unknown as IiiClient + return { + client, + on, + call, + calls, + offSignal, + offConn, + fireSignal: () => signalHandler?.(undefined), + fireConn: (s: IIIConnectionState) => connListener?.(s), + } +} + +function fakeDoc(initial: 'visible' | 'hidden' = 'visible') { + let visibilityState = initial + let handler: (() => void) | null = null + const addEventListener = vi.fn((type: string, h: () => void) => { + if (type === 'visibilitychange') handler = h + }) + const removeEventListener = vi.fn() + const doc = { + get visibilityState() { + return visibilityState + }, + addEventListener, + removeEventListener, + } + return { + doc: doc as unknown as Document, + addEventListener, + removeEventListener, + setVisibility: (s: 'visible' | 'hidden') => { + visibilityState = s + }, + fireVisibilityChange: () => handler?.(), + } +} + +describe('makeTracesChangedHandler', () => { + it('invalidates both trace query keys when not paused', () => { + const { client, invalidateQueries } = fakeQueryClient() + const handler = makeTracesChangedHandler(client, { current: false }) + + handler() + + expect(invalidateQueries).toHaveBeenCalledTimes(2) + expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ['traces'] }) + expect(invalidateQueries).toHaveBeenCalledWith({ + queryKey: ['traceGroups'], + }) + }) + + it('does nothing while paused', () => { + const { client, invalidateQueries } = fakeQueryClient() + const handler = makeTracesChangedHandler(client, { current: true }) + + handler() + + expect(invalidateQueries).not.toHaveBeenCalled() + }) + + it('reads the pause flag live from the ref', () => { + const { client, invalidateQueries } = fakeQueryClient() + const ref = { current: true } + const handler = makeTracesChangedHandler(client, ref) + + handler() + expect(invalidateQueries).not.toHaveBeenCalled() + + ref.current = false + handler() + expect(invalidateQueries).toHaveBeenCalledTimes(2) + }) +}) + +describe('startTracesSubscription', () => { + it('registers the signal handler and subscribes to all sessions on start', () => { + const { client, on, calls } = fakeClient() + startTracesSubscription(client, () => {}) + + expect(on).toHaveBeenCalledWith('ui::traces::changed', expect.any(Function)) + expect(calls).toEqual([ + { + fn: 'ui::subscribe', + payload: { browser_id: 'console-test', session_id: null }, + }, + ]) + }) + + it('routes the pushed signal to the provided callback', () => { + const { client, fireSignal } = fakeClient() + const onSignal = vi.fn() + startTracesSubscription(client, onSignal) + + fireSignal() + + expect(onSignal).toHaveBeenCalledTimes(1) + }) + + it('re-subscribes when the socket reconnects', () => { + const { client, calls, fireConn } = fakeClient() + startTracesSubscription(client, () => {}) + + fireConn('connected') + + const subscribes = calls.filter((c) => c.fn === 'ui::subscribe') + expect(subscribes.length).toBeGreaterThanOrEqual(2) + for (const s of subscribes) { + expect(s.payload).toEqual({ + browser_id: 'console-test', + session_id: null, + }) + } + }) + + it('re-syncs (refetches) on reconnect so a blank initial fetch recovers', () => { + const { client, fireConn } = fakeClient() + const onSignal = vi.fn() + startTracesSubscription(client, onSignal) + + fireConn('connected') + + expect(onSignal).toHaveBeenCalledTimes(1) + }) + + it('does not re-subscribe or re-sync on non-connected transitions', () => { + const { client, calls, fireConn } = fakeClient() + const onSignal = vi.fn() + startTracesSubscription(client, onSignal) + + fireConn('reconnecting') + fireConn('disconnected') + + expect(calls.filter((c) => c.fn === 'ui::subscribe')).toHaveLength(1) + expect(onSignal).not.toHaveBeenCalled() + }) + + it('cleans up the handler, the listener, and unsubscribes on stop', () => { + const { client, calls, offSignal, offConn } = fakeClient() + const stop = startTracesSubscription(client, () => {}) + + stop() + + expect(offSignal).toHaveBeenCalledTimes(1) + expect(offConn).toHaveBeenCalledTimes(1) + expect(calls).toContainEqual({ + fn: 'ui::unsubscribe', + payload: { browser_id: 'console-test', session_id: null }, + }) + }) + + it('re-syncs when the tab becomes visible again (recovers signals dropped while hidden)', () => { + const { client } = fakeClient() + const doc = fakeDoc('visible') + const onSignal = vi.fn() + startTracesSubscription(client, onSignal, doc.doc) + + doc.fireVisibilityChange() + + expect(onSignal).toHaveBeenCalledTimes(1) + }) + + it('does not re-sync on a visibilitychange that leaves the tab hidden', () => { + const { client } = fakeClient() + const doc = fakeDoc('hidden') + const onSignal = vi.fn() + startTracesSubscription(client, onSignal, doc.doc) + + doc.fireVisibilityChange() + + expect(onSignal).not.toHaveBeenCalled() + }) + + it('removes the visibilitychange listener on stop', () => { + const { client } = fakeClient() + const doc = fakeDoc('visible') + const stop = startTracesSubscription(client, () => {}, doc.doc) + + stop() + + expect(doc.removeEventListener).toHaveBeenCalledWith( + 'visibilitychange', + expect.any(Function), + ) + }) +}) diff --git a/console/web/src/lib/devtools-stream.ts b/console/web/src/lib/devtools-stream.ts new file mode 100644 index 000000000..99eec9d3f --- /dev/null +++ b/console/web/src/lib/devtools-stream.ts @@ -0,0 +1,170 @@ +/** + * Live-refresh wiring for the devtools Traces view. + * + * The harness fans out an empty `ui::traces::changed` signal (coalesced from + * the `agent::turn_end` stream) to every all-sessions subscriber. This module + * subscribes to that signal and invalidates the Traces React Query caches so + * the page refetches on a real "a turn's spans just landed" beat instead of + * polling `engine::traces::*` on a 3s timer. + * + * The imperative core (`startTracesSubscription`) and the pure invalidation + * handler (`makeTracesChangedHandler`) are framework-free so they unit-test + * without a DOM; `useTracesLiveRefresh` is the thin React wrapper. + */ + +import type { QueryClient } from '@tanstack/react-query' +import { useQueryClient } from '@tanstack/react-query' +import { useEffect, useRef } from 'react' +import { getIiiClient, type IiiClient } from '@/lib/iii-client' + +/** Dev-only trace-stream diagnostics. Silent in production builds. */ +function dlog(msg: string, data?: unknown): void { + if (import.meta.env?.DEV) { + console.debug(`[traces-live] ${msg}`, data ?? '') + } +} + +/** + * Build the signal handler that refetches the Traces queries. Pause is read + * live from a ref so toggling pause never re-creates the subscription. We skip + * refetching while the tab is hidden to avoid background work; the + * `visibilitychange` re-sync wired up in `startTracesSubscription` catches up + * on return. (The app-wide QueryClient disables `refetchOnWindowFocus`, so + * focus refetch is NOT a recovery path — the visibility listener is.) + */ +export function makeTracesChangedHandler( + qc: QueryClient, + isPausedRef: { current: boolean }, +): () => void { + return () => { + if (isPausedRef.current) { + dlog('signal ignored (paused)') + return + } + if ( + typeof document !== 'undefined' && + document.visibilityState === 'hidden' + ) { + dlog('signal ignored (tab hidden)') + return + } + dlog('signal received → invalidating traces queries') + qc.invalidateQueries({ queryKey: ['traces'] }) + qc.invalidateQueries({ queryKey: ['traceGroups'] }) + } +} + +/** + * Register the `ui::traces::changed` handler, subscribe this browser to all + * sessions, and re-subscribe on every reconnect. Returns a cleanup that + * unregisters the handler + connection listener and unsubscribes. + * + * `ui::subscribe` is a one-shot RPC (not a registered function), so the SDK + * does NOT replay it on reconnect the way it replays `on(...)` handlers — if + * the harness restarted while the socket was down, the subscription would be + * lost. Re-subscribing on each `'connected'` transition closes that gap. + * `ui::subscribe` is idempotent (a `Set.add` in the harness FanoutState), so + * the redundant call on the initial connect is harmless. + * + * On every `'connected'` we also re-sync via `onSignal()` (a refetch). Without + * polling, an initial fetch that raced the WS connect (or the harness coming + * up) would otherwise leave the page blank until the next `agent::turn_end`. + * Refetch-on-connect is event-driven — no interval — so it keeps the pure-push + * model while covering cold-start and reconnect. + * + * Signals that fire while the tab is hidden are intentionally dropped by + * `makeTracesChangedHandler`. To recover them, we re-sync via `onSignal()` + * when the tab becomes visible again. `doc` is injectable so this is testable + * without a DOM; it defaults to the global `document` (undefined in SSR/tests). + */ +export function startTracesSubscription( + client: Pick< + IiiClient, + 'browserId' | 'on' | 'call' | 'addConnectionStateListener' + >, + onSignal: () => void, + doc: + | Pick< + Document, + 'addEventListener' | 'removeEventListener' | 'visibilityState' + > + | undefined = typeof document !== 'undefined' ? document : undefined, +): () => void { + const off = client.on('ui::traces::changed', onSignal) + dlog('subscription started; ui::traces::changed handler registered', { + browserId: client.browserId, + }) + + const subscribe = () => + client + .call('ui::subscribe', { browser_id: client.browserId, session_id: null }) + .then(() => dlog('ui::subscribe ok (all sessions)')) + .catch((err) => dlog('ui::subscribe failed', err)) + + subscribe() + + const offConn = client.addConnectionStateListener((state) => { + dlog('connection state', state) + if (state !== 'connected') return + subscribe() + onSignal() + }) + + let offVisibility: (() => void) | undefined + if (doc) { + const onVisible = () => { + if (doc.visibilityState !== 'visible') return + dlog('tab visible → re-syncing traces queries') + onSignal() + } + doc.addEventListener('visibilitychange', onVisible) + offVisibility = () => doc.removeEventListener('visibilitychange', onVisible) + } + + return () => { + off() + offConn() + offVisibility?.() + client + .call('ui::unsubscribe', { + browser_id: client.browserId, + session_id: null, + }) + .catch(() => {}) + } +} + +/** + * Subscribe the Traces page to live `ui::traces::changed` signals for the + * lifetime of the component. The shared `getIiiClient()` singleton is NOT + * disposed on unmount (it's app-wide), so the explicit cleanup is required. + */ +export function useTracesLiveRefresh({ + isPaused, +}: { + isPaused: boolean +}): void { + const qc = useQueryClient() + const isPausedRef = useRef(isPaused) + useEffect(() => { + isPausedRef.current = isPaused + }, [isPaused]) + + useEffect(() => { + let stop: (() => void) | undefined + let disposed = false + void (async () => { + const client = await getIiiClient() + if (disposed) return + stop = startTracesSubscription( + client, + makeTracesChangedHandler(qc, isPausedRef), + ) + })() + return () => { + disposed = true + stop?.() + } + // Pause is read via the ref, so the subscription is set up once per mount. + }, [qc]) +} diff --git a/harness/src/harness/fanout/index.ts b/harness/src/harness/fanout/index.ts index 70788a56b..305172aab 100644 --- a/harness/src/harness/fanout/index.ts +++ b/harness/src/harness/fanout/index.ts @@ -2,6 +2,7 @@ import type { ISdk } from '../../runtime/iii.js'; import type { FanoutState } from '../ui-subscribe.js'; import { spawnAgentEventsPump } from './agent-events.js'; import { spawnSessionsPoll } from './sessions-poll.js'; +import { spawnTracesChangedPump } from './traces-changed.js'; export type FanoutPumps = { shutdown(): Promise; @@ -10,6 +11,7 @@ export type FanoutPumps = { export function spawnPumps(iii: ISdk, state: FanoutState): FanoutPumps { const agentEventsTrigger = spawnAgentEventsPump(iii, state); const stopSessions = spawnSessionsPoll(iii, state); + const stopTraces = spawnTracesChangedPump(iii, state); return { async shutdown() { try { @@ -18,6 +20,7 @@ export function spawnPumps(iii: ISdk, state: FanoutState): FanoutPumps { // ignore } stopSessions(); + stopTraces(); }, }; } diff --git a/harness/src/harness/fanout/traces-changed.ts b/harness/src/harness/fanout/traces-changed.ts new file mode 100644 index 000000000..d0c7df89b --- /dev/null +++ b/harness/src/harness/fanout/traces-changed.ts @@ -0,0 +1,93 @@ +/** + * Subscribe to `agent::turn_end` and fan-out a coalesced, empty + * `ui::traces::changed::` signal to every all-sessions + * subscriber. Lets the Traces devtools view refetch on a real "a turn's + * spans just landed" beat instead of polling `engine::traces::*` on a timer. + * + * Mirrors `agent-events.ts` (stream trigger + `function_not_found` eviction) + * and `sessions-poll.ts` (`allSubscribers()` fan-out, `() => void` shutdown). + * The handler ignores the frame body — it is a pure "something changed" tick; + * `agent::turn_end` already carries one frame per turn, so the debounce is a + * cheap second line of defence rather than the primary load reducer. + */ + +import type { ISdk, Trigger } from '../../runtime/iii.js'; +import { logger } from '../../runtime/otel.js'; +import type { FanoutState } from '../ui-subscribe.js'; + +const FN_ID = 'harness::fanout::traces_changed_handler'; +/** + * Dedicated turn-end stream (mirrored by the turn-orchestrator producer; see + * `turn-orchestrator/events.ts` `TURN_END_STREAM`). Re-declared locally to + * keep this pump from pulling in the orchestrator module graph, matching the + * convention in `context-compaction/register.ts`. + */ +const TURN_END_STREAM = 'agent::turn_end'; +/** Trailing-edge debounce window: collapse a burst of frames into one push. */ +const COALESCE_MS = 400; + +export function spawnTracesChangedPump(iii: ISdk, state: FanoutState): () => void { + let timer: ReturnType | null = null; + + const flush = () => { + timer = null; + const subscribers = state.allSubscribers(); + logger.info('traces-changed: fanout ui::traces::changed', { + subscribers: subscribers.length, + }); + for (const browser_id of subscribers) { + iii + .trigger({ + function_id: `ui::traces::changed::${browser_id}`, + payload: {}, + timeoutMs: 2_000, + }) + .catch((err) => { + const msg = String(err); + if (/function_not_found/.test(msg)) { + state.evictBrowser(browser_id); + } else { + logger.debug('ui::traces::changed push failed', { browser_id, err: msg }); + } + }); + } + }; + + const handler = iii.registerFunction( + FN_ID, + async () => { + logger.debug('traces-changed: agent::turn_end received'); + if (timer) clearTimeout(timer); + timer = setTimeout(flush, COALESCE_MS); + return null; + }, + { description: 'Internal: agent::turn_end -> coalesced ui::traces::changed fanout.' }, + ); + + let trigger: Trigger | null = null; + try { + trigger = iii.registerTrigger({ + type: 'stream', + function_id: FN_ID, + config: { stream_name: TURN_END_STREAM }, + }); + logger.info('traces-changed pump active: agent::turn_end -> ui::traces::changed'); + } catch (err) { + logger.warn('traces-changed stream subscriber registration failed', { + err: String(err), + }); + } + + return () => { + if (timer) { + clearTimeout(timer); + timer = null; + } + try { + trigger?.unregister(); + } catch {} + try { + handler.unregister(); + } catch {} + }; +} diff --git a/harness/tests/harness/fanout/traces-changed.test.ts b/harness/tests/harness/fanout/traces-changed.test.ts new file mode 100644 index 000000000..0242699c2 --- /dev/null +++ b/harness/tests/harness/fanout/traces-changed.test.ts @@ -0,0 +1,173 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { spawnTracesChangedPump } from '../../../src/harness/fanout/traces-changed.js'; +import { FanoutState } from '../../../src/harness/ui-subscribe.js'; +import type { ISdk } from '../../../src/runtime/iii.js'; + +type Handler = (event: unknown) => Promise; + +const HANDLER_FN_ID = 'harness::fanout::traces_changed_handler'; + +// The traces-changed pump subscribes to the dedicated `agent::turn_end` +// stream and fans out an empty `ui::traces::changed::` signal to +// every all-sessions subscriber, coalescing a burst of turn_end frames into a +// single fan-out. These tests pin the registration shape, the coalescing, the +// all-subscribers targeting, eviction on `function_not_found`, and teardown. +function setup(opts: { + subscribers?: Array<[string, string | null]>; + triggerImpl?: (req: { function_id: string; payload: unknown }) => Promise; +} = {}) { + const handlers = new Map(); + const triggers: Array<{ type?: string; function_id?: string; config?: Record }> = + []; + const sent: Array<{ function_id: string; payload: unknown }> = []; + const unregistered = { trigger: 0, handler: 0 }; + const iii = { + registerFunction: vi.fn((id: string, h: Handler) => { + handlers.set(id, h); + return { + unregister() { + unregistered.handler++; + }, + }; + }), + registerTrigger: vi.fn((t) => { + triggers.push(t); + return { + unregister() { + unregistered.trigger++; + }, + }; + }), + trigger: vi.fn(async (req: { function_id: string; payload: unknown }) => { + sent.push(req); + if (opts.triggerImpl) return opts.triggerImpl(req); + return null; + }), + } as unknown as ISdk; + + const state = new FanoutState(); + for (const [b, sid] of opts.subscribers ?? []) state.subscribe(b, sid); + const stop = spawnTracesChangedPump(iii, state); + return { handlers, triggers, sent, state, stop, unregistered }; +} + +function changedCalls(sent: Array<{ function_id: string; payload: unknown }>) { + return sent.filter((s) => s.function_id.startsWith('ui::traces::changed::')); +} + +beforeEach(() => { + vi.useFakeTimers(); +}); + +afterEach(() => { + vi.useRealTimers(); +}); + +describe('spawnTracesChangedPump registration', () => { + it('subscribes to the dedicated agent::turn_end stream', () => { + const { handlers, triggers } = setup(); + expect([...handlers.keys()]).toContain(HANDLER_FN_ID); + const t = triggers.find((x) => x.function_id === HANDLER_FN_ID); + expect(t?.type).toBe('stream'); + expect(t?.config?.stream_name).toBe('agent::turn_end'); + }); +}); + +describe('coalescing', () => { + it('collapses a burst of turn_end frames into a single fan-out per subscriber', async () => { + const { handlers, sent } = setup({ subscribers: [['b1', null], ['b2', null]] }); + const handler = handlers.get(HANDLER_FN_ID); + + // Three frames inside the coalescing window. + await handler?.({ type: 'turn_end' }); + await handler?.({ type: 'turn_end' }); + await handler?.({ type: 'turn_end' }); + expect(changedCalls(sent)).toHaveLength(0); // nothing yet — still debouncing + + await vi.advanceTimersByTimeAsync(400); + + const changed = changedCalls(sent); + expect(changed.map((c) => c.function_id).sort()).toEqual([ + 'ui::traces::changed::b1', + 'ui::traces::changed::b2', + ]); + expect(changed[0]?.payload).toEqual({}); + }); + + it('fans out again on a frame that arrives after the previous window flushed', async () => { + const { handlers, sent } = setup({ subscribers: [['b1', null]] }); + const handler = handlers.get(HANDLER_FN_ID); + + await handler?.({ type: 'turn_end' }); + await vi.advanceTimersByTimeAsync(400); + await handler?.({ type: 'turn_end' }); + await vi.advanceTimersByTimeAsync(400); + + expect(changedCalls(sent)).toHaveLength(2); + }); +}); + +describe('targeting', () => { + it('only fans out to all-sessions subscribers', async () => { + const { handlers, sent } = setup({ + subscribers: [ + ['b1', null], // all-sessions → should receive + ['b2', 'sess-x'], // specific session → should NOT receive + ], + }); + const handler = handlers.get(HANDLER_FN_ID); + + await handler?.({ type: 'turn_end' }); + await vi.advanceTimersByTimeAsync(400); + + expect(changedCalls(sent).map((c) => c.function_id)).toEqual(['ui::traces::changed::b1']); + }); +}); + +describe('eviction', () => { + it('evicts a browser whose push rejects with function_not_found', async () => { + const { handlers, state } = setup({ + subscribers: [['gone', null]], + triggerImpl: async () => { + throw new Error('function_not_found: ui::traces::changed::gone'); + }, + }); + const handler = handlers.get(HANDLER_FN_ID); + expect(state.browserCount()).toBe(1); + + await handler?.({ type: 'turn_end' }); + await vi.advanceTimersByTimeAsync(400); + + expect(state.browserCount()).toBe(0); + }); + + it('does NOT evict on other errors', async () => { + const { handlers, state } = setup({ + subscribers: [['b1', null]], + triggerImpl: async () => { + throw new Error('timeout'); + }, + }); + const handler = handlers.get(HANDLER_FN_ID); + + await handler?.({ type: 'turn_end' }); + await vi.advanceTimersByTimeAsync(400); + + expect(state.browserCount()).toBe(1); + }); +}); + +describe('teardown', () => { + it('stop() clears a pending timer so no fan-out fires', async () => { + const { handlers, sent, stop, unregistered } = setup({ subscribers: [['b1', null]] }); + const handler = handlers.get(HANDLER_FN_ID); + + await handler?.({ type: 'turn_end' }); + stop(); + await vi.advanceTimersByTimeAsync(400); + + expect(changedCalls(sent)).toHaveLength(0); + expect(unregistered.trigger).toBe(1); + expect(unregistered.handler).toBe(1); + }); +}); From ce2be905a17e07156406883845e38c656432ec32 Mon Sep 17 00:00:00 2001 From: Ytallo Layon Date: Mon, 1 Jun 2026 02:54:20 -0300 Subject: [PATCH 3/4] feat(console): traces page improvements + push live-refresh wiring Wire the Traces page to the ui::traces::changed push signal: drop the 3s refetchInterval from useTraceData/useTraceGroups, call useTracesLiveRefresh, and auto-pause live updates while a detail panel is open (restoring the prior pause intent on close, since polling is no longer the throttle). Also: Traces-view refinements plus extracted, unit-tested helpers (attributeText, minimapMarkers, percentOfTotal, preorder treeFlatten) with spanTree/traceListItem/traceTransform updates and component tweaks across FlameGraph, WaterfallChart, SpanOtelLogsTab, SpanErrorsTab, and the group/ detail panels. --- .../web/src/pages/Traces/api/traces.test.ts | 57 +++++++++ console/web/src/pages/Traces/api/traces.ts | 9 +- .../pages/Traces/components/FlameGraph.tsx | 31 ++--- .../Traces/components/ServiceBreakdown.tsx | 5 +- .../Traces/components/SessionDetailPanel.tsx | 11 +- .../pages/Traces/components/SpanErrorsTab.tsx | 26 ++-- .../Traces/components/SpanOtelLogsTab.tsx | 49 +++++--- .../pages/Traces/components/TraceFilters.tsx | 8 +- .../Traces/components/TraceGroupsView.tsx | 40 +++--- .../Traces/components/WaterfallChart.tsx | 41 ++++--- .../src/pages/Traces/hooks/useTraceData.ts | 34 ++++-- .../src/pages/Traces/hooks/useTraceGroups.ts | 7 +- console/web/src/pages/Traces/index.tsx | 114 ++++++++++++++++-- .../pages/Traces/lib/attributeText.test.ts | 38 ++++++ .../web/src/pages/Traces/lib/attributeText.ts | 26 ++++ .../pages/Traces/lib/minimapMarkers.test.ts | 76 ++++++++++++ .../src/pages/Traces/lib/minimapMarkers.ts | 73 +++++++++++ .../web/src/pages/Traces/lib/percent.test.ts | 36 ++++++ console/web/src/pages/Traces/lib/percent.ts | 14 +++ .../web/src/pages/Traces/lib/spanTree.test.ts | 54 +++++++++ console/web/src/pages/Traces/lib/spanTree.ts | 48 +++++++- .../pages/Traces/lib/traceListItem.test.ts | 25 ++++ .../web/src/pages/Traces/lib/traceListItem.ts | 4 +- .../pages/Traces/lib/traceTransform.test.ts | 47 ++++++++ .../src/pages/Traces/lib/traceTransform.ts | 22 +++- .../src/pages/Traces/lib/treeFlatten.test.ts | 49 ++++++++ .../web/src/pages/Traces/lib/treeFlatten.ts | 29 +++++ 27 files changed, 858 insertions(+), 115 deletions(-) create mode 100644 console/web/src/pages/Traces/api/traces.test.ts create mode 100644 console/web/src/pages/Traces/lib/attributeText.test.ts create mode 100644 console/web/src/pages/Traces/lib/attributeText.ts create mode 100644 console/web/src/pages/Traces/lib/minimapMarkers.test.ts create mode 100644 console/web/src/pages/Traces/lib/minimapMarkers.ts create mode 100644 console/web/src/pages/Traces/lib/percent.test.ts create mode 100644 console/web/src/pages/Traces/lib/percent.ts create mode 100644 console/web/src/pages/Traces/lib/treeFlatten.test.ts create mode 100644 console/web/src/pages/Traces/lib/treeFlatten.ts diff --git a/console/web/src/pages/Traces/api/traces.test.ts b/console/web/src/pages/Traces/api/traces.test.ts new file mode 100644 index 000000000..c64be4a16 --- /dev/null +++ b/console/web/src/pages/Traces/api/traces.test.ts @@ -0,0 +1,57 @@ +// Tests for the traces engine transport. The focus here is the +// exporter-availability signal: a genuinely-disabled memory exporter must +// be distinguishable from a legitimately-empty result so the UI can show +// "no observability" only in the former case (not for every empty filter). + +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const call = vi.fn() +vi.mock('@/lib/iii-client', () => ({ + getIiiClient: vi.fn(async () => ({ call })), +})) + +import { fetchTraces } from './traces' + +beforeEach(() => { + call.mockReset() +}) + +describe('fetchTraces — exporter availability', () => { + it('flags exporterDisabled and returns an empty list when the memory exporter is not enabled', async () => { + call.mockRejectedValueOnce(new Error('memory exporter is not enabled')) + + const res = await fetchTraces() + + expect(res.spans).toEqual([]) + expect(res.exporterDisabled).toBe(true) + }) + + it('does NOT set exporterDisabled for a legitimately empty engine response', async () => { + call.mockResolvedValueOnce({ spans: [], total: 0, offset: 0, limit: 100 }) + + const res = await fetchTraces() + + expect(res.spans).toEqual([]) + expect(res.exporterDisabled).toBeUndefined() + }) + + it('does NOT set exporterDisabled when spans are returned', async () => { + call.mockResolvedValueOnce({ + spans: [{ trace_id: 't1' }], + total: 1, + offset: 0, + limit: 100, + }) + + const res = await fetchTraces() + + expect(res.exporterDisabled).toBeUndefined() + expect(res.spans).toHaveLength(1) + }) + + it('rethrows non-exporter errors instead of masking them as empty', async () => { + call.mockRejectedValueOnce(new Error('connection refused')) + + await expect(fetchTraces()).rejects.toThrow('connection refused') + }) +}) diff --git a/console/web/src/pages/Traces/api/traces.ts b/console/web/src/pages/Traces/api/traces.ts index a2fa7d113..f8fcbc452 100644 --- a/console/web/src/pages/Traces/api/traces.ts +++ b/console/web/src/pages/Traces/api/traces.ts @@ -49,6 +49,13 @@ export interface TracesResponse { total: number offset: number limit: number + /** + * True only when the engine reports the memory exporter is not enabled + * (i.e. observability is genuinely unconfigured). Distinguishes that case + * from a legitimately-empty result (a filter matching nothing, a cleared + * store) so the UI shows "no observability" only when it's actually true. + */ + exporterDisabled?: boolean } export interface TracesFilterParams { @@ -149,7 +156,7 @@ export async function fetchTraces( return await client.call(TRACES_RPC_FUNCTIONS.list, payload) } catch (err) { if (isMemoryExporterNotEnabled(err)) { - return { spans: [], total: 0, offset, limit } + return { spans: [], total: 0, offset, limit, exporterDisabled: true } } throw asError(err, 'Failed to fetch traces') } diff --git a/console/web/src/pages/Traces/components/FlameGraph.tsx b/console/web/src/pages/Traces/components/FlameGraph.tsx index e393b7e68..9729a5408 100644 --- a/console/web/src/pages/Traces/components/FlameGraph.tsx +++ b/console/web/src/pages/Traces/components/FlameGraph.tsx @@ -41,6 +41,7 @@ import { useShowEngineRouting } from '../hooks/useShowEngineRouting' import { buildSpanTree, flattenTree } from '../lib/spanTree' import type { VisualizationSpan, WaterfallData } from '../lib/traceTransform' import { formatDuration } from '../lib/traceUtils' +import { flattenPreorder } from '../lib/treeFlatten' import { IconToggleButton } from './IconToggleButton' interface FlameGraphProps { @@ -149,19 +150,10 @@ function buildFlameNodes(spans: VisualizationSpan[]): FlameNode[] { } function flattenFlameNodes(nodes: FlameNode[]): FlameNode[] { - const result: FlameNode[] = [] - - function traverse(node: FlameNode) { - result.push(node) - for (const child of node.children) { - traverse(child) - } - } - - for (const node of nodes) { - traverse(node) - } - return result + // Iterative (not recursive): deep flame trees (thousands of nested spans) + // would overflow the call stack — the same hazard the span/waterfall + // transforms were rewritten to avoid. + return flattenPreorder(nodes) } /** @@ -267,7 +259,9 @@ export function FlameGraph({ }, [data.spans, allFlameNodes, showCriticalPath, showEngineRouting]) const maxDepth = useMemo( - () => Math.max(0, ...flatNodes.map((n) => n.depth)) + 1, + // reduce, not `Math.max(0, ...spread)`: a wide trace with >~100k visible + // nodes would overflow the argument limit and throw during render. + () => flatNodes.reduce((m, n) => (n.depth > m ? n.depth : m), 0) + 1, [flatNodes], ) @@ -513,6 +507,15 @@ export function FlameGraph({ } } + // Only dispatch when the hover TARGET changes. A same-bar move (or + // empty→empty over whitespace) would otherwise re-render and, since + // `hoveredNode` drives the `draw` callback, force a full canvas redraw of + // every bar on each mouse pixel — the freeze WaterfallChart removed for + // the same reason. The tooltip therefore anchors at the bar's entry point. + const foundId = found?.span.span_id ?? null + const currentId = hoveredNode?.span.span_id ?? null + if (foundId === currentId) return + dispatch({ type: 'SET_HOVERED', node: found, x: e.clientX, y: e.clientY }) } diff --git a/console/web/src/pages/Traces/components/ServiceBreakdown.tsx b/console/web/src/pages/Traces/components/ServiceBreakdown.tsx index 3a57ccc36..7159c347b 100644 --- a/console/web/src/pages/Traces/components/ServiceBreakdown.tsx +++ b/console/web/src/pages/Traces/components/ServiceBreakdown.tsx @@ -1,6 +1,7 @@ import { ChevronRight } from 'lucide-react' import { useMemo, useState } from 'react' import { cn } from '@/lib/utils' +import { percentOfTotal } from '../lib/percent' import { getServiceColor } from '../lib/traceColors' import type { WaterfallData } from '../lib/traceTransform' import { formatDuration, getServiceName } from '../lib/traceUtils' @@ -59,7 +60,9 @@ export function ServiceBreakdown({ data }: ServiceBreakdownProps) { const totalDuration = data.total_duration_ms for (const stats of statsMap.values()) { - stats.percentage = (stats.totalDuration / totalDuration) * 100 + // Guarded: zero-duration traces (total_duration_ms === 0) would + // otherwise yield NaN/Infinity bar widths. Clamped to [0, 100]. + stats.percentage = percentOfTotal(stats.totalDuration, totalDuration) } const serviceStats = Array.from(statsMap.values()).sort( diff --git a/console/web/src/pages/Traces/components/SessionDetailPanel.tsx b/console/web/src/pages/Traces/components/SessionDetailPanel.tsx index b716a9a31..8b93e3db3 100644 --- a/console/web/src/pages/Traces/components/SessionDetailPanel.tsx +++ b/console/web/src/pages/Traces/components/SessionDetailPanel.tsx @@ -61,7 +61,12 @@ interface SessionDetailPanelProps { */ groupAttribute?: 'iii.message.id' | 'iii.session.id' | 'iii.function.id' onClose: () => void - onSpanClick: (span: VisualizationSpan) => void + /** + * Fires with the clicked span AND its own trace's waterfall data. The + * waterfall is forwarded because the parent has no single trace loaded in + * group mode — the span-detail panel needs the right trace context. + */ + onSpanClick: (span: VisualizationSpan, waterfall: WaterfallData) => void selectedSpanId?: string } @@ -183,7 +188,7 @@ const AUTO_OPEN_SPAN_LIMIT = 500 interface TraceCardProps { index: number traceId: string - onSpanClick: (span: VisualizationSpan) => void + onSpanClick: (span: VisualizationSpan, waterfall: WaterfallData) => void selectedSpanId?: string defaultOpen: boolean /** Optional hint sourced from the group's span_count for single-trace groups. */ @@ -349,7 +354,7 @@ function TraceCard({ {waterfall && ( onSpanClick(span, waterfall)} selectedSpanId={selectedSpanId} /> )} diff --git a/console/web/src/pages/Traces/components/SpanErrorsTab.tsx b/console/web/src/pages/Traces/components/SpanErrorsTab.tsx index a1d106f8c..5cd0229dc 100644 --- a/console/web/src/pages/Traces/components/SpanErrorsTab.tsx +++ b/console/web/src/pages/Traces/components/SpanErrorsTab.tsx @@ -1,6 +1,7 @@ import { AlertCircle, CheckCircle2, Copy } from 'lucide-react' import { useMemo } from 'react' import { EmptyState } from '@/components/ui/EmptyState' +import { attributeText } from '../lib/attributeText' import type { VisualizationSpan } from '../lib/traceTransform' import { useCopyToClipboard } from '../lib/traceUtils' @@ -16,15 +17,22 @@ export function SpanErrorsTab({ span }: SpanErrorsTabProps) { const hasError = span.status === 'error' || !!exceptionEvent const eventAttrs = exceptionEvent?.attributes ?? {} - const errorMessage = span.attributes?.['error.message'] as string | undefined - const errorType = span.attributes?.['error.type'] as string | undefined - const errorStack = span.attributes?.['error.stack'] as string | undefined - const exceptionMessage = (span.attributes?.['exception.message'] ?? - eventAttrs['exception.message']) as string | undefined - const exceptionType = (span.attributes?.['exception.type'] ?? - eventAttrs['exception.type']) as string | undefined - const exceptionStacktrace = (span.attributes?.['exception.stacktrace'] ?? - eventAttrs['exception.stacktrace']) as string | undefined + // Span attributes are producer-controlled `unknown` values; coerce to text + // defensively so a non-string (array/object) can't crash `.split('\n')` or + // React rendering and blank the whole Traces view via the ErrorBoundary. + const errorMessage = attributeText(span.attributes?.['error.message']) + const errorType = attributeText(span.attributes?.['error.type']) + const errorStack = attributeText(span.attributes?.['error.stack']) + const exceptionMessage = attributeText( + span.attributes?.['exception.message'] ?? eventAttrs['exception.message'], + ) + const exceptionType = attributeText( + span.attributes?.['exception.type'] ?? eventAttrs['exception.type'], + ) + const exceptionStacktrace = attributeText( + span.attributes?.['exception.stacktrace'] ?? + eventAttrs['exception.stacktrace'], + ) const displayMessage = errorMessage || exceptionMessage const displayType = errorType || exceptionType diff --git a/console/web/src/pages/Traces/components/SpanOtelLogsTab.tsx b/console/web/src/pages/Traces/components/SpanOtelLogsTab.tsx index 3a70942c1..aeaff364d 100644 --- a/console/web/src/pages/Traces/components/SpanOtelLogsTab.tsx +++ b/console/web/src/pages/Traces/components/SpanOtelLogsTab.tsx @@ -96,7 +96,23 @@ function tryParseJson(value: unknown): { function JsonValue({ value }: { value: unknown }) { const [expanded, setExpanded] = useState(false) - const { isJson, parsed, raw } = tryParseJson(value) + // Parse + pretty-print once per value. These attribute values can be + // multi-KB `iii.payload.json` blobs; without memoization the full + // stringify/parse/split ran on every re-render (tab switch, copied-state + // tick, parent re-render). + const { isJson, raw, pretty, lineCount } = useMemo(() => { + const result = tryParseJson(value) + if (!result.isJson) { + return { isJson: false, raw: result.raw, pretty: '', lineCount: 0 } + } + const prettyStr = JSON.stringify(result.parsed, null, 2) + return { + isJson: true, + raw: result.raw, + pretty: prettyStr, + lineCount: prettyStr.split('\n').length, + } + }, [value]) if (!isJson) { return ( @@ -104,9 +120,6 @@ function JsonValue({ value }: { value: unknown }) { ) } - const pretty = JSON.stringify(parsed, null, 2) - const lineCount = pretty.split('\n').length - if (lineCount <= 2) { return ( {raw} @@ -175,20 +188,24 @@ function LogCard({ log, index, firstLogMs }: LogCardProps) { const offsetMs = logMs - firstLogMs const severity = getSeverity(log.severity_text) - const dataAttrs: Array<[string, unknown]> = [] - const metaAttrs: Array<[string, unknown]> = [] - - if (log.attributes) { - for (const [key, value] of Object.entries(log.attributes)) { - if (HIDDEN_ATTRS.has(key)) continue - const { isJson } = tryParseJson(value) - if (isJson || (typeof value === 'string' && value.length > 80)) { - dataAttrs.push([key, value]) - } else { - metaAttrs.push([key, value]) + // Classify attributes once per log: tryParseJson eagerly stringifies each + // value, so re-running it on every render is wasteful for large payloads. + const { dataAttrs, metaAttrs } = useMemo(() => { + const data: Array<[string, unknown]> = [] + const meta: Array<[string, unknown]> = [] + if (log.attributes) { + for (const [key, value] of Object.entries(log.attributes)) { + if (HIDDEN_ATTRS.has(key)) continue + const { isJson } = tryParseJson(value) + if (isJson || (typeof value === 'string' && value.length > 80)) { + data.push([key, value]) + } else { + meta.push([key, value]) + } } } - } + return { dataAttrs: data, metaAttrs: meta } + }, [log.attributes]) return (
= [ { value: 'asc', label: 'asc' }, ] +// Stable empty reference for the attributes filter. `filters.attributes ?? []` +// would allocate a fresh array on every render, which trips AttributesFilter's +// reference-equality value-sync guard and wipes the user's in-progress draft +// on each parent re-render (search keystrokes, streaming stats ticks). +const EMPTY_ATTRIBUTES: [string, string][] = [] + // --- Temp Inputs Reducer --- interface TempInputsState { tempServiceName: string @@ -800,7 +806,7 @@ export function TraceFilters({ ) : null}
onFilterChange( 'attributes', diff --git a/console/web/src/pages/Traces/components/TraceGroupsView.tsx b/console/web/src/pages/Traces/components/TraceGroupsView.tsx index dd09b2f64..ae2e4e08d 100644 --- a/console/web/src/pages/Traces/components/TraceGroupsView.tsx +++ b/console/web/src/pages/Traces/components/TraceGroupsView.tsx @@ -23,29 +23,31 @@ import { groupHeading, summarizeGroup } from '../lib/groupTraces' interface TraceGroupsViewProps { attribute: GroupByAttribute showSystem: boolean - isPaused: boolean - onSelectTrace: (traceId: string) => void /** - * Called with the full TraceGroup when a row is clicked. Routes that - * want a session-style multi-trace detail panel use this; older - * call-sites can ignore it and rely solely on `onSelectTrace`. + * Called with the full TraceGroup when a row is clicked. Preferred path — + * opens a session-style multi-trace detail panel that owns its own fetches. */ onSelectGroup?: (group: TraceGroup) => void - selectedTraceId: string | null + /** + * Legacy fallback for call-sites without a session-detail container: drills + * into the group's first trace via single-trace selection. Only used when + * `onSelectGroup` is not provided. + */ + onSelectTrace?: (traceId: string) => void + /** `group.value` of the currently-open group, for row highlight. */ + selectedGroupValue: string | null } export function TraceGroupsView({ attribute, showSystem, - isPaused, - onSelectTrace, onSelectGroup, - selectedTraceId, + onSelectTrace, + selectedGroupValue, }: TraceGroupsViewProps) { const { groups, isLoading, unavailable } = useTraceGroups({ groupBy: attribute, includeInternal: showSystem, - isPaused, }) if (unavailable) { @@ -91,17 +93,17 @@ export function TraceGroupsView({ attribute={attribute} group={group} isSelected={ - selectedTraceId !== null && - group.trace_ids.includes(selectedTraceId) + selectedGroupValue !== null && group.value === selectedGroupValue } onClick={() => { - // Surface the full group when a session-aware container is - // wired in. For row-highlight purposes we also push the - // first trace_id into the legacy single-trace selection so - // isSelected styling still works. - const firstTrace = group.trace_ids[0] - if (firstTrace) onSelectTrace(firstTrace) - if (onSelectGroup) onSelectGroup(group) + // Prefer the session-aware container; fall back to single-trace + // selection only for call-sites that don't handle groups. + if (onSelectGroup) { + onSelectGroup(group) + } else { + const firstTrace = group.trace_ids[0] + if (firstTrace) onSelectTrace?.(firstTrace) + } }} /> ))} diff --git a/console/web/src/pages/Traces/components/WaterfallChart.tsx b/console/web/src/pages/Traces/components/WaterfallChart.tsx index c69d6ad28..a145ae858 100644 --- a/console/web/src/pages/Traces/components/WaterfallChart.tsx +++ b/console/web/src/pages/Traces/components/WaterfallChart.tsx @@ -65,6 +65,7 @@ import { } from '@/components/ui/Tooltip' import { cn } from '@/lib/utils' import { useShowEngineRouting } from '../hooks/useShowEngineRouting' +import { sampleMinimapMarkers } from '../lib/minimapMarkers' import { formatSpanLabel, getSpanKindIndicator, @@ -505,6 +506,13 @@ export function WaterfallChart({ const spanTree = useMemo(() => buildSpanTree(data.spans), [data.spans]) + // Bounded set of minimap markers — never one DOM node per span. Memoized + // so it only recomputes when the span list changes. + const minimapMarkers = useMemo( + () => sampleMinimapMarkers(data.spans), + [data.spans], + ) + useEffect(() => { const allIds = new Set(data.spans.map((s) => s.span_id)) dispatch({ type: 'SET_ALL_EXPANDED', ids: allIds }) @@ -688,24 +696,21 @@ export function WaterfallChart({ className="relative bg-rule-2 overflow-hidden" style={{ height: MINIMAP_HEIGHT }} > - {data.spans.map((span, i) => { - const isError = span.status === 'error' - return ( -
- ) - })} + {minimapMarkers.map((marker) => ( +
+ ))}
>> hasOtelConfigured: boolean isQueryLoading: boolean + /** Set when the flat-list fetch threw a non-exporter error (network, RPC, + * timeout). Lets the page show a real error state instead of silently + * falling back to the misleading "no observability" empty state. */ + queryError: Error | null refetch: () => void isHoveredRef: React.RefObject flushPendingTraces: () => void @@ -40,7 +47,6 @@ export function useTraceData({ filterParams, showSystem, debouncedSearch, - isPaused, }: UseTraceDataOptions): UseTraceDataReturn { const [traceGroups, setTraceListItems] = useState([]) const [hasOtelConfigured, setHasOtelConfigured] = useState(false) @@ -55,8 +61,9 @@ export function useTraceData({ const { data: tracesData, isLoading: isQueryLoading, + error: queryError, refetch, - } = useQuery({ + } = useQuery({ queryKey: ['traces', filterParams, showSystem, debouncedSearch], queryFn: () => fetchTraces({ @@ -68,13 +75,23 @@ export function useTraceData({ limit: DEFAULT_TRACE_LIMIT, include_internal: showSystem, }), - refetchInterval: isPaused ? false : 3000, + // Live updates arrive via `useTracesLiveRefresh` (ui::traces::changed + // push) — no polling interval. Initial mount fetch + manual Refresh + + // signal-driven invalidation cover refresh. + refetchInterval: false, staleTime: 1000, }) useEffect(() => { if (!tracesData) return + // Observability is configured unless the engine explicitly reports the + // exporter disabled. An empty span list is a normal "no matching traces" + // result and must NOT flip the page to the "no observability" state. + // Set this first, before the fingerprint early-return, so an unchanged + // payload still keeps the flag correct. + setHasOtelConfigured(!tracesData.exporterDisabled) + if (tracesData.spans && tracesData.spans.length > 0) { const traces: TraceListItem[] = tracesData.spans.map(mapSpanToListItem) @@ -100,10 +117,12 @@ export function useTraceData({ } setTraceListItems(traces) - setHasOtelConfigured(true) } else { setTraceListItems([]) - setHasOtelConfigured(false) + // Reset the dedup state so a later non-empty fetch is detected as + // fresh (otherwise the fingerprint/new-trace diff would be stale). + fingerprintRef.current = '' + prevTraceIdsRef.current = new Set() } }, [tracesData]) @@ -121,6 +140,7 @@ export function useTraceData({ setNewTraceIds, hasOtelConfigured, isQueryLoading, + queryError: queryError ?? null, refetch, isHoveredRef, flushPendingTraces, diff --git a/console/web/src/pages/Traces/hooks/useTraceGroups.ts b/console/web/src/pages/Traces/hooks/useTraceGroups.ts index 642562859..7502fa037 100644 --- a/console/web/src/pages/Traces/hooks/useTraceGroups.ts +++ b/console/web/src/pages/Traces/hooks/useTraceGroups.ts @@ -14,8 +14,6 @@ export interface UseTraceGroupsOptions { groupBy: GroupByOption /** Include engine-internal spans. */ includeInternal: boolean - /** When true, suspend auto-refresh (matches the flat-list hook). */ - isPaused: boolean } export interface UseTraceGroupsReturn { @@ -43,7 +41,6 @@ export interface UseTraceGroupsReturn { export function useTraceGroups({ groupBy, includeInternal, - isPaused, }: UseTraceGroupsOptions): UseTraceGroupsReturn { const enabled = groupBy !== 'none' @@ -59,7 +56,9 @@ export function useTraceGroups({ limit: DEFAULT_GROUP_LIMIT, include_internal: includeInternal, }), - refetchInterval: isPaused ? false : 3000, + // Live updates arrive via `useTracesLiveRefresh` (ui::traces::changed + // push), which invalidates the ['traceGroups'] key — no polling interval. + refetchInterval: false, staleTime: 1000, retry: (failureCount, err) => { // Don't retry when the endpoint is missing — the UI hides the diff --git a/console/web/src/pages/Traces/index.tsx b/console/web/src/pages/Traces/index.tsx index 884abf68b..e83067e94 100644 --- a/console/web/src/pages/Traces/index.tsx +++ b/console/web/src/pages/Traces/index.tsx @@ -16,6 +16,7 @@ import { ErrorBoundary } from '@/components/ui/ErrorBoundary' import { Pagination } from '@/components/ui/Pagination' import { Skeleton } from '@/components/ui/Skeleton' import { StatusPanel } from '@/components/ui/StatusPanel' +import { useTracesLiveRefresh } from '@/lib/devtools-stream' import { cn } from '@/lib/utils' import { fetchTraceTree, type TraceGroup } from './api/traces' import { FlameGraph } from './components/FlameGraph' @@ -83,6 +84,7 @@ export function Traces() { setNewTraceIds, hasOtelConfigured, isQueryLoading, + queryError, refetch, isHoveredRef, flushPendingTraces, @@ -90,9 +92,12 @@ export function Traces() { filterParams, showSystem, debouncedSearch, - isPaused, }) + // Replace polling with iii push: refetch the trace queries when the harness + // signals new spans (ui::traces::changed), suspended while paused. + useTracesLiveRefresh({ isPaused }) + const totalPages = Math.max( 1, Math.ceil(traceGroups.length / filterState.pageSize), @@ -157,20 +162,80 @@ export function Traces() { } }, []) + // Live updates are paused while a detail panel is open so the list doesn't + // reorder under the user. Track whether WE auto-paused (vs the user manually + // pausing) so closing the panel restores the prior intent instead of leaving + // the list frozen forever (the old behaviour: select → pause → close → + // permanently stuck, since polling is gone and pause is the only throttle). + const isPausedRef = useRef(isPaused) + useEffect(() => { + isPausedRef.current = isPaused + }, [isPaused]) + const autoPausedRef = useRef(false) + + const autoPause = useCallback(() => { + if (!isPausedRef.current) { + autoPausedRef.current = true + setIsPaused(true) + } + }, []) + + const autoResume = useCallback(() => { + if (autoPausedRef.current) { + autoPausedRef.current = false + setIsPaused(false) + } + }, []) + + const togglePause = useCallback(() => { + // Manual toggle overrides the auto-pause bookkeeping. + autoPausedRef.current = false + setIsPaused((v) => !v) + }, []) + const selectTrace = useCallback( (traceId: string | null) => { + setSelectedGroup(null) setSelectedTraceId(traceId) setSelectedSpan(null) setWaterfallData(null) setSpansError(null) if (traceId) { - setIsPaused(true) + autoPause() loadTraceSpans(traceId) + } else { + autoResume() } }, - [loadTraceSpans], + [loadTraceSpans, autoPause, autoResume], ) + // Group-row selection: open the session detail panel ONLY. The panel + // (SessionDetailPanel) owns its own per-trace tree fetches, so we must not + // route through `selectTrace` here — doing so fired a wasted + // `engine::traces::tree` RPC for the group's first trace and read trace + // detail from the flat-list query (a different data source). + const selectGroup = useCallback( + (group: TraceGroup) => { + setSelectedGroup(group) + setSelectedTraceId(null) + setSelectedSpan(null) + setWaterfallData(null) + setSpansError(null) + autoPause() + }, + [autoPause], + ) + + const closeDetail = useCallback(() => { + setSelectedGroup(null) + setSelectedTraceId(null) + setSelectedSpan(null) + setWaterfallData(null) + setSpansError(null) + autoResume() + }, [autoResume]) + const groupAttribute = filterState.groupBy && filterState.groupBy !== 'none' ? filterState.groupBy @@ -214,7 +279,7 @@ export function Traces() {
- {!hasOtelConfigured ? ( + {queryError ? ( +
+ } + headline="failed to load traces" + detail={queryError.message} + /> +
+ +
+
+ ) : !hasOtelConfigured && !isQueryLoading ? (
this engine does not have the trace exporter registered. configure @@ -269,10 +356,8 @@ export function Traces() { selectTrace(id)} - onSelectGroup={(group) => setSelectedGroup(group)} + selectedGroupValue={selectedGroup?.value ?? null} + onSelectGroup={selectGroup} /> ) : isQueryLoading && traceGroups.length === 0 ? (
@@ -391,11 +476,14 @@ export function Traces() { ? filterState.groupBy : undefined } - onClose={() => { - setSelectedGroup(null) - selectTrace(null) + onClose={closeDetail} + onSpanClick={(span, wf) => { + // Carry the clicked span's OWN trace data up so the + // SpanPanel renders with the right neighbours — in + // group mode there is no single waterfallData loaded. + setSelectedSpan(span) + setWaterfallData(wf) }} - onSpanClick={setSelectedSpan} selectedSpanId={selectedSpan?.span_id} /> ) : selectedTrace ? ( diff --git a/console/web/src/pages/Traces/lib/attributeText.test.ts b/console/web/src/pages/Traces/lib/attributeText.test.ts new file mode 100644 index 000000000..56d90fff4 --- /dev/null +++ b/console/web/src/pages/Traces/lib/attributeText.test.ts @@ -0,0 +1,38 @@ +// Tests for the defensive coercion of untrusted OTel attribute values +// (Record) into renderable text. Span attributes are +// producer-controlled, so a non-string value (array/object/number) must +// never reach a String method like `.split` and crash the React tree. + +import { describe, expect, it } from 'vitest' +import { attributeText } from './attributeText' + +describe('attributeText', () => { + it('passes a string through unchanged', () => { + expect(attributeText('boom\n at foo')).toBe('boom\n at foo') + }) + + it('stringifies numbers and booleans', () => { + expect(attributeText(42)).toBe('42') + expect(attributeText(true)).toBe('true') + }) + + it('JSON-encodes arrays so a stacktrace array renders as text', () => { + expect(attributeText(['frame a', 'frame b'])).toBe('["frame a","frame b"]') + }) + + it('JSON-encodes plain objects', () => { + expect(attributeText({ message: 'x' })).toBe('{"message":"x"}') + }) + + it('returns undefined for null and undefined', () => { + expect(attributeText(null)).toBeUndefined() + expect(attributeText(undefined)).toBeUndefined() + }) + + it('never throws on a value JSON.stringify cannot serialize', () => { + const circular: Record = {} + circular.self = circular + expect(() => attributeText(circular)).not.toThrow() + expect(typeof attributeText(circular)).toBe('string') + }) +}) diff --git a/console/web/src/pages/Traces/lib/attributeText.ts b/console/web/src/pages/Traces/lib/attributeText.ts new file mode 100644 index 000000000..c3418a0a6 --- /dev/null +++ b/console/web/src/pages/Traces/lib/attributeText.ts @@ -0,0 +1,26 @@ +/** + * Coerce an untrusted OTel attribute value into renderable text. + * + * Span attributes cross the engine RPC boundary typed as `unknown` and are + * producer-controlled — a value declared as a string (e.g. + * `exception.stacktrace`, `error.message`) can arrive as an array, object, + * or number. Passing such a value straight to a String method like + * `.split('\n')`, or rendering an object as a React child, throws and + * (under the page ErrorBoundary) blanks the whole Traces view. + * + * Returns `undefined` for nullish input so callers can keep using + * truthiness checks; everything else becomes a string. JSON is used for + * arrays/objects; unserializable values fall back to `String(value)`. + */ +export function attributeText(value: unknown): string | undefined { + if (value == null) return undefined + if (typeof value === 'string') return value + if (typeof value === 'number' || typeof value === 'boolean') { + return String(value) + } + try { + return JSON.stringify(value) + } catch { + return String(value) + } +} diff --git a/console/web/src/pages/Traces/lib/minimapMarkers.test.ts b/console/web/src/pages/Traces/lib/minimapMarkers.test.ts new file mode 100644 index 000000000..60f8cf0ca --- /dev/null +++ b/console/web/src/pages/Traces/lib/minimapMarkers.test.ts @@ -0,0 +1,76 @@ +// Bounds the WaterfallChart minimap to a fixed number of markers. The +// minimap previously mounted one absolutely-positioned DOM node per span +// for ALL spans (2000+ on large traces); this downsamples into a capped +// number of rows while keeping the vertical density overview and surfacing +// error spans. + +import { describe, expect, it } from 'vitest' +import { type MinimapSpan, sampleMinimapMarkers } from './minimapMarkers' + +const span = (overrides: Partial = {}): MinimapSpan => ({ + span_id: 's', + status: 'ok', + start_percent: 0, + width_percent: 10, + ...overrides, +}) + +describe('sampleMinimapMarkers', () => { + it('returns no markers for no spans', () => { + expect(sampleMinimapMarkers([], 200)).toEqual([]) + }) + + it('keeps every span when under the cap, with index-proportional top', () => { + const spans = [ + span({ span_id: 'a' }), + span({ span_id: 'b' }), + span({ span_id: 'c', status: 'error' }), + span({ span_id: 'd' }), + ] + const markers = sampleMinimapMarkers(spans, 200) + expect(markers).toHaveLength(4) + expect(markers[0].topPercent).toBe(0) + expect(markers[2].topPercent).toBe(50) + expect(markers[2].isError).toBe(true) + }) + + it('caps the marker count when there are more spans than the budget', () => { + const spans = Array.from({ length: 5000 }, (_, i) => + span({ span_id: `s${i}` }), + ) + const markers = sampleMinimapMarkers(spans, 200) + expect(markers.length).toBeLessThanOrEqual(200) + expect(markers.length).toBeGreaterThan(0) + }) + + it('emits unique keys so React does not collapse markers', () => { + const spans = Array.from({ length: 1000 }, () => span({ span_id: 'dup' })) + const markers = sampleMinimapMarkers(spans, 200) + const keys = new Set(markers.map((m) => m.key)) + expect(keys.size).toBe(markers.length) + }) + + it('surfaces an error span as its bucket representative', () => { + // One error needle buried in a haystack larger than the budget must + // still appear (errors are the whole point of the minimap). + const spans = Array.from({ length: 1000 }, (_, i) => + span({ span_id: `s${i}`, status: i === 500 ? 'error' : 'ok' }), + ) + const markers = sampleMinimapMarkers(spans, 200) + expect(markers.some((m) => m.isError)).toBe(true) + }) + + it('keeps top values within 0..100 and ascending', () => { + const spans = Array.from({ length: 1000 }, (_, i) => + span({ span_id: `s${i}` }), + ) + const markers = sampleMinimapMarkers(spans, 200) + for (const m of markers) { + expect(m.topPercent).toBeGreaterThanOrEqual(0) + expect(m.topPercent).toBeLessThan(100) + } + for (let i = 1; i < markers.length; i++) { + expect(markers[i].topPercent).toBeGreaterThan(markers[i - 1].topPercent) + } + }) +}) diff --git a/console/web/src/pages/Traces/lib/minimapMarkers.ts b/console/web/src/pages/Traces/lib/minimapMarkers.ts new file mode 100644 index 000000000..ee7a2bd13 --- /dev/null +++ b/console/web/src/pages/Traces/lib/minimapMarkers.ts @@ -0,0 +1,73 @@ +/** + * Downsample a span list into a bounded set of minimap markers. + * + * The WaterfallChart minimap used to mount one absolutely-positioned DOM + * node per span for the entire trace (2000+ nodes on large traces, plus a + * full reconcile whenever the data changed). This buckets the spans into at + * most `maxMarkers` rows, preserving the vertical density overview while + * bounding the DOM node count regardless of trace size. Errors win their + * bucket so a single failed span in a huge trace still shows up. + */ + +export interface MinimapSpan { + span_id: string + status: string + start_percent: number + width_percent: number +} + +export interface MinimapMarker { + /** Stable, collision-free React key. */ + key: string + /** Vertical position 0..100. */ + topPercent: number + leftPercent: number + widthPercent: number + isError: boolean +} + +function toMarker( + span: MinimapSpan, + key: string, + topPercent: number, +): MinimapMarker { + return { + key, + topPercent, + leftPercent: span.start_percent, + widthPercent: span.width_percent, + isError: span.status === 'error', + } +} + +export function sampleMinimapMarkers( + spans: readonly MinimapSpan[], + maxMarkers = 200, +): MinimapMarker[] { + const n = spans.length + if (n === 0) return [] + + // Small traces: render every span (unchanged behaviour). + if (n <= maxMarkers) { + return spans.map((s, i) => toMarker(s, `m${i}`, (i / n) * 100)) + } + + // Large traces: bucket by index. Each bucket yields one representative, + // preferring an error span so failures are never sampled away. + const markers: MinimapMarker[] = [] + for (let b = 0; b < maxMarkers; b++) { + const start = Math.floor((b / maxMarkers) * n) + const end = Math.floor(((b + 1) / maxMarkers) * n) + if (end <= start) continue + + let rep = spans[start] + for (let i = start; i < end; i++) { + if (spans[i].status === 'error') { + rep = spans[i] + break + } + } + markers.push(toMarker(rep, `b${b}`, (b / maxMarkers) * 100)) + } + return markers +} diff --git a/console/web/src/pages/Traces/lib/percent.test.ts b/console/web/src/pages/Traces/lib/percent.test.ts new file mode 100644 index 000000000..78e7cc604 --- /dev/null +++ b/console/web/src/pages/Traces/lib/percent.test.ts @@ -0,0 +1,36 @@ +// Guarded percentage helper. Several trace views divide a part by a total +// duration that can legitimately be 0 (a single instantaneous span, or a +// batch of zero-duration spans), producing NaN/Infinity widths. + +import { describe, expect, it } from 'vitest' +import { percentOfTotal } from './percent' + +describe('percentOfTotal', () => { + it('computes a normal percentage', () => { + expect(percentOfTotal(25, 100)).toBe(25) + }) + + it('returns 0 when the total is 0 (no divide-by-zero NaN)', () => { + expect(percentOfTotal(5, 0)).toBe(0) + }) + + it('returns 0 when the total is negative', () => { + expect(percentOfTotal(5, -10)).toBe(0) + }) + + it('clamps above 100 (overlapping spans can exceed wall-clock total)', () => { + expect(percentOfTotal(150, 100)).toBe(100) + }) + + it('clamps a negative part to 0', () => { + expect(percentOfTotal(-5, 100)).toBe(0) + }) + + it.each([ + ['NaN part', Number.NaN, 100], + ['NaN total', 5, Number.NaN], + ['Infinity total', 5, Number.POSITIVE_INFINITY], + ])('returns 0 for non-finite input (%s)', (_label, part, total) => { + expect(percentOfTotal(part, total)).toBe(0) + }) +}) diff --git a/console/web/src/pages/Traces/lib/percent.ts b/console/web/src/pages/Traces/lib/percent.ts new file mode 100644 index 000000000..e2ff9be98 --- /dev/null +++ b/console/web/src/pages/Traces/lib/percent.ts @@ -0,0 +1,14 @@ +/** + * Compute `part / total * 100`, guarded against the divide-by-zero and + * non-finite inputs that occur for zero-duration traces (a single + * instantaneous span, or a batch of spans sharing one timestamp, makes + * `total_duration_ms === 0`). Returns 0 instead of NaN/Infinity and clamps + * the result to [0, 100] so overlapping spans can't exceed the bar width. + */ +export function percentOfTotal(part: number, total: number): number { + if (!Number.isFinite(part) || !Number.isFinite(total) || total <= 0) { + return 0 + } + const pct = (part / total) * 100 + return Math.min(100, Math.max(0, pct)) +} diff --git a/console/web/src/pages/Traces/lib/spanTree.test.ts b/console/web/src/pages/Traces/lib/spanTree.test.ts index 5ec04e967..d94f47e72 100644 --- a/console/web/src/pages/Traces/lib/spanTree.test.ts +++ b/console/web/src/pages/Traces/lib/spanTree.test.ts @@ -629,3 +629,57 @@ describe('flattenTree — onlyCriticalPath', () => { expect(flat[0].displayDepth).toBe(0) }) }) + +describe('buildSpanTree — malformed parent chains (cycle safety)', () => { + it('keeps a self-parented span as a root instead of dropping it', () => { + const tree = buildSpanTree([ + makeSpan({ span_id: 'a', parent_span_id: 'a' }), + ]) + expect(tree.map((r) => r.span_id)).toEqual(['a']) + // The span must not be pushed into its own children. + expect(tree[0].children).toHaveLength(0) + }) + + it('renders every span in a mutual 2-cycle (a<->b) rather than losing both', () => { + const tree = buildSpanTree([ + makeSpan({ span_id: 'a', parent_span_id: 'b' }), + makeSpan({ span_id: 'b', parent_span_id: 'a' }), + ]) + const ids = tree.map((r) => r.span_id).sort() + expect(ids).toEqual(['a', 'b']) + }) + + it('does not build a child cycle, so critical-path marking terminates', () => { + // buildSpanTree marks the critical path via DFS over children; if the + // cycle leaked into children this call would never return. + const tree = buildSpanTree([ + makeSpan({ span_id: 'a', parent_span_id: 'b' }), + makeSpan({ span_id: 'b', parent_span_id: 'a' }), + ]) + expect(tree.every((n) => n.children.length === 0)).toBe(true) + }) + + it('flattenTree over a previously-cyclic trace terminates and emits every span', () => { + const tree = buildSpanTree([ + makeSpan({ span_id: 'a', parent_span_id: 'b' }), + makeSpan({ span_id: 'b', parent_span_id: 'a' }), + ]) + const flat = flattenTree(tree, { + expandedIds: expandAll(tree), + hideEngineRouting: false, + collapseEngineRoutingPairs: false, + }) + expect(flat.map((r) => r.span_id).sort()).toEqual(['a', 'b']) + }) + + it('leaves a well-formed deep chain unchanged (regression guard)', () => { + const tree = buildSpanTree([ + makeSpan({ span_id: 'a' }), + makeSpan({ span_id: 'b', parent_span_id: 'a' }), + makeSpan({ span_id: 'c', parent_span_id: 'b' }), + ]) + expect(tree.map((r) => r.span_id)).toEqual(['a']) + expect(tree[0].children.map((c) => c.span_id)).toEqual(['b']) + expect(tree[0].children[0].children.map((c) => c.span_id)).toEqual(['c']) + }) +}) diff --git a/console/web/src/pages/Traces/lib/spanTree.ts b/console/web/src/pages/Traces/lib/spanTree.ts index d967d01da..6853e8d6d 100644 --- a/console/web/src/pages/Traces/lib/spanTree.ts +++ b/console/web/src/pages/Traces/lib/spanTree.ts @@ -120,11 +120,55 @@ export function buildSpanTree(spans: VisualizationSpan[]): SpanNode[] { }) }) + // Classify each span's parent chain as either acyclic-to-a-root or + // cyclic. A span is only linked under its parent when its whole ancestor + // chain terminates at a real root without revisiting a node; otherwise it + // is promoted to a root. This keeps a self-parent (`parent === self`) or a + // mutual cycle (a↔b) from (a) dropping the span out of `roots` entirely, + // and (b) building a child cycle that would infinite-loop the + // critical-path DFS below and `flattenTree` downstream. Mirrors the + // `visiting` guard already used by `calculateDepths`. + const SAFE = 1 + const CYCLIC = 2 + const chainMark = new Map() + + function chainReachesRoot(startId: string): boolean { + const path: string[] = [] + let cur: string | undefined = startId + let safe = true + while (cur !== undefined) { + const cached = chainMark.get(cur) + if (cached !== undefined) { + safe = cached === SAFE + break + } + if (path.includes(cur)) { + safe = false + break + } + path.push(cur) + const parentId: string | undefined = spanMap.get(cur)?.parent_span_id + if (!parentId || parentId === cur || !spanMap.has(parentId)) { + safe = true + break + } + cur = parentId + } + for (const id of path) chainMark.set(id, safe ? SAFE : CYCLIC) + return safe + } + spans.forEach((span) => { const node = spanMap.get(span.span_id) if (!node) return - if (span.parent_span_id && spanMap.has(span.parent_span_id)) { - spanMap.get(span.parent_span_id)?.children.push(node) + const parentId = span.parent_span_id + if ( + parentId && + parentId !== span.span_id && + spanMap.has(parentId) && + chainReachesRoot(span.span_id) + ) { + spanMap.get(parentId)?.children.push(node) } else { roots.push(node) } diff --git a/console/web/src/pages/Traces/lib/traceListItem.test.ts b/console/web/src/pages/Traces/lib/traceListItem.test.ts index 0f0c8367b..f3d2b45e2 100644 --- a/console/web/src/pages/Traces/lib/traceListItem.test.ts +++ b/console/web/src/pages/Traces/lib/traceListItem.test.ts @@ -135,6 +135,31 @@ describe('mapSpanToListItem — status normalization', () => { }) }) +describe('mapSpanToListItem — non-string status (unvalidated API boundary)', () => { + // The engine declares status as a string, but some OTel encoders emit it + // as a numeric code. A non-string value must not crash the whole list + // render (the mapper runs inside `spans.map` under the page ErrorBoundary). + it('maps the numeric OTel error code (2) to status="error"', () => { + const span = makeSpan({ status: 2 as unknown as string }) + expect(mapSpanToListItem(span).status).toBe('error') + }) + + it.each([1, 0])('maps numeric non-error code %s to status="ok"', (code) => { + const span = makeSpan({ status: code as unknown as string }) + expect(mapSpanToListItem(span).status).toBe('ok') + }) + + it.each([ + ['null', null], + ['undefined', undefined], + ['object', { code: 2 }], + ])('does not throw and defaults to "ok" for %s status', (_label, raw) => { + const span = makeSpan({ status: raw as unknown as string }) + expect(() => mapSpanToListItem(span)).not.toThrow() + expect(mapSpanToListItem(span).status).toBe('ok') + }) +}) + describe('mapSpanToListItem — semantic attributes', () => { it('reads functionId from OTel faas.invoked_name', () => { const out = mapSpanToListItem( diff --git a/console/web/src/pages/Traces/lib/traceListItem.ts b/console/web/src/pages/Traces/lib/traceListItem.ts index 742fbdcc0..7ff632b71 100644 --- a/console/web/src/pages/Traces/lib/traceListItem.ts +++ b/console/web/src/pages/Traces/lib/traceListItem.ts @@ -6,7 +6,7 @@ import type { StoredSpan } from '../api/traces' import type { TraceListItem } from '../hooks/useTraceData' -import { toMs } from './traceTransform' +import { normalizeSpanStatus, toMs } from './traceTransform' /** * Normalize a span's attributes to a flat object. @@ -71,7 +71,7 @@ export function mapSpanToListItem(span: StoredSpan): TraceListItem { rootOperation: span.name, functionId, topic, - status: span.status.toLowerCase() === 'error' ? 'error' : 'ok', + status: normalizeSpanStatus(span.status) === 'error' ? 'error' : 'ok', startTime, endTime, duration, diff --git a/console/web/src/pages/Traces/lib/traceTransform.test.ts b/console/web/src/pages/Traces/lib/traceTransform.test.ts index 76e99aefa..b4ea78b6b 100644 --- a/console/web/src/pages/Traces/lib/traceTransform.test.ts +++ b/console/web/src/pages/Traces/lib/traceTransform.test.ts @@ -8,6 +8,7 @@ import { describe, expect, it } from 'vitest' import type { SpanTreeNode } from '../api/traces' import { calculateDurationMs, + normalizeSpanStatus, toMs, treeToWaterfallData, } from './traceTransform' @@ -143,3 +144,49 @@ describe('treeToWaterfallData', () => { expect(treeToWaterfallData([])).toBeNull() }) }) + +describe('normalizeSpanStatus', () => { + it.each([ + 'error', + 'Error', + 'ERROR', + '2', + ])('maps string error variant %s to "error"', (raw) => { + expect(normalizeSpanStatus(raw)).toBe('error') + }) + + it.each(['ok', 'OK', '1'])('maps string ok variant %s to "ok"', (raw) => { + expect(normalizeSpanStatus(raw)).toBe('ok') + }) + + it.each([ + 'unset', + 'UNSET', + '0', + ])('maps string unset variant %s to "unset"', (raw) => { + expect(normalizeSpanStatus(raw)).toBe('unset') + }) + + it('maps the numeric OTel error code 2 to "error"', () => { + expect(normalizeSpanStatus(2)).toBe('error') + }) + + it('maps numeric ok/unset codes 1 and 0', () => { + expect(normalizeSpanStatus(1)).toBe('ok') + expect(normalizeSpanStatus(0)).toBe('unset') + }) + + it.each([ + ['null', null], + ['undefined', undefined], + ['empty string', ''], + ['object', { code: 2 }], + ])('does not throw and returns "unset" for %s', (_label, raw) => { + expect(() => normalizeSpanStatus(raw)).not.toThrow() + expect(normalizeSpanStatus(raw)).toBe('unset') + }) + + it('treats unrecognized strings as "unset"', () => { + expect(normalizeSpanStatus('weird')).toBe('unset') + }) +}) diff --git a/console/web/src/pages/Traces/lib/traceTransform.ts b/console/web/src/pages/Traces/lib/traceTransform.ts index 45c10ab43..2df72a54c 100644 --- a/console/web/src/pages/Traces/lib/traceTransform.ts +++ b/console/web/src/pages/Traces/lib/traceTransform.ts @@ -113,17 +113,29 @@ export function calculateDurationMs( } /** - * Get span status from status string + * Normalize a span status into the three UI states. + * + * The engine declares `status` as a string, but some OTel encoders emit + * it as a numeric code (0=unset, 1=ok, 2=error). Accepts `unknown` and + * coerces defensively so a non-string value (number, null, object) + * crossing the RPC boundary can never crash a `.toLowerCase()` call. */ -function getSpanStatus(status: StoredSpan['status']): 'ok' | 'error' | 'unset' { - if (!status) return 'unset' - const lower = status.toLowerCase() +export function normalizeSpanStatus(status: unknown): 'ok' | 'error' | 'unset' { + if (status == null) return 'unset' + const lower = String(status).toLowerCase() if (lower === 'error' || lower === '2') return 'error' if (lower === 'ok' || lower === '1') return 'ok' - if (lower === 'unset' || lower === '0') return 'unset' return 'unset' } +/** + * Get span status from status string. Thin wrapper around + * {@link normalizeSpanStatus} kept for call-site readability. + */ +function getSpanStatus(status: StoredSpan['status']): 'ok' | 'error' | 'unset' { + return normalizeSpanStatus(status) +} + /** * Convert attributes from array-of-tuples to Record. * Handles both `[["key","val"], ...]` (engine format) and already-converted Records. diff --git a/console/web/src/pages/Traces/lib/treeFlatten.test.ts b/console/web/src/pages/Traces/lib/treeFlatten.test.ts new file mode 100644 index 000000000..01bd8dea5 --- /dev/null +++ b/console/web/src/pages/Traces/lib/treeFlatten.test.ts @@ -0,0 +1,49 @@ +// Iterative pre-order flattening shared by the flame view. The flame tree +// can be thousands of nodes deep (long-running workflows / nested tool +// calls), so the flatten MUST NOT recurse — the transforms were already +// rewritten to an explicit stack to survive these traces. + +import { describe, expect, it } from 'vitest' +import { flattenPreorder } from './treeFlatten' + +interface Node { + id: string + children: Node[] +} + +const leaf = (id: string, children: Node[] = []): Node => ({ id, children }) + +describe('flattenPreorder', () => { + it('returns an empty array for no roots', () => { + expect(flattenPreorder([])).toEqual([]) + }) + + it('emits a single node', () => { + expect(flattenPreorder([leaf('a')]).map((n) => n.id)).toEqual(['a']) + }) + + it('emits parents before children (pre-order) and preserves sibling order', () => { + const tree = [leaf('a', [leaf('b'), leaf('c', [leaf('d')])]), leaf('e')] + expect(flattenPreorder(tree).map((n) => n.id)).toEqual([ + 'a', + 'b', + 'c', + 'd', + 'e', + ]) + }) + + it('does not blow the stack on a very deep chain', () => { + // A recursive traversal throws RangeError around ~10k frames; build a + // chain well past that so the regression is unambiguous. + let root = leaf('n0') + const head = root + for (let i = 1; i < 50_000; i++) { + const child = leaf(`n${i}`) + root.children.push(child) + root = child + } + expect(() => flattenPreorder([head])).not.toThrow() + expect(flattenPreorder([head])).toHaveLength(50_000) + }) +}) diff --git a/console/web/src/pages/Traces/lib/treeFlatten.ts b/console/web/src/pages/Traces/lib/treeFlatten.ts new file mode 100644 index 000000000..de9e23b68 --- /dev/null +++ b/console/web/src/pages/Traces/lib/treeFlatten.ts @@ -0,0 +1,29 @@ +/** + * Iterative pre-order flatten of a node tree. + * + * Uses an explicit stack rather than recursion so it survives the deep + * trees (thousands of nested spans in long-running workflows) that would + * otherwise overflow the call stack — the same reason the span/waterfall + * transforms were converted away from recursion. + * + * Generic over any node with a `children` array; emits each node before + * its children, preserving sibling order. + */ +export function flattenPreorder( + roots: readonly T[], +): T[] { + const result: T[] = [] + // Seed the stack in reverse so the first root is popped first. + const stack: T[] = [] + for (let i = roots.length - 1; i >= 0; i--) stack.push(roots[i]) + + while (stack.length > 0) { + const node = stack.pop() as T + result.push(node) + // Push children in reverse so they pop in their original order. + for (let i = node.children.length - 1; i >= 0; i--) { + stack.push(node.children[i]) + } + } + return result +} From 3103098d31eecbee5fe997c84a9880bd4d445f66 Mon Sep 17 00:00:00 2001 From: Ytallo Layon Date: Mon, 1 Jun 2026 12:22:57 -0300 Subject: [PATCH 4/4] style(harness): biome-format traces-changed test --- .../tests/harness/fanout/traces-changed.test.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/harness/tests/harness/fanout/traces-changed.test.ts b/harness/tests/harness/fanout/traces-changed.test.ts index 0242699c2..c79385d80 100644 --- a/harness/tests/harness/fanout/traces-changed.test.ts +++ b/harness/tests/harness/fanout/traces-changed.test.ts @@ -12,10 +12,12 @@ const HANDLER_FN_ID = 'harness::fanout::traces_changed_handler'; // every all-sessions subscriber, coalescing a burst of turn_end frames into a // single fan-out. These tests pin the registration shape, the coalescing, the // all-subscribers targeting, eviction on `function_not_found`, and teardown. -function setup(opts: { - subscribers?: Array<[string, string | null]>; - triggerImpl?: (req: { function_id: string; payload: unknown }) => Promise; -} = {}) { +function setup( + opts: { + subscribers?: Array<[string, string | null]>; + triggerImpl?: (req: { function_id: string; payload: unknown }) => Promise; + } = {}, +) { const handlers = new Map(); const triggers: Array<{ type?: string; function_id?: string; config?: Record }> = []; @@ -75,7 +77,12 @@ describe('spawnTracesChangedPump registration', () => { describe('coalescing', () => { it('collapses a burst of turn_end frames into a single fan-out per subscriber', async () => { - const { handlers, sent } = setup({ subscribers: [['b1', null], ['b2', null]] }); + const { handlers, sent } = setup({ + subscribers: [ + ['b1', null], + ['b2', null], + ], + }); const handler = handlers.get(HANDLER_FN_ID); // Three frames inside the coalescing window.