From b059a259613de44045e92b6aeca893957086cdbe Mon Sep 17 00:00:00 2001 From: milansanjeev <12941259+milansanjeev@users.noreply.github.com> Date: Tue, 1 Sep 2026 17:23:23 +0530 Subject: [PATCH 01/11] feat(app): surface trace total duration and span count in search results DBSearchPage's Results Table view shows scanned rows/elapsed time next to the results, but when viewing a single trace (directTraceId set) there is no glanceable total wall-clock duration or span count for that trace -- users have to open the trace waterfall to see it (#3038). Adds useTraceTotalDuration, which issues a small aggregate query (MIN/MAX/COUNT over the trace's spans) using the resolved source's timestamp/duration/traceId expressions, mirroring the existing DBTraceWaterfallChart getConfig pattern for embedding the traceId (SqlString.escape) into a raw SQL where clause. Renders as a TraceTotalDurationStat next to SearchNumRows. --- packages/app/src/DBSearchPage.tsx | 46 ++++ .../__tests__/useTraceTotalDuration.test.tsx | 221 ++++++++++++++++++ .../app/src/hooks/useTraceTotalDuration.tsx | 98 ++++++++ 3 files changed, 365 insertions(+) create mode 100644 packages/app/src/hooks/__tests__/useTraceTotalDuration.test.tsx create mode 100644 packages/app/src/hooks/useTraceTotalDuration.tsx diff --git a/packages/app/src/DBSearchPage.tsx b/packages/app/src/DBSearchPage.tsx index 7c818cbaba..71f7c3ac52 100644 --- a/packages/app/src/DBSearchPage.tsx +++ b/packages/app/src/DBSearchPage.tsx @@ -110,6 +110,7 @@ import { IS_LOCAL_MODE } from '@/config'; import { useAliasMapFromChartConfig } from '@/hooks/useChartConfig'; import { useExplainQuery } from '@/hooks/useExplainQuery'; import { useResolvedSourceParam } from '@/hooks/useResolvedSourceParam'; +import { useTraceTotalDuration } from '@/hooks/useTraceTotalDuration'; import { withAppNav } from '@/layout'; import { useCreateSavedSearch, @@ -439,6 +440,40 @@ export function SearchNumRows({ ); } +function TraceTotalDurationStat({ + config, + traceId, + enabled, +}: { + config: ChartConfigWithDateRange; + traceId: string; + enabled: boolean; +}) { + const { data, isLoading } = useTraceTotalDuration(config, traceId, { + enabled, + placeholderData: keepPreviousData, + }); + + if (isLoading || !data) { + return null; + } + + return ( + <> + + | + + + {`Total Duration: ${formatDurationMs(data.totalDurationMs)}`} + + + | + + {`Spans: ${data.spanCount.toLocaleString()}`} + + ); +} + function SaveSearchModalComponent({ searchedConfig, opened, @@ -2573,6 +2608,17 @@ export function DBSearchPage() { isSearching={isAnyQueryFetching} isLiveTail={isLive ?? false} /> + {searchedSource?.kind === SourceKind.Trace && + directTraceId && ( + + )} diff --git a/packages/app/src/hooks/__tests__/useTraceTotalDuration.test.tsx b/packages/app/src/hooks/__tests__/useTraceTotalDuration.test.tsx new file mode 100644 index 0000000000..bc2e626130 --- /dev/null +++ b/packages/app/src/hooks/__tests__/useTraceTotalDuration.test.tsx @@ -0,0 +1,221 @@ +import React from 'react'; +import { SourceKind } from '@hyperdx/common-utils/dist/types'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { renderHook, waitFor } from '@testing-library/react'; + +// Mock the clickhouse client +jest.mock('@/clickhouse', () => ({ + useClickhouseClient: jest.fn(), +})); + +// Mock useMetadataWithSettings +jest.mock('@/hooks/useMetadata', () => ({ + useMetadataWithSettings: jest.fn(), +})); + +// Mock useSource +jest.mock('@/source', () => ({ + useSource: jest.fn(), + getDisplayedTimestampValueExpression: jest.fn(() => 'Timestamp'), + getDurationMsExpression: jest.fn(() => '(Duration)/1e6'), +})); + +// Mock the renderChartConfig function +jest.mock('@hyperdx/common-utils/dist/core/renderChartConfig', () => ({ + renderChartConfig: jest.fn(), +})); + +import { useClickhouseClient } from '@/clickhouse'; +import { useMetadataWithSettings } from '@/hooks/useMetadata'; +import { useSource } from '@/source'; +import { renderChartConfig } from '@hyperdx/common-utils/dist/core/renderChartConfig'; + +import { useTraceTotalDuration } from '@/hooks/useTraceTotalDuration'; + +const mockTraceSource = { + id: 'trace-source-1', + kind: SourceKind.Trace, + connection: 'conn-1', + from: { databaseName: 'default', tableName: 'otel_traces' }, + timestampValueExpression: 'Timestamp', + traceIdExpression: 'TraceId', + durationExpression: 'Duration', + durationPrecision: 9, + querySettings: undefined, +}; + +describe('useTraceTotalDuration', () => { + let queryClient: QueryClient; + let wrapper: React.ComponentType<{ children: any }>; + let mockClickhouseClient: any; + let mockStream: any; + let mockReader: any; + + beforeEach(() => { + jest.clearAllMocks(); + + queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + wrapper = ({ children }) => ( + {children} + ); + + mockReader = { read: jest.fn() }; + mockStream = { getReader: jest.fn(() => mockReader) }; + mockClickhouseClient = { + query: jest.fn(() => + Promise.resolve({ + json: jest.fn(), + stream: () => mockStream, + }), + ), + }; + + jest + .mocked(useClickhouseClient) + .mockReturnValue(mockClickhouseClient as any); + jest + .mocked(useMetadataWithSettings) + .mockReturnValue({ getSetting: jest.fn() } as any); + jest.mocked(useSource).mockReturnValue({ + data: mockTraceSource, + isLoading: false, + } as any); + jest.mocked(renderChartConfig).mockResolvedValue({ + sql: 'SELECT MIN(Timestamp) AS minTs, MAX(...) AS maxTs, COUNT(*) AS spanCount FROM otel_traces WHERE TraceId = \'abc\'', + params: {}, + }); + }); + + it('returns 0/0 when traceId is undefined (disabled query)', async () => { + const { result } = renderHook( + () => + useTraceTotalDuration( + { source: 'trace-source-1' } as any, + undefined, + { enabled: true }, + ), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(renderChartConfig).not.toHaveBeenCalled(); + expect(mockClickhouseClient.query).not.toHaveBeenCalled(); + }); + + it('returns 0/0 when the resolved source is not a trace source', async () => { + jest.mocked(useSource).mockReturnValue({ + data: { ...mockTraceSource, kind: SourceKind.Log }, + isLoading: false, + } as any); + + const { result } = renderHook( + () => + useTraceTotalDuration( + { source: 'trace-source-1' } as any, + 'abc123', + { enabled: true }, + ), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.data).toEqual({ totalDurationMs: 0, spanCount: 0 }); + }); + + it('computes wall-clock span from min(start) to max(start+duration) and passes the escaped traceId in the where clause', async () => { + mockClickhouseClient.query.mockResolvedValue({ + json: jest.fn().mockResolvedValue([ + { + minTs: '2024-01-01T00:00:00.000Z', + maxTs: '2024-01-01T00:00:02.280Z', + spanCount: '14', + }, + ]), + }); + + const { result } = renderHook( + () => + useTraceTotalDuration( + { source: 'trace-source-1', connection: 'conn-1' } as any, + 'abc123', + { enabled: true }, + ), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + + expect(result.current.data).toEqual({ + totalDurationMs: 2280, + spanCount: 14, + }); + + expect(renderChartConfig).toHaveBeenCalledTimes(1); + const [aggConfig] = jest.mocked(renderChartConfig).mock.calls[0]; + expect(aggConfig.where).toBe("TraceId = 'abc123'"); + expect(aggConfig.whereLanguage).toBe('sql'); + }); + + it('escapes a traceId containing a single quote before embedding it in the where clause', async () => { + mockClickhouseClient.query.mockResolvedValue({ + json: jest + .fn() + .mockResolvedValue([ + { minTs: '2024-01-01T00:00:00.000Z', maxTs: '2024-01-01T00:00:01.000Z', spanCount: '1' }, + ]), + }); + + renderHook( + () => + useTraceTotalDuration( + { source: 'trace-source-1' } as any, + "abc'123", + { enabled: true }, + ), + { wrapper }, + ); + + await waitFor(() => + expect(renderChartConfig).toHaveBeenCalledTimes(1), + ); + const [aggConfig] = jest.mocked(renderChartConfig).mock.calls[0]; + expect(aggConfig.where).toBe("TraceId = 'abc\\'123'"); + }); + + it('returns 0/0 when the query returns no rows', async () => { + mockClickhouseClient.query.mockResolvedValue({ + json: jest.fn().mockResolvedValue([]), + }); + + const { result } = renderHook( + () => + useTraceTotalDuration( + { source: 'trace-source-1' } as any, + 'abc123', + { enabled: true }, + ), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.data).toEqual({ totalDurationMs: 0, spanCount: 0 }); + }); + + it('does not query while enabled is false', async () => { + const { result } = renderHook( + () => + useTraceTotalDuration( + { source: 'trace-source-1' } as any, + 'abc123', + { enabled: false }, + ), + { wrapper }, + ); + + await new Promise(resolve => setTimeout(resolve, 50)); + expect(result.current.isLoading).toBe(false); + expect(mockClickhouseClient.query).not.toHaveBeenCalled(); + }); +}); \ No newline at end of file diff --git a/packages/app/src/hooks/useTraceTotalDuration.tsx b/packages/app/src/hooks/useTraceTotalDuration.tsx new file mode 100644 index 0000000000..dbf44dff1a --- /dev/null +++ b/packages/app/src/hooks/useTraceTotalDuration.tsx @@ -0,0 +1,98 @@ +import { renderChartConfig } from '@hyperdx/common-utils/dist/core/renderChartConfig'; +import { + ChartConfigWithOptDateRange, + SourceKind, +} from '@hyperdx/common-utils/dist/types'; +import { useQuery, UseQueryOptions } from '@tanstack/react-query'; +import SqlString from 'sqlstring'; + +import { useClickhouseClient } from '@/clickhouse'; +import { + getDisplayedTimestampValueExpression, + getDurationMsExpression, + useSource, +} from '@/source'; + +import { useMetadataWithSettings } from './useMetadata'; + +type TraceTotalDurationResult = { + totalDurationMs: number; + spanCount: number; +}; + +// Wall-clock span of a single trace: MAX(span end) - MIN(span start) across +// every span belonging to the trace, computed server-side so it's correct +// even before the paginated results table has fetched every row (#3038). +export function useTraceTotalDuration( + _config: ChartConfigWithOptDateRange, + traceId: string | undefined, + options?: Omit< + UseQueryOptions, + 'queryKey' | 'queryFn' + >, +) { + const { enabled, ...restOptions } = options ?? {}; + + const config = { + ..._config, + with: undefined, + }; + + const clickhouseClient = useClickhouseClient(); + const metadata = useMetadataWithSettings(); + + const { data: source, isLoading: isSourceLoading } = useSource({ + id: config.source, + }); + + return useQuery({ + queryKey: ['trace-total-duration', config, traceId], + queryFn: async ({ signal }) => { + if (!traceId || !source || source.kind !== SourceKind.Trace) { + return { totalDurationMs: 0, spanCount: 0 }; + } + + const timestampExpr = getDisplayedTimestampValueExpression(source); + const durationMsExpr = getDurationMsExpression(source); + + const aggConfig: ChartConfigWithOptDateRange = { + ...config, + select: `MIN(${timestampExpr}) AS minTs, MAX(${timestampExpr} + toIntervalMillisecond(toInt64(${durationMsExpr}))) AS maxTs, COUNT(*) AS spanCount`, + where: `${source.traceIdExpression} = ${SqlString.escape(traceId)}`, + whereLanguage: 'sql', + orderBy: undefined, + groupBy: undefined, + }; + + const query = await renderChartConfig( + aggConfig, + metadata, + source.querySettings, + ); + const response = await clickhouseClient.query<'JSONEachRow'>({ + query: query.sql, + query_params: query.params, + format: 'JSONEachRow', + abort_signal: signal, + connectionId: config.connection, + }); + const [row] = await response.json<{ + minTs: string; + maxTs: string; + spanCount: string; + }>(); + if (!row) { + return { totalDurationMs: 0, spanCount: 0 }; + } + return { + totalDurationMs: + new Date(row.maxTs).getTime() - new Date(row.minTs).getTime(), + spanCount: Number(row.spanCount), + }; + }, + retry: false, + staleTime: 1000 * 60, + enabled: enabled && !!traceId && !isSourceLoading, + ...restOptions, + }); +} \ No newline at end of file From eefbd84cb5705304d46f97041979869412b3d125 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 1 Sep 2026 11:57:09 +0000 Subject: [PATCH 02/11] chore: add changeset for trace duration search stats Addresses the changeset-bot note on #3051 so this user-visible Search stats-row addition is included in the next @hyperdx/app version bump. --- .changeset/trace-total-duration-search-table.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changeset/trace-total-duration-search-table.md diff --git a/.changeset/trace-total-duration-search-table.md b/.changeset/trace-total-duration-search-table.md new file mode 100644 index 0000000000..132771addb --- /dev/null +++ b/.changeset/trace-total-duration-search-table.md @@ -0,0 +1,8 @@ +--- +'@hyperdx/app': patch +--- + +feat: show a trace's total wall-clock duration and span count in the Search +results stats row when viewing a single trace. Users no longer have to open the +waterfall to see end-to-end duration (summing span rows would double-count +parallel work). From f1b2706e70f9e08bae532c89a3c8808bcb406de3 Mon Sep 17 00:00:00 2001 From: milansanjeev <12941259+milansanjeev@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:35:01 +0530 Subject: [PATCH 03/11] feat(app): move trace total duration/span count stat into the trace waterfall view Move the #3038 stat out of DBSearchPage's Results Table (where the displayed rows/spans can diverge from the actual trace scope once the user edits the search -- a bad place for a "total trace duration" figure to live) and into DBTraceWaterfallChartContainer instead, where the trace's full span set is already fetched for the waterfall itself. Computes MIN(start)/MAX(start+duration)/span count directly from the already-fetched `rows` (client-side, no extra query), consistent with how the waterfall bars already compute each span's own offset/duration (parseTimestampToMs + Duration*1000). Only counts spans, not correlated logs. Renders next to the existing "N spans, N errors" text in the waterfall controls bar. Reverts the earlier DBSearchPage.tsx placement and removes the now-unused useTraceTotalDuration hook/query. --- .../trace-total-duration-search-table.md | 7 +- packages/app/src/DBSearchPage.tsx | 46 ---- .../src/components/DBTraceWaterfallChart.tsx | 29 +++ .../__tests__/DBTraceWaterfallChart.test.tsx | 78 +++++++ .../__tests__/useTraceTotalDuration.test.tsx | 221 ------------------ .../app/src/hooks/useTraceTotalDuration.tsx | 98 -------- 6 files changed, 110 insertions(+), 369 deletions(-) delete mode 100644 packages/app/src/hooks/__tests__/useTraceTotalDuration.test.tsx delete mode 100644 packages/app/src/hooks/useTraceTotalDuration.tsx diff --git a/.changeset/trace-total-duration-search-table.md b/.changeset/trace-total-duration-search-table.md index 132771addb..bcdf4a6223 100644 --- a/.changeset/trace-total-duration-search-table.md +++ b/.changeset/trace-total-duration-search-table.md @@ -2,7 +2,6 @@ '@hyperdx/app': patch --- -feat: show a trace's total wall-clock duration and span count in the Search -results stats row when viewing a single trace. Users no longer have to open the -waterfall to see end-to-end duration (summing span rows would double-count -parallel work). +feat: show a trace's total wall-clock duration and span count in the trace +waterfall view's controls bar. Previously this required manually eyeballing +the timeline (summing span rows would double-count parallel work). diff --git a/packages/app/src/DBSearchPage.tsx b/packages/app/src/DBSearchPage.tsx index 71f7c3ac52..7c818cbaba 100644 --- a/packages/app/src/DBSearchPage.tsx +++ b/packages/app/src/DBSearchPage.tsx @@ -110,7 +110,6 @@ import { IS_LOCAL_MODE } from '@/config'; import { useAliasMapFromChartConfig } from '@/hooks/useChartConfig'; import { useExplainQuery } from '@/hooks/useExplainQuery'; import { useResolvedSourceParam } from '@/hooks/useResolvedSourceParam'; -import { useTraceTotalDuration } from '@/hooks/useTraceTotalDuration'; import { withAppNav } from '@/layout'; import { useCreateSavedSearch, @@ -440,40 +439,6 @@ export function SearchNumRows({ ); } -function TraceTotalDurationStat({ - config, - traceId, - enabled, -}: { - config: ChartConfigWithDateRange; - traceId: string; - enabled: boolean; -}) { - const { data, isLoading } = useTraceTotalDuration(config, traceId, { - enabled, - placeholderData: keepPreviousData, - }); - - if (isLoading || !data) { - return null; - } - - return ( - <> - - | - - - {`Total Duration: ${formatDurationMs(data.totalDurationMs)}`} - - - | - - {`Spans: ${data.spanCount.toLocaleString()}`} - - ); -} - function SaveSearchModalComponent({ searchedConfig, opened, @@ -2608,17 +2573,6 @@ export function DBSearchPage() { isSearching={isAnyQueryFetching} isLiveTail={isLive ?? false} /> - {searchedSource?.kind === SourceKind.Trace && - directTraceId && ( - - )} diff --git a/packages/app/src/components/DBTraceWaterfallChart.tsx b/packages/app/src/components/DBTraceWaterfallChart.tsx index 85ebaca5d0..3a9be396c0 100644 --- a/packages/app/src/components/DBTraceWaterfallChart.tsx +++ b/packages/app/src/components/DBTraceWaterfallChart.tsx @@ -66,6 +66,7 @@ import { useFormatTime } from '@/useFormatTime'; import { CATEGORICAL_PALETTE_TOKENS, COLORS, + formatDurationMs, getChartColorError, getChartColorSuccess, getChartColorSuccessHighlight, @@ -723,6 +724,26 @@ export function DBTraceWaterfallChartContainer({ return nextRows; }, [traceRowsData, logRowsData]); + // Aggregate wall-clock span and span count across every fetched span in the + // trace (not the visible/collapsed subset, and unaffected by the waterfall + // search filters below, since those only flag rows via `__hdx_hidden` + // rather than excluding them) -- a stable, always-accurate summary (#3038). + const traceTotalStats = useMemo(() => { + let minStartMs = Number.MAX_SAFE_INTEGER; + let maxEndMs = 0; + let spanCount = 0; + for (const row of rows) { + if (row.type === SourceKind.Log) continue; + spanCount++; + const startMs = parseTimestampToMs(row.Timestamp); + const endMs = startMs + (row.Duration || 0) * 1000; + if (startMs < minStartMs) minStartMs = startMs; + if (endMs > maxEndMs) maxEndMs = endMs; + } + if (spanCount === 0) return null; + return { totalDurationMs: maxEndMs - minStartMs, spanCount }; + }, [rows]); + // Map each distinct span service to a stable color. Sorting the names first // keeps a service's color stable across renders regardless of row ordering. const serviceColorMap = useMemo(() => { @@ -1413,6 +1434,14 @@ export function DBTraceWaterfallChartContainer({ {errorCountString} + {traceTotalStats && ( + + · Total Duration:{' '} + {formatDurationMs(traceTotalStats.totalDurationMs)} ·{' '} + {traceTotalStats.spanCount} span + {traceTotalStats.spanCount !== 1 ? 's' : ''} + + )} Show: diff --git a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx index f2fecf6a12..a027f9ba5f 100644 --- a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx @@ -312,6 +312,84 @@ describe('DBTraceWaterfallChartContainer', () => { expect(MockTimelineChart.latestProps.rows[1]).toBeTruthy(); // Log row }); + it('renders total duration and span count computed from all fetched spans', async () => { + setupQueryMocks({ traceData: mockTraceData }); + renderComponent(null); // No log table source + await waitForLoading(); + + const stats = screen.getByTestId('trace-total-stats'); + // mockTraceData has one span, Duration: 0.1 (seconds) -> 100ms wall clock + expect(stats.textContent).toContain('Total Duration: 100ms'); + expect(stats.textContent).toContain('1 span'); + }); + + it('counts only spans (not correlated logs) toward the span count/duration stat', async () => { + setupQueryMocks({ + traceData: mockTraceData, + logData: mockLogData, + }); + + renderComponent(); // With log table source + await waitForLoading(); + + // 2 total rows (1 span + 1 log), but the stat should only count the span + const stats = screen.getByTestId('trace-total-stats'); + expect(stats.textContent).toContain('1 span'); + expect(stats.textContent).not.toContain('2 span'); + }); + + it('computes total duration as the wall-clock span across multiple non-overlapping spans', async () => { + const multiSpanData = { + data: [ + { + Body: 'first span', + Timestamp: '2024-01-01T06:00:00.000000000Z', + Duration: 0.1, // ends at +100ms + SpanId: 'span-1', + ParentSpanId: '', + ServiceName: 'test-service', + HyperDXEventType: 'span' as const, + type: 'trace', + } as SpanRow, + { + Body: 'second span', + Timestamp: '2024-01-01T06:00:01.000000000Z', // starts 1s after the first + Duration: 0.5, // ends at +1.5s from the first span's start + SpanId: 'span-2', + ParentSpanId: '', + ServiceName: 'test-service', + HyperDXEventType: 'span' as const, + type: 'trace', + } as SpanRow, + ], + meta: [{ totalCount: 2 }], + }; + + setupQueryMocks({ traceData: multiSpanData }); + renderComponent(null); + await waitForLoading(); + + const stats = screen.getByTestId('trace-total-stats'); + // MIN(start) = 06:00:00.000, MAX(start+duration) = 06:00:01.500 -> 1500ms + expect(stats.textContent).toContain('Total Duration: 1.5s'); + expect(stats.textContent).toContain('2 spans'); + }); + + it('does not render the total duration stat when there are no spans', async () => { + // A log with no matching span never becomes a visible node (correlated + // logs attach to a span), so this renders the existing empty state -- + // just confirms the new stat doesn't render in that case either. + setupQueryMocks({ logData: mockLogData }); + renderComponent(); + + await waitFor(() => { + expect( + screen.getByText('No matching spans or logs found'), + ).toBeInTheDocument(); + }); + expect(screen.queryByTestId('trace-total-stats')).not.toBeInTheDocument(); + }); + it('renders empty state when no data is available', async () => { mockUseOffsetPaginatedQuery.mockReturnValue({ data: emptyData, diff --git a/packages/app/src/hooks/__tests__/useTraceTotalDuration.test.tsx b/packages/app/src/hooks/__tests__/useTraceTotalDuration.test.tsx deleted file mode 100644 index bc2e626130..0000000000 --- a/packages/app/src/hooks/__tests__/useTraceTotalDuration.test.tsx +++ /dev/null @@ -1,221 +0,0 @@ -import React from 'react'; -import { SourceKind } from '@hyperdx/common-utils/dist/types'; -import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; -import { renderHook, waitFor } from '@testing-library/react'; - -// Mock the clickhouse client -jest.mock('@/clickhouse', () => ({ - useClickhouseClient: jest.fn(), -})); - -// Mock useMetadataWithSettings -jest.mock('@/hooks/useMetadata', () => ({ - useMetadataWithSettings: jest.fn(), -})); - -// Mock useSource -jest.mock('@/source', () => ({ - useSource: jest.fn(), - getDisplayedTimestampValueExpression: jest.fn(() => 'Timestamp'), - getDurationMsExpression: jest.fn(() => '(Duration)/1e6'), -})); - -// Mock the renderChartConfig function -jest.mock('@hyperdx/common-utils/dist/core/renderChartConfig', () => ({ - renderChartConfig: jest.fn(), -})); - -import { useClickhouseClient } from '@/clickhouse'; -import { useMetadataWithSettings } from '@/hooks/useMetadata'; -import { useSource } from '@/source'; -import { renderChartConfig } from '@hyperdx/common-utils/dist/core/renderChartConfig'; - -import { useTraceTotalDuration } from '@/hooks/useTraceTotalDuration'; - -const mockTraceSource = { - id: 'trace-source-1', - kind: SourceKind.Trace, - connection: 'conn-1', - from: { databaseName: 'default', tableName: 'otel_traces' }, - timestampValueExpression: 'Timestamp', - traceIdExpression: 'TraceId', - durationExpression: 'Duration', - durationPrecision: 9, - querySettings: undefined, -}; - -describe('useTraceTotalDuration', () => { - let queryClient: QueryClient; - let wrapper: React.ComponentType<{ children: any }>; - let mockClickhouseClient: any; - let mockStream: any; - let mockReader: any; - - beforeEach(() => { - jest.clearAllMocks(); - - queryClient = new QueryClient({ - defaultOptions: { queries: { retry: false } }, - }); - wrapper = ({ children }) => ( - {children} - ); - - mockReader = { read: jest.fn() }; - mockStream = { getReader: jest.fn(() => mockReader) }; - mockClickhouseClient = { - query: jest.fn(() => - Promise.resolve({ - json: jest.fn(), - stream: () => mockStream, - }), - ), - }; - - jest - .mocked(useClickhouseClient) - .mockReturnValue(mockClickhouseClient as any); - jest - .mocked(useMetadataWithSettings) - .mockReturnValue({ getSetting: jest.fn() } as any); - jest.mocked(useSource).mockReturnValue({ - data: mockTraceSource, - isLoading: false, - } as any); - jest.mocked(renderChartConfig).mockResolvedValue({ - sql: 'SELECT MIN(Timestamp) AS minTs, MAX(...) AS maxTs, COUNT(*) AS spanCount FROM otel_traces WHERE TraceId = \'abc\'', - params: {}, - }); - }); - - it('returns 0/0 when traceId is undefined (disabled query)', async () => { - const { result } = renderHook( - () => - useTraceTotalDuration( - { source: 'trace-source-1' } as any, - undefined, - { enabled: true }, - ), - { wrapper }, - ); - - await waitFor(() => expect(result.current.isLoading).toBe(false)); - expect(renderChartConfig).not.toHaveBeenCalled(); - expect(mockClickhouseClient.query).not.toHaveBeenCalled(); - }); - - it('returns 0/0 when the resolved source is not a trace source', async () => { - jest.mocked(useSource).mockReturnValue({ - data: { ...mockTraceSource, kind: SourceKind.Log }, - isLoading: false, - } as any); - - const { result } = renderHook( - () => - useTraceTotalDuration( - { source: 'trace-source-1' } as any, - 'abc123', - { enabled: true }, - ), - { wrapper }, - ); - - await waitFor(() => expect(result.current.isLoading).toBe(false)); - expect(result.current.data).toEqual({ totalDurationMs: 0, spanCount: 0 }); - }); - - it('computes wall-clock span from min(start) to max(start+duration) and passes the escaped traceId in the where clause', async () => { - mockClickhouseClient.query.mockResolvedValue({ - json: jest.fn().mockResolvedValue([ - { - minTs: '2024-01-01T00:00:00.000Z', - maxTs: '2024-01-01T00:00:02.280Z', - spanCount: '14', - }, - ]), - }); - - const { result } = renderHook( - () => - useTraceTotalDuration( - { source: 'trace-source-1', connection: 'conn-1' } as any, - 'abc123', - { enabled: true }, - ), - { wrapper }, - ); - - await waitFor(() => expect(result.current.isLoading).toBe(false)); - - expect(result.current.data).toEqual({ - totalDurationMs: 2280, - spanCount: 14, - }); - - expect(renderChartConfig).toHaveBeenCalledTimes(1); - const [aggConfig] = jest.mocked(renderChartConfig).mock.calls[0]; - expect(aggConfig.where).toBe("TraceId = 'abc123'"); - expect(aggConfig.whereLanguage).toBe('sql'); - }); - - it('escapes a traceId containing a single quote before embedding it in the where clause', async () => { - mockClickhouseClient.query.mockResolvedValue({ - json: jest - .fn() - .mockResolvedValue([ - { minTs: '2024-01-01T00:00:00.000Z', maxTs: '2024-01-01T00:00:01.000Z', spanCount: '1' }, - ]), - }); - - renderHook( - () => - useTraceTotalDuration( - { source: 'trace-source-1' } as any, - "abc'123", - { enabled: true }, - ), - { wrapper }, - ); - - await waitFor(() => - expect(renderChartConfig).toHaveBeenCalledTimes(1), - ); - const [aggConfig] = jest.mocked(renderChartConfig).mock.calls[0]; - expect(aggConfig.where).toBe("TraceId = 'abc\\'123'"); - }); - - it('returns 0/0 when the query returns no rows', async () => { - mockClickhouseClient.query.mockResolvedValue({ - json: jest.fn().mockResolvedValue([]), - }); - - const { result } = renderHook( - () => - useTraceTotalDuration( - { source: 'trace-source-1' } as any, - 'abc123', - { enabled: true }, - ), - { wrapper }, - ); - - await waitFor(() => expect(result.current.isLoading).toBe(false)); - expect(result.current.data).toEqual({ totalDurationMs: 0, spanCount: 0 }); - }); - - it('does not query while enabled is false', async () => { - const { result } = renderHook( - () => - useTraceTotalDuration( - { source: 'trace-source-1' } as any, - 'abc123', - { enabled: false }, - ), - { wrapper }, - ); - - await new Promise(resolve => setTimeout(resolve, 50)); - expect(result.current.isLoading).toBe(false); - expect(mockClickhouseClient.query).not.toHaveBeenCalled(); - }); -}); \ No newline at end of file diff --git a/packages/app/src/hooks/useTraceTotalDuration.tsx b/packages/app/src/hooks/useTraceTotalDuration.tsx deleted file mode 100644 index dbf44dff1a..0000000000 --- a/packages/app/src/hooks/useTraceTotalDuration.tsx +++ /dev/null @@ -1,98 +0,0 @@ -import { renderChartConfig } from '@hyperdx/common-utils/dist/core/renderChartConfig'; -import { - ChartConfigWithOptDateRange, - SourceKind, -} from '@hyperdx/common-utils/dist/types'; -import { useQuery, UseQueryOptions } from '@tanstack/react-query'; -import SqlString from 'sqlstring'; - -import { useClickhouseClient } from '@/clickhouse'; -import { - getDisplayedTimestampValueExpression, - getDurationMsExpression, - useSource, -} from '@/source'; - -import { useMetadataWithSettings } from './useMetadata'; - -type TraceTotalDurationResult = { - totalDurationMs: number; - spanCount: number; -}; - -// Wall-clock span of a single trace: MAX(span end) - MIN(span start) across -// every span belonging to the trace, computed server-side so it's correct -// even before the paginated results table has fetched every row (#3038). -export function useTraceTotalDuration( - _config: ChartConfigWithOptDateRange, - traceId: string | undefined, - options?: Omit< - UseQueryOptions, - 'queryKey' | 'queryFn' - >, -) { - const { enabled, ...restOptions } = options ?? {}; - - const config = { - ..._config, - with: undefined, - }; - - const clickhouseClient = useClickhouseClient(); - const metadata = useMetadataWithSettings(); - - const { data: source, isLoading: isSourceLoading } = useSource({ - id: config.source, - }); - - return useQuery({ - queryKey: ['trace-total-duration', config, traceId], - queryFn: async ({ signal }) => { - if (!traceId || !source || source.kind !== SourceKind.Trace) { - return { totalDurationMs: 0, spanCount: 0 }; - } - - const timestampExpr = getDisplayedTimestampValueExpression(source); - const durationMsExpr = getDurationMsExpression(source); - - const aggConfig: ChartConfigWithOptDateRange = { - ...config, - select: `MIN(${timestampExpr}) AS minTs, MAX(${timestampExpr} + toIntervalMillisecond(toInt64(${durationMsExpr}))) AS maxTs, COUNT(*) AS spanCount`, - where: `${source.traceIdExpression} = ${SqlString.escape(traceId)}`, - whereLanguage: 'sql', - orderBy: undefined, - groupBy: undefined, - }; - - const query = await renderChartConfig( - aggConfig, - metadata, - source.querySettings, - ); - const response = await clickhouseClient.query<'JSONEachRow'>({ - query: query.sql, - query_params: query.params, - format: 'JSONEachRow', - abort_signal: signal, - connectionId: config.connection, - }); - const [row] = await response.json<{ - minTs: string; - maxTs: string; - spanCount: string; - }>(); - if (!row) { - return { totalDurationMs: 0, spanCount: 0 }; - } - return { - totalDurationMs: - new Date(row.maxTs).getTime() - new Date(row.minTs).getTime(), - spanCount: Number(row.spanCount), - }; - }, - retry: false, - staleTime: 1000 * 60, - enabled: enabled && !!traceId && !isSourceLoading, - ...restOptions, - }); -} \ No newline at end of file From d8b218eb5e226fa3125ef9116f77f2d505c11691 Mon Sep 17 00:00:00 2001 From: milansanjeev <12941259+milansanjeev@users.noreply.github.com> Date: Wed, 2 Sep 2026 12:40:34 +0530 Subject: [PATCH 04/11] fix(app): address review findings on trace total duration stat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Drop the span count from the new stat -- it duplicated the existing "N spans, N errors" text with a different scope (all fetched spans vs. the visible/toggled subset), producing contradictory lines like "0 spans, 1 error · Total Duration: 2.28s · 12 spans" once the user toggles a chip or applies a filter. Fold the stat into the same as the existing count instead of a separate floating element. - Rename "Total Duration" to "Total duration" (sentence case, per AGENTS.md UI text convention); update matching test assertions. - Surface the trace query's 50k-row-per-window cap: propagate an `isTruncated` flag from useEventsAroundFocus through useFilteredEventsAroundFocus into the container, and append a "+" to the displayed duration when either window hit the cap, instead of silently presenting a partial total as exact. - Add tests: log exclusion no longer relies on a same-timestamp log row (uses a log timestamped an hour later, which would blow up the reported duration if wrongly included); overlapping-spans case where the true MAX(end) span is not the last one processed (catches a "last write wins" bug the non-overlapping fixture couldn't); chip toggling doesn't change the stat (proves it reflects all fetched spans, not the visible subset); isTruncated flagging via a cheap length-only mock in useEventsAroundFocus rather than a real 50k-row render (which timed out in jsdom). --- .../src/components/DBTraceWaterfallChart.tsx | 44 +++++-- .../__tests__/DBTraceWaterfallChart.test.tsx | 113 ++++++++++++++++-- 2 files changed, 135 insertions(+), 22 deletions(-) diff --git a/packages/app/src/components/DBTraceWaterfallChart.tsx b/packages/app/src/components/DBTraceWaterfallChart.tsx index 3a9be396c0..4070c525af 100644 --- a/packages/app/src/components/DBTraceWaterfallChart.tsx +++ b/packages/app/src/components/DBTraceWaterfallChart.tsx @@ -147,6 +147,10 @@ const SERVICE_COLORS = COLORS.filter( CATEGORICAL_PALETTE_TOKENS[i] !== 'chart-red', ); +// Per-window (before/after focus date) row cap applied in `getConfig` below. +// Exported for the truncation check in `useEventsAroundFocus`. +export const TRACE_WATERFALL_ROW_LIMIT = 50000; + function getTableBody(tableModel: TSource) { if (tableModel?.kind === SourceKind.Trace) { return getSpanEventBody(tableModel) ?? ''; @@ -303,7 +307,7 @@ function getConfig( from: source.from, timestampValueExpression: source.timestampValueExpression, where: `${alias.TraceId} = ${SqlString.escape(traceId)}`, - limit: { limit: 50000 }, + limit: { limit: TRACE_WATERFALL_ROW_LIMIT }, connection: source.connection, }; return { config, alias, type: source.kind }; @@ -384,6 +388,12 @@ export function useEventsAroundFocus({ const isFetching = isBeforeSpanFetching || isAfterSpanFetching; const meta = beforeSpanData?.meta ?? afterSpanData?.meta; const error = beforeSpanError || afterSpanError; + // Either window hitting its own row cap means rows outside the fetched set + // exist but were dropped -- any aggregate over `rows` (e.g. total duration/ + // span count) is then a lower bound, not an exact total. + const isTruncated = + (beforeSpanData?.data?.length ?? 0) >= TRACE_WATERFALL_ROW_LIMIT || + (afterSpanData?.data?.length ?? 0) >= TRACE_WATERFALL_ROW_LIMIT; const getRowWhere = useRowWhere({ meta, aliasMap: alias }); const rows = useMemo(() => { @@ -422,6 +432,7 @@ export function useEventsAroundFocus({ meta, isFetching, error, + isTruncated, }; } @@ -446,6 +457,7 @@ function useFilteredEventsAroundFocus( isFetching: filtered.isFetching || fallback.isFetching, filterError: fallback.error ? undefined : filtered.error, fatalError: fallback.error, + isTruncated: fallback.isTruncated, }; } @@ -455,6 +467,7 @@ function useFilteredEventsAroundFocus( isFetching: filtered.isFetching, filterError: undefined, fatalError: filtered.error, + isTruncated: filtered.isTruncated, }; } @@ -667,6 +680,7 @@ export function DBTraceWaterfallChartContainer({ meta: traceRowsMeta, filterError: traceFilterError, fatalError: traceError, + isTruncated: traceIsTruncated, } = useFilteredEventsAroundFocus({ tableSource: traceTableSource, focusDate, @@ -727,7 +741,9 @@ export function DBTraceWaterfallChartContainer({ // Aggregate wall-clock span and span count across every fetched span in the // trace (not the visible/collapsed subset, and unaffected by the waterfall // search filters below, since those only flag rows via `__hdx_hidden` - // rather than excluding them) -- a stable, always-accurate summary (#3038). + // rather than excluding them) (#3038). If the trace has more spans than fit + // in the fetch window (`traceIsTruncated`), this is a lower bound, not the + // true total -- the caller must surface that rather than claim exactness. const traceTotalStats = useMemo(() => { let minStartMs = Number.MAX_SAFE_INTEGER; let maxEndMs = 0; @@ -741,8 +757,12 @@ export function DBTraceWaterfallChartContainer({ if (endMs > maxEndMs) maxEndMs = endMs; } if (spanCount === 0) return null; - return { totalDurationMs: maxEndMs - minStartMs, spanCount }; - }, [rows]); + return { + totalDurationMs: maxEndMs - minStartMs, + spanCount, + isTruncated: traceIsTruncated, + }; + }, [rows, traceIsTruncated]); // Map each distinct span service to a stable color. Sorting the names first // keeps a service's color stable across renders regardless of row ordering. @@ -1433,15 +1453,15 @@ export function DBTraceWaterfallChartContainer({ {errorCountString} + {traceTotalStats && ( + + {' '} + · Total duration:{' '} + {formatDurationMs(traceTotalStats.totalDurationMs)} + {traceTotalStats.isTruncated ? '+' : ''} + + )} - {traceTotalStats && ( - - · Total Duration:{' '} - {formatDurationMs(traceTotalStats.totalDurationMs)} ·{' '} - {traceTotalStats.spanCount} span - {traceTotalStats.spanCount !== 1 ? 's' : ''} - - )} Show: diff --git a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx index a027f9ba5f..17e7e024f4 100644 --- a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx @@ -17,6 +17,7 @@ import { DBTraceWaterfallChartContainer, getDescendantIds, SpanRow, + TRACE_WATERFALL_ROW_LIMIT, useEventsAroundFocus, } from '@/components/DBTraceWaterfallChart'; import { TimelineChart } from '@/components/TimelineChart'; @@ -312,30 +313,40 @@ describe('DBTraceWaterfallChartContainer', () => { expect(MockTimelineChart.latestProps.rows[1]).toBeTruthy(); // Log row }); - it('renders total duration and span count computed from all fetched spans', async () => { + it('renders total duration computed from all fetched spans', async () => { setupQueryMocks({ traceData: mockTraceData }); renderComponent(null); // No log table source await waitForLoading(); const stats = screen.getByTestId('trace-total-stats'); // mockTraceData has one span, Duration: 0.1 (seconds) -> 100ms wall clock - expect(stats.textContent).toContain('Total Duration: 100ms'); - expect(stats.textContent).toContain('1 span'); + expect(stats.textContent).toContain('Total duration: 100ms'); }); - it('counts only spans (not correlated logs) toward the span count/duration stat', async () => { + it('excludes correlated logs from the total duration computation', async () => { + const laterLogData = { + data: [ + { + ...mockLogData.data[0], + // Far later than the span's own end (start + 100ms) -- if logs were + // wrongly included in the MAX(end) computation, this would blow up + // the reported duration. + Timestamp: '2024-01-01T07:00:00.000000000Z', + }, + ], + meta: [{ totalCount: 1 }], + }; + setupQueryMocks({ traceData: mockTraceData, - logData: mockLogData, + logData: laterLogData, }); renderComponent(); // With log table source await waitForLoading(); - // 2 total rows (1 span + 1 log), but the stat should only count the span const stats = screen.getByTestId('trace-total-stats'); - expect(stats.textContent).toContain('1 span'); - expect(stats.textContent).not.toContain('2 span'); + expect(stats.textContent).toContain('Total duration: 100ms'); }); it('computes total duration as the wall-clock span across multiple non-overlapping spans', async () => { @@ -371,8 +382,64 @@ describe('DBTraceWaterfallChartContainer', () => { const stats = screen.getByTestId('trace-total-stats'); // MIN(start) = 06:00:00.000, MAX(start+duration) = 06:00:01.500 -> 1500ms - expect(stats.textContent).toContain('Total Duration: 1.5s'); - expect(stats.textContent).toContain('2 spans'); + expect(stats.textContent).toContain('Total duration: 1.5s'); + }); + + it('computes total duration as the true max end time even when a later-processed span ends earlier (overlapping spans)', async () => { + const overlappingSpanData = { + data: [ + { + Body: 'outer span', + Timestamp: '2024-01-01T06:00:00.000000000Z', + Duration: 1.0, // ends at +1000ms -- this is the actual max end + SpanId: 'span-1', + ParentSpanId: '', + ServiceName: 'test-service', + HyperDXEventType: 'span' as const, + type: 'trace', + } as SpanRow, + { + Body: 'inner span', + Timestamp: '2024-01-01T06:00:00.100000000Z', // starts 100ms after the outer span + Duration: 0.2, // ends at +300ms -- earlier than the outer span's end, but processed last + SpanId: 'span-2', + ParentSpanId: 'span-1', + ServiceName: 'test-service', + HyperDXEventType: 'span' as const, + type: 'trace', + } as SpanRow, + ], + meta: [{ totalCount: 2 }], + }; + + setupQueryMocks({ traceData: overlappingSpanData }); + renderComponent(null); + await waitForLoading(); + + const stats = screen.getByTestId('trace-total-stats'); + // A "last row wins" bug would report the inner span's earlier end (300ms) + // instead of the true MAX(end) across all spans (1000ms). + expect(stats.textContent).toContain('Total duration: 1s'); + }); + + it('does not change when the Spans chip hides spans from the visible timeline (reflects all fetched spans, not the visible subset)', async () => { + setupQueryMocks({ + traceData: mockTraceData, + logData: mockLogData, + }); + renderComponent(); // With log table source, so the chart stays mounted + await waitForLoading(); + + const statsBefore = screen.getByTestId('trace-total-stats').textContent; + expect(statsBefore).toContain('Total duration: 100ms'); + + await userEvent.click(screen.getByTestId('show-spans-chip')); + + await waitFor(() => { + expect(screen.getByTestId('trace-total-stats').textContent).toBe( + statsBefore, + ); + }); }); it('does not render the total duration stat when there are no spans', async () => { @@ -767,6 +834,32 @@ describe('useEventsAroundFocus', () => { const result = testEventsAroundFocus({ enabled: false }); expect(result.rows.length).toBe(0); }); + + it('flags isTruncated when a window returns exactly the row cap (spans beyond it were dropped)', () => { + // Only `.length` matters for the cap check, so every element shares the + // same trivial object rather than allocating TRACE_WATERFALL_ROW_LIMIT + // distinct ones. + const cappedData = { + data: new Array(TRACE_WATERFALL_ROW_LIMIT).fill(null), + meta: [{ totalCount: TRACE_WATERFALL_ROW_LIMIT }], + }; + + const result = testEventsAroundFocus({ + beforeData: cappedData, + afterData: { data: [], meta: [{ totalCount: 0 }] }, + }); + + expect(result.isTruncated).toBe(true); + }); + + it('does not flag isTruncated when both windows return fewer rows than the cap', () => { + const result = testEventsAroundFocus({ + beforeData: { data: [{ Body: 'a' }], meta: [{ totalCount: 1 }] }, + afterData: { data: [{ Body: 'b' }], meta: [{ totalCount: 1 }] }, + }); + + expect(result.isTruncated).toBe(false); + }); }); describe('getDescendantIds', () => { From d0cd1b5ab8d834839fe600438a74c5aaef5f85d6 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 3 Sep 2026 02:55:31 +0000 Subject: [PATCH 05/11] fix(app): address remaining review notes on trace total duration Fetch one extra waterfall row so a complete window of exactly 50,000 spans is not marked truncated. Compute duration from span rows only, drop the unused spanCount field, document wall-clock vs MCP, and explain the lower-bound marker with a tooltip. --- .../trace-total-duration-search-table.md | 7 +- .../src/components/DBTraceWaterfallChart.tsx | 88 +++++++++++++------ .../__tests__/DBTraceWaterfallChart.test.tsx | 45 +++++++--- 3 files changed, 98 insertions(+), 42 deletions(-) diff --git a/.changeset/trace-total-duration-search-table.md b/.changeset/trace-total-duration-search-table.md index bcdf4a6223..5e033a6a8c 100644 --- a/.changeset/trace-total-duration-search-table.md +++ b/.changeset/trace-total-duration-search-table.md @@ -2,6 +2,7 @@ '@hyperdx/app': patch --- -feat: show a trace's total wall-clock duration and span count in the trace -waterfall view's controls bar. Previously this required manually eyeballing -the timeline (summing span rows would double-count parallel work). +feat: show a trace's total wall-clock duration in the waterfall controls bar +(earliest span start to latest span end). Previously this required eyeballing +the timeline; summing span rows would double-count parallel work. If a fetch +window exceeds the row cap, the figure is marked as a lower bound. diff --git a/packages/app/src/components/DBTraceWaterfallChart.tsx b/packages/app/src/components/DBTraceWaterfallChart.tsx index 4070c525af..d8b602790b 100644 --- a/packages/app/src/components/DBTraceWaterfallChart.tsx +++ b/packages/app/src/components/DBTraceWaterfallChart.tsx @@ -150,6 +150,34 @@ const SERVICE_COLORS = COLORS.filter( // Per-window (before/after focus date) row cap applied in `getConfig` below. // Exported for the truncation check in `useEventsAroundFocus`. export const TRACE_WATERFALL_ROW_LIMIT = 50000; +// Fetch one extra row so a window that contains *exactly* the cap is not +// mistaken for truncation. A full page of LIMIT is ambiguous; LIMIT+1 is not. +const TRACE_WATERFALL_FETCH_LIMIT = TRACE_WATERFALL_ROW_LIMIT + 1; + +export function TraceTotalDurationStat({ + totalDurationMs, + isTruncated, +}: { + totalDurationMs: number; + isTruncated: boolean; +}) { + return ( + + + {' '} + · Total duration: {formatDurationMs(totalDurationMs)} + {isTruncated ? '+' : ''} + + + ); +} function getTableBody(tableModel: TSource) { if (tableModel?.kind === SourceKind.Trace) { @@ -307,7 +335,7 @@ function getConfig( from: source.from, timestampValueExpression: source.timestampValueExpression, where: `${alias.TraceId} = ${SqlString.escape(traceId)}`, - limit: { limit: TRACE_WATERFALL_ROW_LIMIT }, + limit: { limit: TRACE_WATERFALL_FETCH_LIMIT }, connection: source.connection, }; return { config, alias, type: source.kind }; @@ -388,22 +416,27 @@ export function useEventsAroundFocus({ const isFetching = isBeforeSpanFetching || isAfterSpanFetching; const meta = beforeSpanData?.meta ?? afterSpanData?.meta; const error = beforeSpanError || afterSpanError; - // Either window hitting its own row cap means rows outside the fetched set - // exist but were dropped -- any aggregate over `rows` (e.g. total duration/ - // span count) is then a lower bound, not an exact total. - const isTruncated = - (beforeSpanData?.data?.length ?? 0) >= TRACE_WATERFALL_ROW_LIMIT || - (afterSpanData?.data?.length ?? 0) >= TRACE_WATERFALL_ROW_LIMIT; + // LIMIT+1 is a sentinel: a complete window of exactly TRACE_WATERFALL_ROW_LIMIT + // rows must not show as a lower bound. Drop the extra row so the chart still + // renders at most the cap. + const beforeRows = beforeSpanData?.data ?? []; + const afterRows = afterSpanData?.data ?? []; + const beforeTruncated = beforeRows.length > TRACE_WATERFALL_ROW_LIMIT; + const afterTruncated = afterRows.length > TRACE_WATERFALL_ROW_LIMIT; + const isTruncated = beforeTruncated || afterTruncated; + const trimmedBefore = beforeTruncated + ? beforeRows.slice(0, TRACE_WATERFALL_ROW_LIMIT) + : beforeRows; + const trimmedAfter = afterTruncated + ? afterRows.slice(0, TRACE_WATERFALL_ROW_LIMIT) + : afterRows; const getRowWhere = useRowWhere({ meta, aliasMap: alias }); const rows = useMemo(() => { // Sometimes meta has not loaded yet // DO NOT REMOVE, useRowWhere will error if no meta if (!meta || meta.length === 0) return []; - return [ - ...(beforeSpanData?.data ?? []), - ...(afterSpanData?.data ?? []), - ].map(cd => { + return [...trimmedBefore, ...trimmedAfter].map(cd => { // Omit SpanAttributes, SpanEvents and __hdx_hidden from rowWhere id generation. // SpanAttributes and SpanEvents can be large objects, and __hdx_hidden may be a lucene expression. const rowWhereResult = getRowWhere( @@ -425,7 +458,7 @@ export function useEventsAroundFocus({ aliasWith: [], }; }); - }, [afterSpanData, beforeSpanData, meta, getRowWhere, type]); + }, [trimmedAfter, trimmedBefore, meta, getRowWhere, type]); return { rows, @@ -738,18 +771,20 @@ export function DBTraceWaterfallChartContainer({ return nextRows; }, [traceRowsData, logRowsData]); - // Aggregate wall-clock span and span count across every fetched span in the - // trace (not the visible/collapsed subset, and unaffected by the waterfall - // search filters below, since those only flag rows via `__hdx_hidden` - // rather than excluding them) (#3038). If the trace has more spans than fit - // in the fetch window (`traceIsTruncated`), this is a lower bound, not the - // true total -- the caller must surface that rather than claim exactness. + // Wall-clock duration across every fetched span (min start to max end), not + // the visible/collapsed subset. Waterfall search filters only flag rows via + // `__hdx_hidden`, so they do not change this figure (#3038). + // + // This is not the same as MCP `trace_waterfall`'s `totalDurationMs`, which is + // the longest single span. Clock skew or async children that outlive the root + // make the two disagree; the UI number is the envelope of the timeline. + // + // If a fetch window exceeded TRACE_WATERFALL_ROW_LIMIT, this is a lower bound. const traceTotalStats = useMemo(() => { let minStartMs = Number.MAX_SAFE_INTEGER; let maxEndMs = 0; let spanCount = 0; - for (const row of rows) { - if (row.type === SourceKind.Log) continue; + for (const row of traceRowsData) { spanCount++; const startMs = parseTimestampToMs(row.Timestamp); const endMs = startMs + (row.Duration || 0) * 1000; @@ -759,10 +794,9 @@ export function DBTraceWaterfallChartContainer({ if (spanCount === 0) return null; return { totalDurationMs: maxEndMs - minStartMs, - spanCount, isTruncated: traceIsTruncated, }; - }, [rows, traceIsTruncated]); + }, [traceRowsData, traceIsTruncated]); // Map each distinct span service to a stable color. Sorting the names first // keeps a service's color stable across renders regardless of row ordering. @@ -1454,12 +1488,10 @@ export function DBTraceWaterfallChartContainer({ {errorCountString} {traceTotalStats && ( - - {' '} - · Total duration:{' '} - {formatDurationMs(traceTotalStats.totalDurationMs)} - {traceTotalStats.isTruncated ? '+' : ''} - + )} diff --git a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx index 17e7e024f4..5ddc2bbdd4 100644 --- a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx @@ -18,6 +18,7 @@ import { getDescendantIds, SpanRow, TRACE_WATERFALL_ROW_LIMIT, + TraceTotalDurationStat, useEventsAroundFocus, } from '@/components/DBTraceWaterfallChart'; import { TimelineChart } from '@/components/TimelineChart'; @@ -320,7 +321,7 @@ describe('DBTraceWaterfallChartContainer', () => { const stats = screen.getByTestId('trace-total-stats'); // mockTraceData has one span, Duration: 0.1 (seconds) -> 100ms wall clock - expect(stats.textContent).toContain('Total duration: 100ms'); + expect(stats.textContent?.trim()).toBe('· Total duration: 100ms'); }); it('excludes correlated logs from the total duration computation', async () => { @@ -346,7 +347,7 @@ describe('DBTraceWaterfallChartContainer', () => { await waitForLoading(); const stats = screen.getByTestId('trace-total-stats'); - expect(stats.textContent).toContain('Total duration: 100ms'); + expect(stats.textContent?.trim()).toBe('· Total duration: 100ms'); }); it('computes total duration as the wall-clock span across multiple non-overlapping spans', async () => { @@ -382,7 +383,7 @@ describe('DBTraceWaterfallChartContainer', () => { const stats = screen.getByTestId('trace-total-stats'); // MIN(start) = 06:00:00.000, MAX(start+duration) = 06:00:01.500 -> 1500ms - expect(stats.textContent).toContain('Total duration: 1.5s'); + expect(stats.textContent?.trim()).toBe('· Total duration: 1.5s'); }); it('computes total duration as the true max end time even when a later-processed span ends earlier (overlapping spans)', async () => { @@ -419,7 +420,7 @@ describe('DBTraceWaterfallChartContainer', () => { const stats = screen.getByTestId('trace-total-stats'); // A "last row wins" bug would report the inner span's earlier end (300ms) // instead of the true MAX(end) across all spans (1000ms). - expect(stats.textContent).toContain('Total duration: 1s'); + expect(stats.textContent?.trim()).toBe('· Total duration: 1s'); }); it('does not change when the Spans chip hides spans from the visible timeline (reflects all fetched spans, not the visible subset)', async () => { @@ -431,7 +432,7 @@ describe('DBTraceWaterfallChartContainer', () => { await waitForLoading(); const statsBefore = screen.getByTestId('trace-total-stats').textContent; - expect(statsBefore).toContain('Total duration: 100ms'); + expect(statsBefore?.trim()).toBe('· Total duration: 100ms'); await userEvent.click(screen.getByTestId('show-spans-chip')); @@ -457,6 +458,15 @@ describe('DBTraceWaterfallChartContainer', () => { expect(screen.queryByTestId('trace-total-stats')).not.toBeInTheDocument(); }); + it('appends + on the duration label when the fetch window was truncated', () => { + renderWithMantine( + , + ); + expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( + '· Total duration: 100ms+', + ); + }); + it('renders empty state when no data is available', async () => { mockUseOffsetPaginatedQuery.mockReturnValue({ data: emptyData, @@ -835,21 +845,34 @@ describe('useEventsAroundFocus', () => { expect(result.rows.length).toBe(0); }); - it('flags isTruncated when a window returns exactly the row cap (spans beyond it were dropped)', () => { - // Only `.length` matters for the cap check, so every element shares the - // same trivial object rather than allocating TRACE_WATERFALL_ROW_LIMIT - // distinct ones. - const cappedData = { + it('does not flag isTruncated when a window returns exactly the row cap', () => { + const exactCapData = { data: new Array(TRACE_WATERFALL_ROW_LIMIT).fill(null), meta: [{ totalCount: TRACE_WATERFALL_ROW_LIMIT }], }; const result = testEventsAroundFocus({ - beforeData: cappedData, + beforeData: exactCapData, + afterData: { data: [], meta: [{ totalCount: 0 }] }, + }); + + expect(result.isTruncated).toBe(false); + expect(result.rows.length).toBe(TRACE_WATERFALL_ROW_LIMIT); + }); + + it('flags isTruncated when a window returns more rows than the cap, and drops the sentinel row', () => { + const overCapData = { + data: new Array(TRACE_WATERFALL_ROW_LIMIT + 1).fill(null), + meta: [{ totalCount: TRACE_WATERFALL_ROW_LIMIT + 1 }], + }; + + const result = testEventsAroundFocus({ + beforeData: overCapData, afterData: { data: [], meta: [{ totalCount: 0 }] }, }); expect(result.isTruncated).toBe(true); + expect(result.rows.length).toBe(TRACE_WATERFALL_ROW_LIMIT); }); it('does not flag isTruncated when both windows return fewer rows than the cap', () => { From 8fd8e293832dea90ba3f8c160d33ef9efdf740d2 Mon Sep 17 00:00:00 2001 From: Sanjeev Verma Date: Thu, 1 Oct 2026 15:49:08 +0530 Subject: [PATCH 06/11] fix(app): resolve tsc type error and address remaining review feedback on trace total duration - Fix TS2339: traceRowsData's static row type doesn't expose Duration (useEventsAroundFocus builds it via an object-literal spread of an any-typed ClickHouse row plus explicit re-declared keys, so TS infers the element type from only the explicit keys). Cast to any[] for the loop, matching the existing any-typed access pattern used elsewhere in this file (rows.reduce, visibleNodes.map) for the same dynamic shape. - Extend the lower-bound/truncation marker to also fire when the +/-1h dateRange fetch window itself clips the trace (a span starting at/before the window's lower edge, or ending at/after its upper edge), not just the TRACE_WATERFALL_ROW_LIMIT row cap. Previously a trace clipped by the focus window reported a confident-looking exact duration with no indication data was missing. Generalize the tooltip copy to cover both triggers. - Add tests for the new window-edge behavior (lower edge, upper edge, and the not-clipped case), run through the full render chain. - Widen the test file's shared mockDateRange/mockFocusDate, which never actually overlapped the mock span/log timestamps used by the existing duration tests (harmless before this change, since nothing previously compared row timestamps against dateRange) so the new window-edge check doesn't spuriously trip on unrelated tests. --- .../src/components/DBTraceWaterfallChart.tsx | 21 +++-- .../__tests__/DBTraceWaterfallChart.test.tsx | 80 ++++++++++++++++++- 2 files changed, 93 insertions(+), 8 deletions(-) diff --git a/packages/app/src/components/DBTraceWaterfallChart.tsx b/packages/app/src/components/DBTraceWaterfallChart.tsx index d8b602790b..234a86566d 100644 --- a/packages/app/src/components/DBTraceWaterfallChart.tsx +++ b/packages/app/src/components/DBTraceWaterfallChart.tsx @@ -165,7 +165,7 @@ export function TraceTotalDurationStat({ =` (not + // strict) is deliberate: a span landing exactly on the boundary is + // itself ambiguous, and it's safer to over-signal "possibly clipped" + // than to silently under-report (matches the row-cap sentinel above). const traceTotalStats = useMemo(() => { let minStartMs = Number.MAX_SAFE_INTEGER; let maxEndMs = 0; let spanCount = 0; - for (const row of traceRowsData) { + for (const row of traceRowsData as any[]) { spanCount++; const startMs = parseTimestampToMs(row.Timestamp); const endMs = startMs + (row.Duration || 0) * 1000; @@ -792,11 +800,14 @@ export function DBTraceWaterfallChartContainer({ if (endMs > maxEndMs) maxEndMs = endMs; } if (spanCount === 0) return null; + const windowClipped = + minStartMs <= dateRange[0].getTime() || + maxEndMs >= dateRange[1].getTime(); return { totalDurationMs: maxEndMs - minStartMs, - isTruncated: traceIsTruncated, + isTruncated: traceIsTruncated || windowClipped, }; - }, [traceRowsData, traceIsTruncated]); + }, [traceRowsData, traceIsTruncated, dateRange]); // Map each distinct span service to a stable color. Sorting the names first // keeps a service's color stable across renders regardless of row ordering. diff --git a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx index 5ddc2bbdd4..41658fe0e1 100644 --- a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx @@ -148,11 +148,18 @@ describe('DBTraceWaterfallChartContainer', () => { connection: 'conn2', }; + // Wide enough to contain every span/log timestamp used by the tests below + // (06:00:00-07:00:00) with margin on both sides, so the window-clipping + // check in `traceTotalStats` (a span starting at/before the window's lower + // edge, or ending at/after its upper edge, marks the duration as a lower + // bound) doesn't incidentally trip for data that was never meant to be + // testing that behavior. See the dedicated window-clipping tests below for + // cases that deliberately sit at the edge. const mockDateRange = [ - new Date('2024-01-01T00:00:00.000Z'), - new Date('2024-01-01T01:00:00.000Z'), + new Date('2024-01-01T05:00:00.000Z'), + new Date('2024-01-01T08:00:00.000Z'), ] as [Date, Date]; - const mockFocusDate = new Date('2024-01-01T00:30:00.000Z'); + const mockFocusDate = new Date('2024-01-01T06:30:00.000Z'); const mockTraceId = 'test-trace-id'; // Sample data @@ -467,6 +474,73 @@ describe('DBTraceWaterfallChartContainer', () => { ); }); + it('marks the duration as a lower bound when a span starts at/before the fetch window\'s lower edge (possible earlier spans outside the window)', async () => { + const spanAtWindowEdge = { + data: [ + { + ...mockTraceData.data[0], + // mockDateRange's lower edge is 05:00:00 -- a span starting exactly + // there (or earlier) means there could be earlier spans that the + // ±window fetch never requested. + Timestamp: '2024-01-01T05:00:00.000000000Z', + }, + ], + meta: [{ totalCount: 1 }], + }; + setupQueryMocks({ traceData: spanAtWindowEdge }); + renderComponent(null); + await waitForLoading(); + + expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( + '· Total duration: 100ms+', + ); + }); + + it('marks the duration as a lower bound when a span ends at/after the fetch window\'s upper edge (possible later spans outside the window)', async () => { + const spanAtWindowEdge = { + data: [ + { + ...mockTraceData.data[0], + // Ends exactly at mockDateRange's upper edge (08:00:00): starts at + // 07:59:59.900 + Duration 0.1s. Same reasoning as the lower-edge + // case above, mirrored at the other end of the window. + Timestamp: '2024-01-01T07:59:59.900000000Z', + }, + ], + meta: [{ totalCount: 1 }], + }; + setupQueryMocks({ traceData: spanAtWindowEdge }); + renderComponent(null); + await waitForLoading(); + + expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( + '· Total duration: 100ms+', + ); + }); + + it('does not mark the duration as a lower bound when every span sits comfortably inside the fetch window', async () => { + setupQueryMocks({ traceData: mockTraceData }); // 06:00:00, well inside 05:00:00-08:00:00 + renderComponent(null); + await waitForLoading(); + + expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( + '· Total duration: 100ms', + ); + }); + + // Note: the row-cap trigger for isTruncated (as opposed to the + // window-edge trigger tested above) is covered at the `useEventsAroundFocus` + // hook level below ("flags isTruncated when a window returns more rows than + // the cap...") and at the render level by the "appends + on the duration + // label..." test above. A true end-to-end version of that case (actually + // fetching/rendering TRACE_WATERFALL_ROW_LIMIT+1 real rows through + // setupQueryMocks) was tried here and is correct but prohibitively slow + // under full-suite jsdom contention (passes in isolation, times out at 30s+ + // alongside the rest of the file) -- not a good trade for the marginal + // wiring coverage over the window-edge tests above, which already exercise + // the identical `traceTotalStats.isTruncated` -> rendered "+" chain for the + // other trigger. + it('renders empty state when no data is available', async () => { mockUseOffsetPaginatedQuery.mockReturnValue({ data: emptyData, From 178fe7d57e80ccc504b96ceb2b91ee9993ed3b18 Mon Sep 17 00:00:00 2001 From: milansanjeev <12941259+milansanjeev@users.noreply.github.com> Date: Thu, 1 Oct 2026 15:04:21 +0530 Subject: [PATCH 07/11] fix(app): mark clipped traces without a false lower-edge check The row type dropped Duration after spreading a Record, and comparing the earliest start to the window start never sees an earlier span. Treat a missing parent on the earliest span, or a span that runs to the window end, as a lower bound, and keep trimmed query windows stable across renders. --- .../src/components/DBTraceWaterfallChart.tsx | 128 +++++++++++------ .../__tests__/DBTraceWaterfallChart.test.tsx | 135 +++++++++++++----- 2 files changed, 185 insertions(+), 78 deletions(-) diff --git a/packages/app/src/components/DBTraceWaterfallChart.tsx b/packages/app/src/components/DBTraceWaterfallChart.tsx index 00ddbe465c..49ac193608 100644 --- a/packages/app/src/components/DBTraceWaterfallChart.tsx +++ b/packages/app/src/components/DBTraceWaterfallChart.tsx @@ -154,6 +154,10 @@ export const TRACE_WATERFALL_ROW_LIMIT = 50000; // mistaken for truncation. A full page of LIMIT is ambiguous; LIMIT+1 is not. const TRACE_WATERFALL_FETCH_LIMIT = TRACE_WATERFALL_ROW_LIMIT + 1; +// Stable empty fallback. `data ?? []` would allocate a new array every render +// and invalidate every memo downstream of the waterfall rows. +const EMPTY_ROWS: Record[] = []; + export function TraceTotalDurationStat({ totalDurationMs, isTruncated, @@ -165,8 +169,8 @@ export function TraceTotalDurationStat({ @@ -418,18 +422,25 @@ export function useEventsAroundFocus({ const error = beforeSpanError || afterSpanError; // LIMIT+1 is a sentinel: a complete window of exactly TRACE_WATERFALL_ROW_LIMIT // rows must not show as a lower bound. Drop the extra row so the chart still - // renders at most the cap. - const beforeRows = beforeSpanData?.data ?? []; - const afterRows = afterSpanData?.data ?? []; - const beforeTruncated = beforeRows.length > TRACE_WATERFALL_ROW_LIMIT; - const afterTruncated = afterRows.length > TRACE_WATERFALL_ROW_LIMIT; - const isTruncated = beforeTruncated || afterTruncated; - const trimmedBefore = beforeTruncated - ? beforeRows.slice(0, TRACE_WATERFALL_ROW_LIMIT) - : beforeRows; - const trimmedAfter = afterTruncated - ? afterRows.slice(0, TRACE_WATERFALL_ROW_LIMIT) - : afterRows; + // renders at most the cap. Trim inside this memo so a parent re-render does + // not allocate fresh arrays and rebuild the span tree. + const beforeData = beforeSpanData?.data; + const afterData = afterSpanData?.data; + const { isTruncated, trimmedBefore, trimmedAfter } = useMemo(() => { + const beforeRows = beforeData ?? EMPTY_ROWS; + const afterRows = afterData ?? EMPTY_ROWS; + const beforeTruncated = beforeRows.length > TRACE_WATERFALL_ROW_LIMIT; + const afterTruncated = afterRows.length > TRACE_WATERFALL_ROW_LIMIT; + return { + isTruncated: beforeTruncated || afterTruncated, + trimmedBefore: beforeTruncated + ? beforeRows.slice(0, TRACE_WATERFALL_ROW_LIMIT) + : beforeRows, + trimmedAfter: afterTruncated + ? afterRows.slice(0, TRACE_WATERFALL_ROW_LIMIT) + : afterRows, + }; + }, [beforeData, afterData]); const getRowWhere = useRowWhere({ meta, aliasMap: alias }); const rows = useMemo(() => { @@ -446,9 +457,13 @@ export function useEventsAroundFocus({ return { // Keep all fields available for display ...cd, - // Added for typing + // Named fields are repeated for typing. Spreading Record + // drops the index signature once explicit keys are set, so Duration + // would otherwise be missing from the row type. Timestamp: cd?.Timestamp, SpanId: cd?.SpanId, + ParentSpanId: cd?.ParentSpanId, + Duration: cd?.Duration, __hdx_hidden: cd?.__hdx_hidden, type, id: rowWhereResult.where, @@ -775,39 +790,62 @@ export function DBTraceWaterfallChartContainer({ // the visible/collapsed subset. Waterfall search filters only flag rows via // `__hdx_hidden`, so they do not change this figure (#3038). // - // This is not the same as MCP `trace_waterfall`'s `totalDurationMs`, which is - // the longest single span. Clock skew or async children that outlive the root - // make the two disagree; the UI number is the envelope of the timeline. + // `useEventsData` loads a single page on each side of the focus time and does + // not call `fetchNextPage`. Spans outside that page, or outside the requested + // date range, are not included. + // + // MCP `trace_waterfall` reports the longest single span instead. Changing + // that tool is a separate behavior change; this number is the timeline envelope. // - // This is a lower bound if either fetch window was clipped: - // - the TRACE_WATERFALL_ROW_LIMIT row cap (traceIsTruncated), or - // - the ±1h `dateRange` fetch window itself (DBRowSidePanel passes - // oneHourRange) -- a span starting at-or-before the window's lower - // edge, or ending at-or-after its upper edge, means there may be - // earlier/later spans outside the fetched range. `<=`/`>=` (not - // strict) is deliberate: a span landing exactly on the boundary is - // itself ambiguous, and it's safer to over-signal "possibly clipped" - // than to silently under-report (matches the row-cap sentinel above). - const traceTotalStats = useMemo(() => { - let minStartMs = Number.MAX_SAFE_INTEGER; - let maxEndMs = 0; - let spanCount = 0; - for (const row of traceRowsData as any[]) { - spanCount++; - const startMs = parseTimestampToMs(row.Timestamp); - const endMs = startMs + (row.Duration || 0) * 1000; - if (startMs < minStartMs) minStartMs = startMs; + // `windowClipped` is a lower bound for the fetch window, separate from the + // row-cap flag so a cap change does not rescan every span: + // - a span ending at or after `dateRange[1]` may continue past the window + // - the earliest span's parent was not loaded. Both queries filter + // `Timestamp >= dateRange[0]`, so comparing min start to the lower edge + // never sees a span that started earlier. A missing parent on the + // earliest span is that signal. + const traceExtent = useMemo(() => { + if (traceRowsData.length === 0) return null; + const spanIds = new Set(); + for (const row of traceRowsData) { + if (typeof row.SpanId === 'string' && row.SpanId.length > 0) { + spanIds.add(row.SpanId); + } + } + const windowEndMs = dateRange[1].getTime(); + let minStartMs = Number.POSITIVE_INFINITY; + let maxEndMs = Number.NEGATIVE_INFINITY; + let earliestParentMissing = false; + let extendsPastWindowEnd = false; + let sawValid = false; + for (const row of traceRowsData) { + let startMs: number; + try { + startMs = parseTimestampToMs(row.Timestamp); + } catch { + continue; + } + if (!Number.isFinite(startMs)) continue; + const durationSec = Number(row.Duration); + const endMs = + startMs + (Number.isFinite(durationSec) ? durationSec : 0) * 1000; + const parentId = + typeof row.ParentSpanId === 'string' ? row.ParentSpanId : ''; + const parentMissing = parentId.length > 0 && !spanIds.has(parentId); + if (startMs < minStartMs) { + minStartMs = startMs; + earliestParentMissing = parentMissing; + } if (endMs > maxEndMs) maxEndMs = endMs; + if (endMs >= windowEndMs) extendsPastWindowEnd = true; + sawValid = true; } - if (spanCount === 0) return null; - const windowClipped = - minStartMs <= dateRange[0].getTime() || - maxEndMs >= dateRange[1].getTime(); + if (!sawValid) return null; return { totalDurationMs: maxEndMs - minStartMs, - isTruncated: traceIsTruncated || windowClipped, + windowClipped: earliestParentMissing || extendsPastWindowEnd, }; - }, [traceRowsData, traceIsTruncated, dateRange]); + }, [traceRowsData, dateRange]); // Map each distinct span service to a stable color. Sorting the names first // keeps a service's color stable across renders regardless of row ordering. @@ -1496,10 +1534,10 @@ export function DBTraceWaterfallChartContainer({ {errorCountString} - {traceTotalStats && ( + {traceExtent != null && ( )} diff --git a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx index 41658fe0e1..8652919fe9 100644 --- a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx @@ -52,11 +52,13 @@ jest.mock('@/components/TimelineChart', () => { return (
TimelineChart - {props.rows?.map((row: any) => ( -
- {row.events?.map((event: any) => flattenText(event.body))} -
- ))} + {props.rows + ?.slice(0, 20) + .map((row: any) => ( +
+ {row.events?.map((event: any) => flattenText(event.body))} +
+ ))}
); }; @@ -149,12 +151,9 @@ describe('DBTraceWaterfallChartContainer', () => { }; // Wide enough to contain every span/log timestamp used by the tests below - // (06:00:00-07:00:00) with margin on both sides, so the window-clipping - // check in `traceTotalStats` (a span starting at/before the window's lower - // edge, or ending at/after its upper edge, marks the duration as a lower - // bound) doesn't incidentally trip for data that was never meant to be - // testing that behavior. See the dedicated window-clipping tests below for - // cases that deliberately sit at the edge. + // (06:00:00-07:00:00) with margin on both sides, so a span ending at the + // window's upper edge does not trip the lower-bound marker unless a test + // asks for that. const mockDateRange = [ new Date('2024-01-01T05:00:00.000Z'), new Date('2024-01-01T08:00:00.000Z'), @@ -474,20 +473,47 @@ describe('DBTraceWaterfallChartContainer', () => { ); }); - it('marks the duration as a lower bound when a span starts at/before the fetch window\'s lower edge (possible earlier spans outside the window)', async () => { - const spanAtWindowEdge = { + it('shows the lower-bound marker when the trace query exceeds the row cap', async () => { + const span = { + Body: 'capped span', + Timestamp: '2024-01-01T06:00:00.000000000Z', + Duration: 0.1, + SpanId: 'span-1', + ParentSpanId: '', + ServiceName: 'test-service', + HyperDXEventType: 'span' as const, + type: 'trace', + }; + setupQueryMocks({ + traceData: { + data: Array.from({ length: TRACE_WATERFALL_ROW_LIMIT + 1 }, () => span), + meta: [{ totalCount: TRACE_WATERFALL_ROW_LIMIT + 1 }], + }, + }); + renderComponent(null); + await waitForLoading(); + + expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( + '· Total duration: 100ms+', + ); + }); + + it("marks the duration as a lower bound when the earliest span's parent was not loaded", async () => { + const orphan = { data: [ { ...mockTraceData.data[0], - // mockDateRange's lower edge is 05:00:00 -- a span starting exactly - // there (or earlier) means there could be earlier spans that the - // ±window fetch never requested. - Timestamp: '2024-01-01T05:00:00.000000000Z', + // Inside the window. The parent id is not among the fetched spans, + // so the trace may have started before the loaded range. A span that + // merely starts on dateRange[0] is not enough: the queries already + // exclude anything earlier than that edge. + Timestamp: '2024-01-01T06:00:00.000000000Z', + ParentSpanId: 'missing-root', }, ], meta: [{ totalCount: 1 }], }; - setupQueryMocks({ traceData: spanAtWindowEdge }); + setupQueryMocks({ traceData: orphan }); renderComponent(null); await waitForLoading(); @@ -496,14 +522,33 @@ describe('DBTraceWaterfallChartContainer', () => { ); }); - it('marks the duration as a lower bound when a span ends at/after the fetch window\'s upper edge (possible later spans outside the window)', async () => { + it("does not mark a span that starts on the window's lower edge as clipped when its parent was loaded", async () => { + const onLowerEdge = { + data: [ + { + ...mockTraceData.data[0], + Timestamp: '2024-01-01T05:00:00.000000000Z', + ParentSpanId: '', + }, + ], + meta: [{ totalCount: 1 }], + }; + setupQueryMocks({ traceData: onLowerEdge }); + renderComponent(null); + await waitForLoading(); + + expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( + '· Total duration: 100ms', + ); + }); + + it("marks the duration as a lower bound when a span ends at/after the fetch window's upper edge", async () => { const spanAtWindowEdge = { data: [ { ...mockTraceData.data[0], // Ends exactly at mockDateRange's upper edge (08:00:00): starts at - // 07:59:59.900 + Duration 0.1s. Same reasoning as the lower-edge - // case above, mirrored at the other end of the window. + // 07:59:59.900 + Duration 0.1s. Timestamp: '2024-01-01T07:59:59.900000000Z', }, ], @@ -528,19 +573,43 @@ describe('DBTraceWaterfallChartContainer', () => { ); }); - // Note: the row-cap trigger for isTruncated (as opposed to the - // window-edge trigger tested above) is covered at the `useEventsAroundFocus` - // hook level below ("flags isTruncated when a window returns more rows than - // the cap...") and at the render level by the "appends + on the duration - // label..." test above. A true end-to-end version of that case (actually - // fetching/rendering TRACE_WATERFALL_ROW_LIMIT+1 real rows through - // setupQueryMocks) was tried here and is correct but prohibitively slow - // under full-suite jsdom contention (passes in isolation, times out at 30s+ - // alongside the rest of the file) -- not a good trade for the marginal - // wiring coverage over the window-edge tests above, which already exercise - // the identical `traceTotalStats.isTruncated` -> rendered "+" chain for the - // other trigger. + it('keeps the fetched duration when a span is flagged hidden', async () => { + const hiddenSpanData = { + data: [ + { + Body: 'visible span', + Timestamp: '2024-01-01T06:00:00.000000000Z', + Duration: 0.1, + SpanId: 'span-visible', + ParentSpanId: '', + ServiceName: 'test-service', + HyperDXEventType: 'span' as const, + type: 'trace', + __hdx_hidden: false, + } as SpanRow, + { + Body: 'hidden span', + Timestamp: '2024-01-01T06:00:01.000000000Z', + Duration: 0.5, + SpanId: 'span-hidden', + ParentSpanId: '', + ServiceName: 'test-service', + HyperDXEventType: 'span' as const, + type: 'trace', + __hdx_hidden: true, + } as SpanRow, + ], + meta: [{ totalCount: 2 }], + }; + + setupQueryMocks({ traceData: hiddenSpanData }); + renderComponent(null); + await waitForLoading(); + expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( + '· Total duration: 1.5s', + ); + }); it('renders empty state when no data is available', async () => { mockUseOffsetPaginatedQuery.mockReturnValue({ data: emptyData, From 59ee0b9438a53fcc3c3fef7eb0ee195d2cc49f1b Mon Sep 17 00:00:00 2001 From: milansanjeev <12941259+milansanjeev@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:52:22 +0530 Subject: [PATCH 08/11] fix(app): mark a clipped trace from the window edge, not a missing parent A parent that was never exported is a gap in the trace when the child starts inside the fetch window, and an empty parent id is a real root. The lower-bound marker now follows a start pressed against the window, or a span that runs to the window end. Co-authored-by: Cursor --- .../src/components/DBTraceWaterfallChart.tsx | 34 +++++----- .../__tests__/DBTraceWaterfallChart.test.tsx | 67 +++++++++++++++---- 2 files changed, 72 insertions(+), 29 deletions(-) diff --git a/packages/app/src/components/DBTraceWaterfallChart.tsx b/packages/app/src/components/DBTraceWaterfallChart.tsx index 49ac193608..c1daa2264d 100644 --- a/packages/app/src/components/DBTraceWaterfallChart.tsx +++ b/packages/app/src/components/DBTraceWaterfallChart.tsx @@ -153,6 +153,11 @@ export const TRACE_WATERFALL_ROW_LIMIT = 50000; // Fetch one extra row so a window that contains *exactly* the cap is not // mistaken for truncation. A full page of LIMIT is ambiguous; LIMIT+1 is not. const TRACE_WATERFALL_FETCH_LIMIT = TRACE_WATERFALL_ROW_LIMIT + 1; +// The span queries keep `Timestamp >= dateRange[0]`, so a start strictly +// before the window is not returned and `minStart <= dateRange[0]` only +// matches an exact hit. A start this close to the lower edge is the signal +// that the fetch cut off earlier spans. +const WINDOW_START_EDGE_TOLERANCE_MS = 1; // Stable empty fallback. `data ?? []` would allocate a new array every render // and invalidate every memo downstream of the waterfall rows. @@ -800,22 +805,21 @@ export function DBTraceWaterfallChartContainer({ // `windowClipped` is a lower bound for the fetch window, separate from the // row-cap flag so a cap change does not rescan every span: // - a span ending at or after `dateRange[1]` may continue past the window - // - the earliest span's parent was not loaded. Both queries filter - // `Timestamp >= dateRange[0]`, so comparing min start to the lower edge - // never sees a span that started earlier. A missing parent on the - // earliest span is that signal. + // - the earliest span starts within WINDOW_START_EDGE_TOLERANCE_MS of + // `dateRange[0]`. That includes a non-empty parent id that was not + // loaded when the child sits on that edge. + // A missing parent further inside the window is a gap in the export (the + // root was never collected), and an empty ParentSpanId is a real root. + // Neither marks the start. `minStart <= dateRange[0]` is not used: the + // queries already filter `Timestamp >= dateRange[0]`, so it never sees a + // span that began earlier, and a span that did begin earlier was loaded. const traceExtent = useMemo(() => { if (traceRowsData.length === 0) return null; - const spanIds = new Set(); - for (const row of traceRowsData) { - if (typeof row.SpanId === 'string' && row.SpanId.length > 0) { - spanIds.add(row.SpanId); - } - } + const windowStartMs = dateRange[0].getTime(); const windowEndMs = dateRange[1].getTime(); let minStartMs = Number.POSITIVE_INFINITY; let maxEndMs = Number.NEGATIVE_INFINITY; - let earliestParentMissing = false; + let startClipped = false; let extendsPastWindowEnd = false; let sawValid = false; for (const row of traceRowsData) { @@ -829,12 +833,10 @@ export function DBTraceWaterfallChartContainer({ const durationSec = Number(row.Duration); const endMs = startMs + (Number.isFinite(durationSec) ? durationSec : 0) * 1000; - const parentId = - typeof row.ParentSpanId === 'string' ? row.ParentSpanId : ''; - const parentMissing = parentId.length > 0 && !spanIds.has(parentId); if (startMs < minStartMs) { minStartMs = startMs; - earliestParentMissing = parentMissing; + startClipped = + Math.abs(startMs - windowStartMs) <= WINDOW_START_EDGE_TOLERANCE_MS; } if (endMs > maxEndMs) maxEndMs = endMs; if (endMs >= windowEndMs) extendsPastWindowEnd = true; @@ -843,7 +845,7 @@ export function DBTraceWaterfallChartContainer({ if (!sawValid) return null; return { totalDurationMs: maxEndMs - minStartMs, - windowClipped: earliestParentMissing || extendsPastWindowEnd, + windowClipped: startClipped || extendsPastWindowEnd, }; }, [traceRowsData, dateRange]); diff --git a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx index 8652919fe9..f6be4b0849 100644 --- a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx @@ -498,42 +498,83 @@ describe('DBTraceWaterfallChartContainer', () => { ); }); - it("marks the duration as a lower bound when the earliest span's parent was not loaded", async () => { - const orphan = { + it('does not mark a missing parent as clipped when the child starts inside the window', async () => { + const orphanInsideWindow = { data: [ { ...mockTraceData.data[0], - // Inside the window. The parent id is not among the fetched spans, - // so the trace may have started before the loaded range. A span that - // merely starts on dateRange[0] is not enough: the queries already - // exclude anything earlier than that edge. + // Well inside 05:00:00-08:00:00. The parent was never exported; + // that is a gap in the trace, not a fetch cut off at the window. Timestamp: '2024-01-01T06:00:00.000000000Z', ParentSpanId: 'missing-root', }, ], meta: [{ totalCount: 1 }], }; - setupQueryMocks({ traceData: orphan }); + setupQueryMocks({ traceData: orphanInsideWindow }); renderComponent(null); await waitForLoading(); expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( - '· Total duration: 100ms+', + '· Total duration: 100ms', ); }); - it("does not mark a span that starts on the window's lower edge as clipped when its parent was loaded", async () => { - const onLowerEdge = { + it('marks the duration as a lower bound when a missing parent starts at the window start', async () => { + const orphanAtWindowStart = { data: [ { ...mockTraceData.data[0], Timestamp: '2024-01-01T05:00:00.000000000Z', + ParentSpanId: 'missing-root', + }, + ], + meta: [{ totalCount: 1 }], + }; + setupQueryMocks({ traceData: orphanAtWindowStart }); + renderComponent(null); + await waitForLoading(); + + expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( + '· Total duration: 100ms+', + ); + }); + + it('marks the duration as a lower bound when the earliest span starts within a small tolerance of the window start', async () => { + const justInsideEdge = { + data: [ + { + ...mockTraceData.data[0], + // 0.5ms after dateRange[0]. The queries filter Timestamp >= that + // edge, so an exact `minStart <= dateRange[0]` check does not see + // this span as clipped. A root (empty parent) on the edge still is. + Timestamp: '2024-01-01T05:00:00.000500000Z', + ParentSpanId: '', + }, + ], + meta: [{ totalCount: 1 }], + }; + setupQueryMocks({ traceData: justInsideEdge }); + renderComponent(null); + await waitForLoading(); + + expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( + '· Total duration: 100ms+', + ); + }); + + it('does not mark a span that starts before the window when that span was loaded', async () => { + const startedBeforeWindow = { + data: [ + { + ...mockTraceData.data[0], + Timestamp: '2024-01-01T04:59:00.000000000Z', ParentSpanId: '', }, ], meta: [{ totalCount: 1 }], }; - setupQueryMocks({ traceData: onLowerEdge }); + setupQueryMocks({ traceData: startedBeforeWindow }); renderComponent(null); await waitForLoading(); @@ -563,8 +604,8 @@ describe('DBTraceWaterfallChartContainer', () => { ); }); - it('does not mark the duration as a lower bound when every span sits comfortably inside the fetch window', async () => { - setupQueryMocks({ traceData: mockTraceData }); // 06:00:00, well inside 05:00:00-08:00:00 + it('does not mark a root with no parent when it starts inside the window', async () => { + setupQueryMocks({ traceData: mockTraceData }); // 06:00:00, ParentSpanId '', inside 05:00:00-08:00:00 renderComponent(null); await waitForLoading(); From 9f4f58f6f5ac6567550e8974a4492cbebf6a2e41 Mon Sep 17 00:00:00 2001 From: milansanjeev <12941259+milansanjeev@users.noreply.github.com> Date: Fri, 2 Oct 2026 22:29:01 +0530 Subject: [PATCH 09/11] fix(app): treat a missing parent as a clipped trace start The 1ms window-edge tolerance missed a root that started before the window and flagged a span that only started on the edge. A parent id on the earliest loaded span that was not fetched is the start clip. --- .../src/components/DBTraceWaterfallChart.tsx | 71 +++++------ .../__tests__/DBTraceWaterfallChart.test.tsx | 111 ++++++++++++------ 2 files changed, 111 insertions(+), 71 deletions(-) diff --git a/packages/app/src/components/DBTraceWaterfallChart.tsx b/packages/app/src/components/DBTraceWaterfallChart.tsx index c1daa2264d..657480c3e2 100644 --- a/packages/app/src/components/DBTraceWaterfallChart.tsx +++ b/packages/app/src/components/DBTraceWaterfallChart.tsx @@ -153,11 +153,6 @@ export const TRACE_WATERFALL_ROW_LIMIT = 50000; // Fetch one extra row so a window that contains *exactly* the cap is not // mistaken for truncation. A full page of LIMIT is ambiguous; LIMIT+1 is not. const TRACE_WATERFALL_FETCH_LIMIT = TRACE_WATERFALL_ROW_LIMIT + 1; -// The span queries keep `Timestamp >= dateRange[0]`, so a start strictly -// before the window is not returned and `minStart <= dateRange[0]` only -// matches an exact hit. A start this close to the lower edge is the signal -// that the fetch cut off earlier spans. -const WINDOW_START_EDGE_TOLERANCE_MS = 1; // Stable empty fallback. `data ?? []` would allocate a new array every render // and invalidate every memo downstream of the waterfall rows. @@ -174,8 +169,8 @@ export function TraceTotalDurationStat({ @@ -795,34 +790,39 @@ export function DBTraceWaterfallChartContainer({ // the visible/collapsed subset. Waterfall search filters only flag rows via // `__hdx_hidden`, so they do not change this figure (#3038). // - // `useEventsData` loads a single page on each side of the focus time and does - // not call `fetchNextPage`. Spans outside that page, or outside the requested - // date range, are not included. + // `useEventsData` loads one page on each side of the focus time and does not + // call `fetchNextPage`. // - // MCP `trace_waterfall` reports the longest single span instead. Changing - // that tool is a separate behavior change; this number is the timeline envelope. - // - // `windowClipped` is a lower bound for the fetch window, separate from the - // row-cap flag so a cap change does not rescan every span: - // - a span ending at or after `dateRange[1]` may continue past the window - // - the earliest span starts within WINDOW_START_EDGE_TOLERANCE_MS of - // `dateRange[0]`. That includes a non-empty parent id that was not - // loaded when the child sits on that edge. - // A missing parent further inside the window is a gap in the export (the - // root was never collected), and an empty ParentSpanId is a real root. - // Neither marks the start. `minStart <= dateRange[0]` is not used: the - // queries already filter `Timestamp >= dateRange[0]`, so it never sees a - // span that began earlier, and a span that did begin earlier was loaded. + // The fetch is Timestamp >= the window start and Timestamp <= the window + // end, so a span that starts entirely outside the window is never returned. + // `isWindowClipped` is set only from rows that were loaded: + // - a span ends at or after the window end + // - the earliest span has a ParentSpanId that is not in the loaded set + // (its parent started before the window and was filtered out) + // An empty ParentSpanId is a root and does not set the flag. const traceExtent = useMemo(() => { if (traceRowsData.length === 0) return null; - const windowStartMs = dateRange[0].getTime(); + const spanIds = new Set(); + for (const row of traceRowsData) { + if (typeof row.SpanId === 'string' && row.SpanId.length > 0) { + spanIds.add(row.SpanId); + } + } const windowEndMs = dateRange[1].getTime(); let minStartMs = Number.POSITIVE_INFINITY; let maxEndMs = Number.NEGATIVE_INFINITY; - let startClipped = false; - let extendsPastWindowEnd = false; - let sawValid = false; + let isStartClipped = false; + let isPastWindowEnd = false; + let isAnySpanValid = false; for (const row of traceRowsData) { + // timestamp-nano accepts a non-date as a finite instant, which would + // stretch the envelope. Skip those before parsing. + if ( + typeof row.Timestamp !== 'string' || + Number.isNaN(Date.parse(row.Timestamp)) + ) { + continue; + } let startMs: number; try { startMs = parseTimestampToMs(row.Timestamp); @@ -835,17 +835,18 @@ export function DBTraceWaterfallChartContainer({ startMs + (Number.isFinite(durationSec) ? durationSec : 0) * 1000; if (startMs < minStartMs) { minStartMs = startMs; - startClipped = - Math.abs(startMs - windowStartMs) <= WINDOW_START_EDGE_TOLERANCE_MS; + const parentId = + typeof row.ParentSpanId === 'string' ? row.ParentSpanId : ''; + isStartClipped = parentId.length > 0 && !spanIds.has(parentId); } if (endMs > maxEndMs) maxEndMs = endMs; - if (endMs >= windowEndMs) extendsPastWindowEnd = true; - sawValid = true; + if (endMs >= windowEndMs) isPastWindowEnd = true; + isAnySpanValid = true; } - if (!sawValid) return null; + if (!isAnySpanValid) return null; return { totalDurationMs: maxEndMs - minStartMs, - windowClipped: startClipped || extendsPastWindowEnd, + isWindowClipped: isStartClipped || isPastWindowEnd, }; }, [traceRowsData, dateRange]); @@ -1539,7 +1540,7 @@ export function DBTraceWaterfallChartContainer({ {traceExtent != null && ( )} diff --git a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx index f6be4b0849..f59cdce618 100644 --- a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx @@ -498,83 +498,77 @@ describe('DBTraceWaterfallChartContainer', () => { ); }); - it('does not mark a missing parent as clipped when the child starts inside the window', async () => { - const orphanInsideWindow = { + it('marks the duration when the earliest span has a parent that was not loaded', async () => { + const childInsideWindow = { data: [ { ...mockTraceData.data[0], - // Well inside 05:00:00-08:00:00. The parent was never exported; - // that is a gap in the trace, not a fetch cut off at the window. + // Seconds inside 05:00:00-08:00:00. The parent started before the + // window, so the query never returned it. Timestamp: '2024-01-01T06:00:00.000000000Z', + SpanId: 'span-child', ParentSpanId: 'missing-root', }, - ], - meta: [{ totalCount: 1 }], - }; - setupQueryMocks({ traceData: orphanInsideWindow }); - renderComponent(null); - await waitForLoading(); - - expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( - '· Total duration: 100ms', - ); - }); - - it('marks the duration as a lower bound when a missing parent starts at the window start', async () => { - const orphanAtWindowStart = { - data: [ { ...mockTraceData.data[0], - Timestamp: '2024-01-01T05:00:00.000000000Z', - ParentSpanId: 'missing-root', + Body: 'later span', + Timestamp: '2024-01-01T06:00:01.000000000Z', + Duration: 0.1, + SpanId: 'span-later', + ParentSpanId: 'span-child', }, ], - meta: [{ totalCount: 1 }], + meta: [{ totalCount: 2 }], }; - setupQueryMocks({ traceData: orphanAtWindowStart }); + setupQueryMocks({ traceData: childInsideWindow }); renderComponent(null); await waitForLoading(); expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( - '· Total duration: 100ms+', + '· Total duration: 1.1s+', ); }); - it('marks the duration as a lower bound when the earliest span starts within a small tolerance of the window start', async () => { - const justInsideEdge = { + it('does not mark a root span that starts within 1ms of the window edge', async () => { + const rootNearEdge = { data: [ { ...mockTraceData.data[0], - // 0.5ms after dateRange[0]. The queries filter Timestamp >= that - // edge, so an exact `minStart <= dateRange[0]` check does not see - // this span as clipped. A root (empty parent) on the edge still is. Timestamp: '2024-01-01T05:00:00.000500000Z', ParentSpanId: '', }, ], meta: [{ totalCount: 1 }], }; - setupQueryMocks({ traceData: justInsideEdge }); + setupQueryMocks({ traceData: rootNearEdge }); renderComponent(null); await waitForLoading(); expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( - '· Total duration: 100ms+', + '· Total duration: 100ms', ); }); - it('does not mark a span that starts before the window when that span was loaded', async () => { - const startedBeforeWindow = { + it('does not mark a span that starts within 1ms of the window edge when its parent is loaded', async () => { + const childNearEdge = { data: [ { ...mockTraceData.data[0], - Timestamp: '2024-01-01T04:59:00.000000000Z', + Timestamp: '2024-01-01T05:00:00.000500000Z', + SpanId: 'span-child', + ParentSpanId: 'span-parent', + }, + { + ...mockTraceData.data[0], + Body: 'parent span', + Timestamp: '2024-01-01T05:00:00.000500000Z', + SpanId: 'span-parent', ParentSpanId: '', }, ], - meta: [{ totalCount: 1 }], + meta: [{ totalCount: 2 }], }; - setupQueryMocks({ traceData: startedBeforeWindow }); + setupQueryMocks({ traceData: childNearEdge }); renderComponent(null); await waitForLoading(); @@ -614,6 +608,51 @@ describe('DBTraceWaterfallChartContainer', () => { ); }); + it('skips an unparseable timestamp when another span is valid', async () => { + const mixed = { + data: [ + { + ...mockTraceData.data[0], + Timestamp: 'not-a-timestamp', + SpanId: 'span-bad', + ParentSpanId: '', + }, + { + ...mockTraceData.data[0], + Timestamp: '2024-01-01T06:00:00.000000000Z', + Duration: 0.1, + SpanId: 'span-good', + ParentSpanId: '', + }, + ], + meta: [{ totalCount: 2 }], + }; + setupQueryMocks({ traceData: mixed }); + renderComponent(null); + await waitForLoading(); + + expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( + '· Total duration: 100ms', + ); + }); + + it('does not render the total duration stat when every timestamp is unparseable', async () => { + const invalid = { + data: [ + { + ...mockTraceData.data[0], + Timestamp: 'not-a-timestamp', + }, + ], + meta: [{ totalCount: 1 }], + }; + setupQueryMocks({ traceData: invalid }); + renderComponent(null); + await waitForLoading(); + + expect(screen.queryByTestId('trace-total-stats')).not.toBeInTheDocument(); + }); + it('keeps the fetched duration when a span is flagged hidden', async () => { const hiddenSpanData = { data: [ From 26c80d2b6a8824bb3ecc5b9143e2681d354f23ad Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Fri, 2 Oct 2026 15:26:21 -0400 Subject: [PATCH 10/11] chore: Update tests and formatting --- .../trace-total-duration-search-table.md | 5 +- .../src/components/DBTraceWaterfallChart.tsx | 60 ++++++++----------- .../__tests__/DBTraceWaterfallChart.test.tsx | 57 +++++++----------- 3 files changed, 48 insertions(+), 74 deletions(-) diff --git a/.changeset/trace-total-duration-search-table.md b/.changeset/trace-total-duration-search-table.md index 5e033a6a8c..d550e91230 100644 --- a/.changeset/trace-total-duration-search-table.md +++ b/.changeset/trace-total-duration-search-table.md @@ -2,7 +2,4 @@ '@hyperdx/app': patch --- -feat: show a trace's total wall-clock duration in the waterfall controls bar -(earliest span start to latest span end). Previously this required eyeballing -the timeline; summing span rows would double-count parallel work. If a fetch -window exceeds the row cap, the figure is marked as a lower bound. +feat: show a trace's total wall-clock duration in the waterfall controls bar \ No newline at end of file diff --git a/packages/app/src/components/DBTraceWaterfallChart.tsx b/packages/app/src/components/DBTraceWaterfallChart.tsx index 657480c3e2..046f305713 100644 --- a/packages/app/src/components/DBTraceWaterfallChart.tsx +++ b/packages/app/src/components/DBTraceWaterfallChart.tsx @@ -148,7 +148,6 @@ const SERVICE_COLORS = COLORS.filter( ); // Per-window (before/after focus date) row cap applied in `getConfig` below. -// Exported for the truncation check in `useEventsAroundFocus`. export const TRACE_WATERFALL_ROW_LIMIT = 50000; // Fetch one extra row so a window that contains *exactly* the cap is not // mistaken for truncation. A full page of LIMIT is ambiguous; LIMIT+1 is not. @@ -169,7 +168,7 @@ export function TraceTotalDurationStat({ = the window start and Timestamp <= the window - // end, so a span that starts entirely outside the window is never returned. - // `isWindowClipped` is set only from rows that were loaded: - // - a span ends at or after the window end - // - the earliest span has a ParentSpanId that is not in the loaded set - // (its parent started before the window and was filtered out) - // An empty ParentSpanId is a root and does not set the flag. + const validSpanIDs = useMemo(() => { + return new Set( + traceRowsData // only spans in traces can define valid span ids + ?.filter(row => _.isString(row.SpanId) && row.SpanId.length > 0) + .map(row => row.SpanId) ?? [], + ); + }, [traceRowsData]); + const traceExtent = useMemo(() => { if (traceRowsData.length === 0) return null; - const spanIds = new Set(); - for (const row of traceRowsData) { - if (typeof row.SpanId === 'string' && row.SpanId.length > 0) { - spanIds.add(row.SpanId); - } - } + const windowEndMs = dateRange[1].getTime(); + let minStartMs = Number.POSITIVE_INFINITY; let maxEndMs = Number.NEGATIVE_INFINITY; + let isStartClipped = false; - let isPastWindowEnd = false; + let isEndClipped = false; + let isAnySpanValid = false; for (const row of traceRowsData) { // timestamp-nano accepts a non-date as a finite instant, which would @@ -823,6 +814,7 @@ export function DBTraceWaterfallChartContainer({ ) { continue; } + let startMs: number; try { startMs = parseTimestampToMs(row.Timestamp); @@ -830,25 +822,32 @@ export function DBTraceWaterfallChartContainer({ continue; } if (!Number.isFinite(startMs)) continue; + const durationSec = Number(row.Duration); const endMs = startMs + (Number.isFinite(durationSec) ? durationSec : 0) * 1000; if (startMs < minStartMs) { minStartMs = startMs; + + // The start is clipped if the earliest span has a parent that was not fetched const parentId = typeof row.ParentSpanId === 'string' ? row.ParentSpanId : ''; - isStartClipped = parentId.length > 0 && !spanIds.has(parentId); + isStartClipped = parentId.length > 0 && !validSpanIDs.has(parentId); } + if (endMs > maxEndMs) maxEndMs = endMs; - if (endMs >= windowEndMs) isPastWindowEnd = true; + if (endMs >= windowEndMs) isEndClipped = true; isAnySpanValid = true; } if (!isAnySpanValid) return null; + return { + /** Wall-clock duration across every fetched span (min start to max end) */ totalDurationMs: maxEndMs - minStartMs, - isWindowClipped: isStartClipped || isPastWindowEnd, + /** Whether the trace start or end is clipped by the time range */ + isWindowClipped: isStartClipped || isEndClipped, }; - }, [traceRowsData, dateRange]); + }, [traceRowsData, validSpanIDs, dateRange]); // Map each distinct span service to a stable color. Sorting the names first // keeps a service's color stable across renders regardless of row ordering. @@ -972,13 +971,6 @@ export function DBTraceWaterfallChartContainer({ children: SpanRow[]; aliasWith: WithClause[]; }; - const validSpanIDs = useMemo(() => { - return new Set( - traceRowsData // only spans in traces can define valid span ids - ?.filter(row => _.isString(row.SpanId) && row.SpanId.length > 0) - .map(row => row.SpanId) ?? [], - ); - }, [traceRowsData]); const [collapsedIds, setCollapsedIds] = useState>(new Set()); const [showSpanEvents, setShowSpanEvents] = useState(true); diff --git a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx index f59cdce618..f813397b3e 100644 --- a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx @@ -529,51 +529,31 @@ describe('DBTraceWaterfallChartContainer', () => { ); }); - it('does not mark a root span that starts within 1ms of the window edge', async () => { - const rootNearEdge = { + it('does not mark the duration when the earliest span has a parent that was loaded', async () => { + const skewedChildFirst = { data: [ { ...mockTraceData.data[0], - Timestamp: '2024-01-01T05:00:00.000500000Z', - ParentSpanId: '', - }, - ], - meta: [{ totalCount: 1 }], - }; - setupQueryMocks({ traceData: rootNearEdge }); - renderComponent(null); - await waitForLoading(); - - expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( - '· Total duration: 100ms', - ); - }); - - it('does not mark a span that starts within 1ms of the window edge when its parent is loaded', async () => { - const childNearEdge = { - data: [ - { - ...mockTraceData.data[0], - Timestamp: '2024-01-01T05:00:00.000500000Z', + Timestamp: '2024-01-01T06:00:00.000000000Z', SpanId: 'span-child', ParentSpanId: 'span-parent', }, { ...mockTraceData.data[0], Body: 'parent span', - Timestamp: '2024-01-01T05:00:00.000500000Z', + Timestamp: '2024-01-01T06:00:00.050000000Z', SpanId: 'span-parent', ParentSpanId: '', }, ], meta: [{ totalCount: 2 }], }; - setupQueryMocks({ traceData: childNearEdge }); + setupQueryMocks({ traceData: skewedChildFirst }); renderComponent(null); await waitForLoading(); expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( - '· Total duration: 100ms', + '· Total duration: 150ms', ); }); @@ -598,16 +578,6 @@ describe('DBTraceWaterfallChartContainer', () => { ); }); - it('does not mark a root with no parent when it starts inside the window', async () => { - setupQueryMocks({ traceData: mockTraceData }); // 06:00:00, ParentSpanId '', inside 05:00:00-08:00:00 - renderComponent(null); - await waitForLoading(); - - expect(screen.getByTestId('trace-total-stats').textContent?.trim()).toBe( - '· Total duration: 100ms', - ); - }); - it('skips an unparseable timestamp when another span is valid', async () => { const mixed = { data: [ @@ -1098,6 +1068,21 @@ describe('useEventsAroundFocus', () => { expect(result.rows.length).toBe(TRACE_WATERFALL_ROW_LIMIT); }); + it('flags isTruncated when only the after window returns more rows than the cap', () => { + const overCapData = { + data: new Array(TRACE_WATERFALL_ROW_LIMIT + 1).fill(null), + meta: [{ totalCount: TRACE_WATERFALL_ROW_LIMIT + 1 }], + }; + + const result = testEventsAroundFocus({ + beforeData: { data: [{ Body: 'a' }], meta: [{ totalCount: 1 }] }, + afterData: overCapData, + }); + + expect(result.isTruncated).toBe(true); + expect(result.rows.length).toBe(TRACE_WATERFALL_ROW_LIMIT + 1); + }); + it('does not flag isTruncated when both windows return fewer rows than the cap', () => { const result = testEventsAroundFocus({ beforeData: { data: [{ Body: 'a' }], meta: [{ totalCount: 1 }] }, From 6f0604eb3307b9751efce8ee6aa98904d3836ca6 Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Sat, 3 Oct 2026 14:16:23 -0400 Subject: [PATCH 11/11] test: Fix slow test --- .../src/components/DBTraceWaterfallChart.tsx | 10 ++++------ .../__tests__/DBTraceWaterfallChart.test.tsx | 18 ++++++++++-------- .../app/src/components/traceWaterfallLimits.ts | 7 +++++++ 3 files changed, 21 insertions(+), 14 deletions(-) create mode 100644 packages/app/src/components/traceWaterfallLimits.ts diff --git a/packages/app/src/components/DBTraceWaterfallChart.tsx b/packages/app/src/components/DBTraceWaterfallChart.tsx index 046f305713..35e116764f 100644 --- a/packages/app/src/components/DBTraceWaterfallChart.tsx +++ b/packages/app/src/components/DBTraceWaterfallChart.tsx @@ -53,6 +53,10 @@ import { TimelineMinimap, type TimelineViewportController, } from '@/components/TimelineChart'; +import { + TRACE_WATERFALL_FETCH_LIMIT, + TRACE_WATERFALL_ROW_LIMIT, +} from '@/components/traceWaterfallLimits'; import useOffsetPaginatedQuery from '@/hooks/useOffsetPaginatedQuery'; import useRowWhere, { WithClause } from '@/hooks/useRowWhere'; import useWaterfallSearchState from '@/hooks/useWaterfallSearchState'; @@ -147,12 +151,6 @@ const SERVICE_COLORS = COLORS.filter( CATEGORICAL_PALETTE_TOKENS[i] !== 'chart-red', ); -// Per-window (before/after focus date) row cap applied in `getConfig` below. -export const TRACE_WATERFALL_ROW_LIMIT = 50000; -// Fetch one extra row so a window that contains *exactly* the cap is not -// mistaken for truncation. A full page of LIMIT is ambiguous; LIMIT+1 is not. -const TRACE_WATERFALL_FETCH_LIMIT = TRACE_WATERFALL_ROW_LIMIT + 1; - // Stable empty fallback. `data ?? []` would allocate a new array every render // and invalidate every memo downstream of the waterfall rows. const EMPTY_ROWS: Record[] = []; diff --git a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx index f813397b3e..ee7e81ae14 100644 --- a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx @@ -17,15 +17,19 @@ import { DBTraceWaterfallChartContainer, getDescendantIds, SpanRow, - TRACE_WATERFALL_ROW_LIMIT, TraceTotalDurationStat, useEventsAroundFocus, } from '@/components/DBTraceWaterfallChart'; import { TimelineChart } from '@/components/TimelineChart'; +import { TRACE_WATERFALL_ROW_LIMIT } from '@/components/traceWaterfallLimits'; import useOffsetPaginatedQuery from '@/hooks/useOffsetPaginatedQuery'; import useRowWhere from '@/hooks/useRowWhere'; // Mock setup +jest.mock('@/components/traceWaterfallLimits', () => ({ + TRACE_WATERFALL_ROW_LIMIT: 3, + TRACE_WATERFALL_FETCH_LIMIT: 4, +})); jest.mock('@/components/TimelineChart', () => { const flattenText = (value: React.ReactNode): string => { if (value == null || typeof value === 'boolean') { @@ -52,13 +56,11 @@ jest.mock('@/components/TimelineChart', () => { return (
TimelineChart - {props.rows - ?.slice(0, 20) - .map((row: any) => ( -
- {row.events?.map((event: any) => flattenText(event.body))} -
- ))} + {props.rows?.map((row: any) => ( +
+ {row.events?.map((event: any) => flattenText(event.body))} +
+ ))}
); }; diff --git a/packages/app/src/components/traceWaterfallLimits.ts b/packages/app/src/components/traceWaterfallLimits.ts new file mode 100644 index 0000000000..40a9496dfd --- /dev/null +++ b/packages/app/src/components/traceWaterfallLimits.ts @@ -0,0 +1,7 @@ +// Per-window (before/after focus date) row cap for the trace waterfall query. +// Kept in its own module so tests can mock it down to a few rows instead of +// rendering 50k spans. +export const TRACE_WATERFALL_ROW_LIMIT = 50000; +// Fetch one extra row so a window that contains *exactly* the cap is not +// mistaken for truncation. A full page of LIMIT is ambiguous; LIMIT+1 is not. +export const TRACE_WATERFALL_FETCH_LIMIT = TRACE_WATERFALL_ROW_LIMIT + 1;