From a036a05c702ade134e6a343f80fb9f084c0ac010 Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Mon, 14 Sep 2026 00:02:58 -0700 Subject: [PATCH 01/18] feat: Add a trace logs tab to the event side panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading a trace's logs meant hunting for the green rows interleaved in the waterfall, which a log-heavy trace buries. The new tab lists them flat and chronological in the same DBSqlRowTable the search page uses, so it paginates instead of being capped by tree building. It shows on any row with a trace id that resolves a log source: a span, via the trace source's correlated log source, or a log, via its own source with the row you came from highlighted. The trace scope goes in the config's `filters` so the user's filter keeps the whole `where` slot, and that filter is the waterfall's own `logWhere` URL state, so setting it in either place applies to both. Picking a log routes through the existing navigation: a cross-source frame push from a span (breadcrumb + back), a same-source row change from another log. The tab is navigational, not a reading view, so it never becomes the remembered tab preference — landing on a picked log shows its overview, not another logs list. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/trace-logs-side-panel-tab.md | 7 + .../app/src/components/DBRowSidePanel.tsx | 74 ++++ .../src/components/DBRowSidePanel.types.ts | 1 + .../app/src/components/TraceLogsPanel.tsx | 228 ++++++++++++ .../DBRowSidePanel.traceLogsTab.test.tsx | 330 ++++++++++++++++++ .../__tests__/TraceLogsPanel.test.tsx | 170 +++++++++ packages/app/src/hooks/useSidePanelStack.ts | 7 +- .../app/src/hooks/useWaterfallSearchState.tsx | 14 +- 8 files changed, 824 insertions(+), 7 deletions(-) create mode 100644 .changeset/trace-logs-side-panel-tab.md create mode 100644 packages/app/src/components/TraceLogsPanel.tsx create mode 100644 packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx create mode 100644 packages/app/src/components/__tests__/TraceLogsPanel.test.tsx diff --git a/.changeset/trace-logs-side-panel-tab.md b/.changeset/trace-logs-side-panel-tab.md new file mode 100644 index 0000000000..8282d4513d --- /dev/null +++ b/.changeset/trace-logs-side-panel-tab.md @@ -0,0 +1,7 @@ +--- +'@hyperdx/app': minor +--- + +Add a "Trace logs" tab to the event side panel, listing every log in the trace as a flat chronological table. It appears on any row that carries a trace id and resolves a log source — a span (via the trace source's correlated log source) or a log (its own source, with the row you came from highlighted). Previously the only route to a trace's logs was hunting for the green rows interleaved in the waterfall, which a log-heavy trace buries. + +The tab shares the waterfall's log filter, so a filter set in either place applies to both, and picking a log opens it in the panel: a breadcrumb hop from a span, or a row change from another log. Sorting is ascending, which inside a trace is execution order. diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index 36b7d4340a..231e40dfb0 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -80,6 +80,7 @@ import { import LogLevel from './LogLevel'; import SidePanelBreadcrumbs, { BreadcrumbItem } from './SidePanelBreadcrumbs'; import { SpanLinkData } from './SpanLinksSubpanel'; +import TraceLogsPanel from './TraceLogsPanel'; import { ViewTraceCalloutButton } from './ViewTraceCalloutButton'; import styles from '@/../styles/LogSidePanel.module.scss'; @@ -549,6 +550,16 @@ export const DBRowSidePanelInner = ({ ? source.traceSourceId : undefined; + // The source holding this trace's logs: the trace source's correlated log + // source, or the log source itself when the displayed row is already a log + // (where the tab lists its siblings in the same trace). + const traceLogSourceId = isTraceSource(source) + ? childSourceId + : isLogSource(source) + ? source.id + : undefined; + const enableTraceLogs = !!traceId && !!traceLogSourceId; + const enableServiceMap = traceId && traceSourceId; const { data: traceSourceData } = useSource({ id: traceSourceId }); @@ -611,6 +622,36 @@ export const DBRowSidePanelInner = ({ [traceSourceData, handleSourceStackPush, mainContent], ); + const handleTraceLogNavigate = useCallback( + (rowId: string, aliasWith: WithClause[], label: string) => { + if (traceLogSourceId == null) { + return; + } + // Viewing a log already: its trace siblings live in this same source, so + // this is a row change, not a source hop. + if (traceLogSourceId === source.id) { + handleNavigateToRow(rowId, aliasWith, label, SourceKind.Log); + return; + } + handleSourceStackPush({ + sourceId: traceLogSourceId, + rowId, + aliasWith, + label, + sourceKind: SourceKind.Log, + // Every log in the trace sits within seconds of the span we came from. + focusTimestamp: rowFocusTimestamp, + }); + }, + [ + traceLogSourceId, + source.id, + handleNavigateToRow, + handleSourceStackPush, + rowFocusTimestamp, + ], + ); + // "Open trace" on a span link: the link carries only the linked span's // TraceId + SpanId, so resolve it against the current trace source. const handleOpenLinkedTrace = useCallback( @@ -786,6 +827,7 @@ export const DBRowSidePanelInner = ({ if (hasOverviewPanel && !sourceIsTrace) tabs.push(Tab.Overview); if (!sourceIsTrace) tabs.push(Tab.Parsed); if (sourceIsTrace) tabs.push(Tab.Trace); + if (enableTraceLogs) tabs.push(Tab.Logs); if (enableServiceMap) tabs.push(Tab.ServiceMap); tabs.push(Tab.Context); if (rumSessionId != null) tabs.push(Tab.Replay); @@ -794,6 +836,7 @@ export const DBRowSidePanelInner = ({ }, [ hasOverviewPanel, sourceIsTrace, + enableTraceLogs, enableServiceMap, rumSessionId, hasK8sContext, @@ -1020,6 +1063,14 @@ export const DBRowSidePanelInner = ({ }, ] : []), + ...(enableTraceLogs + ? [ + { + text: 'Trace logs', + value: Tab.Logs, + }, + ] + : []), ...(enableServiceMap ? [ { @@ -1097,6 +1148,29 @@ export const DBRowSidePanelInner = ({ )} + {displayedTab === Tab.Logs && traceLogSourceId && traceId && ( + { + console.error(err); + }} + fallbackRender={() => ( +
+ An error occurred while rendering this event. +
+ )} + > + + + )} {displayedTab === Tab.ServiceMap && enableServiceMap && ( { diff --git a/packages/app/src/components/DBRowSidePanel.types.ts b/packages/app/src/components/DBRowSidePanel.types.ts index 78bdc2942a..1af6ad9154 100644 --- a/packages/app/src/components/DBRowSidePanel.types.ts +++ b/packages/app/src/components/DBRowSidePanel.types.ts @@ -6,6 +6,7 @@ export enum Tab { Parsed = 'parsed', Debug = 'debug', Trace = 'trace', + Logs = 'logs', ServiceMap = 'serviceMap', Context = 'context', Replay = 'replay', diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx new file mode 100644 index 0000000000..e4bcdd6078 --- /dev/null +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -0,0 +1,228 @@ +import { use, useCallback, useMemo } from 'react'; +import { useForm } from 'react-hook-form'; +import SqlString from 'sqlstring'; +import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; +import { SourceKind } from '@hyperdx/common-utils/dist/types'; +import { Box, Flex } from '@mantine/core'; +import { IconLogs } from '@tabler/icons-react'; + +import EmptyState from '@/components/EmptyState'; +import SearchWhereInput, { + getStoredLanguage, +} from '@/components/SearchInput/SearchWhereInput'; +import { RowWhereResult, WithClause } from '@/hooks/useRowWhere'; +import useWaterfallSearchState from '@/hooks/useWaterfallSearchState'; +import { getEventBody, useSource } from '@/source'; + +import { + deriveRowSidePanelContextForSource, + RowSidePanelContext, +} from './DBRowSidePanel'; +import { DBSqlRowTable } from './DBRowTable'; + +/** + * Every log in one trace, as a flat chronological table. + * + * Sibling view to the waterfall's interleaved log rows: it shares the same + * `logWhere` URL state, so a filter set in either place applies to both, but + * it isn't bound to the span tree — which is what makes a log-heavy trace + * readable. + */ +export default function TraceLogsPanel({ + logSourceId, + traceId, + dateRange, + highlightedRowId, + onNavigateToLog, + 'data-testid': dataTestId, +}: { + logSourceId: string; + traceId: string; + dateRange: [Date, Date]; + /** Row id of the log this panel was opened from, when it is one of these logs. */ + highlightedRowId?: string; + onNavigateToLog: ( + rowId: string, + aliasWith: WithClause[], + label: string, + ) => void; + 'data-testid'?: string; +}) { + const { data: logSource, isLoading } = useSource({ + id: logSourceId, + kinds: [SourceKind.Log], + }); + + const { + logWhere, + logWhereLanguage, + onSubmit: submitFilters, + } = useWaterfallSearchState({ hasLogSource: true }); + + const logFilterLanguage: 'lucene' | 'sql' = + logWhereLanguage === 'sql' ? 'sql' : 'lucene'; + + const { control, handleSubmit, setValue } = useForm({ + defaultValues: { + logWhere: logWhere ?? '', + // Prefer the URL language so a shared link / reload shows the language + // the filter runs in; fall back to the stored preference. + logWhereLanguage: + logWhereLanguage === 'sql' || logWhereLanguage === 'lucene' + ? logWhereLanguage + : (getStoredLanguage() ?? 'lucene'), + }, + }); + + const onSubmitFilter = useCallback( + (data: { logWhere: string; logWhereLanguage: string }) => { + submitFilters({ + logWhere: data.logWhere, + logWhereLanguage: data.logWhereLanguage, + }); + }, + [submitFilters], + ); + + const traceIdExpression = logSource?.traceIdExpression; + + const config = useMemo(() => { + if (logSource == null || !traceIdExpression) { + return undefined; + } + return { + connection: logSource.connection, + from: logSource.from, + timestampValueExpression: logSource.timestampValueExpression, + implicitColumnExpression: logSource.implicitColumnExpression, + bodyExpression: logSource.bodyExpression, + useTextIndexForImplicitColumn: logSource.useTextIndexForImplicitColumn, + select: logSource.defaultTableSelectExpression ?? '', + // The trace scope goes in `filters` so the user's filter keeps the whole + // `where` slot to itself — the two run in independent languages. + filters: [ + { + type: 'sql' as const, + condition: SqlString.format('?=?', [ + SqlString.raw(traceIdExpression), + traceId, + ]), + }, + ], + where: logWhere ?? '', + whereLanguage: logFilterLanguage, + // Ascending: inside a trace, chronological order is execution order. + orderBy: `${logSource.timestampValueExpression} ASC`, + limit: { limit: 200 }, + dateRange, + }; + }, [ + logSource, + traceIdExpression, + traceId, + logWhere, + logFilterLanguage, + dateRange, + ]); + + const handleRowDetailsClick = useCallback( + (rowWhere: RowWhereResult, row: Record) => { + // The table selects the source's own columns, so the body lands under + // its expression rather than a normalized alias. + const bodyExpression = logSource ? getEventBody(logSource) : undefined; + const body = bodyExpression ? row[bodyExpression] : undefined; + onNavigateToLog( + rowWhere.where, + rowWhere.aliasWith, + typeof body === 'string' && body.length > 0 ? body : 'Log', + ); + }, + [logSource, onNavigateToLog], + ); + + // The logs belong to a different source than the search this panel was + // opened from, so rebind search-url generation to them and drop the + // filter/column actions that only make sense against the searched source. + const parentContext = use(RowSidePanelContext); + const rowSidePanelContextValue = useMemo( + () => + logSource + ? deriveRowSidePanelContextForSource(parentContext, logSource) + : parentContext, + [parentContext, logSource], + ); + + if (isLoading) { + return null; + } + + if (logSource == null) { + return ( + } + title="Correlated log source not found" + description="The log source linked to this trace source could not be loaded. Pick another one in the trace source's settings." + variant="card" + m="sm" + /> + ); + } + + if (config == null) { + return ( + } + title="No trace ID column configured" + description={`Set a trace ID expression on the "${logSource.name}" source to correlate its logs with traces.`} + variant="card" + m="sm" + /> + ); + } + + return ( + + + +
+ + setValue('logWhereLanguage', lang, { shouldDirty: true }) + } + lucenePlaceholder='Filter logs ex. SeverityText:"error"' + sqlPlaceholder="Filter logs ex. SeverityText = 'error'" + data-testid="trace-logs-search-input" + /> + +
+
+ +
+ + + ); +} diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx new file mode 100644 index 0000000000..df50a1652d --- /dev/null +++ b/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx @@ -0,0 +1,330 @@ +import React from 'react'; +import { TSource } from '@hyperdx/common-utils/dist/types'; +import { MantineProvider } from '@mantine/core'; +import { act, render } from '@testing-library/react'; + +// In-memory nuqs so the active tab can be seeded and the stack setters +// inspected. Prefixed with `mock` so jest.mock factories 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(); +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: () => null, + getJSONColumnNames: () => [], + getMapColumnNames: () => [], +})); + +const TRACE_SOURCE_WITH_LOGS = { + id: 'trace-src', + kind: 'trace', + traceIdExpression: 'TraceId', + spanIdExpression: 'SpanId', + logSourceId: 'log-src', + timestampValueExpression: 'Timestamp', + resourceAttributesExpression: 'ResourceAttributes', +}; + +const LOG_SOURCE = { + id: 'log-src', + kind: 'log', + traceSourceId: 'trace-src', + traceIdExpression: 'TraceId', + timestampValueExpression: 'Timestamp', + resourceAttributesExpression: 'ResourceAttributes', +}; + +jest.mock('@/source', () => ({ + __esModule: true, + getEventBody: () => '__hdx_body', + useSource: ({ id }: { id: string | null }) => + id === 'trace-src' + ? { data: TRACE_SOURCE_WITH_LOGS } + : id === 'log-src' + ? { data: LOG_SOURCE } + : { data: undefined }, +})); + +type TabBarProps = { + items?: { text: string; value: string }[]; + activeItem?: unknown; + onClick?: (v: unknown) => void; +}; +const mockTabBarProps: { current: TabBarProps } = { current: {} }; +jest.mock('@/TabBar', () => ({ + __esModule: true, + default: (props: TabBarProps) => { + mockTabBarProps.current = props; + return null; + }, +})); + +type TraceLogsPanelProps = { + logSourceId?: string; + traceId?: string; + highlightedRowId?: string; + onNavigateToLog?: ( + rowId: string, + aliasWith: unknown[], + label: string, + ) => void; +}; +const mockTraceLogsProps: { current: TraceLogsPanelProps } = { current: {} }; +jest.mock('../TraceLogsPanel', () => ({ + __esModule: true, + default: (props: TraceLogsPanelProps) => { + mockTraceLogsProps.current = props; + return null; + }, +})); + +jest.mock('../DBSessionPanel', () => ({ + __esModule: true, + useSessionId: () => ({ rumSessionId: undefined, rumServiceName: undefined }), + DBSessionPanel: () => null, +})); +jest.mock('@/utils/highlightedAttributes', () => ({ + __esModule: true, + getHighlightedAttributesFromData: () => [], +})); +jest.mock('../DBTracePanel', () => ({ __esModule: true, default: () => null })); +jest.mock('../ContextSidePanel', () => ({ + __esModule: true, + default: () => null, +})); +jest.mock('../DBInfraPanel', () => ({ __esModule: true, default: () => null })); +jest.mock('../DBRowOverviewPanel', () => ({ + __esModule: true, + RowOverviewPanel: () => 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('@/useFormatTime', () => ({ + __esModule: true, + FormatTime: () => null, +})); + +// NOTE: imported after the mock factories above. +import { DBRowSidePanelInner } from '@/components/DBRowSidePanel'; +import { Tab } from '@/components/DBRowSidePanel.types'; +import useSidePanelStack from '@/hooks/useSidePanelStack'; + +const TRACE_ID = '7316d5a2ab0dc2efa72258f64a98a405'; +const SPAN_ID = 'e3748131832d6176'; +const TIMESTAMP_VALUE = '2024-05-01T10:00:00.123456789Z'; + +function rowResult(row: Record) { + return { + data: { + data: [row], + meta: [{ name: '__hdx_timestamp_value_0', type: 'DateTime64(9)' }], + }, + isLoading: false, + isSuccess: true, + isError: false, + error: null, + }; +} + +const ROW_WITH_TRACE = { + __hdx_trace_id: TRACE_ID, + __hdx_span_id: SPAN_ID, + __hdx_timestamp: TIMESTAMP_VALUE, + __hdx_timestamp_value_0: TIMESTAMP_VALUE, + __hdx_body: 'a log line', +}; + +function renderPanel(source: TSource, rowId = 'row-1') { + function Harness() { + const sidePanelStack = useSidePanelStack({ initialRowId: rowId }); + return ( + + ); + } + return render( + + + , + ); +} + +function tabValues() { + return (mockTabBarProps.current.items ?? []).map(item => item.value); +} + +function clickLogRow(rowId: string, label = 'another log line') { + act(() => mockTraceLogsProps.current.onNavigateToLog?.(rowId, [], label)); +} + +describe('DBRowSidePanelInner — trace logs tab', () => { + beforeEach(() => { + resetQueryState(); + localStorage.clear(); + mockTabBarProps.current = {}; + mockTraceLogsProps.current = {}; + mockUseRowData.mockReset(); + mockUseRowData.mockReturnValue(rowResult(ROW_WITH_TRACE)); + }); + + it('offers the tab on a span, pointed at the correlated log source', () => { + mockQueryStore.sidePanelTab = Tab.Logs; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + renderPanel(TRACE_SOURCE_WITH_LOGS as TSource); + + expect(tabValues()).toContain(Tab.Logs); + expect(mockTraceLogsProps.current.logSourceId).toBe('log-src'); + expect(mockTraceLogsProps.current.traceId).toBe(TRACE_ID); + // Nothing to highlight: the row on screen is a span, not one of these logs. + expect(mockTraceLogsProps.current.highlightedRowId).toBeUndefined(); + }); + + it('offers the tab on a log, pointed at its own source and highlighting it', () => { + mockQueryStore.sidePanelTab = Tab.Logs; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + renderPanel(LOG_SOURCE as TSource); + + expect(tabValues()).toContain(Tab.Logs); + expect(mockTraceLogsProps.current.logSourceId).toBe('log-src'); + expect(mockTraceLogsProps.current.highlightedRowId).toBe('row-1'); + }); + + it('hides the tab when the trace source has no correlated log source', () => { + const traceSourceWithoutLogs = { + ...TRACE_SOURCE_WITH_LOGS, + logSourceId: undefined, + }; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + renderPanel(traceSourceWithoutLogs as TSource); + + expect(tabValues()).not.toContain(Tab.Logs); + }); + + it('hides the tab when the row carries no trace id', () => { + mockUseRowData.mockReturnValue( + rowResult({ ...ROW_WITH_TRACE, __hdx_trace_id: '' }), + ); + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + renderPanel(LOG_SOURCE as TSource); + + expect(tabValues()).not.toContain(Tab.Logs); + }); + + it('pushes a cross-source frame when a log is picked from a span', () => { + mockQueryStore.sidePanelTab = Tab.Logs; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + renderPanel(TRACE_SOURCE_WITH_LOGS as TSource); + + clickLogRow("TraceId='abc' AND SpanId='def'"); + + const frame = setterFor('sidePanelSourceStack').mock.calls[0][0][0]; + expect(frame).toMatchObject({ + sourceId: 'log-src', + rowId: "TraceId='abc' AND SpanId='def'", + label: 'another log line', + sourceKind: 'log', + // Anchors the destination's row lookup: every log in a trace sits within + // seconds of the span we came from. + focusTimestamp: new Date(TIMESTAMP_VALUE).toISOString(), + }); + // A source hop resets the same-source drilldown trail rather than adding + // to it. + expect(setterFor('sidePanelNavStack')).toHaveBeenCalledWith([]); + }); + + it('stays in the same source when a log is picked from another log', () => { + mockQueryStore.sidePanelTab = Tab.Logs; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + renderPanel(LOG_SOURCE as TSource); + + clickLogRow('row-2'); + + expect(setterFor('sidePanelNavStack').mock.calls[0][0][0]).toMatchObject({ + rowId: 'row-2', + label: 'another log line', + sourceKind: 'log', + }); + // No source hop: the cross-source trail is rewritten unchanged (empty). + expect(setterFor('sidePanelSourceStack')).toHaveBeenCalledWith([]); + }); + + it('lands the picked log on a reading view, not on another logs list', () => { + mockQueryStore.sidePanelTab = Tab.Logs; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + renderPanel(LOG_SOURCE as TSource); + + clickLogRow('row-2'); + + expect(setterFor('sidePanelTab')).toHaveBeenCalledWith(Tab.Overview); + }); +}); diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx new file mode 100644 index 0000000000..33ca43d17f --- /dev/null +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -0,0 +1,170 @@ +import React from 'react'; +import { TLogSource } from '@hyperdx/common-utils/dist/types'; +import { MantineProvider } from '@mantine/core'; +import { act, render, screen } from '@testing-library/react'; + +const mockQueryStore: Record = {}; +jest.mock('nuqs', () => { + const actual = jest.requireActual('nuqs'); + return { + ...actual, + useQueryState: (key: string) => [mockQueryStore[key] ?? null, jest.fn()], + }; +}); + +type RowTableProps = { + config?: Record; + sourceId?: string; + highlightedLineId?: string; + onRowDetailsClick?: ( + rowWhere: { where: string; aliasWith: unknown[] }, + row: Record, + ) => void; +}; +const mockRowTableProps: { current: RowTableProps } = { current: {} }; +jest.mock('../DBRowTable', () => ({ + __esModule: true, + DBSqlRowTable: (props: RowTableProps) => { + mockRowTableProps.current = props; + return null; + }, +})); + +jest.mock('../SearchInput/SearchWhereInput', () => ({ + __esModule: true, + default: () => null, + getStoredLanguage: () => 'lucene', +})); + +// A real context so `use(RowSidePanelContext)` works without pulling in the +// side panel (which imports this component back). +jest.mock('../DBRowSidePanel', () => { + const react = jest.requireActual('react'); + return { + __esModule: true, + RowSidePanelContext: react.createContext({}), + deriveRowSidePanelContextForSource: (parent: unknown) => parent, + }; +}); + +const mockUseSource = jest.fn(); +jest.mock('@/source', () => ({ + __esModule: true, + useSource: (args: unknown) => mockUseSource(args), + getEventBody: (source: { bodyExpression?: string }) => source.bodyExpression, +})); + +// NOTE: imported after the mock factories above. +import TraceLogsPanel from '@/components/TraceLogsPanel'; + +const TRACE_ID = '7316d5a2ab0dc2efa72258f64a98a405'; +const DATE_RANGE: [Date, Date] = [ + new Date('2024-05-01T09:00:00Z'), + new Date('2024-05-01T11:00:00Z'), +]; + +// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion +const LOG_SOURCE = { + id: 'log-src', + kind: 'log', + name: 'Demo logs', + connection: 'conn', + from: { databaseName: 'default', tableName: 'otel_logs' }, + timestampValueExpression: 'Timestamp', + defaultTableSelectExpression: 'Timestamp, ServiceName, SeverityText, Body', + bodyExpression: 'Body', + implicitColumnExpression: 'Body', + traceIdExpression: 'TraceId', +} as TLogSource; + +function renderPanel({ + source = LOG_SOURCE, + onNavigateToLog = jest.fn(), +}: { + source?: TLogSource; + onNavigateToLog?: jest.Mock; +} = {}) { + mockUseSource.mockReturnValue({ data: source, isLoading: false }); + render( + + + , + ); + return { onNavigateToLog }; +} + +describe('TraceLogsPanel', () => { + beforeEach(() => { + Object.keys(mockQueryStore).forEach(k => delete mockQueryStore[k]); + mockRowTableProps.current = {}; + mockUseSource.mockReset(); + }); + + it('scopes the table to the trace without spending the where clause', () => { + renderPanel(); + + const config = mockRowTableProps.current.config!; + expect(config.filters).toEqual([ + { type: 'sql', condition: `TraceId='${TRACE_ID}'` }, + ]); + expect(config.where).toBe(''); + expect(config.select).toBe(LOG_SOURCE.defaultTableSelectExpression); + // Chronological: inside a trace that is execution order. + expect(config.orderBy).toBe('Timestamp ASC'); + expect(config.dateRange).toBe(DATE_RANGE); + }); + + it('applies the waterfall log filter and its language from the URL', () => { + mockQueryStore.logWhere = "SeverityText = 'error'"; + mockQueryStore.logWhereLanguage = 'sql'; + renderPanel(); + + const config = mockRowTableProps.current.config!; + expect(config.where).toBe("SeverityText = 'error'"); + expect(config.whereLanguage).toBe('sql'); + }); + + it("labels a picked row with the log's body", () => { + const { onNavigateToLog } = renderPanel(); + + act(() => + mockRowTableProps.current.onRowDetailsClick?.( + { where: 'row-2', aliasWith: [] }, + { Body: 'upstream timeout' }, + ), + ); + + expect(onNavigateToLog).toHaveBeenCalledWith( + 'row-2', + [], + 'upstream timeout', + ); + }); + + it('falls back to a generic label when the body column is not selected', () => { + const { onNavigateToLog } = renderPanel(); + + act(() => + mockRowTableProps.current.onRowDetailsClick?.( + { where: 'row-2', aliasWith: [] }, + { ServiceName: 'cart' }, + ), + ); + + expect(onNavigateToLog).toHaveBeenCalledWith('row-2', [], 'Log'); + }); + + it('explains itself instead of querying when the log source has no trace id column', () => { + renderPanel({ + source: { ...LOG_SOURCE, traceIdExpression: undefined } as TLogSource, + }); + + expect(screen.getByText('No trace ID column configured')).toBeVisible(); + expect(mockRowTableProps.current.config).toBeUndefined(); + }); +}); diff --git a/packages/app/src/hooks/useSidePanelStack.ts b/packages/app/src/hooks/useSidePanelStack.ts index 2e1c3d95f5..510c8fb032 100644 --- a/packages/app/src/hooks/useSidePanelStack.ts +++ b/packages/app/src/hooks/useSidePanelStack.ts @@ -16,10 +16,11 @@ export const LAST_TAB_STORAGE_KEY = 'hdx-side-panel-last-tab'; /** * Tabs that are a way to *find* another row rather than a way to read the * current one, so they never become the remembered preference. Opening - * Surrounding Context to pick out a neighbour shouldn't cost you the reading - * view you had chosen, nor strand you on another context list once you land. + * Surrounding Context or the trace's logs to pick out a row shouldn't cost you + * the reading view you had chosen, nor strand you on another list once you + * land. */ -const NAVIGATIONAL_TABS: readonly Tab[] = [Tab.Context]; +const NAVIGATIONAL_TABS: readonly Tab[] = [Tab.Context, Tab.Logs]; const EMPTY_SOURCE_STACK: SourceFrame[] = []; const EMPTY_NAV_STACK: NavEntry[] = []; diff --git a/packages/app/src/hooks/useWaterfallSearchState.tsx b/packages/app/src/hooks/useWaterfallSearchState.tsx index 552cdd9bf0..bcf0048fb2 100644 --- a/packages/app/src/hooks/useWaterfallSearchState.tsx +++ b/packages/app/src/hooks/useWaterfallSearchState.tsx @@ -17,15 +17,21 @@ export default function useWaterfallSearchState({ const [isFilterExpanded, setIsFilterExpanded] = useState(isFilterActive); + // Only the keys present are written, so a caller that owns one of the two + // filters (e.g. the trace Logs tab) can submit it without clearing the other. const onSubmit = useCallback( (data: { - traceWhere: string; - logWhere: string; + traceWhere?: string; + logWhere?: string; traceWhereLanguage?: string; logWhereLanguage?: string; }) => { - setTraceWhere(data.traceWhere || null); - setLogWhere(data.logWhere || null); + if (data.traceWhere !== undefined) { + setTraceWhere(data.traceWhere || null); + } + if (data.logWhere !== undefined) { + setLogWhere(data.logWhere || null); + } if (data.traceWhereLanguage !== undefined) { setTraceWhereLanguage(data.traceWhereLanguage || null); } From 0c59d60630b9705ff6f01e397ea9bd0f1ad311e8 Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Mon, 14 Sep 2026 00:24:11 -0700 Subject: [PATCH 02/18] fix: Address review on the trace logs tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The filter input re-seeds from the URL, so browser back/forward can't leave it showing a filter the table isn't running — or write the stale one back. - The query language now follows the URL alone, defaulting to Lucene, so a filter shared with the waterfall runs the same way in both places. The input shows that language; with no filter to interpret, the reader's stored editor preference still wins. - When the logs come from the source the panel was opened from, the table reuses the search's `select`. Its rows then carry the ids the highlight was built from (no hunting through pages for a row id that can never match), and the header's remove-column action lines up with the columns it mutates. Without that select the action is dropped, since it maps by index onto the search's own columns and would drop an unrelated one. - Tests: the partial-write guard in `useWaterfallSearchState`, `Tab.Logs` as a navigational tab (the panel-level test passed with that reverted), the language fallbacks, and both column paths. - Test fixtures use `as unknown as` rather than eslint-disable, which the escape-hatch ratchet counts. Declined: deriving the query window from the trace's extent. The tab gets the same ±1h window as the waterfall beside it, and narrowing to a trace's real extent needs an unbounded scan to discover that extent. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/trace-logs-side-panel-tab.md | 2 +- .../app/src/components/DBRowSidePanel.tsx | 14 +- .../app/src/components/TraceLogsPanel.tsx | 90 ++++++--- .../DBRowSidePanel.traceLogsTab.test.tsx | 42 ++--- .../__tests__/TraceLogsPanel.test.tsx | 178 ++++++++++++++++-- .../__tests__/useSidePanelStack.test.tsx | 31 +-- .../useWaterfallSearchState.test.tsx | 87 +++++++++ 7 files changed, 356 insertions(+), 88 deletions(-) create mode 100644 packages/app/src/hooks/__tests__/useWaterfallSearchState.test.tsx diff --git a/.changeset/trace-logs-side-panel-tab.md b/.changeset/trace-logs-side-panel-tab.md index 8282d4513d..ecbf1ee4a1 100644 --- a/.changeset/trace-logs-side-panel-tab.md +++ b/.changeset/trace-logs-side-panel-tab.md @@ -2,6 +2,6 @@ '@hyperdx/app': minor --- -Add a "Trace logs" tab to the event side panel, listing every log in the trace as a flat chronological table. It appears on any row that carries a trace id and resolves a log source — a span (via the trace source's correlated log source) or a log (its own source, with the row you came from highlighted). Previously the only route to a trace's logs was hunting for the green rows interleaved in the waterfall, which a log-heavy trace buries. +Add a "Trace logs" tab to the event side panel, listing the trace's logs as a flat chronological table over the same time window the waterfall uses. It appears on any row that carries a trace id and resolves a log source — a span (via the trace source's correlated log source) or a log (its own source, with the row you came from highlighted). Previously the only route to a trace's logs was hunting for the green rows interleaved in the waterfall, which a log-heavy trace buries. The tab shares the waterfall's log filter, so a filter set in either place applies to both, and picking a log opens it in the panel: a breadcrumb hop from a span, or a row change from another log. Sorting is ascending, which inside a trace is execution order. diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index 231e40dfb0..2e85cf02a7 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -559,6 +559,9 @@ export const DBRowSidePanelInner = ({ ? source.id : undefined; const enableTraceLogs = !!traceId && !!traceLogSourceId; + // The displayed row is itself one of the logs the tab lists, so picking one + // is a row change rather than a source hop, and the current row is in view. + const isOwnTraceLogSource = traceLogSourceId === source.id; const enableServiceMap = traceId && traceSourceId; @@ -627,9 +630,7 @@ export const DBRowSidePanelInner = ({ if (traceLogSourceId == null) { return; } - // Viewing a log already: its trace siblings live in this same source, so - // this is a row change, not a source hop. - if (traceLogSourceId === source.id) { + if (isOwnTraceLogSource) { handleNavigateToRow(rowId, aliasWith, label, SourceKind.Log); return; } @@ -645,7 +646,7 @@ export const DBRowSidePanelInner = ({ }, [ traceLogSourceId, - source.id, + isOwnTraceLogSource, handleNavigateToRow, handleSourceStackPush, rowFocusTimestamp, @@ -1164,8 +1165,9 @@ export const DBRowSidePanelInner = ({ logSourceId={traceLogSourceId} traceId={traceId} dateRange={oneHourRange} - highlightedRowId={ - traceLogSourceId === source.id ? activeRowId : undefined + highlightedRowId={isOwnTraceLogSource ? activeRowId : undefined} + searchTableConfig={ + isOwnTraceLogSource ? dbSqlRowTableConfig : undefined } onNavigateToLog={handleTraceLogNavigate} /> diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index e4bcdd6078..636cdf0af0 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -1,8 +1,11 @@ -import { use, useCallback, useMemo } from 'react'; +import { use, useCallback, useEffect, useMemo } from 'react'; import { useForm } from 'react-hook-form'; import SqlString from 'sqlstring'; import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; -import { SourceKind } from '@hyperdx/common-utils/dist/types'; +import { + BuilderChartConfigWithDateRange, + SourceKind, +} from '@hyperdx/common-utils/dist/types'; import { Box, Flex } from '@mantine/core'; import { IconLogs } from '@tabler/icons-react'; @@ -33,6 +36,7 @@ export default function TraceLogsPanel({ traceId, dateRange, highlightedRowId, + searchTableConfig, onNavigateToLog, 'data-testid': dataTestId, }: { @@ -41,6 +45,12 @@ export default function TraceLogsPanel({ dateRange: [Date, Date]; /** Row id of the log this panel was opened from, when it is one of these logs. */ highlightedRowId?: string; + /** + * The searched table's config, when this panel's logs come from that same + * source. Its `select` is reused so these rows carry the same ids the search + * built `highlightedRowId` from, and so the columns track the reader's. + */ + searchTableConfig?: BuilderChartConfigWithDateRange; onNavigateToLog: ( rowId: string, aliasWith: WithClause[], @@ -59,21 +69,37 @@ export default function TraceLogsPanel({ onSubmit: submitFilters, } = useWaterfallSearchState({ hasLogSource: true }); - const logFilterLanguage: 'lucene' | 'sql' = - logWhereLanguage === 'sql' ? 'sql' : 'lucene'; + const urlLanguage = + logWhereLanguage === 'sql' || logWhereLanguage === 'lucene' + ? logWhereLanguage + : undefined; + + // The query language follows the URL alone, defaulting to Lucene, so a filter + // shared with the waterfall runs the same way in both places even if a link + // carries `logWhere` without its language. + const logFilterLanguage: 'lucene' | 'sql' = urlLanguage ?? 'lucene'; + + // The input shows the language the filter actually runs in. With no filter + // there is nothing to interpret, so the user's stored editor preference wins. + const displayedLanguage = + urlLanguage ?? (logWhere ? 'lucene' : (getStoredLanguage() ?? 'lucene')); - const { control, handleSubmit, setValue } = useForm({ + const { control, handleSubmit, setValue, reset } = useForm({ defaultValues: { logWhere: logWhere ?? '', - // Prefer the URL language so a shared link / reload shows the language - // the filter runs in; fall back to the stored preference. - logWhereLanguage: - logWhereLanguage === 'sql' || logWhereLanguage === 'lucene' - ? logWhereLanguage - : (getStoredLanguage() ?? 'lucene'), + logWhereLanguage: displayedLanguage, }, }); + // The filter is URL state, which can change without this form touching it + // (browser back/forward, a shared link). Re-seed the input so what's on + // screen is what the table is querying — and so a submit can't write a stale + // value back over the restored one. Submitting is a no-op here: it moves the + // URL to what the form already holds. + useEffect(() => { + reset({ logWhere: logWhere ?? '', logWhereLanguage: displayedLanguage }); + }, [logWhere, displayedLanguage, reset]); + const onSubmitFilter = useCallback( (data: { logWhere: string; logWhereLanguage: string }) => { submitFilters({ @@ -90,14 +116,7 @@ export default function TraceLogsPanel({ if (logSource == null || !traceIdExpression) { return undefined; } - return { - connection: logSource.connection, - from: logSource.from, - timestampValueExpression: logSource.timestampValueExpression, - implicitColumnExpression: logSource.implicitColumnExpression, - bodyExpression: logSource.bodyExpression, - useTextIndexForImplicitColumn: logSource.useTextIndexForImplicitColumn, - select: logSource.defaultTableSelectExpression ?? '', + const scope = { // The trace scope goes in `filters` so the user's filter keeps the whole // `where` slot to itself — the two run in independent languages. filters: [ @@ -116,6 +135,19 @@ export default function TraceLogsPanel({ limit: { limit: 200 }, dateRange, }; + if (searchTableConfig != null) { + return { ...searchTableConfig, ...scope }; + } + return { + connection: logSource.connection, + from: logSource.from, + timestampValueExpression: logSource.timestampValueExpression, + implicitColumnExpression: logSource.implicitColumnExpression, + bodyExpression: logSource.bodyExpression, + useTextIndexForImplicitColumn: logSource.useTextIndexForImplicitColumn, + select: logSource.defaultTableSelectExpression ?? '', + ...scope, + }; }, [ logSource, traceIdExpression, @@ -123,6 +155,7 @@ export default function TraceLogsPanel({ logWhere, logFilterLanguage, dateRange, + searchTableConfig, ]); const handleRowDetailsClick = useCallback( @@ -144,13 +177,18 @@ export default function TraceLogsPanel({ // opened from, so rebind search-url generation to them and drop the // filter/column actions that only make sense against the searched source. const parentContext = use(RowSidePanelContext); - const rowSidePanelContextValue = useMemo( - () => - logSource - ? deriveRowSidePanelContextForSource(parentContext, logSource) - : parentContext, - [parentContext, logSource], - ); + const rowSidePanelContextValue = useMemo(() => { + const derived = logSource + ? deriveRowSidePanelContextForSource(parentContext, logSource) + : parentContext; + if (searchTableConfig != null) { + return derived; + } + // These columns are this table's own, not the searched table's, so the + // header's remove-column action would drop whatever column sits at that + // index in the *search* results. Take it away. + return { ...derived, displayedColumns: undefined, toggleColumn: undefined }; + }, [parentContext, logSource, searchTableConfig]); if (isLoading) { return null; diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx index df50a1652d..e644793a5a 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx @@ -52,7 +52,12 @@ jest.mock('../DBRowDataPanel', () => ({ getMapColumnNames: () => [], })); -const TRACE_SOURCE_WITH_LOGS = { +// Test fixtures carry only the fields the panel reads; TSource's full shape is +// irrelevant here and spelling it out would bury the gating under boilerplate. +const asSource = (source: Record) => + source as unknown as TSource; + +const TRACE_SOURCE_WITH_LOGS = asSource({ id: 'trace-src', kind: 'trace', traceIdExpression: 'TraceId', @@ -60,16 +65,16 @@ const TRACE_SOURCE_WITH_LOGS = { logSourceId: 'log-src', timestampValueExpression: 'Timestamp', resourceAttributesExpression: 'ResourceAttributes', -}; +}); -const LOG_SOURCE = { +const LOG_SOURCE = asSource({ id: 'log-src', kind: 'log', traceSourceId: 'trace-src', traceIdExpression: 'TraceId', timestampValueExpression: 'Timestamp', resourceAttributesExpression: 'ResourceAttributes', -}; +}); jest.mock('@/source', () => ({ __esModule: true, @@ -239,8 +244,7 @@ describe('DBRowSidePanelInner — trace logs tab', () => { it('offers the tab on a span, pointed at the correlated log source', () => { mockQueryStore.sidePanelTab = Tab.Logs; - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - renderPanel(TRACE_SOURCE_WITH_LOGS as TSource); + renderPanel(TRACE_SOURCE_WITH_LOGS); expect(tabValues()).toContain(Tab.Logs); expect(mockTraceLogsProps.current.logSourceId).toBe('log-src'); @@ -251,8 +255,7 @@ describe('DBRowSidePanelInner — trace logs tab', () => { it('offers the tab on a log, pointed at its own source and highlighting it', () => { mockQueryStore.sidePanelTab = Tab.Logs; - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - renderPanel(LOG_SOURCE as TSource); + renderPanel(LOG_SOURCE); expect(tabValues()).toContain(Tab.Logs); expect(mockTraceLogsProps.current.logSourceId).toBe('log-src'); @@ -260,12 +263,9 @@ describe('DBRowSidePanelInner — trace logs tab', () => { }); it('hides the tab when the trace source has no correlated log source', () => { - const traceSourceWithoutLogs = { - ...TRACE_SOURCE_WITH_LOGS, - logSourceId: undefined, - }; - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - renderPanel(traceSourceWithoutLogs as TSource); + renderPanel( + asSource({ ...TRACE_SOURCE_WITH_LOGS, logSourceId: undefined }), + ); expect(tabValues()).not.toContain(Tab.Logs); }); @@ -274,16 +274,14 @@ describe('DBRowSidePanelInner — trace logs tab', () => { mockUseRowData.mockReturnValue( rowResult({ ...ROW_WITH_TRACE, __hdx_trace_id: '' }), ); - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - renderPanel(LOG_SOURCE as TSource); + renderPanel(LOG_SOURCE); expect(tabValues()).not.toContain(Tab.Logs); }); it('pushes a cross-source frame when a log is picked from a span', () => { mockQueryStore.sidePanelTab = Tab.Logs; - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - renderPanel(TRACE_SOURCE_WITH_LOGS as TSource); + renderPanel(TRACE_SOURCE_WITH_LOGS); clickLogRow("TraceId='abc' AND SpanId='def'"); @@ -304,8 +302,7 @@ describe('DBRowSidePanelInner — trace logs tab', () => { it('stays in the same source when a log is picked from another log', () => { mockQueryStore.sidePanelTab = Tab.Logs; - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - renderPanel(LOG_SOURCE as TSource); + renderPanel(LOG_SOURCE); clickLogRow('row-2'); @@ -318,10 +315,11 @@ describe('DBRowSidePanelInner — trace logs tab', () => { expect(setterFor('sidePanelSourceStack')).toHaveBeenCalledWith([]); }); + // That Tab.Logs never *becomes* the remembered reading view is pinned in + // hooks/__tests__/useSidePanelStack.test.tsx; this covers the wiring. it('lands the picked log on a reading view, not on another logs list', () => { mockQueryStore.sidePanelTab = Tab.Logs; - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - renderPanel(LOG_SOURCE as TSource); + renderPanel(LOG_SOURCE); clickLogRow('row-2'); diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx index 33ca43d17f..37e27800a6 100644 --- a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -1,5 +1,9 @@ import React from 'react'; -import { TLogSource } from '@hyperdx/common-utils/dist/types'; +import type { Control } from 'react-hook-form'; +import { + BuilderChartConfigWithDateRange, + TLogSource, +} from '@hyperdx/common-utils/dist/types'; import { MantineProvider } from '@mantine/core'; import { act, render, screen } from '@testing-library/react'; @@ -22,27 +26,69 @@ type RowTableProps = { ) => void; }; const mockRowTableProps: { current: RowTableProps } = { current: {} }; -jest.mock('../DBRowTable', () => ({ - __esModule: true, - DBSqlRowTable: (props: RowTableProps) => { - mockRowTableProps.current = props; - return null; - }, -})); +const mockRowTableContext: { current: Record } = { + current: {}, +}; +jest.mock('../DBRowTable', () => { + const react = jest.requireActual('react'); + const panel = jest.requireMock<{ + RowSidePanelContext: React.Context>; + }>('../DBRowSidePanel'); + return { + __esModule: true, + DBSqlRowTable: (props: RowTableProps) => { + mockRowTableProps.current = props; + // The real table reads these off the context, not its props. + mockRowTableContext.current = react.use(panel.RowSidePanelContext); + return null; + }, + }; +}); -jest.mock('../SearchInput/SearchWhereInput', () => ({ - __esModule: true, - default: () => null, - getStoredLanguage: () => 'lucene', -})); +type FilterForm = { logWhere: string; logWhereLanguage: string }; + +let mockStoredLanguage = 'lucene'; + +jest.mock('../SearchInput/SearchWhereInput', () => { + const rhf = + jest.requireActual('react-hook-form'); + return { + __esModule: true, + default: ({ + control, + name, + }: { + control: Control; + name: keyof FilterForm; + }) => { + // Surface the form's live value so a test can see what the user would. + const value = rhf.useWatch({ control, name }); + const language = rhf.useWatch({ control, name: 'logWhereLanguage' }); + return ( + <> + {String(value ?? '')} + + {String(language ?? '')} + + + ); + }, + getStoredLanguage: () => mockStoredLanguage, + }; +}); // A real context so `use(RowSidePanelContext)` works without pulling in the // side panel (which imports this component back). +const mockToggleColumn = jest.fn(); jest.mock('../DBRowSidePanel', () => { const react = jest.requireActual('react'); return { __esModule: true, - RowSidePanelContext: react.createContext({}), + RowSidePanelContext: react.createContext({ + // What the search page hands down: its own columns and their toggle. + displayedColumns: ['Timestamp', 'Body'], + toggleColumn: mockToggleColumn, + }), deriveRowSidePanelContextForSource: (parent: unknown) => parent, }; }); @@ -63,8 +109,11 @@ const DATE_RANGE: [Date, Date] = [ new Date('2024-05-01T11:00:00Z'), ]; -// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -const LOG_SOURCE = { +// Only the fields the panel reads; the rest of TLogSource is irrelevant here. +const asLogSource = (source: Record) => + source as unknown as TLogSource; + +const LOG_SOURCE = asLogSource({ id: 'log-src', kind: 'log', name: 'Demo logs', @@ -75,27 +124,32 @@ const LOG_SOURCE = { bodyExpression: 'Body', implicitColumnExpression: 'Body', traceIdExpression: 'TraceId', -} as TLogSource; +}); function renderPanel({ source = LOG_SOURCE, onNavigateToLog = jest.fn(), + searchTableConfig, }: { source?: TLogSource; onNavigateToLog?: jest.Mock; + searchTableConfig?: BuilderChartConfigWithDateRange; } = {}) { mockUseSource.mockReturnValue({ data: source, isLoading: false }); - render( + // A fresh element per render: React bails out of re-rendering an identical one. + const tree = () => ( - , + ); - return { onNavigateToLog }; + const { rerender } = render(tree()); + return { onNavigateToLog, rerender: () => rerender(tree()) }; } describe('TraceLogsPanel', () => { @@ -103,6 +157,9 @@ describe('TraceLogsPanel', () => { Object.keys(mockQueryStore).forEach(k => delete mockQueryStore[k]); mockRowTableProps.current = {}; mockUseSource.mockReset(); + mockRowTableContext.current = {}; + mockToggleColumn.mockReset(); + mockStoredLanguage = 'lucene'; }); it('scopes the table to the trace without spending the where clause', () => { @@ -129,6 +186,87 @@ describe('TraceLogsPanel', () => { expect(config.whereLanguage).toBe('sql'); }); + it('runs a URL filter that carries no language as Lucene, as the waterfall does', () => { + // A SQL editor preference must not reinterpret a filter the URL left + // unlabelled — the waterfall would run it as Lucene. + mockStoredLanguage = 'sql'; + mockQueryStore.logWhere = 'SeverityText:"error"'; + renderPanel(); + + expect(mockRowTableProps.current.config?.whereLanguage).toBe('lucene'); + expect(screen.getByTestId('log-filter-language')).toHaveTextContent( + 'lucene', + ); + }); + + it('offers the stored editor preference when there is no filter to interpret', () => { + mockStoredLanguage = 'sql'; + renderPanel(); + + expect(screen.getByTestId('log-filter-language')).toHaveTextContent('sql'); + }); + + describe('when the logs come from the searched source', () => { + const SEARCH_CONFIG: BuilderChartConfigWithDateRange = { + connection: 'conn', + from: { databaseName: 'default', tableName: 'otel_logs' }, + timestampValueExpression: 'Timestamp', + // The reader's chosen columns, which differ from the source default. + select: 'Timestamp, Body', + where: 'ServiceName:"cart"', + whereLanguage: 'lucene', + filters: [{ type: 'sql' as const, condition: "ServiceName = 'cart'" }], + orderBy: 'Timestamp DESC', + dateRange: [new Date(0), new Date(1)] as [Date, Date], + }; + + it("reuses the search's columns so row ids match the highlighted row", () => { + renderPanel({ searchTableConfig: SEARCH_CONFIG }); + + const config = mockRowTableProps.current.config!; + expect(config.select).toBe('Timestamp, Body'); + // ...while the trace scope and ordering still come from this panel. + expect(config.filters).toEqual([ + { type: 'sql', condition: `TraceId='${TRACE_ID}'` }, + ]); + expect(config.where).toBe(''); + expect(config.orderBy).toBe('Timestamp ASC'); + expect(config.dateRange).toBe(DATE_RANGE); + }); + + it('leaves the remove-column action in place, since the columns are shared', () => { + renderPanel({ searchTableConfig: SEARCH_CONFIG }); + + expect(mockRowTableContext.current.toggleColumn).toBe(mockToggleColumn); + }); + }); + + it('drops the remove-column action when showing its own columns', () => { + // Otherwise the header's × maps by index onto the *search* table's select + // and drops an unrelated column from the search results. + renderPanel(); + + expect(mockRowTableContext.current.toggleColumn).toBeUndefined(); + expect(mockRowTableContext.current.displayedColumns).toBeUndefined(); + }); + + it('re-seeds the filter input when the URL filter changes underneath it', () => { + mockQueryStore.logWhere = 'SeverityText:"error"'; + const { rerender } = renderPanel(); + expect(screen.getByTestId('log-filter-value')).toHaveTextContent( + 'SeverityText:"error"', + ); + + // Browser back restores an earlier filter without this form touching it. + mockQueryStore.logWhere = 'ServiceName:"cart"'; + act(() => rerender()); + + expect(screen.getByTestId('log-filter-value')).toHaveTextContent( + 'ServiceName:"cart"', + ); + expect(mockRowTableProps.current.config?.where).toBe('ServiceName:"cart"'); + }); + it("labels a picked row with the log's body", () => { const { onNavigateToLog } = renderPanel(); diff --git a/packages/app/src/hooks/__tests__/useSidePanelStack.test.tsx b/packages/app/src/hooks/__tests__/useSidePanelStack.test.tsx index 5f2580e4fa..f59d3d19b6 100644 --- a/packages/app/src/hooks/__tests__/useSidePanelStack.test.tsx +++ b/packages/app/src/hooks/__tests__/useSidePanelStack.test.tsx @@ -438,19 +438,24 @@ describe('useSidePanelStack', () => { expect(readLastTab()).toBe(Tab.Parsed); }); - it('does not remember a navigational tab as a reading view', () => { - const { result } = renderHook(() => - useSidePanelStack({ initialRowId: 'root-1' }), - ); - act(() => result.current.setTab(Tab.Parsed)); - // Opening Surrounding Context to hunt for a neighbouring row must not cost - // the reader the view they had chosen. - act(() => result.current.setTab(Tab.Context)); - - expect(readLastTab()).toBe(Tab.Parsed); - // It is still the active tab for the current row, just not remembered. - expect(setterFor('sidePanelTab')).toHaveBeenLastCalledWith(Tab.Context); - }); + it.each([Tab.Context, Tab.Logs])( + 'does not remember %s as a reading view', + navigationalTab => { + const { result } = renderHook(() => + useSidePanelStack({ initialRowId: 'root-1' }), + ); + act(() => result.current.setTab(Tab.Parsed)); + // Opening Surrounding Context or the trace's logs to hunt for a row + // must not cost the reader the view they had chosen. + act(() => result.current.setTab(navigationalTab)); + + expect(readLastTab()).toBe(Tab.Parsed); + // It is still the active tab for the current row, just not remembered. + expect(setterFor('sidePanelTab')).toHaveBeenLastCalledWith( + navigationalTab, + ); + }, + ); it('exposes the reading view separately from where the URL points', () => { seedLastTab(Tab.Parsed); diff --git a/packages/app/src/hooks/__tests__/useWaterfallSearchState.test.tsx b/packages/app/src/hooks/__tests__/useWaterfallSearchState.test.tsx new file mode 100644 index 0000000000..4e279f3ebd --- /dev/null +++ b/packages/app/src/hooks/__tests__/useWaterfallSearchState.test.tsx @@ -0,0 +1,87 @@ +import { renderHook } from '@testing-library/react'; + +const mockSetters: Record = {}; +function setterFor(key: string) { + if (!mockSetters[key]) mockSetters[key] = jest.fn(); + return mockSetters[key]; +} + +jest.mock('nuqs', () => { + const actual = jest.requireActual('nuqs'); + return { + ...actual, + useQueryState: (key: string) => { + if (!mockSetters[key]) mockSetters[key] = jest.fn(); + return [null, mockSetters[key]]; + }, + }; +}); + +// NOTE: imported after the mock factory above. +import useWaterfallSearchState from '@/hooks/useWaterfallSearchState'; + +describe('useWaterfallSearchState onSubmit', () => { + beforeEach(() => { + Object.keys(mockSetters).forEach(k => delete mockSetters[k]); + }); + + it('writes only the log filter when only the log filter is submitted', () => { + const { result } = renderHook(() => + useWaterfallSearchState({ hasLogSource: true }), + ); + + result.current.onSubmit({ + logWhere: 'SeverityText:"error"', + logWhereLanguage: 'lucene', + }); + + expect(setterFor('logWhere')).toHaveBeenCalledWith('SeverityText:"error"'); + expect(setterFor('logWhereLanguage')).toHaveBeenCalledWith('lucene'); + expect(setterFor('traceWhere')).not.toHaveBeenCalled(); + expect(setterFor('traceWhereLanguage')).not.toHaveBeenCalled(); + }); + + it('writes only the spans filter when only the spans filter is submitted', () => { + const { result } = renderHook(() => + useWaterfallSearchState({ hasLogSource: true }), + ); + + result.current.onSubmit({ traceWhere: "StatusCode = 'Error'" }); + + expect(setterFor('traceWhere')).toHaveBeenCalledWith( + "StatusCode = 'Error'", + ); + expect(setterFor('logWhere')).not.toHaveBeenCalled(); + }); + + it('clears a filter submitted as empty', () => { + const { result } = renderHook(() => + useWaterfallSearchState({ hasLogSource: true }), + ); + + result.current.onSubmit({ logWhere: '', logWhereLanguage: '' }); + + expect(setterFor('logWhere')).toHaveBeenCalledWith(null); + expect(setterFor('logWhereLanguage')).toHaveBeenCalledWith(null); + }); + + it('writes both filters when the waterfall submits its whole form', () => { + const { result } = renderHook(() => + useWaterfallSearchState({ hasLogSource: true }), + ); + + result.current.onSubmit({ + traceWhere: "StatusCode = 'Error'", + logWhere: 'SeverityText:"error"', + traceWhereLanguage: 'sql', + logWhereLanguage: 'lucene', + }); + + expect(setterFor('traceWhere')).toHaveBeenCalledWith( + "StatusCode = 'Error'", + ); + expect(setterFor('logWhere')).toHaveBeenCalledWith('SeverityText:"error"'); + expect(setterFor('traceWhereLanguage')).toHaveBeenCalledWith('sql'); + expect(setterFor('logWhereLanguage')).toHaveBeenCalledWith('lucene'); + }); +}); From f7740e8e4dbdeee6b979ec7a37de15389febaae6 Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Mon, 14 Sep 2026 00:28:54 -0700 Subject: [PATCH 03/18] fix: Type the trace logs row callback and its test mock `Record` for a clicked row's data was copied from DBSqlRowTable's callback signature; `unknown` carries it fine, since the only read is guarded by a typeof check. The test's mocked table now takes the real BuilderChartConfigWithDateRange instead of a loose record. Co-Authored-By: Claude Opus 5 (1M context) --- packages/app/src/components/TraceLogsPanel.tsx | 2 +- packages/app/src/components/__tests__/TraceLogsPanel.test.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index 636cdf0af0..23dd894c71 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -159,7 +159,7 @@ export default function TraceLogsPanel({ ]); const handleRowDetailsClick = useCallback( - (rowWhere: RowWhereResult, row: Record) => { + (rowWhere: RowWhereResult, row: Record) => { // The table selects the source's own columns, so the body lands under // its expression rather than a normalized alias. const bodyExpression = logSource ? getEventBody(logSource) : undefined; diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx index 37e27800a6..9231899e21 100644 --- a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -17,7 +17,7 @@ jest.mock('nuqs', () => { }); type RowTableProps = { - config?: Record; + config?: BuilderChartConfigWithDateRange; sourceId?: string; highlightedLineId?: string; onRowDetailsClick?: ( From c5dfd7978b028f1dd36fb31338f3fb1bad68dafe Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Mon, 14 Sep 2026 00:46:51 -0700 Subject: [PATCH 04/18] fix: Build the trace logs query through buildSearchChartConfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hand-rolled config restated connection/from/timestamp/select and so silently dropped what the shared builder adds: a log source's `tableFilterExpression`, sample weighting, and the `source` id that `useOffsetPaginatedQuery` resolves to apply the source's querySettings. A source configured to exclude rows excluded them on the search page and in alerts, but not in this tab. Reusing the searched table's config had the same hole from the other side — spreading it and overwriting `filters` discarded the table filter the search page puts first. One branch now, with the search's `select` passed as an override so the shared-columns case still produces comparable row ids. Also: - Re-seed the filter input from the URL only. The stored language preference changes the instant the reader flips the language switch, and keying the effect on it blanked a half-typed filter on the next render. - Pass `highlightedRowId` only alongside the search config it was built from. Only the search page provides one, so from any other opening table (a dashboard tile, a customized select) the id could never match a row here, leaving the table paging through the window hunting for it. Co-Authored-By: Claude Opus 5 (1M context) --- .../app/src/components/DBRowSidePanel.tsx | 18 +++-- .../app/src/components/TraceLogsPanel.tsx | 70 ++++++++++--------- .../DBRowSidePanel.traceLogsTab.test.tsx | 67 +++++++++++++++--- .../__tests__/TraceLogsPanel.test.tsx | 20 ++++++ 4 files changed, 128 insertions(+), 47 deletions(-) diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index 2e85cf02a7..e9f9d7e253 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -560,8 +560,15 @@ export const DBRowSidePanelInner = ({ : undefined; const enableTraceLogs = !!traceId && !!traceLogSourceId; // The displayed row is itself one of the logs the tab lists, so picking one - // is a row change rather than a source hop, and the current row is in view. + // is a row change rather than a source hop. const isOwnTraceLogSource = traceLogSourceId === source.id; + // Only the search page hands down a table config, and only at the root frame. + // When it's there, the tab can share the reader's columns; without it the tab + // falls back to the source's own, and nothing about the opening table carries + // over — including a comparable row id. + const traceLogsSearchConfig = isOwnTraceLogSource + ? dbSqlRowTableConfig + : undefined; const enableServiceMap = traceId && traceSourceId; @@ -1165,9 +1172,12 @@ export const DBRowSidePanelInner = ({ logSourceId={traceLogSourceId} traceId={traceId} dateRange={oneHourRange} - highlightedRowId={isOwnTraceLogSource ? activeRowId : undefined} - searchTableConfig={ - isOwnTraceLogSource ? dbSqlRowTableConfig : undefined + searchTableConfig={traceLogsSearchConfig} + // Row ids are built from the selected columns, so the id this + // panel was opened with is only comparable to the tab's rows when + // the tab is selecting the same columns. + highlightedRowId={ + traceLogsSearchConfig != null ? activeRowId : undefined } onNavigateToLog={handleTraceLogNavigate} /> diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index 23dd894c71..6baf2e2a5d 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -2,6 +2,7 @@ import { use, useCallback, useEffect, useMemo } from 'react'; import { useForm } from 'react-hook-form'; import SqlString from 'sqlstring'; import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; +import { buildSearchChartConfig } from '@hyperdx/common-utils/dist/core/searchChartConfig'; import { BuilderChartConfigWithDateRange, SourceKind, @@ -84,7 +85,7 @@ export default function TraceLogsPanel({ const displayedLanguage = urlLanguage ?? (logWhere ? 'lucene' : (getStoredLanguage() ?? 'lucene')); - const { control, handleSubmit, setValue, reset } = useForm({ + const { control, handleSubmit, setValue, reset, getValues } = useForm({ defaultValues: { logWhere: logWhere ?? '', logWhereLanguage: displayedLanguage, @@ -96,9 +97,16 @@ export default function TraceLogsPanel({ // screen is what the table is querying — and so a submit can't write a stale // value back over the restored one. Submitting is a no-op here: it moves the // URL to what the form already holds. + // + // Keyed on the URL alone: the stored language preference changes the moment + // the reader flips the language switch, and re-seeding on that would blank a + // filter they are halfway through typing. useEffect(() => { - reset({ logWhere: logWhere ?? '', logWhereLanguage: displayedLanguage }); - }, [logWhere, displayedLanguage, reset]); + reset({ + logWhere: logWhere ?? '', + logWhereLanguage: urlLanguage ?? getValues('logWhereLanguage'), + }); + }, [logWhere, urlLanguage, reset, getValues]); const onSubmitFilter = useCallback( (data: { logWhere: string; logWhereLanguage: string }) => { @@ -112,42 +120,38 @@ export default function TraceLogsPanel({ const traceIdExpression = logSource?.traceIdExpression; - const config = useMemo(() => { + const config = useMemo((): BuilderChartConfigWithDateRange | undefined => { if (logSource == null || !traceIdExpression) { return undefined; } - const scope = { - // The trace scope goes in `filters` so the user's filter keeps the whole - // `where` slot to itself — the two run in independent languages. - filters: [ - { - type: 'sql' as const, - condition: SqlString.format('?=?', [ - SqlString.raw(traceIdExpression), - traceId, - ]), - }, - ], - where: logWhere ?? '', - whereLanguage: logFilterLanguage, - // Ascending: inside a trace, chronological order is execution order. - orderBy: `${logSource.timestampValueExpression} ASC`, + return { + // Assembled through the shared builder so this query sees the same row + // set as the search page and alerts do — it carries the source's + // `tableFilterExpression`, sample weighting, and `source` id (which is + // how the source's `querySettings` reach the query). + ...buildSearchChartConfig(logSource, { + // The trace scope goes in `filters` so the user's filter keeps the + // whole `where` slot to itself — the two run in independent languages. + filters: [ + { + type: 'sql', + condition: SqlString.format('?=?', [ + SqlString.raw(traceIdExpression), + traceId, + ]), + }, + ], + where: logWhere ?? '', + whereLanguage: logFilterLanguage, + // Falls back to the source's default columns when these logs aren't + // the searched table's rows. + select: searchTableConfig?.select ?? null, + // Ascending: inside a trace, chronological order is execution order. + orderBy: `${logSource.timestampValueExpression} ASC`, + }), limit: { limit: 200 }, dateRange, }; - if (searchTableConfig != null) { - return { ...searchTableConfig, ...scope }; - } - return { - connection: logSource.connection, - from: logSource.from, - timestampValueExpression: logSource.timestampValueExpression, - implicitColumnExpression: logSource.implicitColumnExpression, - bodyExpression: logSource.bodyExpression, - useTextIndexForImplicitColumn: logSource.useTextIndexForImplicitColumn, - select: logSource.defaultTableSelectExpression ?? '', - ...scope, - }; }, [ logSource, traceIdExpression, diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx index e644793a5a..be3bddb4fd 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx @@ -105,6 +105,7 @@ type TraceLogsPanelProps = { logSourceId?: string; traceId?: string; highlightedRowId?: string; + searchTableConfig?: unknown; onNavigateToLog?: ( rowId: string, aliasWith: unknown[], @@ -175,7 +176,10 @@ jest.mock('@/useFormatTime', () => ({ })); // NOTE: imported after the mock factories above. -import { DBRowSidePanelInner } from '@/components/DBRowSidePanel'; +import { + DBRowSidePanelInner, + RowSidePanelContext, +} from '@/components/DBRowSidePanel'; import { Tab } from '@/components/DBRowSidePanel.types'; import useSidePanelStack from '@/hooks/useSidePanelStack'; @@ -204,17 +208,40 @@ const ROW_WITH_TRACE = { __hdx_body: 'a log line', }; -function renderPanel(source: TSource, rowId = 'row-1') { +// What the search page provides: the table config behind the row, whose +// `select` decides what a row id is made of. +const SEARCH_TABLE_CONFIG = { + connection: 'conn', + from: { databaseName: 'default', tableName: 'otel_logs' }, + timestampValueExpression: 'Timestamp', + select: 'Timestamp, Body', + where: '', + whereLanguage: 'lucene' as const, + dateRange: [new Date(0), new Date(1)] as [Date, Date], +}; + +function renderPanel( + source: TSource, + { rowId = 'row-1', withSearchTableConfig = false } = {}, +) { function Harness() { const sidePanelStack = useSidePanelStack({ initialRowId: rowId }); return ( - + + + ); } return render( @@ -253,15 +280,35 @@ describe('DBRowSidePanelInner — trace logs tab', () => { expect(mockTraceLogsProps.current.highlightedRowId).toBeUndefined(); }); - it('offers the tab on a log, pointed at its own source and highlighting it', () => { + it('offers the tab on a log, pointed at its own source', () => { mockQueryStore.sidePanelTab = Tab.Logs; renderPanel(LOG_SOURCE); expect(tabValues()).toContain(Tab.Logs); expect(mockTraceLogsProps.current.logSourceId).toBe('log-src'); + }); + + it("shares the search's table config, and the row id built from it", () => { + mockQueryStore.sidePanelTab = Tab.Logs; + renderPanel(LOG_SOURCE, { withSearchTableConfig: true }); + + expect(mockTraceLogsProps.current.searchTableConfig).toBe( + SEARCH_TABLE_CONFIG, + ); expect(mockTraceLogsProps.current.highlightedRowId).toBe('row-1'); }); + it('withholds the highlight when the tab selects its own columns', () => { + // A row id is built from the selected columns, so an id from a table + // selecting something else can never match one of these rows — and the + // table would page through the window hunting for it. + mockQueryStore.sidePanelTab = Tab.Logs; + renderPanel(LOG_SOURCE); + + expect(mockTraceLogsProps.current.searchTableConfig).toBeUndefined(); + expect(mockTraceLogsProps.current.highlightedRowId).toBeUndefined(); + }); + it('hides the tab when the trace source has no correlated log source', () => { renderPanel( asSource({ ...TRACE_SOURCE_WITH_LOGS, logSourceId: undefined }), diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx index 9231899e21..d223067a5e 100644 --- a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -176,6 +176,24 @@ describe('TraceLogsPanel', () => { expect(config.dateRange).toBe(DATE_RANGE); }); + it("keeps the source's mandatory table filter ahead of the trace scope", () => { + renderPanel({ + source: asLogSource({ + ...LOG_SOURCE, + tableFilterExpression: "ServiceName != 'internal'", + }), + }); + + const config = mockRowTableProps.current.config!; + // Rows the source is configured to hide must stay hidden here too. + expect(config.filters).toEqual([ + { type: 'sql', condition: "ServiceName != 'internal'" }, + { type: 'sql', condition: `TraceId='${TRACE_ID}'` }, + ]); + // And the source id, which is how its querySettings reach the query. + expect(config.source).toBe('log-src'); + }); + it('applies the waterfall log filter and its language from the URL', () => { mockQueryStore.logWhere = "SeverityText = 'error'"; mockQueryStore.logWhereLanguage = 'sql'; @@ -232,6 +250,8 @@ describe('TraceLogsPanel', () => { expect(config.where).toBe(''); expect(config.orderBy).toBe('Timestamp ASC'); expect(config.dateRange).toBe(DATE_RANGE); + // Source-level fields still come from the log source, not the config. + expect(config.source).toBe('log-src'); }); it('leaves the remove-column action in place, since the columns are shared', () => { From 101d633436ab0e0ac6e631598365468a64de3c53 Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Mon, 14 Sep 2026 01:07:37 -0700 Subject: [PATCH 05/18] fix: Keep the trace logs tab's columns stable, and share the row label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The tab lost the reader's columns (and its highlight) after drilling into another log from it: `dbSqlRowTableConfig` is nulled for nested rows, so the identical list came back with the source's default columns. It now reads the select from the parent context, gated on the searched source still being the one the logs come from — a drilldown changes which row is shown, not which columns the reader chose or which select their ids were built from. - `searchTableConfig` became `selectOverride`: only the select was ever read, and a caller passing a config whose `filters` or `dateRange` mattered would have had them silently overridden. - Row -> breadcrumb label is now one `getTableRowLabel` helper, called from both this tab and Surrounding Context. The latter read the `__hdx_body` alias that only `useRowData` projects, so its label was always the 'Log'/'Span' fallback. Writing a test for the extracted version turned up a second bug in the original: an empty-string body rendered as `""` rather than falling back. - The filter's submit path has a test: the mocked input can now fire `onSubmit`, and the assertion is on the nuqs setters, so a renamed field or a dropped `languageName` fails instead of passing silently. Co-Authored-By: Claude Opus 5 (1M context) --- .../app/src/components/ContextSidePanel.tsx | 18 +++-- .../app/src/components/DBRowSidePanel.tsx | 23 +++---- .../app/src/components/TraceLogsPanel.tsx | 28 ++++---- .../DBRowSidePanel.traceLogsTab.test.tsx | 35 +++++++--- .../__tests__/TraceLogsPanel.test.tsx | 66 +++++++++++++------ .../src/components/__tests__/rowLabel.test.ts | 39 +++++++++++ packages/app/src/components/rowLabel.ts | 32 +++++++++ 7 files changed, 177 insertions(+), 64 deletions(-) create mode 100644 packages/app/src/components/__tests__/rowLabel.test.ts create mode 100644 packages/app/src/components/rowLabel.ts diff --git a/packages/app/src/components/ContextSidePanel.tsx b/packages/app/src/components/ContextSidePanel.tsx index fe2db7ae00..d71ee81d8a 100644 --- a/packages/app/src/components/ContextSidePanel.tsx +++ b/packages/app/src/components/ContextSidePanel.tsx @@ -30,6 +30,7 @@ import { import { ROW_DATA_ALIASES } from './DBRowDataPanel'; import { RowSidePanelContext } from './DBRowSidePanel'; import { DBSqlRowTable } from './DBRowTable'; +import { getTableRowLabel } from './rowLabel'; interface ContextSubpanelProps { source: TSource; @@ -77,16 +78,13 @@ export default function ContextSubpanel({ const { setChildModalOpen } = use(RowSidePanelContext); const handleRowExpandClick = useCallback( - (rowWhere: RowWhereResult, row: Record) => { - const body = row?.[ROW_DATA_ALIASES.BODY]; - const fallback = isTraceSource(source) ? 'Span' : 'Log'; - const label = - typeof body === 'string' && body.length > 0 - ? body - : body != null - ? JSON.stringify(body) - : fallback; - onNavigateToRow?.(rowWhere.where, rowWhere.aliasWith, label, source.kind); + (rowWhere: RowWhereResult, row: Record) => { + onNavigateToRow?.( + rowWhere.where, + rowWhere.aliasWith, + getTableRowLabel(source, row), + source.kind, + ); }, [onNavigateToRow, source], ); diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index e9f9d7e253..24b874917f 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -562,13 +562,16 @@ export const DBRowSidePanelInner = ({ // The displayed row is itself one of the logs the tab lists, so picking one // is a row change rather than a source hop. const isOwnTraceLogSource = traceLogSourceId === source.id; - // Only the search page hands down a table config, and only at the root frame. - // When it's there, the tab can share the reader's columns; without it the tab - // falls back to the source's own, and nothing about the opening table carries - // over — including a comparable row id. - const traceLogsSearchConfig = isOwnTraceLogSource - ? dbSqlRowTableConfig - : undefined; + // The columns the reader picked on the search page, handed to the tab + // whenever these logs are that search's rows. Read from the parent context + // rather than `dbSqlRowTableConfig`, which is nulled for nested rows: a + // drilldown changes which row is shown, not which columns the reader chose or + // which select their ids were built from. Undefined once the searched source + // is no longer the one the logs come from, where neither would hold. + const traceLogsSelect = + isOwnTraceLogSource && parentContext.source?.id === traceLogSourceId + ? parentContext.dbSqlRowTableConfig?.select + : undefined; const enableServiceMap = traceId && traceSourceId; @@ -1172,13 +1175,11 @@ export const DBRowSidePanelInner = ({ logSourceId={traceLogSourceId} traceId={traceId} dateRange={oneHourRange} - searchTableConfig={traceLogsSearchConfig} + selectOverride={traceLogsSelect} // Row ids are built from the selected columns, so the id this // panel was opened with is only comparable to the tab's rows when // the tab is selecting the same columns. - highlightedRowId={ - traceLogsSearchConfig != null ? activeRowId : undefined - } + highlightedRowId={traceLogsSelect != null ? activeRowId : undefined} onNavigateToLog={handleTraceLogNavigate} /> diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index 6baf2e2a5d..bda9fa9903 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -5,6 +5,7 @@ import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; import { buildSearchChartConfig } from '@hyperdx/common-utils/dist/core/searchChartConfig'; import { BuilderChartConfigWithDateRange, + SelectList, SourceKind, } from '@hyperdx/common-utils/dist/types'; import { Box, Flex } from '@mantine/core'; @@ -16,13 +17,14 @@ import SearchWhereInput, { } from '@/components/SearchInput/SearchWhereInput'; import { RowWhereResult, WithClause } from '@/hooks/useRowWhere'; import useWaterfallSearchState from '@/hooks/useWaterfallSearchState'; -import { getEventBody, useSource } from '@/source'; +import { useSource } from '@/source'; import { deriveRowSidePanelContextForSource, RowSidePanelContext, } from './DBRowSidePanel'; import { DBSqlRowTable } from './DBRowTable'; +import { getTableRowLabel } from './rowLabel'; /** * Every log in one trace, as a flat chronological table. @@ -37,7 +39,7 @@ export default function TraceLogsPanel({ traceId, dateRange, highlightedRowId, - searchTableConfig, + selectOverride, onNavigateToLog, 'data-testid': dataTestId, }: { @@ -47,11 +49,11 @@ export default function TraceLogsPanel({ /** Row id of the log this panel was opened from, when it is one of these logs. */ highlightedRowId?: string; /** - * The searched table's config, when this panel's logs come from that same - * source. Its `select` is reused so these rows carry the same ids the search - * built `highlightedRowId` from, and so the columns track the reader's. + * The searched table's SELECT, when this panel's logs are that table's rows. + * Reused so these rows carry the same ids `highlightedRowId` was built from, + * and so the columns track the ones the reader picked. */ - searchTableConfig?: BuilderChartConfigWithDateRange; + selectOverride?: SelectList; onNavigateToLog: ( rowId: string, aliasWith: WithClause[], @@ -145,7 +147,7 @@ export default function TraceLogsPanel({ whereLanguage: logFilterLanguage, // Falls back to the source's default columns when these logs aren't // the searched table's rows. - select: searchTableConfig?.select ?? null, + select: selectOverride ?? null, // Ascending: inside a trace, chronological order is execution order. orderBy: `${logSource.timestampValueExpression} ASC`, }), @@ -159,19 +161,15 @@ export default function TraceLogsPanel({ logWhere, logFilterLanguage, dateRange, - searchTableConfig, + selectOverride, ]); const handleRowDetailsClick = useCallback( (rowWhere: RowWhereResult, row: Record) => { - // The table selects the source's own columns, so the body lands under - // its expression rather than a normalized alias. - const bodyExpression = logSource ? getEventBody(logSource) : undefined; - const body = bodyExpression ? row[bodyExpression] : undefined; onNavigateToLog( rowWhere.where, rowWhere.aliasWith, - typeof body === 'string' && body.length > 0 ? body : 'Log', + logSource ? getTableRowLabel(logSource, row) : 'Log', ); }, [logSource, onNavigateToLog], @@ -185,14 +183,14 @@ export default function TraceLogsPanel({ const derived = logSource ? deriveRowSidePanelContextForSource(parentContext, logSource) : parentContext; - if (searchTableConfig != null) { + if (selectOverride != null) { return derived; } // These columns are this table's own, not the searched table's, so the // header's remove-column action would drop whatever column sits at that // index in the *search* results. Take it away. return { ...derived, displayedColumns: undefined, toggleColumn: undefined }; - }, [parentContext, logSource, searchTableConfig]); + }, [parentContext, logSource, selectOverride]); if (isLoading) { return null; diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx index be3bddb4fd..1510881a68 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { TSource } from '@hyperdx/common-utils/dist/types'; +import { TLogSource, TSource } from '@hyperdx/common-utils/dist/types'; import { MantineProvider } from '@mantine/core'; import { act, render } from '@testing-library/react'; @@ -54,8 +54,8 @@ jest.mock('../DBRowDataPanel', () => ({ // Test fixtures carry only the fields the panel reads; TSource's full shape is // irrelevant here and spelling it out would bury the gating under boilerplate. -const asSource = (source: Record) => - source as unknown as TSource; +const asSource = (source: Record) => + source as unknown as T; const TRACE_SOURCE_WITH_LOGS = asSource({ id: 'trace-src', @@ -105,7 +105,7 @@ type TraceLogsPanelProps = { logSourceId?: string; traceId?: string; highlightedRowId?: string; - searchTableConfig?: unknown; + selectOverride?: unknown; onNavigateToLog?: ( rowId: string, aliasWith: unknown[], @@ -229,6 +229,9 @@ function renderPanel( return ( (LOG_SOURCE) + : undefined, dbSqlRowTableConfig: withSearchTableConfig ? SEARCH_TABLE_CONFIG : undefined, @@ -288,16 +291,32 @@ describe('DBRowSidePanelInner — trace logs tab', () => { expect(mockTraceLogsProps.current.logSourceId).toBe('log-src'); }); - it("shares the search's table config, and the row id built from it", () => { + it("shares the search's columns, and the row id built from them", () => { mockQueryStore.sidePanelTab = Tab.Logs; renderPanel(LOG_SOURCE, { withSearchTableConfig: true }); - expect(mockTraceLogsProps.current.searchTableConfig).toBe( - SEARCH_TABLE_CONFIG, + expect(mockTraceLogsProps.current.selectOverride).toBe( + SEARCH_TABLE_CONFIG.select, ); expect(mockTraceLogsProps.current.highlightedRowId).toBe('row-1'); }); + it("keeps the search's columns after drilling into another log", () => { + // A drilldown changes which row is shown, not which columns the reader + // chose — and the row id it produced was built from those columns. + mockQueryStore.sidePanelTab = Tab.Logs; + mockQueryStore.sidePanelStackRoot = 'row-1'; + mockQueryStore.sidePanelNavStack = [ + { rowId: 'row-2', aliasWith: [], label: 'another log line' }, + ]; + renderPanel(LOG_SOURCE, { withSearchTableConfig: true }); + + expect(mockTraceLogsProps.current.selectOverride).toBe( + SEARCH_TABLE_CONFIG.select, + ); + expect(mockTraceLogsProps.current.highlightedRowId).toBe('row-2'); + }); + it('withholds the highlight when the tab selects its own columns', () => { // A row id is built from the selected columns, so an id from a table // selecting something else can never match one of these rows — and the @@ -305,7 +324,7 @@ describe('DBRowSidePanelInner — trace logs tab', () => { mockQueryStore.sidePanelTab = Tab.Logs; renderPanel(LOG_SOURCE); - expect(mockTraceLogsProps.current.searchTableConfig).toBeUndefined(); + expect(mockTraceLogsProps.current.selectOverride).toBeUndefined(); expect(mockTraceLogsProps.current.highlightedRowId).toBeUndefined(); }); diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx index d223067a5e..f8266acc40 100644 --- a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -5,14 +5,23 @@ import { TLogSource, } from '@hyperdx/common-utils/dist/types'; import { MantineProvider } from '@mantine/core'; -import { act, render, screen } from '@testing-library/react'; +import { act, fireEvent, render, screen } from '@testing-library/react'; const mockQueryStore: Record = {}; +const mockSetters: Record = {}; +function setterFor(key: string) { + if (!mockSetters[key]) mockSetters[key] = jest.fn(); + return mockSetters[key]; +} + jest.mock('nuqs', () => { const actual = jest.requireActual('nuqs'); return { ...actual, - useQueryState: (key: string) => [mockQueryStore[key] ?? null, jest.fn()], + useQueryState: (key: string) => { + if (!mockSetters[key]) mockSetters[key] = jest.fn(); + return [mockQueryStore[key] ?? null, mockSetters[key]]; + }, }; }); @@ -57,9 +66,11 @@ jest.mock('../SearchInput/SearchWhereInput', () => { default: ({ control, name, + onSubmit, }: { control: Control; name: keyof FilterForm; + onSubmit?: () => void; }) => { // Surface the form's live value so a test can see what the user would. const value = rhf.useWatch({ control, name }); @@ -70,6 +81,14 @@ jest.mock('../SearchInput/SearchWhereInput', () => { {String(language ?? '')} + {/* Stands in for pressing enter in the real input. */} + ); }, @@ -129,11 +148,11 @@ const LOG_SOURCE = asLogSource({ function renderPanel({ source = LOG_SOURCE, onNavigateToLog = jest.fn(), - searchTableConfig, + selectOverride, }: { source?: TLogSource; onNavigateToLog?: jest.Mock; - searchTableConfig?: BuilderChartConfigWithDateRange; + selectOverride?: string; } = {}) { mockUseSource.mockReturnValue({ data: source, isLoading: false }); // A fresh element per render: React bails out of re-rendering an identical one. @@ -143,7 +162,7 @@ function renderPanel({ logSourceId="log-src" traceId={TRACE_ID} dateRange={DATE_RANGE} - searchTableConfig={searchTableConfig} + selectOverride={selectOverride} onNavigateToLog={onNavigateToLog} /> @@ -155,6 +174,7 @@ function renderPanel({ describe('TraceLogsPanel', () => { beforeEach(() => { Object.keys(mockQueryStore).forEach(k => delete mockQueryStore[k]); + Object.keys(mockSetters).forEach(k => delete mockSetters[k]); mockRowTableProps.current = {}; mockUseSource.mockReset(); mockRowTableContext.current = {}; @@ -225,24 +245,14 @@ describe('TraceLogsPanel', () => { }); describe('when the logs come from the searched source', () => { - const SEARCH_CONFIG: BuilderChartConfigWithDateRange = { - connection: 'conn', - from: { databaseName: 'default', tableName: 'otel_logs' }, - timestampValueExpression: 'Timestamp', - // The reader's chosen columns, which differ from the source default. - select: 'Timestamp, Body', - where: 'ServiceName:"cart"', - whereLanguage: 'lucene', - filters: [{ type: 'sql' as const, condition: "ServiceName = 'cart'" }], - orderBy: 'Timestamp DESC', - dateRange: [new Date(0), new Date(1)] as [Date, Date], - }; + // The reader's chosen columns, which differ from the source default. + const SEARCH_SELECT = 'Timestamp, Body'; it("reuses the search's columns so row ids match the highlighted row", () => { - renderPanel({ searchTableConfig: SEARCH_CONFIG }); + renderPanel({ selectOverride: SEARCH_SELECT }); const config = mockRowTableProps.current.config!; - expect(config.select).toBe('Timestamp, Body'); + expect(config.select).toBe(SEARCH_SELECT); // ...while the trace scope and ordering still come from this panel. expect(config.filters).toEqual([ { type: 'sql', condition: `TraceId='${TRACE_ID}'` }, @@ -255,7 +265,7 @@ describe('TraceLogsPanel', () => { }); it('leaves the remove-column action in place, since the columns are shared', () => { - renderPanel({ searchTableConfig: SEARCH_CONFIG }); + renderPanel({ selectOverride: SEARCH_SELECT }); expect(mockRowTableContext.current.toggleColumn).toBe(mockToggleColumn); }); @@ -287,6 +297,22 @@ describe('TraceLogsPanel', () => { expect(mockRowTableProps.current.config?.where).toBe('ServiceName:"cart"'); }); + it('submits the log filter without touching the spans filter', async () => { + mockQueryStore.logWhere = 'SeverityText:"error"'; + mockQueryStore.logWhereLanguage = 'sql'; + renderPanel(); + + await act(async () => { + fireEvent.click(screen.getByTestId('submit-log-filter')); + }); + + expect(setterFor('logWhere')).toHaveBeenCalledWith('SeverityText:"error"'); + expect(setterFor('logWhereLanguage')).toHaveBeenCalledWith('sql'); + // The waterfall's own filter is not this form's to write. + expect(setterFor('traceWhere')).not.toHaveBeenCalled(); + expect(setterFor('traceWhereLanguage')).not.toHaveBeenCalled(); + }); + it("labels a picked row with the log's body", () => { const { onNavigateToLog } = renderPanel(); diff --git a/packages/app/src/components/__tests__/rowLabel.test.ts b/packages/app/src/components/__tests__/rowLabel.test.ts new file mode 100644 index 0000000000..6e298da16a --- /dev/null +++ b/packages/app/src/components/__tests__/rowLabel.test.ts @@ -0,0 +1,39 @@ +import { TSource } from '@hyperdx/common-utils/dist/types'; + +import { getTableRowLabel } from '@/components/rowLabel'; + +const asSource = (source: Record) => + source as unknown as TSource; + +const LOG_SOURCE = asSource({ kind: 'log', bodyExpression: 'Body' }); +const TRACE_SOURCE = asSource({ + kind: 'trace', + spanNameExpression: 'SpanName', +}); + +describe('getTableRowLabel', () => { + it("reads the body from the source's own expression, as the table selects it", () => { + expect(getTableRowLabel(LOG_SOURCE, { Body: 'upstream timeout' })).toBe( + 'upstream timeout', + ); + expect(getTableRowLabel(TRACE_SOURCE, { SpanName: 'GET /checkout' })).toBe( + 'GET /checkout', + ); + }); + + it('serializes a body that is not a string', () => { + expect(getTableRowLabel(LOG_SOURCE, { Body: { msg: 'hi' } })).toBe( + '{"msg":"hi"}', + ); + }); + + it('names the kind of event when the row carries no body', () => { + expect(getTableRowLabel(LOG_SOURCE, { ServiceName: 'cart' })).toBe('Log'); + expect(getTableRowLabel(TRACE_SOURCE, {})).toBe('Span'); + expect(getTableRowLabel(LOG_SOURCE, undefined)).toBe('Log'); + }); + + it('names the kind of event for an empty body', () => { + expect(getTableRowLabel(LOG_SOURCE, { Body: '' })).toBe('Log'); + }); +}); diff --git a/packages/app/src/components/rowLabel.ts b/packages/app/src/components/rowLabel.ts new file mode 100644 index 0000000000..f5fa127525 --- /dev/null +++ b/packages/app/src/components/rowLabel.ts @@ -0,0 +1,32 @@ +import { isTraceSource, TSource } from '@hyperdx/common-utils/dist/types'; + +import { getEventBody } from '@/source'; + +/** + * Label for a row picked out of a `DBSqlRowTable` — what the side panel's + * breadcrumb shows for the row you drilled into. + * + * The table selects the source's own expressions, so the body sits under its + * expression, not under the `__hdx_body` alias `useRowData` projects. Falls + * back to the kind of event when the row's select doesn't carry the body (or + * carries it under an alias). + */ +export function getTableRowLabel( + source: TSource, + row: Record | undefined, +): string { + const bodyExpression = getEventBody(source); + const body = bodyExpression ? row?.[bodyExpression] : undefined; + if (typeof body === 'string') { + // An empty body names nothing; fall through to the kind of event. + return body.length > 0 ? body : eventKindLabel(source); + } + if (body != null) { + return JSON.stringify(body); + } + return eventKindLabel(source); +} + +function eventKindLabel(source: TSource): string { + return isTraceSource(source) ? 'Span' : 'Log'; +} From f934f6e9c2d7cac417113b6c9cb8643e6b47eda6 Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Mon, 14 Sep 2026 01:23:40 -0700 Subject: [PATCH 06/18] fix: Treat a blank trace id expression as unconfigured `min(1)` on the schema accepts a single space, and the empty-state gate keyed on falsiness, so a whitespace expression reached the filter and rendered as a bare `=`. Trimmed before the check; the whitespace case is now covered alongside the undefined one. Co-Authored-By: Claude Opus 5 (1M context) --- .../app/src/components/TraceLogsPanel.tsx | 5 +++- .../__tests__/TraceLogsPanel.test.tsx | 25 +++++++++++++------ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index bda9fa9903..137c7923ee 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -120,7 +120,10 @@ export default function TraceLogsPanel({ [submitFilters], ); - const traceIdExpression = logSource?.traceIdExpression; + // Trimmed so a whitespace-only expression falls into the empty state below + // rather than rendering as a bare `=` in the trace filter. The schema allows + // it: `min(1)` accepts ' '. + const traceIdExpression = logSource?.traceIdExpression?.trim(); const config = useMemo((): BuilderChartConfigWithDateRange | undefined => { if (logSource == null || !traceIdExpression) { diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx index f8266acc40..cfaf103134 100644 --- a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -194,6 +194,8 @@ describe('TraceLogsPanel', () => { // Chronological: inside a trace that is execution order. expect(config.orderBy).toBe('Timestamp ASC'); expect(config.dateRange).toBe(DATE_RANGE); + // One page to start; the table pages on scroll from here. + expect(config.limit).toEqual({ limit: 200 }); }); it("keeps the source's mandatory table filter ahead of the trace scope", () => { @@ -343,12 +345,19 @@ describe('TraceLogsPanel', () => { expect(onNavigateToLog).toHaveBeenCalledWith('row-2', [], 'Log'); }); - it('explains itself instead of querying when the log source has no trace id column', () => { - renderPanel({ - source: { ...LOG_SOURCE, traceIdExpression: undefined } as TLogSource, - }); - - expect(screen.getByText('No trace ID column configured')).toBeVisible(); - expect(mockRowTableProps.current.config).toBeUndefined(); - }); + it.each([ + ['undefined', undefined], + // `min(1)` accepts a space, so this reaches the panel from a source config. + ['whitespace', ' '], + ])( + 'explains itself instead of querying when the trace id column is %s', + (_label, traceIdExpression) => { + renderPanel({ + source: asLogSource({ ...LOG_SOURCE, traceIdExpression }), + }); + + expect(screen.getByText('No trace ID column configured')).toBeVisible(); + expect(mockRowTableProps.current.config).toBeUndefined(); + }, + ); }); From 690f14e1954b047203dd6e6aefa55cf21dc2ffad Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Mon, 14 Sep 2026 01:38:51 -0700 Subject: [PATCH 07/18] test: Cover the trace logs tab's unresolved source and id escaping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "correlated log source not found" branch had no test — every render helper mocked a live source. Also pins that a trace id carrying a quote is escaped by the `?=?` placeholder rather than landing in the filter raw. Co-Authored-By: Claude Opus 5 (1M context) --- .../app/src/components/TraceLogsPanel.tsx | 3 +- .../__tests__/TraceLogsPanel.test.tsx | 28 ++++++++++++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index 137c7923ee..3c2c81fbc4 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -121,8 +121,7 @@ export default function TraceLogsPanel({ ); // Trimmed so a whitespace-only expression falls into the empty state below - // rather than rendering as a bare `=` in the trace filter. The schema allows - // it: `min(1)` accepts ' '. + // rather than rendering as a bare `=` in the trace filter. const traceIdExpression = logSource?.traceIdExpression?.trim(); const config = useMemo((): BuilderChartConfigWithDateRange | undefined => { diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx index cfaf103134..dce68e0bdd 100644 --- a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -149,18 +149,24 @@ function renderPanel({ source = LOG_SOURCE, onNavigateToLog = jest.fn(), selectOverride, + traceId = TRACE_ID, }: { - source?: TLogSource; + /** Null for a source that no longer resolves. */ + source?: TLogSource | null; onNavigateToLog?: jest.Mock; selectOverride?: string; + traceId?: string; } = {}) { - mockUseSource.mockReturnValue({ data: source, isLoading: false }); + mockUseSource.mockReturnValue({ + data: source ?? undefined, + isLoading: false, + }); // A fresh element per render: React bails out of re-rendering an identical one. const tree = () => ( { expect(onNavigateToLog).toHaveBeenCalledWith('row-2', [], 'Log'); }); + it('escapes a trace id that carries SQL metacharacters', () => { + renderPanel({ traceId: "abc' OR 1=1--" }); + + expect(mockRowTableProps.current.config?.filters).toEqual([ + { type: 'sql', condition: "TraceId='abc\\' OR 1=1--'" }, + ]); + }); + + it('explains itself when the correlated log source no longer resolves', () => { + renderPanel({ source: null }); + + expect(screen.getByText('Correlated log source not found')).toBeVisible(); + expect(mockRowTableProps.current.config).toBeUndefined(); + }); + it.each([ ['undefined', undefined], - // `min(1)` accepts a space, so this reaches the panel from a source config. ['whitespace', ' '], ])( 'explains itself instead of querying when the trace id column is %s', From edb456913ed7c78cd1e511e52655273ace0b65f5 Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Mon, 14 Sep 2026 01:56:47 -0700 Subject: [PATCH 08/18] fix: Highlight a trace log only where its row id is comparable Reusing the search's select made the columns stable across a drilldown but didn't make every nested row id comparable: Surrounding Context falls back to the source's own columns once a stack is active, so a row picked there carries an id built from different columns while the tab still reports the search's select. Three hops in, that id could never match and the table paged the window hunting for it. The highlight is now passed only at the root frame, where the id demonstrably came from the table whose select the tab reuses; the columns stay stable regardless. Also extracts `resolveWhereLanguage` next to `getStoredLanguage`, so the tab and the waterfall read the shared `logWhereLanguage` param through one function instead of two independently written ternaries, and asserts the `highlightedLineId` wiring that deleting the prop would otherwise leave green. Co-Authored-By: Claude Opus 5 (1M context) --- .../app/src/components/DBRowSidePanel.tsx | 17 ++++++++++---- .../src/components/DBTraceWaterfallChart.tsx | 23 ++++++++----------- .../SearchInput/SearchWhereInput.tsx | 14 +++++++++++ .../app/src/components/TraceLogsPanel.tsx | 6 ++--- .../DBRowSidePanel.traceLogsTab.test.tsx | 7 ++++-- .../__tests__/DBTraceWaterfallChart.test.tsx | 2 ++ .../__tests__/TraceLogsPanel.test.tsx | 13 +++++++++++ 7 files changed, 58 insertions(+), 24 deletions(-) diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index 24b874917f..ef5211402f 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -1176,10 +1176,19 @@ export const DBRowSidePanelInner = ({ traceId={traceId} dateRange={oneHourRange} selectOverride={traceLogsSelect} - // Row ids are built from the selected columns, so the id this - // panel was opened with is only comparable to the tab's rows when - // the tab is selecting the same columns. - highlightedRowId={traceLogsSelect != null ? activeRowId : undefined} + // Row ids are built from the selected columns, so the row on screen + // is only findable among the tab's rows when the table that + // produced its id selected the same ones. That holds at the root + // frame — the search's table, whose select the tab reuses. Deeper + // in, the id could have come from any panel that pushed a frame + // (Surrounding Context falls back to the source's own columns for + // nested rows), and an id that cannot match leaves the table paging + // through the window hunting for it. + highlightedRowId={ + traceLogsSelect != null && !hasActiveStacks + ? activeRowId + : undefined + } onNavigateToLog={handleTraceLogNavigate} /> diff --git a/packages/app/src/components/DBTraceWaterfallChart.tsx b/packages/app/src/components/DBTraceWaterfallChart.tsx index 85ebaca5d0..4d8cb7d442 100644 --- a/packages/app/src/components/DBTraceWaterfallChart.tsx +++ b/packages/app/src/components/DBTraceWaterfallChart.tsx @@ -47,6 +47,7 @@ import { ContactSupportText } from '@/components/ContactSupportText'; import { ErrorCollapse } from '@/components/Error/ErrorCollapse'; import SearchWhereInput, { getStoredLanguage, + resolveWhereLanguage, } from '@/components/SearchInput/SearchWhereInput'; import { TimelineChart, @@ -609,12 +610,12 @@ export function DBTraceWaterfallChartContainer({ hasLogSource: !!logTableSource, }); - // Each filter runs in its own language. Defaults come from the URL (shared - // link / reload fidelity), falling back to the stored preference. - const traceFilterLanguage: 'lucene' | 'sql' = - traceWhereLanguage === 'sql' ? 'sql' : 'lucene'; - const logFilterLanguage: 'lucene' | 'sql' = - logWhereLanguage === 'sql' ? 'sql' : 'lucene'; + // Each filter runs in its own language, read from the URL by the same helper + // the Trace logs tab uses, so a shared `logWhere` runs identically in both. + const traceUrlLanguage = resolveWhereLanguage(traceWhereLanguage); + const logUrlLanguage = resolveWhereLanguage(logWhereLanguage); + const traceFilterLanguage: 'lucene' | 'sql' = traceUrlLanguage ?? 'lucene'; + const logFilterLanguage: 'lucene' | 'sql' = logUrlLanguage ?? 'lucene'; const { control, handleSubmit, setValue } = useForm({ defaultValues: { @@ -622,14 +623,8 @@ export function DBTraceWaterfallChartContainer({ logWhere: logWhere ?? '', // Prefer each input's URL language so a shared link / reload shows the // same language the filter runs in; fall back to the stored preference. - traceWhereLanguage: - traceWhereLanguage === 'sql' || traceWhereLanguage === 'lucene' - ? traceWhereLanguage - : (getStoredLanguage() ?? 'lucene'), - logWhereLanguage: - logWhereLanguage === 'sql' || logWhereLanguage === 'lucene' - ? logWhereLanguage - : (getStoredLanguage() ?? 'lucene'), + traceWhereLanguage: traceUrlLanguage ?? getStoredLanguage() ?? 'lucene', + logWhereLanguage: logUrlLanguage ?? getStoredLanguage() ?? 'lucene', }, }); diff --git a/packages/app/src/components/SearchInput/SearchWhereInput.tsx b/packages/app/src/components/SearchInput/SearchWhereInput.tsx index e405c7d77d..065a8040df 100644 --- a/packages/app/src/components/SearchInput/SearchWhereInput.tsx +++ b/packages/app/src/components/SearchInput/SearchWhereInput.tsx @@ -31,6 +31,20 @@ export function getStoredLanguage(): 'sql' | 'lucene' | null { return null; } +/** + * The WHERE language a URL param names, or undefined when it names none. + * + * One reading of the param for every view that shares it, so the same URL can't + * run as Lucene in one and SQL in another. Callers supply their own fallback: + * `?? 'lucene'` for what a query runs as, `?? getStoredLanguage()` for what an + * input offers when the URL is silent. + */ +export function resolveWhereLanguage( + urlValue: string | null | undefined, +): 'sql' | 'lucene' | undefined { + return urlValue === 'sql' || urlValue === 'lucene' ? urlValue : undefined; +} + function setStoredLanguage(lang: 'sql' | 'lucene'): void { try { if (typeof window !== 'undefined') diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index 3c2c81fbc4..4ac4f468cb 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -14,6 +14,7 @@ import { IconLogs } from '@tabler/icons-react'; import EmptyState from '@/components/EmptyState'; import SearchWhereInput, { getStoredLanguage, + resolveWhereLanguage, } from '@/components/SearchInput/SearchWhereInput'; import { RowWhereResult, WithClause } from '@/hooks/useRowWhere'; import useWaterfallSearchState from '@/hooks/useWaterfallSearchState'; @@ -72,10 +73,7 @@ export default function TraceLogsPanel({ onSubmit: submitFilters, } = useWaterfallSearchState({ hasLogSource: true }); - const urlLanguage = - logWhereLanguage === 'sql' || logWhereLanguage === 'lucene' - ? logWhereLanguage - : undefined; + const urlLanguage = resolveWhereLanguage(logWhereLanguage); // The query language follows the URL alone, defaulting to Lucene, so a filter // shared with the waterfall runs the same way in both places even if a link diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx index 1510881a68..d5b75e1a4e 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx @@ -303,7 +303,7 @@ describe('DBRowSidePanelInner — trace logs tab', () => { it("keeps the search's columns after drilling into another log", () => { // A drilldown changes which row is shown, not which columns the reader - // chose — and the row id it produced was built from those columns. + // chose, so the list must not silently switch column sets. mockQueryStore.sidePanelTab = Tab.Logs; mockQueryStore.sidePanelStackRoot = 'row-1'; mockQueryStore.sidePanelNavStack = [ @@ -314,7 +314,10 @@ describe('DBRowSidePanelInner — trace logs tab', () => { expect(mockTraceLogsProps.current.selectOverride).toBe( SEARCH_TABLE_CONFIG.select, ); - expect(mockTraceLogsProps.current.highlightedRowId).toBe('row-2'); + // The highlight goes, though: a nested row's id may have been built by + // another panel selecting other columns (Surrounding Context falls back to + // the source's own), and an unmatchable id costs a paging hunt. + expect(mockTraceLogsProps.current.highlightedRowId).toBeUndefined(); }); it('withholds the highlight when the tab selects its own columns', () => { diff --git a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx index f2fecf6a12..3e351027be 100644 --- a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx @@ -85,6 +85,8 @@ jest.mock('@/components/SearchInput/SearchWhereInput', () => ({
{name}
), getStoredLanguage: () => 'lucene', + resolveWhereLanguage: (v: unknown) => + v === 'sql' || v === 'lucene' ? v : undefined, })); const makeWaterfallSearchState = () => ({ diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx index dce68e0bdd..7a723c8048 100644 --- a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -93,6 +93,8 @@ jest.mock('../SearchInput/SearchWhereInput', () => { ); }, getStoredLanguage: () => mockStoredLanguage, + resolveWhereLanguage: (v: unknown) => + v === 'sql' || v === 'lucene' ? v : undefined, }; }); @@ -150,12 +152,14 @@ function renderPanel({ onNavigateToLog = jest.fn(), selectOverride, traceId = TRACE_ID, + highlightedRowId, }: { /** Null for a source that no longer resolves. */ source?: TLogSource | null; onNavigateToLog?: jest.Mock; selectOverride?: string; traceId?: string; + highlightedRowId?: string; } = {}) { mockUseSource.mockReturnValue({ data: source ?? undefined, @@ -169,6 +173,7 @@ function renderPanel({ traceId={traceId} dateRange={DATE_RANGE} selectOverride={selectOverride} + highlightedRowId={highlightedRowId} onNavigateToLog={onNavigateToLog} /> @@ -351,6 +356,14 @@ describe('TraceLogsPanel', () => { expect(onNavigateToLog).toHaveBeenCalledWith('row-2', [], 'Log'); }); + it('hands the row to highlight to the table', () => { + renderPanel({ highlightedRowId: "Timestamp='2024-05-01 10:00:00'" }); + + expect(mockRowTableProps.current.highlightedLineId).toBe( + "Timestamp='2024-05-01 10:00:00'", + ); + }); + it('escapes a trace id that carries SQL metacharacters', () => { renderPanel({ traceId: "abc' OR 1=1--" }); From d98d95e0c5dcae5945cd5dde8438e40e014f6849 Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Tue, 15 Sep 2026 00:32:01 -0700 Subject: [PATCH 09/18] refactor: Cut the trace logs tab back to its core The tab's own filter input and its shared-columns/highlight plumbing were where every review finding landed, for features the tab doesn't need: - No filter input. The tab still honours the log filter set in the Trace tab's waterfall, and "Open in search" covers anything more. Removes the form, the URL re-seed effect, the submit path, the language resolution, `resolveWhereLanguage` (and its edit to the waterfall), and the partial-write change to `useWaterfallSearchState`. - No shared columns or row highlight. The table shows the log source's own columns, so there is no row-id provenance to reason about and no way to leave the table paging for an id that cannot match. - `getTableRowLabel` is gone with its Surrounding Context fix, which is an unrelated pre-existing bug and belongs in its own change. - Three empty states collapse into one, since the reader's remedy is the same for all three: check the source's settings. Adds the side padding the table was missing and an "Open in search" link. Production diff: 480 lines -> 284. Nothing user-facing is lost beyond in-tab filtering and the highlight. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/trace-logs-side-panel-tab.md | 4 +- .../app/src/components/ContextSidePanel.tsx | 18 +- .../app/src/components/DBRowSidePanel.tsx | 24 -- .../src/components/DBTraceWaterfallChart.tsx | 23 +- .../SearchInput/SearchWhereInput.tsx | 14 - .../app/src/components/TraceLogsPanel.tsx | 236 ++++++--------- .../DBRowSidePanel.traceLogsTab.test.tsx | 97 +----- .../__tests__/DBTraceWaterfallChart.test.tsx | 2 - .../__tests__/TraceLogsPanel.test.tsx | 281 ++++++------------ .../src/components/__tests__/rowLabel.test.ts | 39 --- packages/app/src/components/rowLabel.ts | 32 -- .../useWaterfallSearchState.test.tsx | 87 ------ .../app/src/hooks/useWaterfallSearchState.tsx | 14 +- 13 files changed, 219 insertions(+), 652 deletions(-) delete mode 100644 packages/app/src/components/__tests__/rowLabel.test.ts delete mode 100644 packages/app/src/components/rowLabel.ts delete mode 100644 packages/app/src/hooks/__tests__/useWaterfallSearchState.test.tsx diff --git a/.changeset/trace-logs-side-panel-tab.md b/.changeset/trace-logs-side-panel-tab.md index ecbf1ee4a1..44166fd4ec 100644 --- a/.changeset/trace-logs-side-panel-tab.md +++ b/.changeset/trace-logs-side-panel-tab.md @@ -2,6 +2,6 @@ '@hyperdx/app': minor --- -Add a "Trace logs" tab to the event side panel, listing the trace's logs as a flat chronological table over the same time window the waterfall uses. It appears on any row that carries a trace id and resolves a log source — a span (via the trace source's correlated log source) or a log (its own source, with the row you came from highlighted). Previously the only route to a trace's logs was hunting for the green rows interleaved in the waterfall, which a log-heavy trace buries. +Add a "Trace logs" tab to the event side panel, listing the trace's logs as a flat chronological table over the same time window the waterfall uses. It appears on any row that carries a trace id and resolves a log source — a span (via the trace source's correlated log source) or a log (its own source). Previously the only route to a trace's logs was hunting for the green rows interleaved in the waterfall, which a log-heavy trace buries. -The tab shares the waterfall's log filter, so a filter set in either place applies to both, and picking a log opens it in the panel: a breadcrumb hop from a span, or a row change from another log. Sorting is ascending, which inside a trace is execution order. +The tab honours the log filter set in the Trace tab's waterfall, and "Open in search" hands the same query to the search page — the trace scope plus whatever filter is set, in the language that filter runs in. Picking a log opens it in the panel: a breadcrumb hop from a span, or a row change from another log. Sorting is ascending, which inside a trace is execution order. diff --git a/packages/app/src/components/ContextSidePanel.tsx b/packages/app/src/components/ContextSidePanel.tsx index d71ee81d8a..fe2db7ae00 100644 --- a/packages/app/src/components/ContextSidePanel.tsx +++ b/packages/app/src/components/ContextSidePanel.tsx @@ -30,7 +30,6 @@ import { import { ROW_DATA_ALIASES } from './DBRowDataPanel'; import { RowSidePanelContext } from './DBRowSidePanel'; import { DBSqlRowTable } from './DBRowTable'; -import { getTableRowLabel } from './rowLabel'; interface ContextSubpanelProps { source: TSource; @@ -78,13 +77,16 @@ export default function ContextSubpanel({ const { setChildModalOpen } = use(RowSidePanelContext); const handleRowExpandClick = useCallback( - (rowWhere: RowWhereResult, row: Record) => { - onNavigateToRow?.( - rowWhere.where, - rowWhere.aliasWith, - getTableRowLabel(source, row), - source.kind, - ); + (rowWhere: RowWhereResult, row: Record) => { + const body = row?.[ROW_DATA_ALIASES.BODY]; + const fallback = isTraceSource(source) ? 'Span' : 'Log'; + const label = + typeof body === 'string' && body.length > 0 + ? body + : body != null + ? JSON.stringify(body) + : fallback; + onNavigateToRow?.(rowWhere.where, rowWhere.aliasWith, label, source.kind); }, [onNavigateToRow, source], ); diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index ef5211402f..3dba206a67 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -562,16 +562,6 @@ export const DBRowSidePanelInner = ({ // The displayed row is itself one of the logs the tab lists, so picking one // is a row change rather than a source hop. const isOwnTraceLogSource = traceLogSourceId === source.id; - // The columns the reader picked on the search page, handed to the tab - // whenever these logs are that search's rows. Read from the parent context - // rather than `dbSqlRowTableConfig`, which is nulled for nested rows: a - // drilldown changes which row is shown, not which columns the reader chose or - // which select their ids were built from. Undefined once the searched source - // is no longer the one the logs come from, where neither would hold. - const traceLogsSelect = - isOwnTraceLogSource && parentContext.source?.id === traceLogSourceId - ? parentContext.dbSqlRowTableConfig?.select - : undefined; const enableServiceMap = traceId && traceSourceId; @@ -1175,20 +1165,6 @@ export const DBRowSidePanelInner = ({ logSourceId={traceLogSourceId} traceId={traceId} dateRange={oneHourRange} - selectOverride={traceLogsSelect} - // Row ids are built from the selected columns, so the row on screen - // is only findable among the tab's rows when the table that - // produced its id selected the same ones. That holds at the root - // frame — the search's table, whose select the tab reuses. Deeper - // in, the id could have come from any panel that pushed a frame - // (Surrounding Context falls back to the source's own columns for - // nested rows), and an id that cannot match leaves the table paging - // through the window hunting for it. - highlightedRowId={ - traceLogsSelect != null && !hasActiveStacks - ? activeRowId - : undefined - } onNavigateToLog={handleTraceLogNavigate} /> diff --git a/packages/app/src/components/DBTraceWaterfallChart.tsx b/packages/app/src/components/DBTraceWaterfallChart.tsx index 4d8cb7d442..85ebaca5d0 100644 --- a/packages/app/src/components/DBTraceWaterfallChart.tsx +++ b/packages/app/src/components/DBTraceWaterfallChart.tsx @@ -47,7 +47,6 @@ import { ContactSupportText } from '@/components/ContactSupportText'; import { ErrorCollapse } from '@/components/Error/ErrorCollapse'; import SearchWhereInput, { getStoredLanguage, - resolveWhereLanguage, } from '@/components/SearchInput/SearchWhereInput'; import { TimelineChart, @@ -610,12 +609,12 @@ export function DBTraceWaterfallChartContainer({ hasLogSource: !!logTableSource, }); - // Each filter runs in its own language, read from the URL by the same helper - // the Trace logs tab uses, so a shared `logWhere` runs identically in both. - const traceUrlLanguage = resolveWhereLanguage(traceWhereLanguage); - const logUrlLanguage = resolveWhereLanguage(logWhereLanguage); - const traceFilterLanguage: 'lucene' | 'sql' = traceUrlLanguage ?? 'lucene'; - const logFilterLanguage: 'lucene' | 'sql' = logUrlLanguage ?? 'lucene'; + // Each filter runs in its own language. Defaults come from the URL (shared + // link / reload fidelity), falling back to the stored preference. + const traceFilterLanguage: 'lucene' | 'sql' = + traceWhereLanguage === 'sql' ? 'sql' : 'lucene'; + const logFilterLanguage: 'lucene' | 'sql' = + logWhereLanguage === 'sql' ? 'sql' : 'lucene'; const { control, handleSubmit, setValue } = useForm({ defaultValues: { @@ -623,8 +622,14 @@ export function DBTraceWaterfallChartContainer({ logWhere: logWhere ?? '', // Prefer each input's URL language so a shared link / reload shows the // same language the filter runs in; fall back to the stored preference. - traceWhereLanguage: traceUrlLanguage ?? getStoredLanguage() ?? 'lucene', - logWhereLanguage: logUrlLanguage ?? getStoredLanguage() ?? 'lucene', + traceWhereLanguage: + traceWhereLanguage === 'sql' || traceWhereLanguage === 'lucene' + ? traceWhereLanguage + : (getStoredLanguage() ?? 'lucene'), + logWhereLanguage: + logWhereLanguage === 'sql' || logWhereLanguage === 'lucene' + ? logWhereLanguage + : (getStoredLanguage() ?? 'lucene'), }, }); diff --git a/packages/app/src/components/SearchInput/SearchWhereInput.tsx b/packages/app/src/components/SearchInput/SearchWhereInput.tsx index 065a8040df..e405c7d77d 100644 --- a/packages/app/src/components/SearchInput/SearchWhereInput.tsx +++ b/packages/app/src/components/SearchInput/SearchWhereInput.tsx @@ -31,20 +31,6 @@ export function getStoredLanguage(): 'sql' | 'lucene' | null { return null; } -/** - * The WHERE language a URL param names, or undefined when it names none. - * - * One reading of the param for every view that shares it, so the same URL can't - * run as Lucene in one and SQL in another. Callers supply their own fallback: - * `?? 'lucene'` for what a query runs as, `?? getStoredLanguage()` for what an - * input offers when the URL is silent. - */ -export function resolveWhereLanguage( - urlValue: string | null | undefined, -): 'sql' | 'lucene' | undefined { - return urlValue === 'sql' || urlValue === 'lucene' ? urlValue : undefined; -} - function setStoredLanguage(lang: 'sql' | 'lucene'): void { try { if (typeof window !== 'undefined') diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index 4ac4f468cb..180b4bcefc 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -1,60 +1,43 @@ -import { use, useCallback, useEffect, useMemo } from 'react'; -import { useForm } from 'react-hook-form'; +import { use, useCallback, useMemo } from 'react'; +import Link from 'next/link'; import SqlString from 'sqlstring'; -import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; import { buildSearchChartConfig } from '@hyperdx/common-utils/dist/core/searchChartConfig'; import { BuilderChartConfigWithDateRange, - SelectList, SourceKind, } from '@hyperdx/common-utils/dist/types'; -import { Box, Flex } from '@mantine/core'; -import { IconLogs } from '@tabler/icons-react'; +import { Button, Flex, Group } from '@mantine/core'; +import { IconExternalLink, IconLogs } from '@tabler/icons-react'; import EmptyState from '@/components/EmptyState'; -import SearchWhereInput, { - getStoredLanguage, - resolveWhereLanguage, -} from '@/components/SearchInput/SearchWhereInput'; import { RowWhereResult, WithClause } from '@/hooks/useRowWhere'; import useWaterfallSearchState from '@/hooks/useWaterfallSearchState'; -import { useSource } from '@/source'; +import { getEventBody, useSource } from '@/source'; import { deriveRowSidePanelContextForSource, RowSidePanelContext, } from './DBRowSidePanel'; import { DBSqlRowTable } from './DBRowTable'; -import { getTableRowLabel } from './rowLabel'; /** - * Every log in one trace, as a flat chronological table. + * One trace's logs, as a flat chronological table. * - * Sibling view to the waterfall's interleaved log rows: it shares the same - * `logWhere` URL state, so a filter set in either place applies to both, but - * it isn't bound to the span tree — which is what makes a log-heavy trace - * readable. + * Sibling view to the waterfall's interleaved log rows, for when a trace logs + * enough that reading them inside the span tree stops working. The waterfall + * owns the log filter; this honours whatever is set there, and hands the same + * query to the search page for anything more. */ export default function TraceLogsPanel({ logSourceId, traceId, dateRange, - highlightedRowId, - selectOverride, onNavigateToLog, 'data-testid': dataTestId, }: { logSourceId: string; traceId: string; dateRange: [Date, Date]; - /** Row id of the log this panel was opened from, when it is one of these logs. */ - highlightedRowId?: string; - /** - * The searched table's SELECT, when this panel's logs are that table's rows. - * Reused so these rows carry the same ids `highlightedRowId` was built from, - * and so the columns track the ones the reader picked. - */ - selectOverride?: SelectList; onNavigateToLog: ( rowId: string, aliasWith: WithClause[], @@ -67,63 +50,24 @@ export default function TraceLogsPanel({ kinds: [SourceKind.Log], }); - const { - logWhere, - logWhereLanguage, - onSubmit: submitFilters, - } = useWaterfallSearchState({ hasLogSource: true }); - - const urlLanguage = resolveWhereLanguage(logWhereLanguage); - - // The query language follows the URL alone, defaulting to Lucene, so a filter - // shared with the waterfall runs the same way in both places even if a link - // carries `logWhere` without its language. - const logFilterLanguage: 'lucene' | 'sql' = urlLanguage ?? 'lucene'; - - // The input shows the language the filter actually runs in. With no filter - // there is nothing to interpret, so the user's stored editor preference wins. - const displayedLanguage = - urlLanguage ?? (logWhere ? 'lucene' : (getStoredLanguage() ?? 'lucene')); + // Read-only: the waterfall's Logs filter is the one input for both views. + const { logWhere, logWhereLanguage } = useWaterfallSearchState({}); + const logFilterLanguage = logWhereLanguage === 'sql' ? 'sql' : 'lucene'; - const { control, handleSubmit, setValue, reset, getValues } = useForm({ - defaultValues: { - logWhere: logWhere ?? '', - logWhereLanguage: displayedLanguage, - }, - }); - - // The filter is URL state, which can change without this form touching it - // (browser back/forward, a shared link). Re-seed the input so what's on - // screen is what the table is querying — and so a submit can't write a stale - // value back over the restored one. Submitting is a no-op here: it moves the - // URL to what the form already holds. - // - // Keyed on the URL alone: the stored language preference changes the moment - // the reader flips the language switch, and re-seeding on that would blank a - // filter they are halfway through typing. - useEffect(() => { - reset({ - logWhere: logWhere ?? '', - logWhereLanguage: urlLanguage ?? getValues('logWhereLanguage'), - }); - }, [logWhere, urlLanguage, reset, getValues]); + // Trimmed so a whitespace-only expression counts as unconfigured rather than + // rendering as a bare `=` in the trace filter. + const traceIdExpression = logSource?.traceIdExpression?.trim(); - const onSubmitFilter = useCallback( - (data: { logWhere: string; logWhereLanguage: string }) => { - submitFilters({ - logWhere: data.logWhere, - logWhereLanguage: data.logWhereLanguage, - }); - }, - [submitFilters], + const traceFilter = useMemo( + () => + traceIdExpression + ? SqlString.format('?=?', [SqlString.raw(traceIdExpression), traceId]) + : undefined, + [traceIdExpression, traceId], ); - // Trimmed so a whitespace-only expression falls into the empty state below - // rather than rendering as a bare `=` in the trace filter. - const traceIdExpression = logSource?.traceIdExpression?.trim(); - const config = useMemo((): BuilderChartConfigWithDateRange | undefined => { - if (logSource == null || !traceIdExpression) { + if (logSource == null || traceFilter == null) { return undefined; } return { @@ -132,88 +76,88 @@ export default function TraceLogsPanel({ // `tableFilterExpression`, sample weighting, and `source` id (which is // how the source's `querySettings` reach the query). ...buildSearchChartConfig(logSource, { - // The trace scope goes in `filters` so the user's filter keeps the - // whole `where` slot to itself — the two run in independent languages. - filters: [ - { - type: 'sql', - condition: SqlString.format('?=?', [ - SqlString.raw(traceIdExpression), - traceId, - ]), - }, - ], + // The trace scope goes in `filters` so the filter keeps the whole + // `where` slot to itself — the two run in independent languages. + filters: [{ type: 'sql', condition: traceFilter }], where: logWhere ?? '', whereLanguage: logFilterLanguage, - // Falls back to the source's default columns when these logs aren't - // the searched table's rows. - select: selectOverride ?? null, // Ascending: inside a trace, chronological order is execution order. orderBy: `${logSource.timestampValueExpression} ASC`, }), limit: { limit: 200 }, dateRange, }; + }, [logSource, traceFilter, logWhere, logFilterLanguage, dateRange]); + + const parentContext = use(RowSidePanelContext); + + // The same rows in the full search page: the trace scope plus whatever + // filter is set, in the language that filter runs in so the two combine. + const searchUrl = useMemo(() => { + const { generateSearchUrl } = parentContext; + if (generateSearchUrl == null || logSource == null || !traceIdExpression) { + return undefined; + } + const traceCondition = + logFilterLanguage === 'sql' + ? traceFilter + : `${traceIdExpression}:"${traceId}"`; + return generateSearchUrl({ + where: logWhere ? `${traceCondition} AND (${logWhere})` : traceCondition, + whereLanguage: logFilterLanguage, + source: logSource, + }); }, [ + parentContext, logSource, traceIdExpression, + traceFilter, traceId, - logWhere, logFilterLanguage, - dateRange, - selectOverride, + logWhere, ]); const handleRowDetailsClick = useCallback( (rowWhere: RowWhereResult, row: Record) => { + // The table selects the source's own expressions, so the body sits under + // its expression rather than a normalized alias. + const body = logSource ? row[getEventBody(logSource) ?? ''] : undefined; onNavigateToLog( rowWhere.where, rowWhere.aliasWith, - logSource ? getTableRowLabel(logSource, row) : 'Log', + typeof body === 'string' && body.length > 0 ? body : 'Log', ); }, [logSource, onNavigateToLog], ); - // The logs belong to a different source than the search this panel was - // opened from, so rebind search-url generation to them and drop the - // filter/column actions that only make sense against the searched source. - const parentContext = use(RowSidePanelContext); - const rowSidePanelContextValue = useMemo(() => { - const derived = logSource - ? deriveRowSidePanelContextForSource(parentContext, logSource) - : parentContext; - if (selectOverride != null) { - return derived; - } - // These columns are this table's own, not the searched table's, so the - // header's remove-column action would drop whatever column sits at that - // index in the *search* results. Take it away. - return { ...derived, displayedColumns: undefined, toggleColumn: undefined }; - }, [parentContext, logSource, selectOverride]); + // These logs come from a different source than the search this panel was + // opened from, so rebind search-url generation to them. The column actions go + // regardless: this table selects the source's own columns, so the header's + // remove-column action would drop whatever column sits at that index in the + // searched table instead. + const rowSidePanelContextValue = useMemo( + () => + logSource + ? { + ...deriveRowSidePanelContextForSource(parentContext, logSource), + displayedColumns: undefined, + toggleColumn: undefined, + } + : parentContext, + [parentContext, logSource], + ); if (isLoading) { return null; } - if (logSource == null) { - return ( - } - title="Correlated log source not found" - description="The log source linked to this trace source could not be loaded. Pick another one in the trace source's settings." - variant="card" - m="sm" - /> - ); - } - - if (config == null) { + if (config == null || logSource == null) { return ( } - title="No trace ID column configured" - description={`Set a trace ID expression on the "${logSource.name}" source to correlate its logs with traces.`} + title="Correlated logs unavailable" + description="This trace's log source is missing, or has no trace ID expression configured. Check the source's settings." variant="card" m="sm" /> @@ -225,37 +169,29 @@ export default function TraceLogsPanel({ - -
- + + + )}
({ // Test fixtures carry only the fields the panel reads; TSource's full shape is // irrelevant here and spelling it out would bury the gating under boilerplate. -const asSource = (source: Record) => - source as unknown as T; +const asSource = (source: Record) => + source as unknown as TSource; const TRACE_SOURCE_WITH_LOGS = asSource({ id: 'trace-src', @@ -104,8 +104,6 @@ jest.mock('@/TabBar', () => ({ type TraceLogsPanelProps = { logSourceId?: string; traceId?: string; - highlightedRowId?: string; - selectOverride?: unknown; onNavigateToLog?: ( rowId: string, aliasWith: unknown[], @@ -176,10 +174,7 @@ jest.mock('@/useFormatTime', () => ({ })); // NOTE: imported after the mock factories above. -import { - DBRowSidePanelInner, - RowSidePanelContext, -} from '@/components/DBRowSidePanel'; +import { DBRowSidePanelInner } from '@/components/DBRowSidePanel'; import { Tab } from '@/components/DBRowSidePanel.types'; import useSidePanelStack from '@/hooks/useSidePanelStack'; @@ -208,43 +203,17 @@ const ROW_WITH_TRACE = { __hdx_body: 'a log line', }; -// What the search page provides: the table config behind the row, whose -// `select` decides what a row id is made of. -const SEARCH_TABLE_CONFIG = { - connection: 'conn', - from: { databaseName: 'default', tableName: 'otel_logs' }, - timestampValueExpression: 'Timestamp', - select: 'Timestamp, Body', - where: '', - whereLanguage: 'lucene' as const, - dateRange: [new Date(0), new Date(1)] as [Date, Date], -}; - -function renderPanel( - source: TSource, - { rowId = 'row-1', withSearchTableConfig = false } = {}, -) { +function renderPanel(source: TSource, { rowId = 'row-1' } = {}) { function Harness() { const sidePanelStack = useSidePanelStack({ initialRowId: rowId }); return ( - (LOG_SOURCE) - : undefined, - dbSqlRowTableConfig: withSearchTableConfig - ? SEARCH_TABLE_CONFIG - : undefined, - }} - > - - + ); } return render( @@ -279,8 +248,6 @@ describe('DBRowSidePanelInner — trace logs tab', () => { expect(tabValues()).toContain(Tab.Logs); expect(mockTraceLogsProps.current.logSourceId).toBe('log-src'); expect(mockTraceLogsProps.current.traceId).toBe(TRACE_ID); - // Nothing to highlight: the row on screen is a span, not one of these logs. - expect(mockTraceLogsProps.current.highlightedRowId).toBeUndefined(); }); it('offers the tab on a log, pointed at its own source', () => { @@ -291,46 +258,6 @@ describe('DBRowSidePanelInner — trace logs tab', () => { expect(mockTraceLogsProps.current.logSourceId).toBe('log-src'); }); - it("shares the search's columns, and the row id built from them", () => { - mockQueryStore.sidePanelTab = Tab.Logs; - renderPanel(LOG_SOURCE, { withSearchTableConfig: true }); - - expect(mockTraceLogsProps.current.selectOverride).toBe( - SEARCH_TABLE_CONFIG.select, - ); - expect(mockTraceLogsProps.current.highlightedRowId).toBe('row-1'); - }); - - it("keeps the search's columns after drilling into another log", () => { - // A drilldown changes which row is shown, not which columns the reader - // chose, so the list must not silently switch column sets. - mockQueryStore.sidePanelTab = Tab.Logs; - mockQueryStore.sidePanelStackRoot = 'row-1'; - mockQueryStore.sidePanelNavStack = [ - { rowId: 'row-2', aliasWith: [], label: 'another log line' }, - ]; - renderPanel(LOG_SOURCE, { withSearchTableConfig: true }); - - expect(mockTraceLogsProps.current.selectOverride).toBe( - SEARCH_TABLE_CONFIG.select, - ); - // The highlight goes, though: a nested row's id may have been built by - // another panel selecting other columns (Surrounding Context falls back to - // the source's own), and an unmatchable id costs a paging hunt. - expect(mockTraceLogsProps.current.highlightedRowId).toBeUndefined(); - }); - - it('withholds the highlight when the tab selects its own columns', () => { - // A row id is built from the selected columns, so an id from a table - // selecting something else can never match one of these rows — and the - // table would page through the window hunting for it. - mockQueryStore.sidePanelTab = Tab.Logs; - renderPanel(LOG_SOURCE); - - expect(mockTraceLogsProps.current.selectOverride).toBeUndefined(); - expect(mockTraceLogsProps.current.highlightedRowId).toBeUndefined(); - }); - it('hides the tab when the trace source has no correlated log source', () => { renderPanel( asSource({ ...TRACE_SOURCE_WITH_LOGS, logSourceId: undefined }), diff --git a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx index 3e351027be..f2fecf6a12 100644 --- a/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx @@ -85,8 +85,6 @@ jest.mock('@/components/SearchInput/SearchWhereInput', () => ({
{name}
), getStoredLanguage: () => 'lucene', - resolveWhereLanguage: (v: unknown) => - v === 'sql' || v === 'lucene' ? v : undefined, })); const makeWaterfallSearchState = () => ({ diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx index 7a723c8048..46a814c7a5 100644 --- a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -1,34 +1,37 @@ import React from 'react'; -import type { Control } from 'react-hook-form'; -import { - BuilderChartConfigWithDateRange, - TLogSource, -} from '@hyperdx/common-utils/dist/types'; +import { BuilderChartConfigWithDateRange } from '@hyperdx/common-utils/dist/types'; +import { TLogSource } from '@hyperdx/common-utils/dist/types'; import { MantineProvider } from '@mantine/core'; -import { act, fireEvent, render, screen } from '@testing-library/react'; +import { act, render, screen } from '@testing-library/react'; const mockQueryStore: Record = {}; -const mockSetters: Record = {}; -function setterFor(key: string) { - if (!mockSetters[key]) mockSetters[key] = jest.fn(); - return mockSetters[key]; -} - jest.mock('nuqs', () => { const actual = jest.requireActual('nuqs'); return { ...actual, - useQueryState: (key: string) => { - if (!mockSetters[key]) mockSetters[key] = jest.fn(); - return [mockQueryStore[key] ?? null, mockSetters[key]]; - }, + useQueryState: (key: string) => [mockQueryStore[key] ?? null, jest.fn()], }; }); +jest.mock('next/link', () => ({ + __esModule: true, + default: ({ + href, + children, + ...rest + }: { + href: string; + children: React.ReactNode; + }) => ( + + {children} + + ), +})); + type RowTableProps = { config?: BuilderChartConfigWithDateRange; sourceId?: string; - highlightedLineId?: string; onRowDetailsClick?: ( rowWhere: { where: string; aliasWith: unknown[] }, row: Record, @@ -54,61 +57,27 @@ jest.mock('../DBRowTable', () => { }; }); -type FilterForm = { logWhere: string; logWhereLanguage: string }; - -let mockStoredLanguage = 'lucene'; - -jest.mock('../SearchInput/SearchWhereInput', () => { - const rhf = - jest.requireActual('react-hook-form'); - return { - __esModule: true, - default: ({ - control, - name, - onSubmit, - }: { - control: Control; - name: keyof FilterForm; - onSubmit?: () => void; - }) => { - // Surface the form's live value so a test can see what the user would. - const value = rhf.useWatch({ control, name }); - const language = rhf.useWatch({ control, name: 'logWhereLanguage' }); - return ( - <> - {String(value ?? '')} - - {String(language ?? '')} - - {/* Stands in for pressing enter in the real input. */} - - - ); - }, - getStoredLanguage: () => mockStoredLanguage, - resolveWhereLanguage: (v: unknown) => - v === 'sql' || v === 'lucene' ? v : undefined, - }; -}); - // A real context so `use(RowSidePanelContext)` works without pulling in the // side panel (which imports this component back). -const mockToggleColumn = jest.fn(); jest.mock('../DBRowSidePanel', () => { const react = jest.requireActual('react'); return { __esModule: true, RowSidePanelContext: react.createContext({ - // What the search page hands down: its own columns and their toggle. + // What the search page hands down: its own columns and their toggle, and + // a builder for links back into the full search. displayedColumns: ['Timestamp', 'Body'], - toggleColumn: mockToggleColumn, + toggleColumn: jest.fn(), + generateSearchUrl: ({ + where, + whereLanguage, + source, + }: { + where: string; + whereLanguage: string; + source?: { id: string }; + }) => + `/search?where=${encodeURIComponent(where)}&whereLanguage=${whereLanguage}&source=${source?.id}`, }), deriveRowSidePanelContextForSource: (parent: unknown) => parent, }; @@ -150,47 +119,45 @@ const LOG_SOURCE = asLogSource({ function renderPanel({ source = LOG_SOURCE, onNavigateToLog = jest.fn(), - selectOverride, traceId = TRACE_ID, - highlightedRowId, }: { /** Null for a source that no longer resolves. */ source?: TLogSource | null; onNavigateToLog?: jest.Mock; - selectOverride?: string; traceId?: string; - highlightedRowId?: string; } = {}) { mockUseSource.mockReturnValue({ data: source ?? undefined, isLoading: false, }); - // A fresh element per render: React bails out of re-rendering an identical one. - const tree = () => ( + render( - + , + ); + return { onNavigateToLog }; +} + +function searchWhere() { + const href = screen + .getByTestId('trace-logs-open-in-search') + .getAttribute('href')!; + return decodeURIComponent( + new URLSearchParams(href.split('?')[1]).get('where')!, ); - const { rerender } = render(tree()); - return { onNavigateToLog, rerender: () => rerender(tree()) }; } describe('TraceLogsPanel', () => { beforeEach(() => { Object.keys(mockQueryStore).forEach(k => delete mockQueryStore[k]); - Object.keys(mockSetters).forEach(k => delete mockSetters[k]); mockRowTableProps.current = {}; - mockUseSource.mockReset(); mockRowTableContext.current = {}; - mockToggleColumn.mockReset(); - mockStoredLanguage = 'lucene'; + mockUseSource.mockReset(); }); it('scopes the table to the trace without spending the where clause', () => { @@ -205,7 +172,6 @@ describe('TraceLogsPanel', () => { // Chronological: inside a trace that is execution order. expect(config.orderBy).toBe('Timestamp ASC'); expect(config.dateRange).toBe(DATE_RANGE); - // One page to start; the table pages on scroll from here. expect(config.limit).toEqual({ limit: 200 }); }); @@ -227,7 +193,7 @@ describe('TraceLogsPanel', () => { expect(config.source).toBe('log-src'); }); - it('applies the waterfall log filter and its language from the URL', () => { + it("applies the waterfall's log filter and its language", () => { mockQueryStore.logWhere = "SeverityText = 'error'"; mockQueryStore.logWhereLanguage = 'sql'; renderPanel(); @@ -237,95 +203,23 @@ describe('TraceLogsPanel', () => { expect(config.whereLanguage).toBe('sql'); }); - it('runs a URL filter that carries no language as Lucene, as the waterfall does', () => { - // A SQL editor preference must not reinterpret a filter the URL left - // unlabelled — the waterfall would run it as Lucene. - mockStoredLanguage = 'sql'; - mockQueryStore.logWhere = 'SeverityText:"error"'; - renderPanel(); - - expect(mockRowTableProps.current.config?.whereLanguage).toBe('lucene'); - expect(screen.getByTestId('log-filter-language')).toHaveTextContent( - 'lucene', - ); - }); - - it('offers the stored editor preference when there is no filter to interpret', () => { - mockStoredLanguage = 'sql'; - renderPanel(); - - expect(screen.getByTestId('log-filter-language')).toHaveTextContent('sql'); - }); - - describe('when the logs come from the searched source', () => { - // The reader's chosen columns, which differ from the source default. - const SEARCH_SELECT = 'Timestamp, Body'; - - it("reuses the search's columns so row ids match the highlighted row", () => { - renderPanel({ selectOverride: SEARCH_SELECT }); - - const config = mockRowTableProps.current.config!; - expect(config.select).toBe(SEARCH_SELECT); - // ...while the trace scope and ordering still come from this panel. - expect(config.filters).toEqual([ - { type: 'sql', condition: `TraceId='${TRACE_ID}'` }, - ]); - expect(config.where).toBe(''); - expect(config.orderBy).toBe('Timestamp ASC'); - expect(config.dateRange).toBe(DATE_RANGE); - // Source-level fields still come from the log source, not the config. - expect(config.source).toBe('log-src'); - }); - - it('leaves the remove-column action in place, since the columns are shared', () => { - renderPanel({ selectOverride: SEARCH_SELECT }); + it('escapes a trace id that carries SQL metacharacters', () => { + renderPanel({ traceId: "abc' OR 1=1--" }); - expect(mockRowTableContext.current.toggleColumn).toBe(mockToggleColumn); - }); + expect(mockRowTableProps.current.config?.filters).toEqual([ + { type: 'sql', condition: "TraceId='abc\\' OR 1=1--'" }, + ]); }); - it('drops the remove-column action when showing its own columns', () => { - // Otherwise the header's × maps by index onto the *search* table's select - // and drops an unrelated column from the search results. + it('drops the remove-column action, which belongs to the searched table', () => { + // Otherwise the header's × maps by index onto the search table's select and + // drops an unrelated column from the search results. renderPanel(); expect(mockRowTableContext.current.toggleColumn).toBeUndefined(); expect(mockRowTableContext.current.displayedColumns).toBeUndefined(); }); - it('re-seeds the filter input when the URL filter changes underneath it', () => { - mockQueryStore.logWhere = 'SeverityText:"error"'; - const { rerender } = renderPanel(); - expect(screen.getByTestId('log-filter-value')).toHaveTextContent( - 'SeverityText:"error"', - ); - - // Browser back restores an earlier filter without this form touching it. - mockQueryStore.logWhere = 'ServiceName:"cart"'; - act(() => rerender()); - - expect(screen.getByTestId('log-filter-value')).toHaveTextContent( - 'ServiceName:"cart"', - ); - expect(mockRowTableProps.current.config?.where).toBe('ServiceName:"cart"'); - }); - - it('submits the log filter without touching the spans filter', async () => { - mockQueryStore.logWhere = 'SeverityText:"error"'; - mockQueryStore.logWhereLanguage = 'sql'; - renderPanel(); - - await act(async () => { - fireEvent.click(screen.getByTestId('submit-log-filter')); - }); - - expect(setterFor('logWhere')).toHaveBeenCalledWith('SeverityText:"error"'); - expect(setterFor('logWhereLanguage')).toHaveBeenCalledWith('sql'); - // The waterfall's own filter is not this form's to write. - expect(setterFor('traceWhere')).not.toHaveBeenCalled(); - expect(setterFor('traceWhereLanguage')).not.toHaveBeenCalled(); - }); - it("labels a picked row with the log's body", () => { const { onNavigateToLog } = renderPanel(); @@ -356,41 +250,48 @@ describe('TraceLogsPanel', () => { expect(onNavigateToLog).toHaveBeenCalledWith('row-2', [], 'Log'); }); - it('hands the row to highlight to the table', () => { - renderPanel({ highlightedRowId: "Timestamp='2024-05-01 10:00:00'" }); + describe('open in search', () => { + it('carries the trace scope in the filter language, with the filter', () => { + mockQueryStore.logWhere = 'SeverityText:"error"'; + renderPanel(); - expect(mockRowTableProps.current.highlightedLineId).toBe( - "Timestamp='2024-05-01 10:00:00'", - ); - }); + expect(searchWhere()).toBe( + `TraceId:"${TRACE_ID}" AND (SeverityText:"error")`, + ); + }); - it('escapes a trace id that carries SQL metacharacters', () => { - renderPanel({ traceId: "abc' OR 1=1--" }); + it('scopes to the trace alone when no filter is set', () => { + renderPanel(); - expect(mockRowTableProps.current.config?.filters).toEqual([ - { type: 'sql', condition: "TraceId='abc\\' OR 1=1--'" }, - ]); - }); + expect(searchWhere()).toBe(`TraceId:"${TRACE_ID}"`); + }); - it('explains itself when the correlated log source no longer resolves', () => { - renderPanel({ source: null }); + it('writes the trace scope as SQL when the filter runs as SQL', () => { + mockQueryStore.logWhereLanguage = 'sql'; + mockQueryStore.logWhere = "SeverityText = 'error'"; + renderPanel(); - expect(screen.getByText('Correlated log source not found')).toBeVisible(); - expect(mockRowTableProps.current.config).toBeUndefined(); + expect(searchWhere()).toBe( + `TraceId='${TRACE_ID}' AND (SeverityText = 'error')`, + ); + }); }); it.each([ - ['undefined', undefined], - ['whitespace', ' '], - ])( - 'explains itself instead of querying when the trace id column is %s', - (_label, traceIdExpression) => { - renderPanel({ - source: asLogSource({ ...LOG_SOURCE, traceIdExpression }), - }); - - expect(screen.getByText('No trace ID column configured')).toBeVisible(); - expect(mockRowTableProps.current.config).toBeUndefined(); - }, - ); + ['the source no longer resolves', { source: null }], + [ + 'the source has no trace id column', + { source: asLogSource({ ...LOG_SOURCE, traceIdExpression: undefined }) }, + ], + // `min(1)` on the schema accepts a space, so this reaches the panel. + [ + 'the trace id column is whitespace', + { source: asLogSource({ ...LOG_SOURCE, traceIdExpression: ' ' }) }, + ], + ])('explains itself instead of querying when %s', (_label, props) => { + renderPanel(props); + + expect(screen.getByText('Correlated logs unavailable')).toBeVisible(); + expect(mockRowTableProps.current.config).toBeUndefined(); + }); }); diff --git a/packages/app/src/components/__tests__/rowLabel.test.ts b/packages/app/src/components/__tests__/rowLabel.test.ts deleted file mode 100644 index 6e298da16a..0000000000 --- a/packages/app/src/components/__tests__/rowLabel.test.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { TSource } from '@hyperdx/common-utils/dist/types'; - -import { getTableRowLabel } from '@/components/rowLabel'; - -const asSource = (source: Record) => - source as unknown as TSource; - -const LOG_SOURCE = asSource({ kind: 'log', bodyExpression: 'Body' }); -const TRACE_SOURCE = asSource({ - kind: 'trace', - spanNameExpression: 'SpanName', -}); - -describe('getTableRowLabel', () => { - it("reads the body from the source's own expression, as the table selects it", () => { - expect(getTableRowLabel(LOG_SOURCE, { Body: 'upstream timeout' })).toBe( - 'upstream timeout', - ); - expect(getTableRowLabel(TRACE_SOURCE, { SpanName: 'GET /checkout' })).toBe( - 'GET /checkout', - ); - }); - - it('serializes a body that is not a string', () => { - expect(getTableRowLabel(LOG_SOURCE, { Body: { msg: 'hi' } })).toBe( - '{"msg":"hi"}', - ); - }); - - it('names the kind of event when the row carries no body', () => { - expect(getTableRowLabel(LOG_SOURCE, { ServiceName: 'cart' })).toBe('Log'); - expect(getTableRowLabel(TRACE_SOURCE, {})).toBe('Span'); - expect(getTableRowLabel(LOG_SOURCE, undefined)).toBe('Log'); - }); - - it('names the kind of event for an empty body', () => { - expect(getTableRowLabel(LOG_SOURCE, { Body: '' })).toBe('Log'); - }); -}); diff --git a/packages/app/src/components/rowLabel.ts b/packages/app/src/components/rowLabel.ts deleted file mode 100644 index f5fa127525..0000000000 --- a/packages/app/src/components/rowLabel.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { isTraceSource, TSource } from '@hyperdx/common-utils/dist/types'; - -import { getEventBody } from '@/source'; - -/** - * Label for a row picked out of a `DBSqlRowTable` — what the side panel's - * breadcrumb shows for the row you drilled into. - * - * The table selects the source's own expressions, so the body sits under its - * expression, not under the `__hdx_body` alias `useRowData` projects. Falls - * back to the kind of event when the row's select doesn't carry the body (or - * carries it under an alias). - */ -export function getTableRowLabel( - source: TSource, - row: Record | undefined, -): string { - const bodyExpression = getEventBody(source); - const body = bodyExpression ? row?.[bodyExpression] : undefined; - if (typeof body === 'string') { - // An empty body names nothing; fall through to the kind of event. - return body.length > 0 ? body : eventKindLabel(source); - } - if (body != null) { - return JSON.stringify(body); - } - return eventKindLabel(source); -} - -function eventKindLabel(source: TSource): string { - return isTraceSource(source) ? 'Span' : 'Log'; -} diff --git a/packages/app/src/hooks/__tests__/useWaterfallSearchState.test.tsx b/packages/app/src/hooks/__tests__/useWaterfallSearchState.test.tsx deleted file mode 100644 index 4e279f3ebd..0000000000 --- a/packages/app/src/hooks/__tests__/useWaterfallSearchState.test.tsx +++ /dev/null @@ -1,87 +0,0 @@ -import { renderHook } from '@testing-library/react'; - -const mockSetters: Record = {}; -function setterFor(key: string) { - if (!mockSetters[key]) mockSetters[key] = jest.fn(); - return mockSetters[key]; -} - -jest.mock('nuqs', () => { - const actual = jest.requireActual('nuqs'); - return { - ...actual, - useQueryState: (key: string) => { - if (!mockSetters[key]) mockSetters[key] = jest.fn(); - return [null, mockSetters[key]]; - }, - }; -}); - -// NOTE: imported after the mock factory above. -import useWaterfallSearchState from '@/hooks/useWaterfallSearchState'; - -describe('useWaterfallSearchState onSubmit', () => { - beforeEach(() => { - Object.keys(mockSetters).forEach(k => delete mockSetters[k]); - }); - - it('writes only the log filter when only the log filter is submitted', () => { - const { result } = renderHook(() => - useWaterfallSearchState({ hasLogSource: true }), - ); - - result.current.onSubmit({ - logWhere: 'SeverityText:"error"', - logWhereLanguage: 'lucene', - }); - - expect(setterFor('logWhere')).toHaveBeenCalledWith('SeverityText:"error"'); - expect(setterFor('logWhereLanguage')).toHaveBeenCalledWith('lucene'); - expect(setterFor('traceWhere')).not.toHaveBeenCalled(); - expect(setterFor('traceWhereLanguage')).not.toHaveBeenCalled(); - }); - - it('writes only the spans filter when only the spans filter is submitted', () => { - const { result } = renderHook(() => - useWaterfallSearchState({ hasLogSource: true }), - ); - - result.current.onSubmit({ traceWhere: "StatusCode = 'Error'" }); - - expect(setterFor('traceWhere')).toHaveBeenCalledWith( - "StatusCode = 'Error'", - ); - expect(setterFor('logWhere')).not.toHaveBeenCalled(); - }); - - it('clears a filter submitted as empty', () => { - const { result } = renderHook(() => - useWaterfallSearchState({ hasLogSource: true }), - ); - - result.current.onSubmit({ logWhere: '', logWhereLanguage: '' }); - - expect(setterFor('logWhere')).toHaveBeenCalledWith(null); - expect(setterFor('logWhereLanguage')).toHaveBeenCalledWith(null); - }); - - it('writes both filters when the waterfall submits its whole form', () => { - const { result } = renderHook(() => - useWaterfallSearchState({ hasLogSource: true }), - ); - - result.current.onSubmit({ - traceWhere: "StatusCode = 'Error'", - logWhere: 'SeverityText:"error"', - traceWhereLanguage: 'sql', - logWhereLanguage: 'lucene', - }); - - expect(setterFor('traceWhere')).toHaveBeenCalledWith( - "StatusCode = 'Error'", - ); - expect(setterFor('logWhere')).toHaveBeenCalledWith('SeverityText:"error"'); - expect(setterFor('traceWhereLanguage')).toHaveBeenCalledWith('sql'); - expect(setterFor('logWhereLanguage')).toHaveBeenCalledWith('lucene'); - }); -}); diff --git a/packages/app/src/hooks/useWaterfallSearchState.tsx b/packages/app/src/hooks/useWaterfallSearchState.tsx index bcf0048fb2..552cdd9bf0 100644 --- a/packages/app/src/hooks/useWaterfallSearchState.tsx +++ b/packages/app/src/hooks/useWaterfallSearchState.tsx @@ -17,21 +17,15 @@ export default function useWaterfallSearchState({ const [isFilterExpanded, setIsFilterExpanded] = useState(isFilterActive); - // Only the keys present are written, so a caller that owns one of the two - // filters (e.g. the trace Logs tab) can submit it without clearing the other. const onSubmit = useCallback( (data: { - traceWhere?: string; - logWhere?: string; + traceWhere: string; + logWhere: string; traceWhereLanguage?: string; logWhereLanguage?: string; }) => { - if (data.traceWhere !== undefined) { - setTraceWhere(data.traceWhere || null); - } - if (data.logWhere !== undefined) { - setLogWhere(data.logWhere || null); - } + setTraceWhere(data.traceWhere || null); + setLogWhere(data.logWhere || null); if (data.traceWhereLanguage !== undefined) { setTraceWhereLanguage(data.traceWhereLanguage || null); } From 49746110eecf9869cdb4d7a69d9815325a9585ca Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Tue, 15 Sep 2026 00:39:27 -0700 Subject: [PATCH 10/18] test: Type the source fixtures instead of casting them The `as unknown as` casts each cost an eslint warning, and main's cap now sits at exactly its own count, so any addition fails the merge's lint. These fixtures need ~8 more fields to satisfy the real schemas, which is cheaper than a warning and reads better besides. Co-Authored-By: Claude Opus 5 (1M context) --- .../DBRowSidePanel.traceLogsTab.test.tsx | 49 ++++++++++++------- .../__tests__/TraceLogsPanel.test.tsx | 25 +++++----- 2 files changed, 42 insertions(+), 32 deletions(-) diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx index 6aa0398d9f..92535ec8f0 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx @@ -1,5 +1,10 @@ import React from 'react'; -import { TSource } from '@hyperdx/common-utils/dist/types'; +import { + SourceKind, + TLogSource, + TSource, + TTraceSource, +} from '@hyperdx/common-utils/dist/types'; import { MantineProvider } from '@mantine/core'; import { act, render } from '@testing-library/react'; @@ -52,29 +57,37 @@ jest.mock('../DBRowDataPanel', () => ({ getMapColumnNames: () => [], })); -// Test fixtures carry only the fields the panel reads; TSource's full shape is -// irrelevant here and spelling it out would bury the gating under boilerplate. -const asSource = (source: Record) => - source as unknown as TSource; - -const TRACE_SOURCE_WITH_LOGS = asSource({ +const TRACE_SOURCE_WITH_LOGS: TTraceSource = { id: 'trace-src', - kind: 'trace', + kind: SourceKind.Trace, + name: 'Demo traces', + connection: 'conn', + from: { databaseName: 'default', tableName: 'otel_traces' }, + timestampValueExpression: 'Timestamp', + defaultTableSelectExpression: 'Timestamp, ServiceName, SpanName', + durationExpression: 'Duration', + durationPrecision: 9, traceIdExpression: 'TraceId', spanIdExpression: 'SpanId', - logSourceId: 'log-src', - timestampValueExpression: 'Timestamp', + parentSpanIdExpression: 'ParentSpanId', + spanNameExpression: 'SpanName', + spanKindExpression: 'SpanKind', resourceAttributesExpression: 'ResourceAttributes', -}); + logSourceId: 'log-src', +}; -const LOG_SOURCE = asSource({ +const LOG_SOURCE: TLogSource = { id: 'log-src', - kind: 'log', - traceSourceId: 'trace-src', - traceIdExpression: 'TraceId', + kind: SourceKind.Log, + name: 'Demo logs', + connection: 'conn', + from: { databaseName: 'default', tableName: 'otel_logs' }, timestampValueExpression: 'Timestamp', + defaultTableSelectExpression: 'Timestamp, ServiceName, SeverityText, Body', + traceIdExpression: 'TraceId', + traceSourceId: 'trace-src', resourceAttributesExpression: 'ResourceAttributes', -}); +}; jest.mock('@/source', () => ({ __esModule: true, @@ -259,9 +272,7 @@ describe('DBRowSidePanelInner — trace logs tab', () => { }); it('hides the tab when the trace source has no correlated log source', () => { - renderPanel( - asSource({ ...TRACE_SOURCE_WITH_LOGS, logSourceId: undefined }), - ); + renderPanel({ ...TRACE_SOURCE_WITH_LOGS, logSourceId: undefined }); expect(tabValues()).not.toContain(Tab.Logs); }); diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx index 46a814c7a5..7c90136808 100644 --- a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -1,6 +1,9 @@ import React from 'react'; -import { BuilderChartConfigWithDateRange } from '@hyperdx/common-utils/dist/types'; -import { TLogSource } from '@hyperdx/common-utils/dist/types'; +import { + BuilderChartConfigWithDateRange, + SourceKind, + TLogSource, +} from '@hyperdx/common-utils/dist/types'; import { MantineProvider } from '@mantine/core'; import { act, render, screen } from '@testing-library/react'; @@ -99,13 +102,9 @@ const DATE_RANGE: [Date, Date] = [ new Date('2024-05-01T11:00:00Z'), ]; -// Only the fields the panel reads; the rest of TLogSource is irrelevant here. -const asLogSource = (source: Record) => - source as unknown as TLogSource; - -const LOG_SOURCE = asLogSource({ +const LOG_SOURCE: TLogSource = { id: 'log-src', - kind: 'log', + kind: SourceKind.Log, name: 'Demo logs', connection: 'conn', from: { databaseName: 'default', tableName: 'otel_logs' }, @@ -114,7 +113,7 @@ const LOG_SOURCE = asLogSource({ bodyExpression: 'Body', implicitColumnExpression: 'Body', traceIdExpression: 'TraceId', -}); +}; function renderPanel({ source = LOG_SOURCE, @@ -177,10 +176,10 @@ describe('TraceLogsPanel', () => { it("keeps the source's mandatory table filter ahead of the trace scope", () => { renderPanel({ - source: asLogSource({ + source: { ...LOG_SOURCE, tableFilterExpression: "ServiceName != 'internal'", - }), + }, }); const config = mockRowTableProps.current.config!; @@ -281,12 +280,12 @@ describe('TraceLogsPanel', () => { ['the source no longer resolves', { source: null }], [ 'the source has no trace id column', - { source: asLogSource({ ...LOG_SOURCE, traceIdExpression: undefined }) }, + { source: { ...LOG_SOURCE, traceIdExpression: undefined } }, ], // `min(1)` on the schema accepts a space, so this reaches the panel. [ 'the trace id column is whitespace', - { source: asLogSource({ ...LOG_SOURCE, traceIdExpression: ' ' }) }, + { source: { ...LOG_SOURCE, traceIdExpression: ' ' } }, ], ])('explains itself instead of querying when %s', (_label, props) => { renderPanel(props); From e23298cd756310d56d3646532deddca2376ca50a Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Tue, 15 Sep 2026 15:44:47 -0700 Subject: [PATCH 11/18] fix: Stop the trace logs tab reading the waterfall's filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading the shared `logWhere` param gave the tab a filter with nothing on screen naming it and no way to clear it — and on a log row, where no waterfall is offered at all, a filter authored against another source's columns serialises to `(1 = 0)`, so the tab read as "this trace logged nothing". The tab now lists the trace unfiltered; "Open in search" is the way on to anything narrower. That removes the language resolution too, so the rest follows: - The trace scope is one `buildDirectTraceWhereClause` call rather than a hand-rolled `SqlString.format`, and it goes in `where` rather than `filters` now that nothing else wants that slot. - "Open in search" builds its own URL. Through the context's `generateSearchUrl` it inherited the search page's time range and filter pills, so the link landed on a different row set than the tab shows; it also had to write the trace scope as a Lucene field name built from a free-form SQL expression, which for e.g. `hex(TraceId)` resolves to `(1 = 0)`. One SQL `where` plus the tab's own window, no branches. - `deriveRowSidePanelContextForSource` is gone from the tab's provider: the only reader under it is the table, which takes two fields this then clears anyway. - `ROW_LOOKUP_WINDOW_LEAD_HOURS`' comment now names this tab's span → log push, which is the direction that constant's lead actually carries. Panel: 204 lines -> 180. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/trace-logs-side-panel-tab.md | 2 +- .../app/src/components/TraceLogsPanel.tsx | 92 +++++++------------ .../__tests__/TraceLogsPanel.test.tsx | 76 ++++----------- packages/app/src/utils/rowTimestamps.ts | 17 ++-- 4 files changed, 64 insertions(+), 123 deletions(-) diff --git a/.changeset/trace-logs-side-panel-tab.md b/.changeset/trace-logs-side-panel-tab.md index 44166fd4ec..d8846177d7 100644 --- a/.changeset/trace-logs-side-panel-tab.md +++ b/.changeset/trace-logs-side-panel-tab.md @@ -4,4 +4,4 @@ Add a "Trace logs" tab to the event side panel, listing the trace's logs as a flat chronological table over the same time window the waterfall uses. It appears on any row that carries a trace id and resolves a log source — a span (via the trace source's correlated log source) or a log (its own source). Previously the only route to a trace's logs was hunting for the green rows interleaved in the waterfall, which a log-heavy trace buries. -The tab honours the log filter set in the Trace tab's waterfall, and "Open in search" hands the same query to the search page — the trace scope plus whatever filter is set, in the language that filter runs in. Picking a log opens it in the panel: a breadcrumb hop from a span, or a row change from another log. Sorting is ascending, which inside a trace is execution order. +The tab lists the trace unfiltered; "Open in search" hands the same query — same source, same trace, same window — to the search page for anything narrower. Picking a log opens it in the panel: a breadcrumb hop from a span, or a row change from another log. Sorting is ascending, which inside a trace is execution order. diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index 180b4bcefc..4f28ca2ca2 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -1,6 +1,5 @@ import { use, useCallback, useMemo } from 'react'; import Link from 'next/link'; -import SqlString from 'sqlstring'; import { buildSearchChartConfig } from '@hyperdx/common-utils/dist/core/searchChartConfig'; import { BuilderChartConfigWithDateRange, @@ -11,22 +10,18 @@ import { IconExternalLink, IconLogs } from '@tabler/icons-react'; import EmptyState from '@/components/EmptyState'; import { RowWhereResult, WithClause } from '@/hooks/useRowWhere'; -import useWaterfallSearchState from '@/hooks/useWaterfallSearchState'; import { getEventBody, useSource } from '@/source'; +import { buildDirectTraceWhereClause } from '@/utils/directTrace'; -import { - deriveRowSidePanelContextForSource, - RowSidePanelContext, -} from './DBRowSidePanel'; +import { RowSidePanelContext } from './DBRowSidePanel'; import { DBSqlRowTable } from './DBRowTable'; /** * One trace's logs, as a flat chronological table. * * Sibling view to the waterfall's interleaved log rows, for when a trace logs - * enough that reading them inside the span tree stops working. The waterfall - * owns the log filter; this honours whatever is set there, and hands the same - * query to the search page for anything more. + * enough that reading them inside the span tree stops working. Shows the whole + * trace, unfiltered — "Open in search" is the way on to anything narrower. */ export default function TraceLogsPanel({ logSourceId, @@ -50,24 +45,20 @@ export default function TraceLogsPanel({ kinds: [SourceKind.Log], }); - // Read-only: the waterfall's Logs filter is the one input for both views. - const { logWhere, logWhereLanguage } = useWaterfallSearchState({}); - const logFilterLanguage = logWhereLanguage === 'sql' ? 'sql' : 'lucene'; - // Trimmed so a whitespace-only expression counts as unconfigured rather than // rendering as a bare `=` in the trace filter. const traceIdExpression = logSource?.traceIdExpression?.trim(); - const traceFilter = useMemo( + const traceWhere = useMemo( () => traceIdExpression - ? SqlString.format('?=?', [SqlString.raw(traceIdExpression), traceId]) + ? buildDirectTraceWhereClause(traceIdExpression, traceId) : undefined, [traceIdExpression, traceId], ); const config = useMemo((): BuilderChartConfigWithDateRange | undefined => { - if (logSource == null || traceFilter == null) { + if (logSource == null || traceWhere == null) { return undefined; } return { @@ -76,46 +67,34 @@ export default function TraceLogsPanel({ // `tableFilterExpression`, sample weighting, and `source` id (which is // how the source's `querySettings` reach the query). ...buildSearchChartConfig(logSource, { - // The trace scope goes in `filters` so the filter keeps the whole - // `where` slot to itself — the two run in independent languages. - filters: [{ type: 'sql', condition: traceFilter }], - where: logWhere ?? '', - whereLanguage: logFilterLanguage, + where: traceWhere, + whereLanguage: 'sql', // Ascending: inside a trace, chronological order is execution order. orderBy: `${logSource.timestampValueExpression} ASC`, }), limit: { limit: 200 }, dateRange, }; - }, [logSource, traceFilter, logWhere, logFilterLanguage, dateRange]); - - const parentContext = use(RowSidePanelContext); + }, [logSource, traceWhere, dateRange]); - // The same rows in the full search page: the trace scope plus whatever - // filter is set, in the language that filter runs in so the two combine. + // The same rows in the full search page. Built here rather than through the + // context's `generateSearchUrl`, which stamps the search page's own time + // range and re-applies its filter pills — both would land the reader on a + // different row set than the tab is showing. const searchUrl = useMemo(() => { - const { generateSearchUrl } = parentContext; - if (generateSearchUrl == null || logSource == null || !traceIdExpression) { + if (logSource == null || traceWhere == null) { return undefined; } - const traceCondition = - logFilterLanguage === 'sql' - ? traceFilter - : `${traceIdExpression}:"${traceId}"`; - return generateSearchUrl({ - where: logWhere ? `${traceCondition} AND (${logWhere})` : traceCondition, - whereLanguage: logFilterLanguage, - source: logSource, + const params = new URLSearchParams({ + source: logSource.id, + where: traceWhere, + whereLanguage: 'sql', + from: dateRange[0].getTime().toString(), + to: dateRange[1].getTime().toString(), + isLive: 'false', }); - }, [ - parentContext, - logSource, - traceIdExpression, - traceFilter, - traceId, - logFilterLanguage, - logWhere, - ]); + return `/search?${params.toString()}`; + }, [logSource, traceWhere, dateRange]); const handleRowDetailsClick = useCallback( (rowWhere: RowWhereResult, row: Record) => { @@ -131,21 +110,18 @@ export default function TraceLogsPanel({ [logSource, onNavigateToLog], ); - // These logs come from a different source than the search this panel was - // opened from, so rebind search-url generation to them. The column actions go - // regardless: this table selects the source's own columns, so the header's + const parentContext = use(RowSidePanelContext); + + // This table selects the log source's own columns, so the header's // remove-column action would drop whatever column sits at that index in the - // searched table instead. + // searched table instead. Take it away. const rowSidePanelContextValue = useMemo( - () => - logSource - ? { - ...deriveRowSidePanelContextForSource(parentContext, logSource), - displayedColumns: undefined, - toggleColumn: undefined, - } - : parentContext, - [parentContext, logSource], + () => ({ + ...parentContext, + displayedColumns: undefined, + toggleColumn: undefined, + }), + [parentContext], ); if (isLoading) { diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx index 7c90136808..3c760d2564 100644 --- a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -7,15 +7,6 @@ import { import { MantineProvider } from '@mantine/core'; import { act, render, screen } from '@testing-library/react'; -const mockQueryStore: Record = {}; -jest.mock('nuqs', () => { - const actual = jest.requireActual('nuqs'); - return { - ...actual, - useQueryState: (key: string) => [mockQueryStore[key] ?? null, jest.fn()], - }; -}); - jest.mock('next/link', () => ({ __esModule: true, default: ({ @@ -142,31 +133,26 @@ function renderPanel({ return { onNavigateToLog }; } -function searchWhere() { +function searchParams() { const href = screen .getByTestId('trace-logs-open-in-search') .getAttribute('href')!; - return decodeURIComponent( - new URLSearchParams(href.split('?')[1]).get('where')!, - ); + return new URLSearchParams(href.split('?')[1]); } describe('TraceLogsPanel', () => { beforeEach(() => { - Object.keys(mockQueryStore).forEach(k => delete mockQueryStore[k]); mockRowTableProps.current = {}; mockRowTableContext.current = {}; mockUseSource.mockReset(); }); - it('scopes the table to the trace without spending the where clause', () => { + it('scopes the table to the trace', () => { renderPanel(); const config = mockRowTableProps.current.config!; - expect(config.filters).toEqual([ - { type: 'sql', condition: `TraceId='${TRACE_ID}'` }, - ]); - expect(config.where).toBe(''); + expect(config.where).toBe(`TraceId = '${TRACE_ID}'`); + expect(config.whereLanguage).toBe('sql'); expect(config.select).toBe(LOG_SOURCE.defaultTableSelectExpression); // Chronological: inside a trace that is execution order. expect(config.orderBy).toBe('Timestamp ASC'); @@ -174,7 +160,7 @@ describe('TraceLogsPanel', () => { expect(config.limit).toEqual({ limit: 200 }); }); - it("keeps the source's mandatory table filter ahead of the trace scope", () => { + it("keeps the source's mandatory table filter", () => { renderPanel({ source: { ...LOG_SOURCE, @@ -186,28 +172,17 @@ describe('TraceLogsPanel', () => { // Rows the source is configured to hide must stay hidden here too. expect(config.filters).toEqual([ { type: 'sql', condition: "ServiceName != 'internal'" }, - { type: 'sql', condition: `TraceId='${TRACE_ID}'` }, ]); // And the source id, which is how its querySettings reach the query. expect(config.source).toBe('log-src'); }); - it("applies the waterfall's log filter and its language", () => { - mockQueryStore.logWhere = "SeverityText = 'error'"; - mockQueryStore.logWhereLanguage = 'sql'; - renderPanel(); - - const config = mockRowTableProps.current.config!; - expect(config.where).toBe("SeverityText = 'error'"); - expect(config.whereLanguage).toBe('sql'); - }); - it('escapes a trace id that carries SQL metacharacters', () => { renderPanel({ traceId: "abc' OR 1=1--" }); - expect(mockRowTableProps.current.config?.filters).toEqual([ - { type: 'sql', condition: "TraceId='abc\\' OR 1=1--'" }, - ]); + expect(mockRowTableProps.current.config?.where).toBe( + "TraceId = 'abc\\' OR 1=1--'", + ); }); it('drops the remove-column action, which belongs to the searched table', () => { @@ -249,31 +224,16 @@ describe('TraceLogsPanel', () => { expect(onNavigateToLog).toHaveBeenCalledWith('row-2', [], 'Log'); }); - describe('open in search', () => { - it('carries the trace scope in the filter language, with the filter', () => { - mockQueryStore.logWhere = 'SeverityText:"error"'; - renderPanel(); - - expect(searchWhere()).toBe( - `TraceId:"${TRACE_ID}" AND (SeverityText:"error")`, - ); - }); - - it('scopes to the trace alone when no filter is set', () => { - renderPanel(); - - expect(searchWhere()).toBe(`TraceId:"${TRACE_ID}"`); - }); - - it('writes the trace scope as SQL when the filter runs as SQL', () => { - mockQueryStore.logWhereLanguage = 'sql'; - mockQueryStore.logWhere = "SeverityText = 'error'"; - renderPanel(); + it('links to a search that reproduces the table', () => { + renderPanel(); - expect(searchWhere()).toBe( - `TraceId='${TRACE_ID}' AND (SeverityText = 'error')`, - ); - }); + const params = searchParams(); + expect(params.get('where')).toBe(`TraceId = '${TRACE_ID}'`); + expect(params.get('whereLanguage')).toBe('sql'); + expect(params.get('source')).toBe('log-src'); + // The tab's window, not whatever range the search page was left on. + expect(params.get('from')).toBe(DATE_RANGE[0].getTime().toString()); + expect(params.get('to')).toBe(DATE_RANGE[1].getTime().toString()); }); it.each([ diff --git a/packages/app/src/utils/rowTimestamps.ts b/packages/app/src/utils/rowTimestamps.ts index 802051e1f3..4d183202a5 100644 --- a/packages/app/src/utils/rowTimestamps.ts +++ b/packages/app/src/utils/rowTimestamps.ts @@ -99,12 +99,17 @@ export function resolveRowTimestampAnchor({ * * 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. + * lookback is sized for "View Trace" (log → the span the log belongs to): 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 lead carries the opposite direction — the Trace logs tab's span → log push, + * where the destination log is at or after the origin span's start. It is enough + * because that tab only lists logs inside its own ±1h window + * (`oneHourRange` in `DBRowSidePanel`), so every log it can hand over lands + * within the lead. Shrink the lead and those hops fall back to `useRowData`'s + * unbounded scan. */ export const ROW_LOOKUP_WINDOW_LOOKBACK_HOURS = 4; export const ROW_LOOKUP_WINDOW_LEAD_HOURS = 1; From a954f6c9049ffbe687bd9ee8467d2adf5c092832 Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Tue, 15 Sep 2026 20:49:46 -0700 Subject: [PATCH 12/18] fix: Drop the searched table's cell filter action in the trace logs tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DBRowTableFieldWithPopover` reads `onPropertyAddClick` off the row side panel context, and on the search page that writes into the *searched* source's filter state. From a span row this table lists another source's rows, so the popover offered to filter a trace search by a log column — an unknown identifier against otel_traces. Nulled alongside the column actions, which the last round's plain spread had stopped doing. Filtering belongs in search, which "Open in search" opens; that link is now a plain const past the config guard, since its preconditions were the config's and the `searchUrl &&` wrapper could never be false. Co-Authored-By: Claude Opus 5 (1M context) --- .../app/src/components/TraceLogsPanel.tsx | 69 +++++++++---------- .../__tests__/TraceLogsPanel.test.tsx | 9 ++- 2 files changed, 38 insertions(+), 40 deletions(-) diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index 4f28ca2ca2..15bece5817 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -77,25 +77,6 @@ export default function TraceLogsPanel({ }; }, [logSource, traceWhere, dateRange]); - // The same rows in the full search page. Built here rather than through the - // context's `generateSearchUrl`, which stamps the search page's own time - // range and re-applies its filter pills — both would land the reader on a - // different row set than the tab is showing. - const searchUrl = useMemo(() => { - if (logSource == null || traceWhere == null) { - return undefined; - } - const params = new URLSearchParams({ - source: logSource.id, - where: traceWhere, - whereLanguage: 'sql', - from: dateRange[0].getTime().toString(), - to: dateRange[1].getTime().toString(), - isLive: 'false', - }); - return `/search?${params.toString()}`; - }, [logSource, traceWhere, dateRange]); - const handleRowDetailsClick = useCallback( (rowWhere: RowWhereResult, row: Record) => { // The table selects the source's own expressions, so the body sits under @@ -112,12 +93,15 @@ export default function TraceLogsPanel({ const parentContext = use(RowSidePanelContext); - // This table selects the log source's own columns, so the header's - // remove-column action would drop whatever column sits at that index in the - // searched table instead. Take it away. + // Both of the searched table's row actions are wrong for this table, which + // lists another source's rows with its own columns: the cell popover's + // filter buttons would write a log column into the searched source's + // filters, and the header's remove-column action maps by index onto the + // searched select. Filtering lives in search, which the link below opens. const rowSidePanelContextValue = useMemo( () => ({ ...parentContext, + onPropertyAddClick: undefined, displayedColumns: undefined, toggleColumn: undefined, }), @@ -128,7 +112,7 @@ export default function TraceLogsPanel({ return null; } - if (config == null || logSource == null) { + if (config == null || logSource == null || traceWhere == null) { return ( } @@ -140,6 +124,19 @@ export default function TraceLogsPanel({ ); } + // The same rows in the full search page. Built here rather than through the + // context's `generateSearchUrl`, which stamps the search page's own time + // range and re-applies its filter pills — both would land the reader on a + // different row set than the tab is showing. + const searchUrl = `/search?${new URLSearchParams({ + source: logSource.id, + where: traceWhere, + whereLanguage: 'sql', + from: dateRange[0].getTime().toString(), + to: dateRange[1].getTime().toString(), + isLive: 'false', + }).toString()}`; + return ( - {searchUrl && ( - - - - )} + + +
{ // a builder for links back into the full search. displayedColumns: ['Timestamp', 'Body'], toggleColumn: jest.fn(), + onPropertyAddClick: jest.fn(), generateSearchUrl: ({ where, whereLanguage, @@ -185,13 +186,15 @@ describe('TraceLogsPanel', () => { ); }); - it('drops the remove-column action, which belongs to the searched table', () => { - // Otherwise the header's × maps by index onto the search table's select and - // drops an unrelated column from the search results. + it("drops the searched table's row actions", () => { + // The header's × maps by index onto the search table's select, and the + // cell popover's filter buttons write into the searched source's filters — + // a log column against a trace search, from a span row. renderPanel(); expect(mockRowTableContext.current.toggleColumn).toBeUndefined(); expect(mockRowTableContext.current.displayedColumns).toBeUndefined(); + expect(mockRowTableContext.current.onPropertyAddClick).toBeUndefined(); }); it("labels a picked row with the log's body", () => { From 56d5c1964a384aadd4ff7d252d64a028015679a9 Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Wed, 16 Sep 2026 11:15:44 -0700 Subject: [PATCH 13/18] fix: Carry select and pre-encode the trace logs search link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The search page's query falls back to the source's columns when `select` is absent, but its SELECT input is seeded from the URL, so the link landed on a blank one — and submitting from there wrote `select=''` back. Pass the same columns the tab's table queries. `where` and `select` are read by `parseAsStringEncoded`, which decodes a level beyond the query string's own, so both go in pre-encoded the way nuqs writes them. Plain values survived by luck (its decode is a no-op, or throws and falls back); a select like `formatDateTime(Timestamp, '%F')` would not have. `orderBy` stays out: the tab is ascending because inside a trace that is execution order, but a search page landing oldest-first would surprise. The link reproduces the row set, not the sort. Co-Authored-By: Claude Opus 5 (1M context) --- packages/app/src/components/TraceLogsPanel.tsx | 10 +++++++++- .../src/components/__tests__/TraceLogsPanel.test.tsx | 12 ++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index 15bece5817..9cc2e40514 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -128,9 +128,17 @@ export default function TraceLogsPanel({ // context's `generateSearchUrl`, which stamps the search page's own time // range and re-applies its filter pills — both would land the reader on a // different row set than the tab is showing. + // + // `where` and `select` are read back by `parseAsStringEncoded`, which decodes + // one level beyond the query string's own, so they go in pre-encoded the way + // nuqs writes them. `select` is passed rather than left to the page's + // fallback: the query defaults to the source's columns either way, but the + // SELECT input is seeded from the URL, so omitting it lands the reader on a + // blank one. const searchUrl = `/search?${new URLSearchParams({ source: logSource.id, - where: traceWhere, + select: encodeURIComponent(logSource.defaultTableSelectExpression), + where: encodeURIComponent(traceWhere), whereLanguage: 'sql', from: dateRange[0].getTime().toString(), to: dateRange[1].getTime().toString(), diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx index a027f5b1e6..5136d5c093 100644 --- a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -231,12 +231,20 @@ describe('TraceLogsPanel', () => { renderPanel(); const params = searchParams(); - expect(params.get('where')).toBe(`TraceId = '${TRACE_ID}'`); - expect(params.get('whereLanguage')).toBe('sql'); expect(params.get('source')).toBe('log-src'); + expect(params.get('whereLanguage')).toBe('sql'); // The tab's window, not whatever range the search page was left on. expect(params.get('from')).toBe(DATE_RANGE[0].getTime().toString()); expect(params.get('to')).toBe(DATE_RANGE[1].getTime().toString()); + // One level of encoding beyond the query string's own, which is what + // `parseAsStringEncoded` decodes on the way in. + expect(decodeURIComponent(params.get('where')!)).toBe( + `TraceId = '${TRACE_ID}'`, + ); + // Carried explicitly so the page's SELECT input isn't blank. + expect(decodeURIComponent(params.get('select')!)).toBe( + LOG_SOURCE.defaultTableSelectExpression, + ); }); it.each([ From 3d07907aa353e14b95867eef2e2de4c571d15411 Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Wed, 16 Sep 2026 13:35:44 -0700 Subject: [PATCH 14/18] fix: Give the trace logs table its order, identity and sort state One `orderBy` now feeds all three consumers instead of an inline string: - The search link carries it. `useDefaultOrderBy` always ends in DESC, so the linked page reversed the chronological order the tab exists to provide. - `initialSortBy` seeds the header. `config.orderBy` never reaches the table's own sort state, so the timestamp column showed no arrow and the first click re-applied the order already in effect. - `tableId` scopes the persisted column widths and wrap-lines preference, which were landing in the bucket shared by every unnamed table. Co-Authored-By: Claude Opus 5 (1M context) --- .../app/src/components/TraceLogsPanel.tsx | 23 ++++++++++++++++--- .../__tests__/TraceLogsPanel.test.tsx | 15 ++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index 9cc2e40514..ff88c938e9 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -12,6 +12,7 @@ import EmptyState from '@/components/EmptyState'; import { RowWhereResult, WithClause } from '@/hooks/useRowWhere'; import { getEventBody, useSource } from '@/source'; import { buildDirectTraceWhereClause } from '@/utils/directTrace'; +import { parseAsSortingStateString } from '@/utils/queryParsers'; import { RowSidePanelContext } from './DBRowSidePanel'; import { DBSqlRowTable } from './DBRowTable'; @@ -57,6 +58,11 @@ export default function TraceLogsPanel({ [traceIdExpression, traceId], ); + // Ascending: inside a trace, chronological order is execution order. + const orderBy = logSource + ? `${logSource.timestampValueExpression} ASC` + : undefined; + const config = useMemo((): BuilderChartConfigWithDateRange | undefined => { if (logSource == null || traceWhere == null) { return undefined; @@ -69,13 +75,12 @@ export default function TraceLogsPanel({ ...buildSearchChartConfig(logSource, { where: traceWhere, whereLanguage: 'sql', - // Ascending: inside a trace, chronological order is execution order. - orderBy: `${logSource.timestampValueExpression} ASC`, + orderBy, }), limit: { limit: 200 }, dateRange, }; - }, [logSource, traceWhere, dateRange]); + }, [logSource, traceWhere, orderBy, dateRange]); const handleRowDetailsClick = useCallback( (rowWhere: RowWhereResult, row: Record) => { @@ -124,6 +129,9 @@ export default function TraceLogsPanel({ ); } + const parsedSort = parseAsSortingStateString.parse(orderBy ?? ''); + const sortBy = parsedSort ? [parsedSort] : undefined; + // The same rows in the full search page. Built here rather than through the // context's `generateSearchUrl`, which stamps the search page's own time // range and re-applies its filter pills — both would land the reader on a @@ -140,6 +148,7 @@ export default function TraceLogsPanel({ select: encodeURIComponent(logSource.defaultTableSelectExpression), where: encodeURIComponent(traceWhere), whereLanguage: 'sql', + orderBy: encodeURIComponent(orderBy ?? ''), from: dateRange[0].getTime().toString(), to: dateRange[1].getTime().toString(), isLive: 'false', @@ -171,6 +180,14 @@ export default function TraceLogsPanel({ sourceId={logSource.id} config={config} queryKeyPrefix="trace-logs" + // Scopes the persisted column widths and wrap-lines preference to + // this table; without it they land in the bucket shared by every + // table that doesn't name itself. + tableId="trace-logs" + // So the timestamp header shows the order the query already runs + // in, and the reader's first click flips it rather than re-asserting + // it. + initialSortBy={sortBy} isLive={false} showExpandButton={false} onRowDetailsClick={handleRowDetailsClick} diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx index 5136d5c093..16989c61f4 100644 --- a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -26,6 +26,8 @@ jest.mock('next/link', () => ({ type RowTableProps = { config?: BuilderChartConfigWithDateRange; sourceId?: string; + tableId?: string; + initialSortBy?: { id: string; desc: boolean }[]; onRowDetailsClick?: ( rowWhere: { where: string; aliasWith: unknown[] }, row: Record, @@ -245,6 +247,19 @@ describe('TraceLogsPanel', () => { expect(decodeURIComponent(params.get('select')!)).toBe( LOG_SOURCE.defaultTableSelectExpression, ); + // And the tab's order, so the linked page doesn't reverse it. + expect(decodeURIComponent(params.get('orderBy')!)).toBe('Timestamp ASC'); + }); + + it('tells the table its own identity and sort state', () => { + renderPanel(); + + // Persisted column widths belong to this table, not the unnamed bucket. + expect(mockRowTableProps.current.tableId).toBe('trace-logs'); + // The header shows the order the query already runs in. + expect(mockRowTableProps.current.initialSortBy).toEqual([ + { id: 'Timestamp', desc: false }, + ]); }); it.each([ From 9eefc5808b060bdaa598eca760bf41c506489d82 Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Wed, 16 Sep 2026 17:20:45 -0700 Subject: [PATCH 15/18] fix: Let the source decide the ordering of the linked search Driving the feature by hand showed the `orderBy` I'd added to the link was honoured only on the same-source path: arriving with a different `source` than the page currently holds, its "source changed, reset select/orderBy to the defaults" behaviour discards it, so the destination listed the trace's logs newest-first regardless. Half-honoured is worse than absent, and the page's own default is the better ordering anyway: `optimizeDefaultOrderBy` derives it from the table's sorting key, so it scans better than a bare `Timestamp ASC`. This tab's ascending order is a reading aid for one trace, not a property of the row set the link hands over. Co-Authored-By: Claude Opus 5 (1M context) --- packages/app/src/components/TraceLogsPanel.tsx | 6 +++++- .../app/src/components/__tests__/TraceLogsPanel.test.tsx | 5 +++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index ff88c938e9..2e7c907831 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -143,12 +143,16 @@ export default function TraceLogsPanel({ // fallback: the query defaults to the source's columns either way, but the // SELECT input is seeded from the URL, so omitting it lands the reader on a // blank one. + // + // No `orderBy`: the search page derives its own from the table's sorting key + // (`optimizeDefaultOrderBy`), which is both the idiom there and the cheaper + // scan. This tab's ascending order is a reading aid for one trace, not a + // property of the row set the link hands over. const searchUrl = `/search?${new URLSearchParams({ source: logSource.id, select: encodeURIComponent(logSource.defaultTableSelectExpression), where: encodeURIComponent(traceWhere), whereLanguage: 'sql', - orderBy: encodeURIComponent(orderBy ?? ''), from: dateRange[0].getTime().toString(), to: dateRange[1].getTime().toString(), isLive: 'false', diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx index 16989c61f4..9e7647790e 100644 --- a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -247,8 +247,9 @@ describe('TraceLogsPanel', () => { expect(decodeURIComponent(params.get('select')!)).toBe( LOG_SOURCE.defaultTableSelectExpression, ); - // And the tab's order, so the linked page doesn't reverse it. - expect(decodeURIComponent(params.get('orderBy')!)).toBe('Timestamp ASC'); + // No ordering: the search page derives its own from the table's sorting + // key, which is the idiom there and the cheaper scan. + expect(params.get('orderBy')).toBeNull(); }); it('tells the table its own identity and sort state', () => { From 748fd888a048445e660506bb1ecfbd15228d75bb Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Sun, 20 Sep 2026 18:59:11 -0700 Subject: [PATCH 16/18] refactor: Cut the trace logs tab's comments back to what isn't obvious What's left states a constraint the code can't: the source schema's `min(1)` accepting a space, what the shared config builder carries, why all three of the searched table's row actions are wrong here, and the extra decode `parseAsStringEncoded` performs. The two docblock edits in shared files stay because this change made their existing text false. Co-Authored-By: Claude Opus 5 (1M context) --- .../app/src/components/DBRowSidePanel.tsx | 8 +-- .../app/src/components/TraceLogsPanel.tsx | 54 ++++--------------- .../DBRowSidePanel.traceLogsTab.test.tsx | 7 --- .../__tests__/TraceLogsPanel.test.tsx | 22 +------- packages/app/src/hooks/useSidePanelStack.ts | 7 ++- packages/app/src/utils/rowTimestamps.ts | 10 ++-- 6 files changed, 21 insertions(+), 87 deletions(-) diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index 3dba206a67..65552b75da 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -550,17 +550,12 @@ export const DBRowSidePanelInner = ({ ? source.traceSourceId : undefined; - // The source holding this trace's logs: the trace source's correlated log - // source, or the log source itself when the displayed row is already a log - // (where the tab lists its siblings in the same trace). const traceLogSourceId = isTraceSource(source) ? childSourceId : isLogSource(source) ? source.id : undefined; const enableTraceLogs = !!traceId && !!traceLogSourceId; - // The displayed row is itself one of the logs the tab lists, so picking one - // is a row change rather than a source hop. const isOwnTraceLogSource = traceLogSourceId === source.id; const enableServiceMap = traceId && traceSourceId; @@ -630,6 +625,7 @@ export const DBRowSidePanelInner = ({ if (traceLogSourceId == null) { return; } + // The row on screen is one of the logs listed, so this is a row change. if (isOwnTraceLogSource) { handleNavigateToRow(rowId, aliasWith, label, SourceKind.Log); return; @@ -640,7 +636,7 @@ export const DBRowSidePanelInner = ({ aliasWith, label, sourceKind: SourceKind.Log, - // Every log in the trace sits within seconds of the span we came from. + // Every log in the trace is within seconds of the origin span. focusTimestamp: rowFocusTimestamp, }); }, diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index 2e7c907831..58e2d8e2b8 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -17,13 +17,6 @@ import { parseAsSortingStateString } from '@/utils/queryParsers'; import { RowSidePanelContext } from './DBRowSidePanel'; import { DBSqlRowTable } from './DBRowTable'; -/** - * One trace's logs, as a flat chronological table. - * - * Sibling view to the waterfall's interleaved log rows, for when a trace logs - * enough that reading them inside the span tree stops working. Shows the whole - * trace, unfiltered — "Open in search" is the way on to anything narrower. - */ export default function TraceLogsPanel({ logSourceId, traceId, @@ -46,8 +39,7 @@ export default function TraceLogsPanel({ kinds: [SourceKind.Log], }); - // Trimmed so a whitespace-only expression counts as unconfigured rather than - // rendering as a bare `=` in the trace filter. + // `min(1)` on the schema accepts a space, which would render as a bare `=`. const traceIdExpression = logSource?.traceIdExpression?.trim(); const traceWhere = useMemo( @@ -58,7 +50,7 @@ export default function TraceLogsPanel({ [traceIdExpression, traceId], ); - // Ascending: inside a trace, chronological order is execution order. + // Inside a trace, chronological order is execution order. const orderBy = logSource ? `${logSource.timestampValueExpression} ASC` : undefined; @@ -68,10 +60,7 @@ export default function TraceLogsPanel({ return undefined; } return { - // Assembled through the shared builder so this query sees the same row - // set as the search page and alerts do — it carries the source's - // `tableFilterExpression`, sample weighting, and `source` id (which is - // how the source's `querySettings` reach the query). + // Carries the source's tableFilterExpression, sample weighting and id. ...buildSearchChartConfig(logSource, { where: traceWhere, whereLanguage: 'sql', @@ -84,8 +73,7 @@ export default function TraceLogsPanel({ const handleRowDetailsClick = useCallback( (rowWhere: RowWhereResult, row: Record) => { - // The table selects the source's own expressions, so the body sits under - // its expression rather than a normalized alias. + // The table selects the source's expressions, not normalized aliases. const body = logSource ? row[getEventBody(logSource) ?? ''] : undefined; onNavigateToLog( rowWhere.where, @@ -98,11 +86,8 @@ export default function TraceLogsPanel({ const parentContext = use(RowSidePanelContext); - // Both of the searched table's row actions are wrong for this table, which - // lists another source's rows with its own columns: the cell popover's - // filter buttons would write a log column into the searched source's - // filters, and the header's remove-column action maps by index onto the - // searched select. Filtering lives in search, which the link below opens. + // These rows aren't the searched table's: its cell filter would write a log + // column into that source's filters, and its column removal maps by index. const rowSidePanelContextValue = useMemo( () => ({ ...parentContext, @@ -132,22 +117,11 @@ export default function TraceLogsPanel({ const parsedSort = parseAsSortingStateString.parse(orderBy ?? ''); const sortBy = parsedSort ? [parsedSort] : undefined; - // The same rows in the full search page. Built here rather than through the - // context's `generateSearchUrl`, which stamps the search page's own time - // range and re-applies its filter pills — both would land the reader on a - // different row set than the tab is showing. - // - // `where` and `select` are read back by `parseAsStringEncoded`, which decodes - // one level beyond the query string's own, so they go in pre-encoded the way - // nuqs writes them. `select` is passed rather than left to the page's - // fallback: the query defaults to the source's columns either way, but the - // SELECT input is seeded from the URL, so omitting it lands the reader on a - // blank one. - // - // No `orderBy`: the search page derives its own from the table's sorting key - // (`optimizeDefaultOrderBy`), which is both the idiom there and the cheaper - // scan. This tab's ascending order is a reading aid for one trace, not a - // property of the row set the link hands over. + // Not `generateSearchUrl`: it stamps the search page's time range and filter + // pills, landing the reader on a different row set. `where` and `select` are + // read by `parseAsStringEncoded`, which decodes one level past the query + // string's own; `select` also seeds the page's SELECT input, which is blank + // without it. const searchUrl = `/search?${new URLSearchParams({ source: logSource.id, select: encodeURIComponent(logSource.defaultTableSelectExpression), @@ -184,13 +158,7 @@ export default function TraceLogsPanel({ sourceId={logSource.id} config={config} queryKeyPrefix="trace-logs" - // Scopes the persisted column widths and wrap-lines preference to - // this table; without it they land in the bucket shared by every - // table that doesn't name itself. tableId="trace-logs" - // So the timestamp header shows the order the query already runs - // in, and the reader's first click flips it rather than re-asserting - // it. initialSortBy={sortBy} isLive={false} showExpandButton={false} diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx index 92535ec8f0..a6a4da0a6f 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanel.traceLogsTab.test.tsx @@ -298,12 +298,8 @@ describe('DBRowSidePanelInner — trace logs tab', () => { rowId: "TraceId='abc' AND SpanId='def'", label: 'another log line', sourceKind: 'log', - // Anchors the destination's row lookup: every log in a trace sits within - // seconds of the span we came from. focusTimestamp: new Date(TIMESTAMP_VALUE).toISOString(), }); - // A source hop resets the same-source drilldown trail rather than adding - // to it. expect(setterFor('sidePanelNavStack')).toHaveBeenCalledWith([]); }); @@ -318,12 +314,9 @@ describe('DBRowSidePanelInner — trace logs tab', () => { label: 'another log line', sourceKind: 'log', }); - // No source hop: the cross-source trail is rewritten unchanged (empty). expect(setterFor('sidePanelSourceStack')).toHaveBeenCalledWith([]); }); - // That Tab.Logs never *becomes* the remembered reading view is pinned in - // hooks/__tests__/useSidePanelStack.test.tsx; this covers the wiring. it('lands the picked log on a reading view, not on another logs list', () => { mockQueryStore.sidePanelTab = Tab.Logs; renderPanel(LOG_SOURCE); diff --git a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx index 9e7647790e..3b73199810 100644 --- a/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx +++ b/packages/app/src/components/__tests__/TraceLogsPanel.test.tsx @@ -46,22 +46,17 @@ jest.mock('../DBRowTable', () => { __esModule: true, DBSqlRowTable: (props: RowTableProps) => { mockRowTableProps.current = props; - // The real table reads these off the context, not its props. mockRowTableContext.current = react.use(panel.RowSidePanelContext); return null; }, }; }); -// A real context so `use(RowSidePanelContext)` works without pulling in the -// side panel (which imports this component back). jest.mock('../DBRowSidePanel', () => { const react = jest.requireActual('react'); return { __esModule: true, RowSidePanelContext: react.createContext({ - // What the search page hands down: its own columns and their toggle, and - // a builder for links back into the full search. displayedColumns: ['Timestamp', 'Body'], toggleColumn: jest.fn(), onPropertyAddClick: jest.fn(), @@ -157,7 +152,6 @@ describe('TraceLogsPanel', () => { expect(config.where).toBe(`TraceId = '${TRACE_ID}'`); expect(config.whereLanguage).toBe('sql'); expect(config.select).toBe(LOG_SOURCE.defaultTableSelectExpression); - // Chronological: inside a trace that is execution order. expect(config.orderBy).toBe('Timestamp ASC'); expect(config.dateRange).toBe(DATE_RANGE); expect(config.limit).toEqual({ limit: 200 }); @@ -172,11 +166,9 @@ describe('TraceLogsPanel', () => { }); const config = mockRowTableProps.current.config!; - // Rows the source is configured to hide must stay hidden here too. expect(config.filters).toEqual([ { type: 'sql', condition: "ServiceName != 'internal'" }, ]); - // And the source id, which is how its querySettings reach the query. expect(config.source).toBe('log-src'); }); @@ -189,9 +181,6 @@ describe('TraceLogsPanel', () => { }); it("drops the searched table's row actions", () => { - // The header's × maps by index onto the search table's select, and the - // cell popover's filter buttons write into the searched source's filters — - // a log column against a trace search, from a span row. renderPanel(); expect(mockRowTableContext.current.toggleColumn).toBeUndefined(); @@ -235,29 +224,22 @@ describe('TraceLogsPanel', () => { const params = searchParams(); expect(params.get('source')).toBe('log-src'); expect(params.get('whereLanguage')).toBe('sql'); - // The tab's window, not whatever range the search page was left on. expect(params.get('from')).toBe(DATE_RANGE[0].getTime().toString()); expect(params.get('to')).toBe(DATE_RANGE[1].getTime().toString()); - // One level of encoding beyond the query string's own, which is what - // `parseAsStringEncoded` decodes on the way in. + // `parseAsStringEncoded` decodes one level past the query string's own. expect(decodeURIComponent(params.get('where')!)).toBe( `TraceId = '${TRACE_ID}'`, ); - // Carried explicitly so the page's SELECT input isn't blank. expect(decodeURIComponent(params.get('select')!)).toBe( LOG_SOURCE.defaultTableSelectExpression, ); - // No ordering: the search page derives its own from the table's sorting - // key, which is the idiom there and the cheaper scan. expect(params.get('orderBy')).toBeNull(); }); it('tells the table its own identity and sort state', () => { renderPanel(); - // Persisted column widths belong to this table, not the unnamed bucket. expect(mockRowTableProps.current.tableId).toBe('trace-logs'); - // The header shows the order the query already runs in. expect(mockRowTableProps.current.initialSortBy).toEqual([ { id: 'Timestamp', desc: false }, ]); @@ -269,7 +251,7 @@ describe('TraceLogsPanel', () => { 'the source has no trace id column', { source: { ...LOG_SOURCE, traceIdExpression: undefined } }, ], - // `min(1)` on the schema accepts a space, so this reaches the panel. + // `min(1)` on the schema accepts a space. [ 'the trace id column is whitespace', { source: { ...LOG_SOURCE, traceIdExpression: ' ' } }, diff --git a/packages/app/src/hooks/useSidePanelStack.ts b/packages/app/src/hooks/useSidePanelStack.ts index 510c8fb032..8b6fe15577 100644 --- a/packages/app/src/hooks/useSidePanelStack.ts +++ b/packages/app/src/hooks/useSidePanelStack.ts @@ -15,10 +15,9 @@ export const LAST_TAB_STORAGE_KEY = 'hdx-side-panel-last-tab'; /** * Tabs that are a way to *find* another row rather than a way to read the - * current one, so they never become the remembered preference. Opening - * Surrounding Context or the trace's logs to pick out a row shouldn't cost you - * the reading view you had chosen, nor strand you on another list once you - * land. + * current one, so they never become the remembered preference. Opening one to + * pick out a row shouldn't cost you the reading view you had chosen, nor + * strand you on another list once you land. */ const NAVIGATIONAL_TABS: readonly Tab[] = [Tab.Context, Tab.Logs]; diff --git a/packages/app/src/utils/rowTimestamps.ts b/packages/app/src/utils/rowTimestamps.ts index 4d183202a5..9bcee0032b 100644 --- a/packages/app/src/utils/rowTimestamps.ts +++ b/packages/app/src/utils/rowTimestamps.ts @@ -103,13 +103,9 @@ export function resolveRowTimestampAnchor({ * 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 lead carries the opposite direction — the Trace logs tab's span → log push, - * where the destination log is at or after the origin span's start. It is enough - * because that tab only lists logs inside its own ±1h window - * (`oneHourRange` in `DBRowSidePanel`), so every log it can hand over lands - * within the lead. Shrink the lead and those hops fall back to `useRowData`'s - * unbounded scan. + * The lead carries the opposite direction, the Trace logs tab's span → log + * push, and is enough only because that tab lists logs within its own ±1h + * window. */ export const ROW_LOOKUP_WINDOW_LOOKBACK_HOURS = 4; export const ROW_LOOKUP_WINDOW_LEAD_HOURS = 1; From 2e4d987d10a923f0d5b9f18c6d012e59bdbfbb41 Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Sun, 20 Sep 2026 19:37:52 -0700 Subject: [PATCH 17/18] fix: Open the trace logs search as a fresh navigation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Title-case the tab to match its siblings, and drop `next/link` from the search link. Client-side navigation keeps the search page mounted, so it inherits the outgoing source's state, and its reset declines to rewrite select/orderBy when the form is only catching up to the config — deliberately, since a link may have carried them. The page therefore kept an ORDER BY derived for the previous source: on a schema whose log sorting key is `TimestampTime`, switching back to traces then queried otel_traces with `ORDER BY (TimestampTime, Timestamp)`, which that table has no column for. A plain anchor boots the page from the URL instead. Measured against a seeded stack: the link no longer plants an orderBy, and switching back to traces now derives the trace source's own. Co-Authored-By: Claude Opus 5 (1M context) --- packages/app/src/components/DBRowSidePanel.tsx | 2 +- packages/app/src/components/TraceLogsPanel.tsx | 3 +-- .../components/__tests__/TraceLogsPanel.test.tsx | 16 ---------------- packages/app/src/utils/rowTimestamps.ts | 2 +- 4 files changed, 3 insertions(+), 20 deletions(-) diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index 65552b75da..17f3412748 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -1063,7 +1063,7 @@ export const DBRowSidePanelInner = ({ ...(enableTraceLogs ? [ { - text: 'Trace logs', + text: 'Trace Logs', value: Tab.Logs, }, ] diff --git a/packages/app/src/components/TraceLogsPanel.tsx b/packages/app/src/components/TraceLogsPanel.tsx index 58e2d8e2b8..c515f618ef 100644 --- a/packages/app/src/components/TraceLogsPanel.tsx +++ b/packages/app/src/components/TraceLogsPanel.tsx @@ -1,5 +1,4 @@ import { use, useCallback, useMemo } from 'react'; -import Link from 'next/link'; import { buildSearchChartConfig } from '@hyperdx/common-utils/dist/core/searchChartConfig'; import { BuilderChartConfigWithDateRange, @@ -145,7 +144,7 @@ export default function TraceLogsPanel({