From 183ca7186cd3e208d0cb5d8f8f704a3e690b1ff4 Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Wed, 5 Aug 2026 08:38:42 -0400 Subject: [PATCH 1/3] fix: Bound the side panel's row lookup after 'View Trace' to a time window --- .../view-trace-row-lookup-time-filter.md | 6 + .../app/src/components/DBRowDataPanel.tsx | 26 +- .../app/src/components/DBRowOverviewPanel.tsx | 4 +- .../app/src/components/DBRowSidePanel.tsx | 43 ++ .../src/components/DBRowSidePanel.types.ts | 9 + .../__tests__/DBRowDataPanel.test.ts | 162 +++++++ ...BRowSidePanel.viewTraceTimeFilter.test.tsx | 416 ++++++++++++++++++ .../src/utils/__tests__/rowTimestamps.test.ts | 216 +++++++++ packages/app/src/utils/rowTimestamps.ts | 93 ++++ packages/common-utils/src/core/utils.ts | 13 +- 10 files changed, 983 insertions(+), 5 deletions(-) create mode 100644 .changeset/view-trace-row-lookup-time-filter.md create mode 100644 packages/app/src/components/__tests__/DBRowSidePanel.viewTraceTimeFilter.test.tsx create mode 100644 packages/app/src/utils/__tests__/rowTimestamps.test.ts create mode 100644 packages/app/src/utils/rowTimestamps.ts diff --git a/.changeset/view-trace-row-lookup-time-filter.md b/.changeset/view-trace-row-lookup-time-filter.md new file mode 100644 index 0000000000..d3833091f8 --- /dev/null +++ b/.changeset/view-trace-row-lookup-time-filter.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/app': patch +'@hyperdx/common-utils': patch +--- + +fix: Bound the side panel's row lookup after "View Trace" to a time window diff --git a/packages/app/src/components/DBRowDataPanel.tsx b/packages/app/src/components/DBRowDataPanel.tsx index 7a6f6f0a77..c448746c32 100644 --- a/packages/app/src/components/DBRowDataPanel.tsx +++ b/packages/app/src/components/DBRowDataPanel.tsx @@ -17,10 +17,13 @@ import { getEventBody, } from '@/source'; import { getSelectExpressionsForHighlightedAttributes } from '@/utils/highlightedAttributes'; +import { getTimestampValueSelects } from '@/utils/rowTimestamps'; import { DBRowJsonViewer } from './DBRowJsonViewer'; import { getActiveInfraCorrelations } from './infraCorrelations'; +// The source's own `timestampValueExpression` columns are projected too, under +// the `__hdx_timestamp_value_` aliases owned by `@/utils/rowTimestamps`. export enum ROW_DATA_ALIASES { TIMESTAMP = '__hdx_timestamp', BODY = '__hdx_body', @@ -41,13 +44,26 @@ export function useRowData({ source, rowId, aliasWith, + dateRange, }: { source: TSource; rowId: string | undefined | null; aliasWith?: WithClause[]; + /** + * Optional window to bound the lookup by. Applied against the source's + * `timestampValueExpression` (not the displayed timestamp), so a row id that + * carries no timestamp of its own — e.g. the `TraceId`/`SpanId` pair "View + * Trace" synthesizes — can still prune parts instead of scanning the table. + * Callers must memoize the tuple; it participates in the query key. + */ + dateRange?: [Date, Date]; }) { const eventBodyExpr = getEventBody(source); + const timestampValueExpr = source.timestampValueExpression?.trim() + ? source.timestampValueExpression + : undefined; + const searchedTraceIdExpr = isLogSource(source) || isTraceSource(source) ? source.traceIdExpression @@ -90,6 +106,7 @@ export function useRowData({ valueExpression: getDisplayedTimestampValueExpression(source), alias: ROW_DATA_ALIASES.TIMESTAMP, }, + ...getTimestampValueSelects(timestampValueExpr), ...(eventBodyExpr ? [ { @@ -191,9 +208,12 @@ export function useRowData({ from: source.from, limit: { limit: 1 }, ...(aliasWith && aliasWith.length > 0 ? { with: aliasWith } : {}), + ...(dateRange && timestampValueExpr + ? { dateRange, timestampValueExpression: timestampValueExpr } + : {}), }, { - queryKey: ['row_side_panel', rowId, aliasWith, source], + queryKey: ['row_side_panel', rowId, aliasWith, source, dateRange], enabled: rowId != null, }, ); @@ -288,18 +308,20 @@ export function RowDataPanel({ source, rowId, aliasWith, + dateRange, flush = false, 'data-testid': dataTestId, }: { source: TSource; rowId: string | undefined | null; aliasWith?: WithClause[]; + dateRange?: [Date, Date]; // When true, drop the horizontal margin so content aligns flush with // surrounding chrome (e.g. the tab bar in the trace span detail panel). flush?: boolean; 'data-testid'?: string; }) { - const { data } = useRowData({ source, rowId, aliasWith }); + const { data } = useRowData({ source, rowId, aliasWith, dateRange }); const firstRow = useMemo(() => { const firstRow = { ...(data?.data?.[0] ?? {}) }; diff --git a/packages/app/src/components/DBRowOverviewPanel.tsx b/packages/app/src/components/DBRowOverviewPanel.tsx index fc6d9c548b..71efd24550 100644 --- a/packages/app/src/components/DBRowOverviewPanel.tsx +++ b/packages/app/src/components/DBRowOverviewPanel.tsx @@ -27,6 +27,7 @@ export function RowOverviewPanel({ source, rowId, aliasWith, + dateRange, hideHeader = false, flush = false, 'data-testid': dataTestId, @@ -34,6 +35,7 @@ export function RowOverviewPanel({ source: TSource; rowId: string | undefined | null; aliasWith?: WithClause[]; + dateRange?: [Date, Date]; hideHeader?: boolean; // When true, drop the horizontal padding so content aligns flush with // surrounding chrome (e.g. the tab bar in the trace span detail panel). @@ -41,7 +43,7 @@ export function RowOverviewPanel({ 'data-testid'?: string; }) { const contentPx = flush ? 0 : 'md'; - const { data } = useRowData({ source, rowId, aliasWith }); + const { data } = useRowData({ source, rowId, aliasWith, dateRange }); const { onPropertyAddClick, generateSearchUrl, onOpenLinkedTrace } = useContext(RowSidePanelContext); diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index 4d91dae2d5..fc163d924d 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -51,6 +51,7 @@ import { SearchConfig } from '@/types'; import { FormatTime } from '@/useFormatTime'; import { formatDistanceToNowStrictShort } from '@/utils'; import { getHighlightedAttributesFromData } from '@/utils/highlightedAttributes'; +import { resolveRowTimestampAnchor } from '@/utils/rowTimestamps'; import { useZIndex, ZIndexContext } from '@/zIndex'; import ServiceMapSidePanel from './ServiceMap/ServiceMapSidePanel'; @@ -278,6 +279,28 @@ export const DBRowSidePanelInner = ({ const activeRowId = skipRowQuery ? undefined : resolvedRowId; const activeAliasWith = skipRowQuery ? undefined : resolvedAliasWith; + // A cross-source frame's row id is synthesized from ids alone ("View Trace" + // builds `TraceId = … AND SpanId = …`), so on its own the lookup has no + // timestamp predicate and scans every part. When the pushing panel stamped + // the origin row's timestamp onto the frame, bound the lookup to the same + // ±1h window the trace waterfall and service map already use. + const frameFocusTimestamp = activeSourceFrame?.focusTimestamp; + const frameDateRange = useMemo<[Date, Date] | undefined>(() => { + // Nav entries carry a full row id (timestamp included), so their lookups + // are already bounded and must not be narrowed by the frame's window — + // surrounding context can walk arbitrarily far from the frame's anchor. + if (leafNav != null || frameFocusTimestamp == null) { + return undefined; + } + const focus = new Date(frameFocusTimestamp); + if (isNaN(focus.getTime())) { + return undefined; + } + return [add(focus, { minutes: -60 }), add(focus, { minutes: 60 })]; + }, [leafNav, frameFocusTimestamp]); + + const activeDateRange = skipRowQuery ? undefined : frameDateRange; + const { data: rowData, isLoading: isRowLoading, @@ -288,6 +311,7 @@ export const DBRowSidePanelInner = ({ source, rowId: activeRowId, aliasWith: activeAliasWith, + dateRange: activeDateRange, }); const hasActiveStacks = activeSourceFrame != null || leafNav != null; @@ -508,6 +532,22 @@ export const DBRowSidePanelInner = ({ ? traceSourceData.spanIdExpression : undefined; + // The current row's own timestamp, read from the source's + // `timestampValueExpression` rather than the displayed expression (which + // may not be in the ordering key). Undefined when the source exposes no + // better-than-day precision, so a composite `"EventDate, EventTime"` can't + // anchor a window on `EventDate`'s midnight. + const rowMeta = rowData?.meta; + const rowFocusTimestamp = useMemo( + () => + resolveRowTimestampAnchor({ + timestampValueExpression: source.timestampValueExpression, + row: normalizedRow, + meta: rowMeta, + })?.toISOString(), + [source.timestampValueExpression, normalizedRow, rowMeta], + ); + const traceSpanRowId = useMemo(() => { const clauses: string[] = []; if (traceIdExpression && traceId) { @@ -877,6 +917,7 @@ export const DBRowSidePanelInner = ({ label: mainContent || 'Log', sourceKind: traceSourceData.kind as SourceKind, aliasWith: [], + focusTimestamp: rowFocusTimestamp, }); } }} @@ -970,6 +1011,7 @@ export const DBRowSidePanelInner = ({ source={source} rowId={activeRowId} aliasWith={activeAliasWith} + dateRange={activeDateRange} hideHeader={true} /> @@ -1034,6 +1076,7 @@ export const DBRowSidePanelInner = ({ source={source} rowId={activeRowId} aliasWith={activeAliasWith} + dateRange={activeDateRange} /> )} diff --git a/packages/app/src/components/DBRowSidePanel.types.ts b/packages/app/src/components/DBRowSidePanel.types.ts index f36b22b5c1..0d7723559a 100644 --- a/packages/app/src/components/DBRowSidePanel.types.ts +++ b/packages/app/src/components/DBRowSidePanel.types.ts @@ -29,6 +29,15 @@ const SourceFrameSchema = z.object({ label: z.string(), sourceKind: z.nativeEnum(SourceKind).optional(), originTab: z.nativeEnum(Tab).optional(), + /** + * ISO timestamp of the row the push originated from, when the pushing panel + * knows the destination row sits at roughly the same point in time (e.g. + * "View Trace" jumps to the span the current log belongs to). Lets the + * destination bound its row lookup to a window. Optional — frames pushed + * without a trustworthy anchor (eg. span links, which may point at a distant + * trace) leave it unset. + */ + focusTimestamp: z.string().optional(), }); export type SourceFrame = z.infer; diff --git a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts index 6e7d002179..c6025a23e6 100644 --- a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts +++ b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts @@ -77,6 +77,168 @@ describe('DBRowDataPanel', () => { expect(config.select).not.toContainEqual({ valueExpression: '*' }); }); + describe('time filtering', () => { + // A row id synthesized from ids alone (e.g. "View Trace" builds + // TraceId + SpanId) has no timestamp of its own, so without a dateRange the + // lookup scans every part. + it('omits the time filter when no dateRange is given', () => { + renderHook(() => useRowData({ source, rowId: "id='abc123'" })); + + const [config] = mockUseQueriedChartConfig.mock.calls[0]; + expect(config.dateRange).toBeUndefined(); + expect(config.timestampValueExpression).toBeUndefined(); + }); + + it('filters on timestampValueExpression when a dateRange is given', () => { + const dateRange: [Date, Date] = [ + new Date('2024-01-01T00:00:00Z'), + new Date('2024-01-01T02:00:00Z'), + ]; + + renderHook(() => useRowData({ source, rowId: "id='abc123'", dateRange })); + + const [config, options] = mockUseQueriedChartConfig.mock.calls[0]; + expect(config.dateRange).toBe(dateRange); + expect(config.timestampValueExpression).toBe('Timestamp'); + // Filtered and unfiltered lookups for the same row must not share a + // cache entry. + expect(options.queryKey).toContain(dateRange); + }); + + // The filter has to cover every timestamp column in the sort key, so the + // multi-column expression is passed through whole rather than truncated to + // its first token. + it('passes a multi-column timestampValueExpression through to the filter', () => { + const multiColumnSource: TLogSource = { + ...source, + timestampValueExpression: 'EventDate, EventTime', + }; + + renderHook(() => + useRowData({ + source: multiColumnSource, + rowId: "id='abc123'", + dateRange: [ + new Date('2024-01-01T00:00:00Z'), + new Date('2024-01-01T02:00:00Z'), + ], + }), + ); + + const [config] = mockUseQueriedChartConfig.mock.calls[0]; + expect(config.timestampValueExpression).toBe('EventDate, EventTime'); + }); + + // renderChartConfig needs both halves to emit a filter, so a source with no + // usable timestamp expression must not contribute a lone dateRange. + it('omits the filter when the source has no timestamp expression', () => { + const sourceWithoutTimestamp: TLogSource = { + ...source, + timestampValueExpression: ' ', + }; + + renderHook(() => + useRowData({ + source: sourceWithoutTimestamp, + rowId: "id='abc123'", + dateRange: [ + new Date('2024-01-01T00:00:00Z'), + new Date('2024-01-01T02:00:00Z'), + ], + }), + ); + + const [config] = mockUseQueriedChartConfig.mock.calls[0]; + expect(config.dateRange).toBeUndefined(); + expect(config.timestampValueExpression).toBeUndefined(); + }); + + it('gives bounded and unbounded lookups of the same row different query keys', () => { + renderHook(() => useRowData({ source, rowId: "id='abc123'" })); + renderHook(() => + useRowData({ + source, + rowId: "id='abc123'", + dateRange: [ + new Date('2024-01-01T00:00:00Z'), + new Date('2024-01-01T02:00:00Z'), + ], + }), + ); + + const [, unboundedOptions] = mockUseQueriedChartConfig.mock.calls[0]; + const [, boundedOptions] = mockUseQueriedChartConfig.mock.calls[1]; + expect(boundedOptions.queryKey).not.toEqual(unboundedOptions.queryKey); + }); + }); + + describe('__hdx_timestamp_value_', () => { + function timestampValueSelects(config: { + select: { alias?: string }[]; + }): { alias?: string }[] { + return config.select.filter(s => + s.alias?.startsWith('__hdx_timestamp_value_'), + ); + } + + it("selects the source's timestampValueExpression, not the displayed one", () => { + const sourceWithDisplayedTimestamp: TLogSource = { + ...source, + displayedTimestampValueExpression: 'ObservedTimestamp', + }; + + renderHook(() => + useRowData({ + source: sourceWithDisplayedTimestamp, + rowId: "id='abc123'", + }), + ); + + const [config] = mockUseQueriedChartConfig.mock.calls[0]; + expect(config.select).toContainEqual({ + valueExpression: 'ObservedTimestamp', + alias: '__hdx_timestamp', + }); + expect(timestampValueSelects(config)).toEqual([ + { valueExpression: 'Timestamp', alias: '__hdx_timestamp_value_0' }, + ]); + }); + + // Every column is projected so the anchor can be resolved from the + // highest-precision one at read time; picking the first token here would + // pin the anchor to `EventDate`'s midnight. + it('selects every column of a multi-column timestamp expression', () => { + const multiColumnSource: TLogSource = { + ...source, + timestampValueExpression: 'EventDate, EventTime', + }; + + renderHook(() => + useRowData({ source: multiColumnSource, rowId: "id='abc123'" }), + ); + + const [config] = mockUseQueriedChartConfig.mock.calls[0]; + expect(timestampValueSelects(config)).toEqual([ + { valueExpression: 'EventDate', alias: '__hdx_timestamp_value_0' }, + { valueExpression: 'EventTime', alias: '__hdx_timestamp_value_1' }, + ]); + }); + + it('is not selected when the source has no timestamp expression', () => { + const sourceWithoutTimestamp: TLogSource = { + ...source, + timestampValueExpression: ' ', + }; + + renderHook(() => + useRowData({ source: sourceWithoutTimestamp, rowId: "id='abc123'" }), + ); + + const [config] = mockUseQueriedChartConfig.mock.calls[0]; + expect(timestampValueSelects(config)).toEqual([]); + }); + }); + // Regression test for the OSS #2357 conflict-resolution merge. The // composed result wraps `Event Attributes` in a length check from // origin/main AND passes `mapColumns={mapColumns}` through to the diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.viewTraceTimeFilter.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.viewTraceTimeFilter.test.tsx new file mode 100644 index 0000000000..d329be77a5 --- /dev/null +++ b/packages/app/src/components/__tests__/DBRowSidePanel.viewTraceTimeFilter.test.tsx @@ -0,0 +1,416 @@ +import React from 'react'; +import { TSource } from '@hyperdx/common-utils/dist/types'; +import { MantineProvider } from '@mantine/core'; +import { fireEvent, render, screen } from '@testing-library/react'; + +// Controlled, in-memory replacement for nuqs' useQueryState so each side-panel +// URL param can be seeded and its setter inspected independently. Values are +// the already-parsed shapes the component consumes (arrays / strings), not URL +// strings. Prefixed with `mock` so jest.mock's factory may reference them. +const mockQueryStore: Record = {}; +const mockSetters: Record = {}; + +function setterFor(key: string) { + if (!mockSetters[key]) mockSetters[key] = jest.fn(); + return mockSetters[key]; +} +function resetQueryState() { + Object.keys(mockQueryStore).forEach(k => delete mockQueryStore[k]); + Object.keys(mockSetters).forEach(k => delete mockSetters[k]); +} + +jest.mock('nuqs', () => { + const actual = jest.requireActual('nuqs'); + return { + ...actual, + useQueryState: (key: string, parser?: { defaultValue?: unknown }) => { + const hasValue = Object.prototype.hasOwnProperty.call( + mockQueryStore, + key, + ); + const fallback = + parser && 'defaultValue' in parser ? parser.defaultValue : null; + const value = hasValue ? mockQueryStore[key] : (fallback ?? null); + if (!mockSetters[key]) mockSetters[key] = jest.fn(); + return [value, mockSetters[key]]; + }, + }; +}); + +const mockUseRowData = jest.fn(); +const mockRowDataPanel = jest.fn(); +jest.mock('../DBRowDataPanel', () => ({ + __esModule: true, + useRowData: (args: unknown) => mockUseRowData(args), + ROW_DATA_ALIASES: { + TIMESTAMP: '__hdx_timestamp', + DURATION_MS: '__hdx_duration', + SPAN_KIND: '__hdx_span_kind', + SERVICE_NAME: '__hdx_service_name', + SEVERITY_TEXT: '__hdx_severity_text', + }, + rowHasK8sContext: () => false, + RowDataPanel: (props: unknown) => { + mockRowDataPanel(props); + return null; + }, + getJSONColumnNames: () => [], + getMapColumnNames: () => [], +})); + +const mockRowOverviewPanel = jest.fn(); +jest.mock('../DBRowOverviewPanel', () => ({ + __esModule: true, + RowOverviewPanel: (props: unknown) => { + mockRowOverviewPanel(props); + return null; + }, +})); + +const TRACE_SOURCE = { + id: 'trace-src', + kind: 'trace', + traceIdExpression: 'TraceId', + spanIdExpression: 'SpanId', + // Makes hasOverviewPanel true, so the landed frame can render the Overview tab. + resourceAttributesExpression: 'ResourceAttributes', +}; + +// A second log source, used as a cross-source destination whose default tab is +// Overview (a trace destination lands on the Trace tab instead, which renders +// the row detail from inside the waterfall). +const LOG_DEST_SOURCE = { + id: 'log-dest', + kind: 'log', + resourceAttributesExpression: 'ResourceAttributes', +}; + +jest.mock('@/source', () => ({ + __esModule: true, + getEventBody: () => undefined, + useSource: ({ id }: { id: string | null }) => + id === 'trace-src' + ? { data: TRACE_SOURCE } + : id === 'log-dest' + ? { data: LOG_DEST_SOURCE } + : { data: undefined }, +})); + +jest.mock('../DBSessionPanel', () => ({ + __esModule: true, + useSessionId: () => ({ rumSessionId: undefined, rumServiceName: undefined }), + DBSessionPanel: () => null, +})); + +jest.mock('@/utils/highlightedAttributes', () => ({ + __esModule: true, + getHighlightedAttributesFromData: () => [], +})); + +// Heavy leaf components / chart deps the panel imports but never renders for +// this row shape. +jest.mock('../DBTracePanel', () => ({ __esModule: true, default: () => null })); +jest.mock('../ContextSidePanel', () => ({ + __esModule: true, + default: () => null, +})); +jest.mock('../DBInfraPanel', () => ({ __esModule: true, default: () => null })); +jest.mock('../DBRowSidePanelErrorState', () => ({ + __esModule: true, + DBRowSidePanelErrorState: () => null, +})); +jest.mock('../DBRowSidePanelHeader', () => ({ + __esModule: true, + default: () => null, +})); +jest.mock('../SidePanelBreadcrumbs', () => ({ + __esModule: true, + default: () => null, +})); +jest.mock('../LogLevel', () => ({ __esModule: true, default: () => null })); +jest.mock('../ServiceMap/ServiceMapSidePanel', () => ({ + __esModule: true, + default: () => null, +})); +jest.mock('../TimelineChart/utils', () => ({ + __esModule: true, + renderMs: () => '', +})); +jest.mock('../DrawerUtils', () => ({ + __esModule: true, + DrawerFullWidthToggle: () => null, + INITIAL_DRAWER_WIDTH_PERCENT: 50, +})); +jest.mock('@/LogSidePanelElements', () => ({ + __esModule: true, + KeyboardShortcutsModal: () => null, +})); +jest.mock('@/TabBar', () => ({ __esModule: true, default: () => null })); +jest.mock('@/useFormatTime', () => ({ + __esModule: true, + FormatTime: () => null, +})); + +// NOTE: this import is intentionally placed after the mock factories above, +// which close over the `mock*` helpers declared at the top of this file. +import { DBRowSidePanelInner } from '@/components/DBRowSidePanel'; +import useSidePanelStack from '@/hooks/useSidePanelStack'; + +// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion +const ROOT_SOURCE = { + id: 'log-src', + kind: 'log', + traceSourceId: 'trace-src', + timestampValueExpression: 'Timestamp', + resourceAttributesExpression: 'ResourceAttributes', +} as TSource; + +const TRACE_ID = '7316d5a2ab0dc2efa72258f64a98a405'; +const SPAN_ID = 'e3748131832d6176'; +const TRACE_SPAN_ROW_ID = `TraceId='${TRACE_ID}' AND SpanId='${SPAN_ID}'`; + +// The source's `timestampValueExpression` value, deliberately different from the +// displayed timestamp so a test can tell which one the window is anchored to. +const TIMESTAMP_VALUE = '2024-05-01T10:00:00.123456789Z'; +const DISPLAYED_TIMESTAMP = '2024-05-01T22:00:00.000000000Z'; + +function rowResult({ + row, + meta, +}: { + row: Record; + meta: { name: string; type: string }[]; +}) { + return { + data: { data: [row], meta }, + isLoading: false, + isSuccess: true, + isError: false, + error: null, + }; +} + +function InnerHarness({ + rowId, + source = ROOT_SOURCE, +}: { + rowId: string; + source?: TSource; +}) { + const sidePanelStack = useSidePanelStack({ initialRowId: rowId }); + return ( + + ); +} + +function renderInner(rowId: string, source?: TSource) { + return render( + + + , + ); +} + +/** Args of the last useRowData call, i.e. the one the render settled on. */ +function lastRowDataArgs() { + const { calls } = mockUseRowData.mock; + return calls[calls.length - 1][0]; +} + +function pushedFrame() { + return setterFor('sidePanelSourceStack').mock.calls[0][0][0]; +} + +function hourWindow(isoTimestamp: string) { + const ms = new Date(isoTimestamp).getTime(); + return [new Date(ms - 60 * 60 * 1000), new Date(ms + 60 * 60 * 1000)]; +} + +describe('DBRowSidePanelInner, "View Trace" row lookup time filter', () => { + beforeEach(() => { + resetQueryState(); + mockUseRowData.mockReset(); + mockRowOverviewPanel.mockReset(); + mockRowDataPanel.mockReset(); + mockUseRowData.mockReturnValue( + rowResult({ + row: { + __hdx_trace_id: TRACE_ID, + __hdx_span_id: SPAN_ID, + __hdx_timestamp: DISPLAYED_TIMESTAMP, + __hdx_timestamp_value_0: TIMESTAMP_VALUE, + }, + meta: [{ name: '__hdx_timestamp_value_0', type: 'DateTime64(9)' }], + }), + ); + }); + + it("stamps the row's timestampValueExpression timestamp onto the pushed frame", () => { + renderInner('row-1'); + + fireEvent.click(screen.getByTestId('side-panel-view-trace')); + + expect(pushedFrame()).toMatchObject({ + sourceId: 'trace-src', + rowId: TRACE_SPAN_ROW_ID, + focusTimestamp: new Date(TIMESTAMP_VALUE).toISOString(), + }); + // Not the displayed timestamp, which may point at another column entirely. + expect(pushedFrame().focusTimestamp).not.toBe( + new Date(DISPLAYED_TIMESTAMP).toISOString(), + ); + }); + + // Regression: a composite "EventDate, EventTime" sort key leads with the + // day-precision partition column. Anchoring the frame on it would center the + // destination window on midnight and the span lookup would find no row. + it('anchors a composite timestamp on its fine column, not the date', () => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const compositeSource = { + ...ROOT_SOURCE, + timestampValueExpression: 'EventDate, EventTime', + } as TSource; + + mockUseRowData.mockReturnValue( + rowResult({ + row: { + __hdx_trace_id: TRACE_ID, + __hdx_span_id: SPAN_ID, + __hdx_timestamp_value_0: '2024-05-01', + __hdx_timestamp_value_1: TIMESTAMP_VALUE, + }, + meta: [ + { name: '__hdx_timestamp_value_0', type: 'Date' }, + { name: '__hdx_timestamp_value_1', type: 'DateTime64(9)' }, + ], + }), + ); + + renderInner('row-1', compositeSource); + + fireEvent.click(screen.getByTestId('side-panel-view-trace')); + + expect(pushedFrame().focusTimestamp).toBe( + new Date(TIMESTAMP_VALUE).toISOString(), + ); + }); + + // No usable anchor must leave the lookup unbounded rather than bound it to a + // window around midnight. + it.each([ + [ + 'the row carries no timestamp value', + { + row: { __hdx_trace_id: TRACE_ID, __hdx_span_id: SPAN_ID }, + meta: [], + }, + ], + [ + 'every timestamp column is day-precision', + { + row: { + __hdx_trace_id: TRACE_ID, + __hdx_span_id: SPAN_ID, + __hdx_timestamp_value_0: '2024-05-01', + }, + meta: [{ name: '__hdx_timestamp_value_0', type: 'Date' }], + }, + ], + ])('omits the frame timestamp when %s', (_label, result) => { + mockUseRowData.mockReturnValue(rowResult(result)); + + renderInner('row-1'); + + fireEvent.click(screen.getByTestId('side-panel-view-trace')); + + expect(pushedFrame().focusTimestamp).toBeUndefined(); + }); + + describe('with a landed frame', () => { + function seedFrame(frame: Record) { + mockQueryStore['sidePanelSourceStack'] = [ + { + sourceId: 'trace-src', + rowId: TRACE_SPAN_ROW_ID, + aliasWith: [], + label: 'Log', + sourceKind: 'trace', + ...frame, + }, + ]; + mockQueryStore['sidePanelStackRoot'] = 'row-1'; + } + + it('bounds the lookup to a 1h window around the frame timestamp', () => { + seedFrame({ focusTimestamp: TIMESTAMP_VALUE }); + + renderInner('row-1'); + + expect(lastRowDataArgs()).toMatchObject({ + rowId: TRACE_SPAN_ROW_ID, + dateRange: hourWindow(TIMESTAMP_VALUE), + }); + }); + + // The tab panels re-run the same lookup; an unbounded copy in one of them + // would both scan the table and split the query cache. + it.each([ + ['overview', () => mockRowOverviewPanel], + ['parsed', () => mockRowDataPanel], + ])('passes the same window to the %s tab', (tab, getSpy) => { + seedFrame({ + sourceId: 'log-dest', + sourceKind: 'log', + focusTimestamp: TIMESTAMP_VALUE, + }); + mockQueryStore['sidePanelTab'] = tab; + + renderInner('row-1'); + + expect(getSpy()).toHaveBeenCalledWith( + expect.objectContaining({ dateRange: hourWindow(TIMESTAMP_VALUE) }), + ); + }); + + it('leaves the lookup unbounded when the frame carries no timestamp', () => { + seedFrame({}); + + renderInner('row-1'); + + expect(lastRowDataArgs().dateRange).toBeUndefined(); + }); + + it('leaves the lookup unbounded when the frame timestamp is unparseable', () => { + seedFrame({ focusTimestamp: 'not-a-timestamp' }); + + renderInner('row-1'); + + expect(lastRowDataArgs().dateRange).toBeUndefined(); + }); + + // Same-source drilldowns (surrounding context) carry a full row id whose + // timestamp is already pinned, and can walk arbitrarily far from the frame's + // anchor, so the frame's window must not be applied to them. + it('leaves the lookup unbounded for a nav entry on top of the frame', () => { + seedFrame({ focusTimestamp: TIMESTAMP_VALUE }); + mockQueryStore['sidePanelNavStack'] = [ + { + rowId: + "Timestamp=parseDateTime64BestEffort('2024-05-02 09:00:00', 9)", + aliasWith: [], + label: 'Neighbour', + }, + ]; + + renderInner('row-1'); + + expect(lastRowDataArgs().dateRange).toBeUndefined(); + }); + }); +}); diff --git a/packages/app/src/utils/__tests__/rowTimestamps.test.ts b/packages/app/src/utils/__tests__/rowTimestamps.test.ts new file mode 100644 index 0000000000..c00723fc7a --- /dev/null +++ b/packages/app/src/utils/__tests__/rowTimestamps.test.ts @@ -0,0 +1,216 @@ +import { + getTimestampValueSelects, + resolveRowTimestampAnchor, + timestampValueAlias, +} from '@/utils/rowTimestamps'; + +describe('getTimestampValueSelects', () => { + it('projects a single-column expression under the first alias', () => { + expect(getTimestampValueSelects('Timestamp')).toEqual([ + { valueExpression: 'Timestamp', alias: '__hdx_timestamp_value_0' }, + ]); + }); + + it('projects every column of a composite expression', () => { + expect(getTimestampValueSelects('EventDate, EventTime')).toEqual([ + { valueExpression: 'EventDate', alias: '__hdx_timestamp_value_0' }, + { valueExpression: 'EventTime', alias: '__hdx_timestamp_value_1' }, + ]); + }); + + // splitAndTrimWithBracket keeps bracketed argument lists intact, so a + // function call with its own comma stays one token. + it('does not split inside brackets', () => { + expect( + getTimestampValueSelects('toDate(EventTime), toDateTime64(EventTime, 9)'), + ).toEqual([ + { + valueExpression: 'toDate(EventTime)', + alias: '__hdx_timestamp_value_0', + }, + { + valueExpression: 'toDateTime64(EventTime, 9)', + alias: '__hdx_timestamp_value_1', + }, + ]); + }); + + it.each([[undefined], [''], [' ']])( + 'projects nothing for %p', + expression => { + expect(getTimestampValueSelects(expression)).toEqual([]); + }, + ); +}); + +describe('resolveRowTimestampAnchor', () => { + const TIMESTAMP = '2024-05-01T14:23:11.123456789Z'; + + function metaFor(types: string[]) { + return types.map((type, index) => ({ + name: timestampValueAlias(index), + type, + })); + } + + it('resolves a single DateTime64 column', () => { + expect( + resolveRowTimestampAnchor({ + timestampValueExpression: 'Timestamp', + row: { __hdx_timestamp_value_0: TIMESTAMP }, + meta: metaFor(['DateTime64(9)']), + }), + ).toEqual(new Date(TIMESTAMP)); + }); + + // Regression: a composite "EventDate, EventTime" sort key leads with the + // day-precision partition column. Anchoring on it puts the instant at + // midnight, and a narrow window around midnight excludes the event. + it('skips the day-precision column of a composite expression', () => { + expect( + resolveRowTimestampAnchor({ + timestampValueExpression: 'EventDate, EventTime', + row: { + __hdx_timestamp_value_0: '2024-05-01', + __hdx_timestamp_value_1: TIMESTAMP, + }, + meta: metaFor(['Date', 'DateTime64(9)']), + }), + ).toEqual(new Date(TIMESTAMP)); + }); + + it('resolves the fine column regardless of token order', () => { + expect( + resolveRowTimestampAnchor({ + timestampValueExpression: 'EventTime, EventDate', + row: { + __hdx_timestamp_value_0: TIMESTAMP, + __hdx_timestamp_value_1: '2024-05-01', + }, + meta: metaFor(['DateTime64(9)', 'Date32']), + }), + ).toEqual(new Date(TIMESTAMP)); + }); + + it('prefers the highest-precision column', () => { + const coarse = '2024-05-01T14:23:11Z'; + expect( + resolveRowTimestampAnchor({ + timestampValueExpression: 'EventSecond, EventNano', + row: { + __hdx_timestamp_value_0: coarse, + __hdx_timestamp_value_1: TIMESTAMP, + }, + meta: metaFor(['DateTime', 'DateTime64(9)']), + }), + ).toEqual(new Date(TIMESTAMP)); + }); + + it('breaks precision ties on the earlier token', () => { + const later = '2024-05-01T18:00:00.000Z'; + expect( + resolveRowTimestampAnchor({ + timestampValueExpression: 'EventTime, ObservedTime', + row: { + __hdx_timestamp_value_0: TIMESTAMP, + __hdx_timestamp_value_1: later, + }, + meta: metaFor(['DateTime64(9)', 'DateTime64(9)']), + }), + ).toEqual(new Date(TIMESTAMP)); + }); + + it('looks through a Nullable wrapper and a timezone argument', () => { + expect( + resolveRowTimestampAnchor({ + timestampValueExpression: 'Timestamp', + row: { __hdx_timestamp_value_0: TIMESTAMP }, + meta: metaFor(["Nullable(DateTime64(3, 'UTC'))"]), + }), + ).toEqual(new Date(TIMESTAMP)); + }); + + it('treats a numeric value as unix seconds', () => { + expect( + resolveRowTimestampAnchor({ + timestampValueExpression: 'Timestamp', + row: { __hdx_timestamp_value_0: 1714573391 }, + meta: metaFor(['DateTime']), + }), + ).toEqual(new Date(1714573391 * 1000)); + }); + + // Every rejection path returns undefined so callers fall back to an + // unbounded lookup instead of a window around a bogus instant. + it.each([ + [ + 'every column is day-precision', + { + timestampValueExpression: 'EventDate, EventDate32', + row: { + __hdx_timestamp_value_0: '2024-05-01', + __hdx_timestamp_value_1: '2024-05-01', + }, + meta: metaFor(['Date', 'Date32']), + }, + ], + [ + 'the column type is not a timestamp', + { + timestampValueExpression: 'Timestamp', + row: { __hdx_timestamp_value_0: TIMESTAMP }, + meta: metaFor(['String']), + }, + ], + [ + 'meta has no entry for the alias', + { + timestampValueExpression: 'Timestamp', + row: { __hdx_timestamp_value_0: TIMESTAMP }, + meta: [{ name: 'Timestamp', type: 'DateTime64(9)' }], + }, + ], + [ + 'the value is missing from the row', + { + timestampValueExpression: 'Timestamp', + row: {}, + meta: metaFor(['DateTime64(9)']), + }, + ], + [ + 'the value is unparseable', + { + timestampValueExpression: 'Timestamp', + row: { __hdx_timestamp_value_0: 'not-a-timestamp' }, + meta: metaFor(['DateTime64(9)']), + }, + ], + [ + 'meta is unavailable', + { + timestampValueExpression: 'Timestamp', + row: { __hdx_timestamp_value_0: TIMESTAMP }, + meta: undefined, + }, + ], + [ + 'the row is missing', + { + timestampValueExpression: 'Timestamp', + row: null, + meta: metaFor(['DateTime64(9)']), + }, + ], + [ + 'the source has no timestamp expression', + { + timestampValueExpression: ' ', + row: { __hdx_timestamp_value_0: TIMESTAMP }, + meta: metaFor(['DateTime64(9)']), + }, + ], + ])('returns undefined when %s', (_label, args) => { + expect(resolveRowTimestampAnchor(args)).toBeUndefined(); + }); +}); diff --git a/packages/app/src/utils/rowTimestamps.ts b/packages/app/src/utils/rowTimestamps.ts new file mode 100644 index 0000000000..d077f8e545 --- /dev/null +++ b/packages/app/src/utils/rowTimestamps.ts @@ -0,0 +1,93 @@ +import { ColumnMetaType } from '@hyperdx/common-utils/dist/clickhouse'; +import { + classifyTimestampType, + splitAndTrimWithBracket, +} from '@hyperdx/common-utils/dist/core/utils'; + +/** + * Alias for the i-th column of a source's (possibly composite) + * `timestampValueExpression`, as projected by `useRowData`. + */ +export function timestampValueAlias(index: number): string { + return `__hdx_timestamp_value_${index}`; +} + +/** + * Select entries projecting every column of a `timestampValueExpression`. + * + * All tokens are projected rather than just the first because which one carries + * the event's real precision isn't knowable from the expression alone — the + * conventional `"EventDate, EventTime"` sort key leads with a day-precision + * `Date` used for partition pruning. The row response's `meta` types settle it; + * see `resolveRowTimestampAnchor`. + */ +export function getTimestampValueSelects( + timestampValueExpression: string | undefined, +): { valueExpression: string; alias: string }[] { + if (!timestampValueExpression?.trim()) { + return []; + } + return splitAndTrimWithBracket(timestampValueExpression).map( + (valueExpression, index) => ({ + valueExpression, + alias: timestampValueAlias(index), + }), + ); +} + +/** + * The instant a row happened, resolved from the highest-precision timestamp + * column the row query actually returned. + * + * Returns undefined when no DateTime-typed token came back — every token is + * `Date`-typed, the values are missing, or `meta` is unavailable. Callers must + * treat that as "no usable anchor" rather than falling back to a day-precision + * value: that would place the instant at midnight, and anything deriving a + * narrow window from it would exclude every event outside that midnight window. + */ +export function resolveRowTimestampAnchor({ + timestampValueExpression, + row, + meta, +}: { + timestampValueExpression: string | undefined; + row: Record | undefined | null; + meta: ColumnMetaType[] | undefined; +}): Date | undefined { + if (!timestampValueExpression?.trim() || row == null || meta == null) { + return undefined; + } + + let best: { precision: number; date: Date } | undefined; + + splitAndTrimWithBracket(timestampValueExpression).forEach((_, index) => { + const alias = timestampValueAlias(index); + const classified = classifyTimestampType( + meta.find(m => m.name === alias)?.type, + ); + // Day-precision columns can't locate the event within its day. + if (classified == null || classified.kind === 'date') { + return; + } + + const rawValue = row[alias]; + if (rawValue == null) { + return; + } + const date = + typeof rawValue === 'number' + ? new Date(rawValue * 1000) + : new Date(rawValue); + if (isNaN(date.getTime())) { + return; + } + + // Highest precision wins; on a tie the earlier token does, matching + // `pickBucketTimestampColumn`. + if (best == null || classified.precision > best.precision) { + best = { precision: classified.precision, date }; + } + }); + + return best?.date; +} diff --git a/packages/common-utils/src/core/utils.ts b/packages/common-utils/src/core/utils.ts index 5ca7bb0e53..c4871b1fea 100644 --- a/packages/common-utils/src/core/utils.ts +++ b/packages/common-utils/src/core/utils.ts @@ -121,9 +121,18 @@ export function getFirstTimestampValueExpression(valueExpression: string) { return splitAndTrimWithBracket(valueExpression)[0]; } -type TimestampTypeKind = 'date' | 'datetime' | 'datetime64'; +export type TimestampTypeKind = 'date' | 'datetime' | 'datetime64'; -function classifyTimestampType(type: string | undefined): { +/** + * Classify a ClickHouse timestamp type into its kind and sub-second precision. + * + * `kind: 'date'` means day precision — a value read from such a column lands at + * midnight and can't locate an event within its day. Callers that need an + * instant use this to skip those columns rather than silently anchor to midnight. + * + * Returns null for anything that isn't a Date/DateTime/DateTime64. + */ +export function classifyTimestampType(type: string | undefined): { kind: TimestampTypeKind; precision: number; } | null { From c1fa069e0e7f8d6230ea73241e86bbc0376c0da5 Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Wed, 5 Aug 2026 14:05:56 -0400 Subject: [PATCH 2/3] fix: Retry unbounded query if bounded trace lookup fails --- .../app/src/components/DBRowDataPanel.tsx | 267 ++++++++++-------- .../app/src/components/DBRowSidePanel.tsx | 30 +- .../__tests__/DBRowDataPanel.test.ts | 160 ++++++++++- ...BRowSidePanel.viewTraceTimeFilter.test.tsx | 33 ++- .../src/utils/__tests__/rowTimestamps.test.ts | 41 +++ packages/app/src/utils/rowTimestamps.ts | 37 +++ 6 files changed, 427 insertions(+), 141 deletions(-) diff --git a/packages/app/src/components/DBRowDataPanel.tsx b/packages/app/src/components/DBRowDataPanel.tsx index c448746c32..d57c6c1a20 100644 --- a/packages/app/src/components/DBRowDataPanel.tsx +++ b/packages/app/src/components/DBRowDataPanel.tsx @@ -95,129 +95,165 @@ export function useRowData({ ? source.knownColumnsListExpression?.trim() : undefined; - const queryResult = useQueriedChartConfig( + const baseConfig = { + connection: source.connection, + select: [ + { + valueExpression: knownColumns || '*', + }, + { + valueExpression: getDisplayedTimestampValueExpression(source), + alias: ROW_DATA_ALIASES.TIMESTAMP, + }, + ...getTimestampValueSelects(timestampValueExpr), + ...(eventBodyExpr + ? [ + { + valueExpression: eventBodyExpr, + alias: ROW_DATA_ALIASES.BODY, + }, + ] + : []), + ...(searchedTraceIdExpr + ? [ + { + valueExpression: searchedTraceIdExpr, + alias: ROW_DATA_ALIASES.TRACE_ID, + }, + ] + : []), + ...(searchedSpanIdExpr + ? [ + { + valueExpression: searchedSpanIdExpr, + alias: ROW_DATA_ALIASES.SPAN_ID, + }, + ] + : []), + ...(severityTextExpr + ? [ + { + valueExpression: severityTextExpr, + alias: ROW_DATA_ALIASES.SEVERITY_TEXT, + }, + ] + : []), + ...((isLogSource(source) || isTraceSource(source)) && + source.serviceNameExpression + ? [ + { + valueExpression: source.serviceNameExpression, + alias: ROW_DATA_ALIASES.SERVICE_NAME, + }, + ] + : []), + ...('resourceAttributesExpression' in source && + source.resourceAttributesExpression + ? [ + { + valueExpression: source.resourceAttributesExpression, + alias: ROW_DATA_ALIASES.RESOURCE_ATTRIBUTES, + }, + ] + : []), + ...((isLogSource(source) || isTraceSource(source)) && + source.eventAttributesExpression + ? [ + { + valueExpression: source.eventAttributesExpression, + alias: ROW_DATA_ALIASES.EVENT_ATTRIBUTES, + }, + ] + : []), + ...(source.kind === SourceKind.Trace && source.spanEventsValueExpression + ? [ + { + valueExpression: `${source.spanEventsValueExpression}.Attributes[indexOf(${source.spanEventsValueExpression}.Name, 'exception')]`, + alias: ROW_DATA_ALIASES.EVENTS_EXCEPTION_ATTRIBUTES, + }, + { + valueExpression: source.spanEventsValueExpression, + alias: ROW_DATA_ALIASES.SPAN_EVENTS, + }, + ] + : []), + ...(source.kind === SourceKind.Trace && source.durationExpression + ? [ + { + valueExpression: getDurationMsExpression(source), + alias: ROW_DATA_ALIASES.DURATION_MS, + }, + ] + : []), + ...(source.kind === SourceKind.Trace && source.spanKindExpression + ? [ + { + valueExpression: source.spanKindExpression, + alias: ROW_DATA_ALIASES.SPAN_KIND, + }, + ] + : []), + ...(source.kind === SourceKind.Trace && source.spanLinksValueExpression + ? [ + { + valueExpression: source.spanLinksValueExpression, + alias: ROW_DATA_ALIASES.SPAN_LINKS, + }, + ] + : []), + ...selectHighlightedRowAttributes, + ], + where: rowId ?? '0=1', + from: source.from, + limit: { limit: 1 }, + ...(aliasWith && aliasWith.length > 0 ? { with: aliasWith } : {}), + }; + + const baseQueryKey = ['row_side_panel', rowId, aliasWith, source]; + // Both halves of the filter are needed for `renderChartConfig` to emit one, so + // a source with no usable timestamp expression can't be bounded at all. + const hasWindow = dateRange != null && timestampValueExpr != null; + + const boundedResult = useQueriedChartConfig( { - connection: source.connection, - select: [ - { - valueExpression: knownColumns || '*', - }, - { - valueExpression: getDisplayedTimestampValueExpression(source), - alias: ROW_DATA_ALIASES.TIMESTAMP, - }, - ...getTimestampValueSelects(timestampValueExpr), - ...(eventBodyExpr - ? [ - { - valueExpression: eventBodyExpr, - alias: ROW_DATA_ALIASES.BODY, - }, - ] - : []), - ...(searchedTraceIdExpr - ? [ - { - valueExpression: searchedTraceIdExpr, - alias: ROW_DATA_ALIASES.TRACE_ID, - }, - ] - : []), - ...(searchedSpanIdExpr - ? [ - { - valueExpression: searchedSpanIdExpr, - alias: ROW_DATA_ALIASES.SPAN_ID, - }, - ] - : []), - ...(severityTextExpr - ? [ - { - valueExpression: severityTextExpr, - alias: ROW_DATA_ALIASES.SEVERITY_TEXT, - }, - ] - : []), - ...((isLogSource(source) || isTraceSource(source)) && - source.serviceNameExpression - ? [ - { - valueExpression: source.serviceNameExpression, - alias: ROW_DATA_ALIASES.SERVICE_NAME, - }, - ] - : []), - ...('resourceAttributesExpression' in source && - source.resourceAttributesExpression - ? [ - { - valueExpression: source.resourceAttributesExpression, - alias: ROW_DATA_ALIASES.RESOURCE_ATTRIBUTES, - }, - ] - : []), - ...((isLogSource(source) || isTraceSource(source)) && - source.eventAttributesExpression - ? [ - { - valueExpression: source.eventAttributesExpression, - alias: ROW_DATA_ALIASES.EVENT_ATTRIBUTES, - }, - ] - : []), - ...(source.kind === SourceKind.Trace && source.spanEventsValueExpression - ? [ - { - valueExpression: `${source.spanEventsValueExpression}.Attributes[indexOf(${source.spanEventsValueExpression}.Name, 'exception')]`, - alias: ROW_DATA_ALIASES.EVENTS_EXCEPTION_ATTRIBUTES, - }, - { - valueExpression: source.spanEventsValueExpression, - alias: ROW_DATA_ALIASES.SPAN_EVENTS, - }, - ] - : []), - ...(source.kind === SourceKind.Trace && source.durationExpression - ? [ - { - valueExpression: getDurationMsExpression(source), - alias: ROW_DATA_ALIASES.DURATION_MS, - }, - ] - : []), - ...(source.kind === SourceKind.Trace && source.spanKindExpression - ? [ - { - valueExpression: source.spanKindExpression, - alias: ROW_DATA_ALIASES.SPAN_KIND, - }, - ] - : []), - ...(source.kind === SourceKind.Trace && source.spanLinksValueExpression - ? [ - { - valueExpression: source.spanLinksValueExpression, - alias: ROW_DATA_ALIASES.SPAN_LINKS, - }, - ] - : []), - ...selectHighlightedRowAttributes, - ], - where: rowId ?? '0=1', - from: source.from, - limit: { limit: 1 }, - ...(aliasWith && aliasWith.length > 0 ? { with: aliasWith } : {}), - ...(dateRange && timestampValueExpr + ...baseConfig, + ...(hasWindow ? { dateRange, timestampValueExpression: timestampValueExpr } : {}), }, { - queryKey: ['row_side_panel', rowId, aliasWith, source, dateRange], - enabled: rowId != null, + queryKey: [...baseQueryKey, dateRange], + enabled: rowId != null && hasWindow, }, ); + // The window may be derived from a *different* row than the one we're + // looking for (eg. looking up a span based on a log's timestamp). If the + // window excludes the row, the bounded query returns zero rows. + const isBoundedEmpty = + hasWindow && + boundedResult.isSuccess && + boundedResult.data?.isComplete !== false && // Defensive check against chunked queries + boundedResult.data?.data?.length === 0; + + const isFallbackActive = !hasWindow || isBoundedEmpty; + + // Key is identical to the unbounded config so this shares cache entries + // with the call sites that never pass a `dateRange`, letting the retry + // often resolve from cache instead of scanning. + const fallbackResult = useQueriedChartConfig(baseConfig, { + queryKey: [...baseQueryKey, undefined], + enabled: rowId != null && isFallbackActive, + }); + + const queryResult = isFallbackActive ? fallbackResult : boundedResult; + + // The bounded result is known-empty by the time the retry is enabled, so + // report loading until it settles rather than briefly claiming the row is + // absent. + const isLoading = + queryResult.isLoading || (isBoundedEmpty && queryResult.isPending); + // Normalize resource and event attributes to always use flat keys for both JSON and Map columns const normalizedData = useMemo(() => { if (!queryResult.data?.data?.[0]) { @@ -248,6 +284,7 @@ export function useRowData({ return { ...queryResult, data: normalizedData, + isLoading, }; } diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index fc163d924d..673af432ac 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -51,7 +51,10 @@ import { SearchConfig } from '@/types'; import { FormatTime } from '@/useFormatTime'; import { formatDistanceToNowStrictShort } from '@/utils'; import { getHighlightedAttributesFromData } from '@/utils/highlightedAttributes'; -import { resolveRowTimestampAnchor } from '@/utils/rowTimestamps'; +import { + getRowLookupWindow, + resolveRowTimestampAnchor, +} from '@/utils/rowTimestamps'; import { useZIndex, ZIndexContext } from '@/zIndex'; import ServiceMapSidePanel from './ServiceMap/ServiceMapSidePanel'; @@ -282,22 +285,17 @@ export const DBRowSidePanelInner = ({ // A cross-source frame's row id is synthesized from ids alone ("View Trace" // builds `TraceId = … AND SpanId = …`), so on its own the lookup has no // timestamp predicate and scans every part. When the pushing panel stamped - // the origin row's timestamp onto the frame, bound the lookup to the same - // ±1h window the trace waterfall and service map already use. + // the origin row's timestamp onto the frame, bound the lookup to a window + // around it. `useRowData` retries unbounded if that window misses. const frameFocusTimestamp = activeSourceFrame?.focusTimestamp; - const frameDateRange = useMemo<[Date, Date] | undefined>(() => { - // Nav entries carry a full row id (timestamp included), so their lookups - // are already bounded and must not be narrowed by the frame's window — - // surrounding context can walk arbitrarily far from the frame's anchor. - if (leafNav != null || frameFocusTimestamp == null) { - return undefined; - } - const focus = new Date(frameFocusTimestamp); - if (isNaN(focus.getTime())) { - return undefined; - } - return [add(focus, { minutes: -60 }), add(focus, { minutes: 60 })]; - }, [leafNav, frameFocusTimestamp]); + const frameDateRange = useMemo<[Date, Date] | undefined>( + () => + // Nav entries carry a full row id (timestamp included), so their lookups + // are already bounded and must not be narrowed by the frame's window — + // surrounding context can walk arbitrarily far from the frame's anchor. + leafNav != null ? undefined : getRowLookupWindow(frameFocusTimestamp), + [leafNav, frameFocusTimestamp], + ); const activeDateRange = skipRowQuery ? undefined : frameDateRange; diff --git a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts index c6025a23e6..211a0f4a4c 100644 --- a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts +++ b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts @@ -154,7 +154,6 @@ describe('DBRowDataPanel', () => { }); it('gives bounded and unbounded lookups of the same row different query keys', () => { - renderHook(() => useRowData({ source, rowId: "id='abc123'" })); renderHook(() => useRowData({ source, @@ -166,9 +165,162 @@ describe('DBRowDataPanel', () => { }), ); - const [, unboundedOptions] = mockUseQueriedChartConfig.mock.calls[0]; - const [, boundedOptions] = mockUseQueriedChartConfig.mock.calls[1]; - expect(boundedOptions.queryKey).not.toEqual(unboundedOptions.queryKey); + const [, boundedOptions] = mockUseQueriedChartConfig.mock.calls[0]; + const [, fallbackOptions] = mockUseQueriedChartConfig.mock.calls[1]; + expect(boundedOptions.queryKey).not.toEqual(fallbackOptions.queryKey); + }); + }); + + // A window derived from the origin row's instant but filtered against the + // destination's timestamp can exclude the row being looked up — a long span + // starts before a window centered on a log it emitted late in its life. Zero + // rows is a query success, so the lookup has to retry unbounded rather than + // report the row as missing. + describe('unbounded fallback', () => { + const DATE_RANGE: [Date, Date] = [ + new Date('2024-01-01T00:00:00Z'), + new Date('2024-01-01T02:00:00Z'), + ]; + const ROW = { __hdx_timestamp: '2024-01-01T01:00:00Z' }; + + function lookupResult(overrides: Record = {}) { + return { + data: { data: [], meta: [], rows: 0, isComplete: true }, + isLoading: false, + isPending: false, + isError: false, + isSuccess: true, + ...overrides, + }; + } + + // Dispatch on the query key rather than a call counter, so the mock is + // indifferent to how many times the hook renders. The unbounded lookup is + // the one whose key ends in `undefined` instead of the window. + function mockLookups({ + bounded, + fallback, + }: { + bounded: ReturnType; + fallback: ReturnType; + }) { + mockUseQueriedChartConfig.mockImplementation((_config, options) => + options.queryKey[options.queryKey.length - 1] === undefined + ? fallback + : bounded, + ); + } + + function renderLookup(dateRange?: [Date, Date]) { + return renderHook(() => + useRowData({ source, rowId: "id='abc123'", dateRange }), + ); + } + + function enabledFlags() { + const [, boundedOptions] = mockUseQueriedChartConfig.mock.calls[0]; + const [, fallbackOptions] = mockUseQueriedChartConfig.mock.calls[1]; + return { + bounded: boundedOptions.enabled, + fallback: fallbackOptions.enabled, + }; + } + + it('does not run when the bounded lookup finds the row', () => { + mockLookups({ + bounded: lookupResult({ + data: { data: [ROW], meta: [], rows: 1, isComplete: true }, + }), + fallback: lookupResult(), + }); + + const { result } = renderLookup(DATE_RANGE); + + expect(enabledFlags()).toEqual({ bounded: true, fallback: false }); + expect(result.current.data?.data).toEqual([ROW]); + }); + + it('runs when the bounded lookup comes back empty', () => { + mockLookups({ bounded: lookupResult(), fallback: lookupResult() }); + + renderLookup(DATE_RANGE); + + expect(enabledFlags()).toEqual({ bounded: true, fallback: true }); + }); + + it('serves the row the bounded lookup missed', () => { + mockLookups({ + bounded: lookupResult(), + fallback: lookupResult({ + data: { data: [ROW], meta: [], rows: 1, isComplete: true }, + }), + }); + + const { result } = renderLookup(DATE_RANGE); + + expect(result.current.data?.data).toEqual([ROW]); + }); + + // An error isn't evidence the row is outside the window, and retrying + // unbounded would hide it from `DBRowSidePanelErrorState`. + it('does not run when the bounded lookup errors', () => { + const error = new Error('boom'); + mockLookups({ + bounded: lookupResult({ + data: undefined, + isSuccess: false, + isError: true, + error, + }), + fallback: lookupResult(), + }); + + const { result } = renderLookup(DATE_RANGE); + + expect(enabledFlags()).toEqual({ bounded: true, fallback: false }); + expect(result.current.isError).toBe(true); + expect(result.current.error).toBe(error); + }); + + it('is the only lookup that runs when there is no window', () => { + mockLookups({ bounded: lookupResult(), fallback: lookupResult() }); + + renderLookup(); + + expect(enabledFlags()).toEqual({ bounded: false, fallback: true }); + }); + + // Otherwise the panel would flash an absent row between the bounded lookup + // settling empty and the retry resolving. + it('reports loading while in flight', () => { + mockLookups({ + bounded: lookupResult(), + fallback: lookupResult({ + data: undefined, + isSuccess: false, + isLoading: false, + isPending: true, + }), + }); + + const { result } = renderLookup(DATE_RANGE); + + expect(result.current.isLoading).toBe(true); + }); + + // A chunked query publishes partial results as successes, so an empty + // first chunk must not read as "no such row". + it('does not run on an incomplete bounded result', () => { + mockLookups({ + bounded: lookupResult({ + data: { data: [], meta: [], rows: 0, isComplete: false }, + }), + fallback: lookupResult(), + }); + + renderLookup(DATE_RANGE); + + expect(enabledFlags()).toEqual({ bounded: true, fallback: false }); }); }); diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.viewTraceTimeFilter.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.viewTraceTimeFilter.test.tsx index d329be77a5..03856d06f9 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanel.viewTraceTimeFilter.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanel.viewTraceTimeFilter.test.tsx @@ -155,6 +155,7 @@ jest.mock('@/useFormatTime', () => ({ // which close over the `mock*` helpers declared at the top of this file. import { DBRowSidePanelInner } from '@/components/DBRowSidePanel'; import useSidePanelStack from '@/hooks/useSidePanelStack'; +import { getRowLookupWindow } from '@/utils/rowTimestamps'; // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion const ROOT_SOURCE = { @@ -227,9 +228,12 @@ function pushedFrame() { return setterFor('sidePanelSourceStack').mock.calls[0][0][0]; } -function hourWindow(isoTimestamp: string) { - const ms = new Date(isoTimestamp).getTime(); - return [new Date(ms - 60 * 60 * 1000), new Date(ms + 60 * 60 * 1000)]; +/** + * Delegates to the real helper: these tests assert the window reaches the + * lookup, while its bounds are pinned in `utils/__tests__/rowTimestamps.test.ts`. + */ +function lookupWindow(isoTimestamp: string) { + return getRowLookupWindow(isoTimestamp); } describe('DBRowSidePanelInner, "View Trace" row lookup time filter', () => { @@ -347,17 +351,34 @@ describe('DBRowSidePanelInner, "View Trace" row lookup time filter', () => { mockQueryStore['sidePanelStackRoot'] = 'row-1'; } - it('bounds the lookup to a 1h window around the frame timestamp', () => { + it('bounds the lookup to a window around the frame timestamp', () => { seedFrame({ focusTimestamp: TIMESTAMP_VALUE }); renderInner('row-1'); expect(lastRowDataArgs()).toMatchObject({ rowId: TRACE_SPAN_ROW_ID, - dateRange: hourWindow(TIMESTAMP_VALUE), + dateRange: lookupWindow(TIMESTAMP_VALUE), }); }); + // Regression: the window is anchored on the origin log but filtered against + // the destination span's *start*, so a symmetric hour excluded any span that + // ran longer than that and logged late in its life. + it('reaches back past the start of a long-running span', () => { + const spanStart = new Date('2024-05-01T08:50:00.000Z'); + // 70 minutes into the span — outside a symmetric hour around the log. + seedFrame({ focusTimestamp: TIMESTAMP_VALUE }); + + renderInner('row-1'); + + const [start, end] = lastRowDataArgs().dateRange; + expect(start.getTime()).toBeLessThan(spanStart.getTime()); + expect(end.getTime()).toBeGreaterThan( + new Date(TIMESTAMP_VALUE).getTime(), + ); + }); + // The tab panels re-run the same lookup; an unbounded copy in one of them // would both scan the table and split the query cache. it.each([ @@ -374,7 +395,7 @@ describe('DBRowSidePanelInner, "View Trace" row lookup time filter', () => { renderInner('row-1'); expect(getSpy()).toHaveBeenCalledWith( - expect.objectContaining({ dateRange: hourWindow(TIMESTAMP_VALUE) }), + expect.objectContaining({ dateRange: lookupWindow(TIMESTAMP_VALUE) }), ); }); diff --git a/packages/app/src/utils/__tests__/rowTimestamps.test.ts b/packages/app/src/utils/__tests__/rowTimestamps.test.ts index c00723fc7a..97d604c81c 100644 --- a/packages/app/src/utils/__tests__/rowTimestamps.test.ts +++ b/packages/app/src/utils/__tests__/rowTimestamps.test.ts @@ -1,6 +1,9 @@ import { + getRowLookupWindow, getTimestampValueSelects, resolveRowTimestampAnchor, + ROW_LOOKUP_WINDOW_LEAD_HOURS, + ROW_LOOKUP_WINDOW_LOOKBACK_HOURS, timestampValueAlias, } from '@/utils/rowTimestamps'; @@ -214,3 +217,41 @@ describe('resolveRowTimestampAnchor', () => { expect(resolveRowTimestampAnchor(args)).toBeUndefined(); }); }); + +describe('getRowLookupWindow', () => { + it('reaches further back than forward', () => { + // The whole point of the window: the destination span starts at or before + // the origin log, so a symmetric window drops long-running spans. + expect(ROW_LOOKUP_WINDOW_LOOKBACK_HOURS).toBeGreaterThan( + ROW_LOOKUP_WINDOW_LEAD_HOURS, + ); + }); + + it('spans 4h back and 1h forward from the anchor', () => { + expect(getRowLookupWindow('2024-05-02T12:00:00.000Z')).toEqual([ + new Date('2024-05-02T08:00:00.000Z'), + new Date('2024-05-02T13:00:00.000Z'), + ]); + }); + + // A log emitted well into a long span is the case a symmetric hour missed. + it('covers a span that started hours before the log it anchors on', () => { + const spanStart = new Date('2024-05-02T09:00:00.000Z'); + const logInstant = '2024-05-02T12:30:00.000Z'; + + const [start, end] = getRowLookupWindow(logInstant)!; + + expect(start.getTime()).toBeLessThan(spanStart.getTime()); + expect(end.getTime()).toBeGreaterThan(new Date(logInstant).getTime()); + }); + + it.each([ + ['undefined', undefined], + ['null', null], + ['empty', ''], + ['blank', ' '], + ['unparseable', 'not-a-timestamp'], + ])('returns undefined when the anchor is %s', (_label, focusTimestamp) => { + expect(getRowLookupWindow(focusTimestamp)).toBeUndefined(); + }); +}); diff --git a/packages/app/src/utils/rowTimestamps.ts b/packages/app/src/utils/rowTimestamps.ts index d077f8e545..802051e1f3 100644 --- a/packages/app/src/utils/rowTimestamps.ts +++ b/packages/app/src/utils/rowTimestamps.ts @@ -1,3 +1,4 @@ +import { add } from 'date-fns'; import { ColumnMetaType } from '@hyperdx/common-utils/dist/clickhouse'; import { classifyTimestampType, @@ -91,3 +92,39 @@ export function resolveRowTimestampAnchor({ return best?.date; } + +/** + * How far a cross-source row lookup's window reaches on either side of the + * origin row's instant. + * + * Asymmetric because the window is derived from the *origin* row's instant but + * filtered against the *destination* source's `timestampValueExpression`. The + * only push that carries an anchor today is "View Trace" (log → the span the log + * belongs to), and a span always starts at or before the logs that reference it + * while the traces schema's `Timestamp` is the span's *start* — so a symmetric + * window silently drops any span that ran longer than the window and logged late + * in its life. The lookback is therefore longer, and the lead only has to + * cover clock skew between the log and span emitters. + */ +export const ROW_LOOKUP_WINDOW_LOOKBACK_HOURS = 4; +export const ROW_LOOKUP_WINDOW_LEAD_HOURS = 1; + +/** + * Window to bound a cross-source row lookup by, given the origin row's instant. + * Returns undefined when focusTimestamp is not a valid date. + */ +export function getRowLookupWindow( + focusTimestamp: string | null | undefined, +): [Date, Date] | undefined { + if (!focusTimestamp?.trim()) { + return undefined; + } + const focus = new Date(focusTimestamp); + if (isNaN(focus.getTime())) { + return undefined; + } + return [ + add(focus, { hours: -ROW_LOOKUP_WINDOW_LOOKBACK_HOURS }), + add(focus, { hours: ROW_LOOKUP_WINDOW_LEAD_HOURS }), + ]; +} From dff67479b7ccdad292810bb7bc6429a21ed894c3 Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Thu, 6 Aug 2026 15:49:22 -0400 Subject: [PATCH 3/3] chore: Lint fix --- .../__tests__/DBRowSidePanel.viewTraceTimeFilter.test.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.viewTraceTimeFilter.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.viewTraceTimeFilter.test.tsx index 03856d06f9..16d0dd4055 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanel.viewTraceTimeFilter.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanel.viewTraceTimeFilter.test.tsx @@ -275,7 +275,6 @@ describe('DBRowSidePanelInner, "View Trace" row lookup time filter', () => { // day-precision partition column. Anchoring the frame on it would center the // destination window on midnight and the span lookup would find no row. it('anchors a composite timestamp on its fine column, not the date', () => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion const compositeSource = { ...ROOT_SOURCE, timestampValueExpression: 'EventDate, EventTime',