diff --git a/.changeset/trace-span-follows-opened-result.md b/.changeset/trace-span-follows-opened-result.md new file mode 100644 index 0000000000..0e6eb514c1 --- /dev/null +++ b/.changeset/trace-span-follows-opened-result.md @@ -0,0 +1,5 @@ +--- +'@hyperdx/app': patch +--- + +Fix the trace waterfall sticking on the previously opened span when another span of the same trace is opened, including from a session replay. Closing the result clears the selected span. Opening a different result selects that result's span, including when the waterfall stays mounted on cached data. diff --git a/packages/app/src/SessionSidePanel.tsx b/packages/app/src/SessionSidePanel.tsx index a5ff7414ae..943c947feb 100644 --- a/packages/app/src/SessionSidePanel.tsx +++ b/packages/app/src/SessionSidePanel.tsx @@ -38,6 +38,7 @@ import SidePanelBreadcrumbs, { BreadcrumbItem, } from '@/components/SidePanelBreadcrumbs'; import { useCloseOnClickOutside } from '@/hooks/useCloseOnClickOutside'; +import { useClearSpanSelection } from '@/hooks/useOpenedRowSpanSelection'; import useResizable from '@/hooks/useResizable'; import { WithClause } from '@/hooks/useRowWhere'; import { @@ -129,6 +130,8 @@ export default function SessionSidePanel({ initialRowId: selectedEvent?.rowId, }); + const clearSpanSelection = useClearSpanSelection(); + const handleBackToSession = useCallback(() => { setSelectedEvent(null); sidePanelStack.clearTrail(); @@ -136,18 +139,20 @@ export default function SessionSidePanel({ const handleEventNavigate = useCallback( (rowId: string, aliasWith: WithClause[]) => { + clearSpanSelection(); sidePanelStack.clearTrail(); setSelectedEvent({ rowId, aliasWith, sessionId }); }, - [setSelectedEvent, sidePanelStack, sessionId], + [clearSpanSelection, setSelectedEvent, sidePanelStack, sessionId], ); // X / Esc-at-root closes the whole panel and clears the session-panel params. const handleClose = useCallback(() => { + clearSpanSelection(); setSelectedEvent(null); sidePanelStack.clearTrail(); onClose(); - }, [setSelectedEvent, sidePanelStack, onClose]); + }, [clearSpanSelection, setSelectedEvent, sidePanelStack, onClose]); useHotkeys(['esc'], handleClose, { enabled: !selectedEvent }); diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index cf0650612d..30e4b718d8 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -9,6 +9,7 @@ import { } from 'react'; import { add } from 'date-fns'; import { isString } from 'lodash'; +import { useQueryState } from 'nuqs'; import { ErrorBoundary, FallbackProps } from 'react-error-boundary'; import { useHotkeys } from 'react-hotkeys-hook'; import SqlString from 'sqlstring'; @@ -37,6 +38,7 @@ import { import { IconCopy, IconKeyboard, IconShare, IconX } from '@tabler/icons-react'; import { useCloseOnClickOutside } from '@/hooks/useCloseOnClickOutside'; +import { useOpenedRowSpanSelection } from '@/hooks/useOpenedRowSpanSelection'; import useResizable from '@/hooks/useResizable'; import { WithClause } from '@/hooks/useRowWhere'; import useSidePanelStack, { @@ -77,6 +79,7 @@ import { DrawerFullWidthToggle, INITIAL_DRAWER_WIDTH_PERCENT, } from './DrawerUtils'; +import { eventRowWhereParser } from './eventRowWhere'; import LogLevel from './LogLevel'; import SidePanelBreadcrumbs, { BreadcrumbItem } from './SidePanelBreadcrumbs'; import { SpanLinkData } from './SpanLinksSubpanel'; @@ -203,6 +206,7 @@ function SidePanelHeaderActions({ size="sm" onClick={onClose} aria-label="Close" + data-testid="side-panel-close" > @@ -407,18 +411,22 @@ export const DBRowSidePanelInner = ({ [pushSource], ); + const clearSpanSelection = useOpenedRowSpanSelection(initialRowId); + const handlePanelBack = useCallback(() => { // Pop one level (nav → source), restoring the tab active before that level // was entered. When the trail is empty, leave the panel: hand off to an - // embedding parent (session) or close. + // embedding parent (session) or close. The search drawer clears the span + // in its own close handler; a parent (the session) does not. if (popOne() === 'none') { if (onNavigateToParent) { + clearSpanSelection(); onNavigateToParent(); } else { onClose(); } } - }, [popOne, onNavigateToParent, onClose]); + }, [popOne, onNavigateToParent, onClose, clearSpanSelection]); useHotkeys(['esc'], handlePanelBack); @@ -760,7 +768,23 @@ export const DBRowSidePanelInner = ({ const items: BreadcrumbItem[] = []; if (parentBreadcrumbs) { - items.push(...parentBreadcrumbs); + // A parent crumb (the session name) leaves this row without going + // through Back or Close. Clear the span on that click too. + items.push( + ...parentBreadcrumbs.map(item => { + const onClick = item.onClick; + if (onClick == null) { + return item; + } + return { + ...item, + onClick: () => { + clearSpanSelection(); + onClick(); + }, + }; + }), + ); } const crumbSourceStack = sourceStack; @@ -822,6 +846,7 @@ export const DBRowSidePanelInner = ({ source.kind, handleBreadcrumbNavigation, parentBreadcrumbs, + clearSpanSelection, ]); const availableTabs = useMemo(() => { @@ -1327,6 +1352,10 @@ export default function DBRowSidePanelErrorBoundary({ }, [isFullWidth, setSize]); const { clear: clearTraceWaterfallSearchState } = useWaterfallSearchState({}); + const [, setEventRowWhere] = useQueryState( + 'eventRowWhere', + eventRowWhereParser, + ); const sidePanelStack = useSidePanelStack({ initialRowId: rowId }); @@ -1336,8 +1365,16 @@ export default function DBRowSidePanelErrorBoundary({ // Clear waterfall search state on close, so that filters don't // persist when reopening another trace. clearTraceWaterfallSearchState(); + // The selected span is in the URL. Leaving it there makes the next open, + // and any shared row link, show the previous span. + setEventRowWhere(null); onClose(); - }, [sidePanelStack, onClose, clearTraceWaterfallSearchState]); + }, [ + sidePanelStack, + onClose, + clearTraceWaterfallSearchState, + setEventRowWhere, + ]); useCloseOnClickOutside({ // Only close on outside click at the root level. When the user has diff --git a/packages/app/src/components/DBTracePanel.tsx b/packages/app/src/components/DBTracePanel.tsx index 822eb208d4..f29346d023 100644 --- a/packages/app/src/components/DBTracePanel.tsx +++ b/packages/app/src/components/DBTracePanel.tsx @@ -10,14 +10,12 @@ import { useAtom } from 'jotai'; import { atomWithStorage } from 'jotai/utils'; import { useQueryState } from 'nuqs'; import { useForm, useWatch } from 'react-hook-form'; -import { z } from 'zod'; import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; import { isLogSource, isTraceSource, SourceKind, TSource, - WithClauseSchema, } from '@hyperdx/common-utils/dist/types'; import { ActionIcon, @@ -38,10 +36,10 @@ import { import { DBTraceWaterfallChartContainer } from '@/components/DBTraceWaterfallChart'; import { SQLInlineEditorControlled } from '@/components/SQLEditor/SQLInlineEditor'; import useResizable from '@/hooks/useResizable'; +import { RowHighlightHint } from '@/hooks/useRowHighlightHint'; import { WithClause } from '@/hooks/useRowWhere'; import { useSource, useUpdateSource } from '@/source'; import TabBar from '@/TabBar'; -import { parseAsJsonEncoded } from '@/utils/queryParsers'; import DBInfraPanel from './DBInfraPanel'; import { RowDataPanel, rowHasK8sContext, useRowData } from './DBRowDataPanel'; @@ -50,6 +48,7 @@ import { deriveRowSidePanelContextForSource, RowSidePanelContext, } from './DBRowSidePanel'; +import { eventRowWhereParser } from './eventRowWhere'; import SourceSchemaPreview, { isSourceSchemaPreviewEnabled, } from './SourceSchemaPreview'; @@ -57,28 +56,6 @@ import { SourceSelectControlled } from './SourceSelect'; import resizeStyles from '@/../styles/ResizablePanel.module.scss'; -type EventRowWhere = { - id: string; - type: string; - aliasWith: WithClause[]; - // The trace this span selection was made in. Used to gate a selection left in - // the URL from a previous trace so it can't render against a different one. - traceId?: string; -}; - -// Validate the persisted span selection so a stale / hand-edited `eventRowWhere` -// (valid JSON but wrong shape) resolves to null instead of feeding a malformed -// row id into the span detail query. -const eventRowWhereSchema = z.object({ - id: z.string(), - type: z.string(), - aliasWith: z.array(WithClauseSchema), - traceId: z.string().optional(), -}); -const eventRowWhereParser = parseAsJsonEncoded( - eventRowWhereSchema.parse, -); - enum SpanDetailTab { Overview = 'overview', Parsed = 'parsed', @@ -255,11 +232,7 @@ export default function DBTracePanel({ // Passed in from side panel to try to identify which // span in the chart to highlight first without constructing // a full row where clause - initialRowHighlightHint?: { - timestamp: string; - spanId: string; - body: string; - }; + initialRowHighlightHint?: RowHighlightHint; emptyState?: ReactNode; 'data-testid'?: string; }) { diff --git a/packages/app/src/components/DBTraceWaterfallChart.tsx b/packages/app/src/components/DBTraceWaterfallChart.tsx index 759fdebe29..eaadd947ee 100644 --- a/packages/app/src/components/DBTraceWaterfallChart.tsx +++ b/packages/app/src/components/DBTraceWaterfallChart.tsx @@ -1,11 +1,4 @@ -import { - ReactNode, - useCallback, - useEffect, - useMemo, - useRef, - useState, -} from 'react'; +import { ReactNode, useCallback, useMemo, useState } from 'react'; import _, { omit } from 'lodash'; import { useForm } from 'react-hook-form'; import SqlString from 'sqlstring'; @@ -59,6 +52,10 @@ import { } from '@/components/traceWaterfallLimits'; import { useColumns } from '@/hooks/useMetadata'; import useOffsetPaginatedQuery from '@/hooks/useOffsetPaginatedQuery'; +import { + RowHighlightHint, + useRowHighlightHint, +} from '@/hooks/useRowHighlightHint'; import useRowWhere, { WithClause } from '@/hooks/useRowWhere'; import useWaterfallSearchState from '@/hooks/useWaterfallSearchState'; import { @@ -686,11 +683,7 @@ export function DBTraceWaterfallChartContainer({ aliasWith: WithClause[]; }) => void; highlightedRowWhere?: string | null; - initialRowHighlightHint?: { - timestamp: string; - spanId: string; - body: string; - }; + initialRowHighlightHint?: RowHighlightHint; emptyState?: ReactNode; /** Extra controls rendered in the waterfall controls bar (e.g. the correlated logs source selector). */ controlsExtra?: ReactNode; @@ -967,53 +960,13 @@ export function DBTraceWaterfallChartContainer({ logRowsMeta, ]); - // Auto-select the originating span once when the panel opens — but only if - // nothing is already selected. Two cases must NOT trigger a (re-)select: - // - the user explicitly cleared the selection (closing the span detail), and - // - a selection was restored from the URL on load (deep link / reload). - // We mark the hint applied as soon as a selection exists or we apply it - // ourselves, so it never fires twice for the same hint; it re-fires only when - // the hint genuinely changes (different originating span / trace). - const appliedHighlightHintRef = useRef(null); - - useEffect(() => { - appliedHighlightHintRef.current = null; - }, [traceId]); - - useEffect(() => { - if (!initialRowHighlightHint || !onClick) { - return; - } - - const hintKey = `${initialRowHighlightHint.timestamp}|${initialRowHighlightHint.spanId}|${initialRowHighlightHint.body}`; - if (appliedHighlightHintRef.current === hintKey) { - return; - } - - // A selection already exists (restored from the URL, or user-chosen). Honor - // it and record the hint so we never override it — now or later. - if (highlightedRowWhere != null) { - appliedHighlightHintRef.current = hintKey; - return; - } - - const initialRowHighlightIndex = rows.findIndex(row => { - return ( - row.Timestamp === initialRowHighlightHint.timestamp && - row.SpanId === initialRowHighlightHint.spanId && - row.Body === initialRowHighlightHint.body - ); - }); - - if (initialRowHighlightIndex !== -1) { - appliedHighlightHintRef.current = hintKey; - onClick({ - id: rows[initialRowHighlightIndex].id, - type: rows[initialRowHighlightIndex].type ?? '', - aliasWith: rows[initialRowHighlightIndex].aliasWith, - }); - } - }, [initialRowHighlightHint, rows, onClick, highlightedRowWhere]); + useRowHighlightHint({ + traceId, + initialRowHighlightHint, + highlightedRowWhere, + rows, + onClick, + }); // 3 Edge-cases // 1. No spans, just logs (ex. sampling) diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.spanSelection.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.spanSelection.test.tsx new file mode 100644 index 0000000000..fe72300553 --- /dev/null +++ b/packages/app/src/components/__tests__/DBRowSidePanel.spanSelection.test.tsx @@ -0,0 +1,255 @@ +import { SourceKind, TLogSource } from '@hyperdx/common-utils/dist/types'; +import { MantineProvider } from '@mantine/core'; +import { fireEvent, render, screen } from '@testing-library/react'; + +// Controlled stand-in for nuqs. Values are the parsed shapes the panel reads. +// Prefixed with `mock` so the hoisted jest.mock factory can close over them. +const mockQueryStore: Record = {}; +const mockSetters: Record = {}; + +function seedParam(key: string, value: unknown) { + mockQueryStore[key] = value; +} +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); + const setters = mockSetters; + if (!setters[key]) setters[key] = jest.fn(); + return [value, setters[key]]; + }, + }; +}); + +jest.mock('../DBRowDataPanel', () => ({ + __esModule: true, + useRowData: () => ({ + data: undefined, + isLoading: false, + isSuccess: false, + isError: false, + error: null, + }), + ROW_DATA_ALIASES: { + DURATION_MS: '__hdx_duration', + SPAN_KIND: '__hdx_span_kind', + SERVICE_NAME: '__hdx_service_name', + SEVERITY_TEXT: '__hdx_severity_text', + }, + rowHasK8sContext: () => false, + RowDataPanel: () => null, +})); + +jest.mock('@/source', () => ({ + __esModule: true, + getEventBody: () => '__hdx_body', + useSource: () => ({ data: undefined, isLoading: false, isSuccess: false }), +})); + +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('../LogLevel', () => ({ __esModule: true, default: () => null })); +jest.mock('../ServiceMap/ServiceMapSidePanel', () => ({ + __esModule: true, + default: () => null, +})); +jest.mock('../TimelineChart/utils', () => ({ + __esModule: true, + renderMs: () => '', +})); +jest.mock('../DrawerUtils', () => ({ + __esModule: true, + DrawerFullWidthToggle: () => null, + INITIAL_DRAWER_WIDTH_PERCENT: 50, +})); +jest.mock('@/LogSidePanelElements', () => ({ + __esModule: true, + KeyboardShortcutsModal: () => null, +})); +jest.mock('@/TabBar', () => ({ __esModule: true, default: () => null })); +jest.mock('@/useFormatTime', () => ({ + __esModule: true, + FormatTime: () => null, +})); + +import DBRowSidePanelErrorBoundary, { + DBRowSidePanelInner, +} from '@/components/DBRowSidePanel'; +import useSidePanelStack from '@/hooks/useSidePanelStack'; + +const ROOT_SOURCE: TLogSource = { + id: 'log-src', + kind: SourceKind.Log, + name: 'Logs', + connection: 'conn', + from: { databaseName: 'default', tableName: 'otel_logs' }, + timestampValueExpression: 'Timestamp', + defaultTableSelectExpression: 'Timestamp, Body', + traceSourceId: 'trace-src', +}; + +const urlSpan = { + id: 'span-where', + type: 'trace', + aliasWith: [], + traceId: 'trace-1', +}; + +function renderBoundary(rowId: string) { + return render( + + + , + ); +} + +function InnerHarness({ + rowId, + onClose, + onNavigateToParent, + parentBreadcrumbs, +}: { + rowId: string; + onClose: () => void; + onNavigateToParent?: () => void; + parentBreadcrumbs?: { label: string; onClick?: () => void }[]; +}) { + const sidePanelStack = useSidePanelStack({ initialRowId: rowId }); + return ( + + ); +} + +describe('DBRowSidePanelErrorBoundary span selection', () => { + beforeEach(() => { + resetQueryState(); + seedParam('eventRowWhere', urlSpan); + }); + + it('keeps an eventRowWhere that arrived with the URL on the first render', () => { + renderBoundary('row-a'); + + expect(setterFor('eventRowWhere')).not.toHaveBeenCalled(); + }); + + it('clears the span when the opened row changes', () => { + const view = renderBoundary('row-a'); + + view.rerender( + + + , + ); + + expect(setterFor('eventRowWhere')).toHaveBeenCalledWith(null); + }); + + it('clears the span when the drawer closes', () => { + renderBoundary('row-a'); + + fireEvent.click(screen.getByLabelText('Close')); + + expect(setterFor('eventRowWhere')).toHaveBeenCalledTimes(1); + expect(setterFor('eventRowWhere')).toHaveBeenCalledWith(null); + }); + + it('clears the span when leaving an embedded event for its parent', () => { + const onNavigateToParent = jest.fn(); + render( + + + , + ); + + fireEvent.click(screen.getByLabelText('Back')); + + expect(setterFor('eventRowWhere')).toHaveBeenCalledTimes(1); + expect(setterFor('eventRowWhere')).toHaveBeenCalledWith(null); + expect(onNavigateToParent).toHaveBeenCalled(); + }); + + it('clears the span when the session breadcrumb is clicked', () => { + const onSessionClick = jest.fn(); + render( + + + , + ); + + fireEvent.click(screen.getByTestId('side-panel-breadcrumb-0')); + + expect(setterFor('eventRowWhere')).toHaveBeenCalledTimes(1); + expect(setterFor('eventRowWhere')).toHaveBeenCalledWith(null); + expect(onSessionClick).toHaveBeenCalled(); + }); +}); diff --git a/packages/app/src/components/eventRowWhere.ts b/packages/app/src/components/eventRowWhere.ts new file mode 100644 index 0000000000..dfe6d1e592 --- /dev/null +++ b/packages/app/src/components/eventRowWhere.ts @@ -0,0 +1,22 @@ +import { z } from 'zod'; +import { WithClauseSchema } from '@hyperdx/common-utils/dist/types'; + +import { parseAsJsonEncoded } from '@/utils/queryParsers'; + +// Validate the persisted span selection so a stale / hand-edited `eventRowWhere` +// (valid JSON but wrong shape) resolves to null instead of feeding a malformed +// row id into the span detail query. +const eventRowWhereSchema = z.object({ + id: z.string(), + type: z.string(), + aliasWith: z.array(WithClauseSchema), + // The trace this span selection was made in. Used to gate a selection left in + // the URL from a previous trace so it can't render against a different one. + traceId: z.string().optional(), +}); + +export type EventRowWhere = z.infer; + +export const eventRowWhereParser = parseAsJsonEncoded( + eventRowWhereSchema.parse, +); diff --git a/packages/app/src/hooks/__tests__/useOpenedRowSpanSelection.test.tsx b/packages/app/src/hooks/__tests__/useOpenedRowSpanSelection.test.tsx new file mode 100644 index 0000000000..765f5c0c71 --- /dev/null +++ b/packages/app/src/hooks/__tests__/useOpenedRowSpanSelection.test.tsx @@ -0,0 +1,171 @@ +import { useQueryState } from 'nuqs'; +import { render } from '@testing-library/react'; + +import { eventRowWhereParser } from '@/components/eventRowWhere'; +import { useOpenedRowSpanSelection } from '@/hooks/useOpenedRowSpanSelection'; +import { + RowHighlightHint, + useRowHighlightHint, +} from '@/hooks/useRowHighlightHint'; + +type StoredSpan = { + id: string; + type: string; + aliasWith: unknown[]; + traceId: string; +}; + +// Mirrors nuqs: each useQueryState keeps its own state and hears updates +// through a subscription registered in useInsertionEffect, which runs before +// useLayoutEffect. Prefixed with `mock` so the hoisted jest.mock factory can +// close over it. +const mockSpanStore: { + value: unknown; + sets: unknown[]; + listeners: Set<(value: unknown) => void>; +} = { + value: null, + sets: [], + listeners: new Set(), +}; + +jest.mock('nuqs', () => { + const React = jest.requireActual('react'); + const actual = jest.requireActual('nuqs'); + return { + ...actual, + useQueryState: (key: string) => { + const [value, setValue] = React.useState( + key === 'eventRowWhere' ? mockSpanStore.value : null, + ); + React.useInsertionEffect(() => { + if (key !== 'eventRowWhere') { + return undefined; + } + const listener = (next: unknown) => setValue(next); + mockSpanStore.listeners.add(listener); + return () => { + mockSpanStore.listeners.delete(listener); + }; + }, [key]); + const set = React.useCallback( + (next: unknown) => { + if (key !== 'eventRowWhere') { + return; + } + mockSpanStore.value = next; + mockSpanStore.sets.push(next); + mockSpanStore.listeners.forEach(listener => listener(next)); + }, + [key], + ); + return [value, set]; + }, + }; +}); + +const hintA: RowHighlightHint = { + timestamp: 't1', + spanId: 'span-a', + body: 'a', +}; +const hintB: RowHighlightHint = { + timestamp: 't2', + spanId: 'span-b', + body: 'b', +}; + +function row(id: string, hint: RowHighlightHint) { + return { + id, + type: 'trace', + aliasWith: [], + Timestamp: hint.timestamp, + SpanId: hint.spanId, + Body: hint.body, + }; +} + +const rows = [row('where-a', hintA), row('where-b', hintB)]; + +const spanA: StoredSpan = { + id: 'where-a', + type: 'trace', + aliasWith: [], + traceId: 'trace-1', +}; + +function Waterfall({ hint }: { hint: RowHighlightHint }) { + const [eventRowWhere, setEventRowWhere] = useQueryState( + 'eventRowWhere', + eventRowWhereParser, + ); + const highlightedRowWhere = + eventRowWhere != null && eventRowWhere.traceId === 'trace-1' + ? eventRowWhere.id + : null; + + useRowHighlightHint({ + traceId: 'trace-1', + initialRowHighlightHint: hint, + highlightedRowWhere, + rows, + onClick: where => setEventRowWhere({ ...where, traceId: 'trace-1' }), + }); + return null; +} + +function Panel({ + rowId, + hint, + showTrace = true, +}: { + rowId: string; + hint: RowHighlightHint; + showTrace?: boolean; +}) { + useOpenedRowSpanSelection(rowId); + return showTrace ? : null; +} + +describe('useOpenedRowSpanSelection', () => { + beforeEach(() => { + mockSpanStore.value = spanA; + mockSpanStore.sets = []; + mockSpanStore.listeners = new Set(); + }); + + it('keeps a URL selection on the first mount', () => { + render(); + + expect(mockSpanStore.sets).toEqual([]); + expect(mockSpanStore.value).toEqual(spanA); + }); + + it('clears the previous span before a cached waterfall applies the next hint', () => { + const view = render(); + + view.rerender(); + + expect(mockSpanStore.sets[0]).toBeNull(); + expect(mockSpanStore.value).toEqual({ + id: 'where-b', + type: 'trace', + aliasWith: [], + traceId: 'trace-1', + }); + }); + + it('selects the new row when the trace view remounts after the row changes', () => { + mockSpanStore.value = null; + const view = render(); + expect(mockSpanStore.value).toMatchObject({ id: 'where-a' }); + + view.rerender(); + view.rerender(); + expect(mockSpanStore.value).toBeNull(); + + view.rerender(); + expect(mockSpanStore.value).toMatchObject({ id: 'where-b' }); + }); +}); diff --git a/packages/app/src/hooks/__tests__/useRowHighlightHint.test.tsx b/packages/app/src/hooks/__tests__/useRowHighlightHint.test.tsx new file mode 100644 index 0000000000..c9e13ee35d --- /dev/null +++ b/packages/app/src/hooks/__tests__/useRowHighlightHint.test.tsx @@ -0,0 +1,206 @@ +import { renderHook } from '@testing-library/react'; + +import { + RowHighlightHint, + useRowHighlightHint, +} from '@/hooks/useRowHighlightHint'; + +const hintA: RowHighlightHint = { + timestamp: 't1', + spanId: 'span-a', + body: 'a', +}; +const hintB: RowHighlightHint = { + timestamp: 't2', + spanId: 'span-b', + body: 'b', +}; + +function row(id: string, hint: RowHighlightHint) { + return { + id, + type: 'trace', + aliasWith: [], + Timestamp: hint.timestamp, + SpanId: hint.spanId, + Body: hint.body, + }; +} + +const rows = [row('where-a', hintA), row('where-b', hintB)]; + +function renderHint( + overrides: Partial[0]> = {}, +) { + const onClick = jest.fn(); + const view = renderHook( + (props: Parameters[0]) => + useRowHighlightHint(props), + { + initialProps: { + traceId: 'trace-1', + initialRowHighlightHint: hintA, + highlightedRowWhere: null as string | null, + rows, + onClick, + ...overrides, + }, + }, + ); + return { ...view, onClick }; +} + +describe('useRowHighlightHint', () => { + it('yields to a selection that is already present on the first hint', () => { + const { onClick, rerender } = renderHint({ + highlightedRowWhere: 'where-existing', + }); + + expect(onClick).not.toHaveBeenCalled(); + + rerender({ + traceId: 'trace-1', + initialRowHighlightHint: hintA, + highlightedRowWhere: 'where-existing', + rows, + onClick, + }); + + expect(onClick).not.toHaveBeenCalled(); + }); + + it('selects the matching row when nothing is selected', () => { + const { onClick } = renderHint(); + + expect(onClick).toHaveBeenCalledWith({ + id: 'where-a', + type: 'trace', + aliasWith: [], + }); + }); + + it('replaces the selection when a later hint arrives', () => { + const { onClick, rerender } = renderHint(); + + rerender({ + traceId: 'trace-1', + initialRowHighlightHint: hintB, + highlightedRowWhere: 'where-a', + rows, + onClick, + }); + + expect(onClick).toHaveBeenLastCalledWith({ + id: 'where-b', + type: 'trace', + aliasWith: [], + }); + }); + + it('does not select again for the same hint', () => { + const { onClick, rerender } = renderHint(); + + rerender({ + traceId: 'trace-1', + initialRowHighlightHint: hintA, + highlightedRowWhere: 'where-a', + rows, + onClick, + }); + + expect(onClick).toHaveBeenCalledTimes(1); + }); + + it('yields again after the trace changes, then follows the next hint', () => { + const { onClick, rerender } = renderHint({ + highlightedRowWhere: 'where-existing', + }); + + rerender({ + traceId: 'trace-2', + initialRowHighlightHint: hintB, + highlightedRowWhere: 'where-existing', + rows, + onClick, + }); + + expect(onClick).not.toHaveBeenCalled(); + + rerender({ + traceId: 'trace-2', + initialRowHighlightHint: hintA, + highlightedRowWhere: 'where-existing', + rows, + onClick, + }); + + expect(onClick).toHaveBeenCalledWith({ + id: 'where-a', + type: 'trace', + aliasWith: [], + }); + }); + + it('waits until the hinted row is loaded', () => { + const { onClick, rerender } = renderHint({ rows: [] }); + + expect(onClick).not.toHaveBeenCalled(); + + rerender({ + traceId: 'trace-1', + initialRowHighlightHint: hintA, + highlightedRowWhere: null, + rows, + onClick, + }); + + expect(onClick).toHaveBeenCalledWith({ + id: 'where-a', + type: 'trace', + aliasWith: [], + }); + }); + + it('keeps an existing selection when remounted with a different hint', () => { + const first = renderHint({ highlightedRowWhere: 'where-a' }); + expect(first.onClick).not.toHaveBeenCalled(); + first.unmount(); + + const remounted = renderHint({ + initialRowHighlightHint: hintB, + highlightedRowWhere: 'where-a', + }); + + expect(remounted.onClick).not.toHaveBeenCalled(); + }); + + it('selects the new hint when remounted after the selection was cleared', () => { + const first = renderHint({ highlightedRowWhere: 'where-a' }); + first.unmount(); + + const remounted = renderHint({ + initialRowHighlightHint: hintB, + highlightedRowWhere: null, + }); + + expect(remounted.onClick).toHaveBeenCalledWith({ + id: 'where-b', + type: 'trace', + aliasWith: [], + }); + }); + + it('does not restore a selection the user cleared while the hint is unchanged', () => { + const { onClick, rerender } = renderHint(); + + rerender({ + traceId: 'trace-1', + initialRowHighlightHint: hintA, + highlightedRowWhere: null, + rows, + onClick, + }); + + expect(onClick).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/app/src/hooks/useOpenedRowSpanSelection.ts b/packages/app/src/hooks/useOpenedRowSpanSelection.ts new file mode 100644 index 0000000000..adc2d7ab5d --- /dev/null +++ b/packages/app/src/hooks/useOpenedRowSpanSelection.ts @@ -0,0 +1,38 @@ +import { useCallback, useLayoutEffect, useRef } from 'react'; +import { useQueryState } from 'nuqs'; + +import { eventRowWhereParser } from '@/components/eventRowWhere'; + +// The waterfall treats the first hint after mount as a restored selection and +// keeps it. Drop the previous span when the opened row changes so the next +// view follows the new row. The first mount keeps a selection that arrived +// with the URL. +// +// useLayoutEffect, not useEffect. Child effects run first, so a parent +// useEffect clears the span only after the waterfall has applied the new +// hint and marked it done. The next render then skips that hint. +export function useClearSpanSelection() { + const [, setEventRowWhere] = useQueryState( + 'eventRowWhere', + eventRowWhereParser, + ); + + return useCallback(() => { + setEventRowWhere(null); + }, [setEventRowWhere]); +} + +export function useOpenedRowSpanSelection(rowId: string | undefined) { + const clearSpanSelection = useClearSpanSelection(); + const openedRowIdRef = useRef(rowId); + + useLayoutEffect(() => { + if (openedRowIdRef.current === rowId) { + return; + } + openedRowIdRef.current = rowId; + clearSpanSelection(); + }, [rowId, clearSpanSelection]); + + return clearSpanSelection; +} diff --git a/packages/app/src/hooks/useRowHighlightHint.ts b/packages/app/src/hooks/useRowHighlightHint.ts new file mode 100644 index 0000000000..3a10311636 --- /dev/null +++ b/packages/app/src/hooks/useRowHighlightHint.ts @@ -0,0 +1,83 @@ +import { useEffect, useRef } from 'react'; + +import { WithClause } from '@/hooks/useRowWhere'; + +export type RowHighlightHint = { + timestamp: string; + spanId: string; + body: string; +}; + +type HighlightRow = { + id: string; + type?: string; + aliasWith: WithClause[]; + Timestamp?: unknown; + SpanId?: unknown; + Body?: unknown; +}; + +// Only the first hint after mount or a trace change yields to an existing +// selection (restored from the URL, or picked by the user). A later hint +// change while this hook stays mounted replaces the selection. A remount +// cannot tell a new row from that restored selection, so the side panel +// clears the span when its opened row changes. +export function useRowHighlightHint({ + traceId, + initialRowHighlightHint, + highlightedRowWhere, + rows, + onClick, +}: { + traceId: string; + initialRowHighlightHint?: RowHighlightHint; + highlightedRowWhere?: string | null; + rows: HighlightRow[]; + onClick?: (rowWhere: { + id: string; + type: string; + aliasWith: WithClause[]; + }) => void; +}) { + const appliedHighlightHintRef = useRef(null); + + useEffect(() => { + appliedHighlightHintRef.current = null; + }, [traceId]); + + useEffect(() => { + if (!initialRowHighlightHint || !onClick) { + return; + } + + const hintKey = `${initialRowHighlightHint.timestamp}|${initialRowHighlightHint.spanId}|${initialRowHighlightHint.body}`; + if (appliedHighlightHintRef.current === hintKey) { + return; + } + + if ( + appliedHighlightHintRef.current == null && + highlightedRowWhere != null + ) { + appliedHighlightHintRef.current = hintKey; + return; + } + + const initialRowHighlightIndex = rows.findIndex(row => { + return ( + row.Timestamp === initialRowHighlightHint.timestamp && + row.SpanId === initialRowHighlightHint.spanId && + row.Body === initialRowHighlightHint.body + ); + }); + + if (initialRowHighlightIndex !== -1) { + appliedHighlightHintRef.current = hintKey; + onClick({ + id: rows[initialRowHighlightIndex].id, + type: rows[initialRowHighlightIndex].type ?? '', + aliasWith: rows[initialRowHighlightIndex].aliasWith, + }); + } + }, [initialRowHighlightHint, rows, onClick, highlightedRowWhere]); +} diff --git a/packages/app/tests/e2e/components/SidePanelComponent.ts b/packages/app/tests/e2e/components/SidePanelComponent.ts index b8caefe762..753513db3e 100644 --- a/packages/app/tests/e2e/components/SidePanelComponent.ts +++ b/packages/app/tests/e2e/components/SidePanelComponent.ts @@ -229,6 +229,35 @@ export class SidePanelComponent { await this.getWaterfallSpan(name).first().click({ timeout: 10_000 }); } + /** + * The waterfall label currently marked selected. The highlighted class sits + * on the span's own `role="button"` row. + */ + getHighlightedWaterfallSpan(name: string) { + return this.panelContainer + .locator('[role="button"][class*="traceTimelineLabelHighlighted"]') + .filter({ hasText: name }); + } + + /** + * The span detail drawer beside the waterfall. Trace sources don't render an + * Overview tab of their own, so `tab-overview` plus the close control only + * exist together on this drawer. The tab bar's spacer is also `flex-grow-1`, + * so that class alone matches the tab chrome and misses the overview body. + * The overview root is `flex-grow-1 overflow-auto`. + */ + get spanDetailsPanel() { + const closeSpanDetails = this.page.getByRole('button', { + name: 'Close span details', + }); + return this.panelContainer + .locator('div') + .filter({ has: closeSpanDetails }) + .filter({ has: this.page.getByTestId('tab-overview') }) + .filter({ has: this.page.locator('.flex-grow-1.overflow-auto') }) + .last(); + } + /** * Rows in the span detail's "Span Links" section. Each row's open action * (`span-link-open-trace`) shows the linked span's name once the target diff --git a/packages/app/tests/e2e/components/TableComponent.ts b/packages/app/tests/e2e/components/TableComponent.ts index 5d08d65ead..51b2d53451 100644 --- a/packages/app/tests/e2e/components/TableComponent.ts +++ b/packages/app/tests/e2e/components/TableComponent.ts @@ -67,6 +67,44 @@ export class TableComponent { await this.firstRow.click(); } + async clickRowContaining(text: string) { + const row = this.getRows().filter({ hasText: text }).first(); + // The leading cell is the selection checkbox. Its click toggles selection + // and does not open the row. The content button does. An open side panel + // covers the right side of the table, so aim at the button's left edge. + // On a short viewport with the nav expanded that edge is covered too; the + // click still has to reach this button, since it is what switches the row. + const button = row.getByRole('button', { + name: /^(View details for log entry|Expand log row|Collapse log row)$/, + }); + if (await this.isCoveredBySidePanel(button)) { + // A pointer click at these coordinates lands on the drawer, which + // covers the row. The button is what switches the open row. + await button.evaluate(el => { + if (el instanceof HTMLElement) { + el.click(); + } + }); + return; + } + await button.click({ position: { x: 4, y: 8 } }); + } + + private async isCoveredBySidePanel(target: Locator): Promise { + const panel = this.page.getByTestId('row-side-panel'); + if ((await panel.count()) === 0 || !(await panel.isVisible())) { + return false; + } + const [targetBox, panelBox] = await Promise.all([ + target.boundingBox(), + panel.boundingBox(), + ]); + if (targetBox == null || panelBox == null) { + return false; + } + return panelBox.x <= targetBox.x + 8; + } + /** * Open a row in the side panel via its hover button. The button only exists * once the `expand` row-click preference is set — without it a plain diff --git a/packages/app/tests/e2e/features/traces-workflow.spec.ts b/packages/app/tests/e2e/features/traces-workflow.spec.ts index 6e4261170e..ec8b9d39b9 100644 --- a/packages/app/tests/e2e/features/traces-workflow.spec.ts +++ b/packages/app/tests/e2e/features/traces-workflow.spec.ts @@ -155,3 +155,92 @@ test.describe('Advanced Search Workflow - Traces', { tag: '@traces' }, () => { }); }); }); + +// trace-0 is four spans in one trace. Opening a result and then another must +// follow the span that was just opened, including a click inside the waterfall +// that is already on screen. +test.describe( + 'Trace waterfall span selection', + { tag: ['@traces', '@full-stack'] }, + () => { + let searchPage: SearchPage; + + test.beforeEach(async ({ page }) => { + searchPage = new SearchPage(page); + await searchPage.goto(); + await searchPage.selectSource(DEFAULT_TRACES_SOURCE_NAME); + await searchPage.timePicker.selectRelativeTime('Last 1 days'); + await searchPage.switchToLuceneMode(); + }); + + async function openSpanInWaterfall(spanId: string, spanName: string) { + await searchPage.performSearch(`SpanId:"${spanId}"`); + await expect(searchPage.table.firstRow).toBeVisible(); + await searchPage.table.clickFirstRow(); + await expect(searchPage.sidePanel.container).toBeVisible(); + await expect(searchPage.sidePanel.getTab('trace')).toBeVisible({ + timeout: 15_000, + }); + await searchPage.sidePanel.clickTab('trace'); + await expect( + searchPage.sidePanel.getWaterfallSpan(spanName).first(), + ).toBeVisible({ timeout: 15_000 }); + } + + test('Opening a span, closing it, opening another span from the same trace', async () => { + await openSpanInWaterfall('span-0-0', 'GET /api/logs'); + await expect( + searchPage.sidePanel.getHighlightedWaterfallSpan('GET /api/logs'), + ).toBeVisible({ timeout: 15_000 }); + await expect(searchPage.sidePanel.spanDetailsPanel).toContainText( + 'GET /api/logs', + { timeout: 15_000 }, + ); + + await searchPage.sidePanel.close(); + await expect(searchPage.sidePanel.container).toBeHidden(); + + await openSpanInWaterfall('span-0-1', 'POST /api/traces'); + await expect( + searchPage.sidePanel.getHighlightedWaterfallSpan('POST /api/traces'), + ).toBeVisible({ timeout: 15_000 }); + await expect( + searchPage.sidePanel.getHighlightedWaterfallSpan('GET /api/logs'), + ).toBeHidden(); + await expect(searchPage.sidePanel.spanDetailsPanel).toContainText( + 'POST /api/traces', + { timeout: 15_000 }, + ); + await expect(searchPage.sidePanel.spanDetailsPanel).not.toContainText( + 'GET /api/logs', + ); + }); + + test('Opening a span, then selecting another span from the same trace without closing the side panel', async () => { + await searchPage.performSearch('TraceId:"trace-0"'); + await searchPage.table.clickRowContaining('GET /api/logs'); + await expect(searchPage.sidePanel.container).toBeVisible(); + await expect(searchPage.sidePanel.getTab('trace')).toBeVisible({ + timeout: 15_000, + }); + await searchPage.sidePanel.clickTab('trace'); + await expect( + searchPage.sidePanel.getHighlightedWaterfallSpan('GET /api/logs'), + ).toBeVisible({ timeout: 15_000 }); + + await searchPage.table.clickRowContaining('POST /api/traces'); + + await expect(searchPage.sidePanel.container).toBeVisible(); + await expect( + searchPage.sidePanel.getHighlightedWaterfallSpan('POST /api/traces'), + ).toBeVisible({ timeout: 15_000 }); + await expect( + searchPage.sidePanel.getHighlightedWaterfallSpan('GET /api/logs'), + ).toBeHidden(); + await expect(searchPage.sidePanel.spanDetailsPanel).toContainText( + 'POST /api/traces', + { timeout: 15_000 }, + ); + }); + }, +);