From 5a21d173bf55455c69b6764fe237f8eb6b170866 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Wed, 4 Nov 2026 03:10:42 +0000 Subject: [PATCH 1/5] fix(trace-panel): start the waterfall higher in the trace side panel The trace side panel stacked a lot of fixed chrome above the waterfall: a bordered box repeating the span name that the panel title already shows, a ServiceName chip repeating the service from the metadata row, and a full row for the zoom hint. On a 1080p screen the first span sat past the middle of the viewport. - Skip the body box on trace spans when the current breadcrumb shows the span name in full. Long names, and View Trace hops where the title is the originating log, keep it. - Stop adding the implicit service-name chip on trace panels. Log panels keep it, and configured highlighted attributes are unaffected. - Move the zoom hint into the timeline's empty top-left corner, and tighten the gaps after the minimap, around the trace attributes, and above the first row (still clear of the hover time label). - Move the breadcrumb label limits into a small module so the side panel can apply the same truncation rule without importing the component. Co-Authored-By: Claude Opus 5.5 --- .changeset/compact-trace-side-panel.md | 11 + .../DBHighlightedAttributesList.tsx | 2 +- .../app/src/components/DBRowSidePanel.tsx | 25 +- .../src/components/DBRowSidePanelHeader.tsx | 15 +- .../src/components/DBTraceWaterfallChart.tsx | 13 +- .../src/components/SidePanelBreadcrumbs.tsx | 11 +- .../TimelineChart/TimelineChart.tsx | 28 +- .../__tests__/TimelineChart.test.tsx | 40 +++ .../DBRowSidePanel.compactHeader.test.tsx | 292 ++++++++++++++++++ .../__tests__/DBRowSidePanelHeader.test.tsx | 49 ++- .../__tests__/SidePanelBreadcrumbs.test.tsx | 25 ++ .../components/sidePanelBreadcrumbLimits.ts | 18 ++ 12 files changed, 492 insertions(+), 37 deletions(-) create mode 100644 .changeset/compact-trace-side-panel.md create mode 100644 packages/app/src/components/TimelineChart/__tests__/TimelineChart.test.tsx create mode 100644 packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.tsx create mode 100644 packages/app/src/components/sidePanelBreadcrumbLimits.ts diff --git a/.changeset/compact-trace-side-panel.md b/.changeset/compact-trace-side-panel.md new file mode 100644 index 0000000000..feb0782f49 --- /dev/null +++ b/.changeset/compact-trace-side-panel.md @@ -0,0 +1,11 @@ +--- +'@hyperdx/app': patch +--- + +fix: start the trace waterfall higher in the side panel + +Trace spans no longer repeat their name in a box under the panel title, or the +service as a chip next to the metadata row that already shows it. The +"⌘/Ctrl + scroll to zoom" hint moves into the timeline's top-left corner +instead of taking its own row. Long span names that the title truncates keep +the box. Log panels keep their body box and service chip. diff --git a/packages/app/src/components/DBHighlightedAttributesList.tsx b/packages/app/src/components/DBHighlightedAttributesList.tsx index 90c01f64de..a243e4f234 100644 --- a/packages/app/src/components/DBHighlightedAttributesList.tsx +++ b/packages/app/src/components/DBHighlightedAttributesList.tsx @@ -41,7 +41,7 @@ export function DBHighlightedAttributesList({ const hiddenAttributesCount = attributes.length - sortedAttributes.length; return ( - + {sortedAttributes.map(({ displayedKey, value, sql, lucene, source }) => ( (() => { const tabs: Tab[] = []; if (hasOverviewPanel && !sourceIsTrace) tabs.push(Tab.Overview); @@ -1075,6 +1087,7 @@ export const DBRowSidePanelInner = ({ attributes={highlightedAttributeValues} mainContent={mainContent} mainContentHeader={mainContentColumn} + bodyShownInTitle={isBodyShownInTitle} severityText={severityText} rowData={normalizedRow} /> diff --git a/packages/app/src/components/DBRowSidePanelHeader.tsx b/packages/app/src/components/DBRowSidePanelHeader.tsx index a253d71fe9..8995a9ab7c 100644 --- a/packages/app/src/components/DBRowSidePanelHeader.tsx +++ b/packages/app/src/components/DBRowSidePanelHeader.tsx @@ -24,12 +24,17 @@ export default function DBRowSidePanelHeader({ // source has neither body nor implicit column configured), the body // paper is suppressed entirely; the highlighted attributes still render. bodyConfigured = true, + // When `true`, the panel title already shows `mainContent` in full (a + // trace span's name, for example), so the body paper is omitted rather + // than repeating the title directly below it. + bodyShownInTitle = false, severityText, rowData, }: { mainContent?: string; mainContentHeader?: string; bodyConfigured?: boolean; + bodyShownInTitle?: boolean; attributes?: HighlightedAttribute[]; severityText?: string; rowData?: Record; @@ -85,7 +90,7 @@ export default function DBRowSidePanelHeader({ return ( <> - {!bodyConfigured ? null : mainContent ? ( + {!bodyConfigured || bodyShownInTitle ? null : mainContent ? ( )} - - - + {attributesWithDefault.length > 0 && ( + + + + )} ); } diff --git a/packages/app/src/components/DBTraceWaterfallChart.tsx b/packages/app/src/components/DBTraceWaterfallChart.tsx index 7811018e2f..15c7206f71 100644 --- a/packages/app/src/components/DBTraceWaterfallChart.tsx +++ b/packages/app/src/components/DBTraceWaterfallChart.tsx @@ -1394,12 +1394,7 @@ export function DBTraceWaterfallChartContainer({ return ( <> {showMinimap && ( - - - + )} {isFilterExpanded && (
@@ -1618,7 +1613,11 @@ export function DBTraceWaterfallChartContainer({ {!isFetching && !error && highlightedAttributeValues?.length > 0 && ( - + + + )}
{ return items.map((item, i) => { const isLast = i === items.length - 1; - const isSingle = items.length === 1; - const maxLen = isSingle - ? MAX_LABEL_LENGTH_SINGLE - : isLast - ? MAX_LABEL_LENGTH_CURRENT - : MAX_LABEL_LENGTH_PREVIOUS; + const maxLen = getBreadcrumbMaxLabelLength(i, items.length); const truncatedLabel = truncate(item.label, maxLen); const needsTooltip = item.label.length > maxLen; diff --git a/packages/app/src/components/TimelineChart/TimelineChart.tsx b/packages/app/src/components/TimelineChart/TimelineChart.tsx index 03f5b7ac77..dea3911a44 100644 --- a/packages/app/src/components/TimelineChart/TimelineChart.tsx +++ b/packages/app/src/components/TimelineChart/TimelineChart.tsx @@ -77,7 +77,10 @@ type TimelineChartProps = { const MIN_VIEWPORT_FRAC = 0.02; const axisHeight = 24; -const rowsMarginTop = 32; +// Gap between the axis and the first row. The hover time label +// (TimelineMouseCursor) reaches about 37px below the top of the chart, so the +// first row starts at axisHeight + rowsMarginTop = 40px to stay clear of it. +const rowsMarginTop = 16; const maxScale = 100; export const TimelineChart = memo(function (props: TimelineChartProps) { @@ -404,12 +407,6 @@ export const TimelineChart = memo(function (props: TimelineChartProps) { return ( - - - ⌘/Ctrl + scroll to zoom - - -
+ {/* The corner above the label column is otherwise empty, so the + zoom hint lives here instead of in its own row above the chart. */}
+ > + + ⌘/Ctrl + scroll to + zoom + +
{rowVirtualizer.getVirtualItems().map(virtualRow => { const row = rows[virtualRow.index]; diff --git a/packages/app/src/components/TimelineChart/__tests__/TimelineChart.test.tsx b/packages/app/src/components/TimelineChart/__tests__/TimelineChart.test.tsx new file mode 100644 index 0000000000..db50b02d0a --- /dev/null +++ b/packages/app/src/components/TimelineChart/__tests__/TimelineChart.test.tsx @@ -0,0 +1,40 @@ +import { screen } from '@testing-library/react'; + +import { TimelineChart } from '@/components/TimelineChart/TimelineChart'; +import type { TTimelineEvent } from '@/components/TimelineChart/TimelineChartRowEvents'; + +function makeRow(id: string, start: number, end: number) { + const event: TTimelineEvent = { + id, + start, + end, + tooltip: id, + color: 'white', + backgroundColor: 'gray', + body: {id}, + minWidthPx: 1, + }; + return { id, label: {id}, events: [event] }; +} + +describe('TimelineChart', () => { + it('shows the zoom hint in the corner above the labels, inside the chart', () => { + renderWithMantine( + , + ); + + const hint = screen.getByTestId('timeline-zoom-hint'); + expect(hint).toHaveTextContent('⌘/Ctrl + scroll to zoom'); + // Lives in the sticky corner cell of the scroll container rather than in + // a row of its own above the chart. + expect(hint.closest('.timelineCorner')).not.toBeNull(); + expect(hint.closest('.timelineContainer')).not.toBeNull(); + }); +}); diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.tsx new file mode 100644 index 0000000000..6b3c80108e --- /dev/null +++ b/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.tsx @@ -0,0 +1,292 @@ +import React from 'react'; +import { TSource } from '@hyperdx/common-utils/dist/types'; +import { MantineProvider } from '@mantine/core'; +import { render } from '@testing-library/react'; + +import { makeLogSource, makeTraceSource } from '@/llm/__fixtures__/sources'; + +// In-memory nuqs so a test can seed the side panel stack. Prefixed with +// `mock` so jest.mock factories may reference it. +const mockQueryStore: Record = {}; +jest.mock('nuqs', () => { + const actual = jest.requireActual('nuqs'); + return { + ...actual, + useQueryState: (key: string, parser?: { defaultValue?: unknown }) => [ + key in mockQueryStore + ? mockQueryStore[key] + : parser && 'defaultValue' in parser + ? parser.defaultValue + : null, + jest.fn(), + ], + }; +}); + +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: () => [], +})); + +// Resolves the trace source a "View Trace" frame points at. +const mockStackSource: { current: TSource | undefined } = { + current: undefined, +}; +jest.mock('@/source', () => ({ + __esModule: true, + getEventBody: () => 'SpanName', + useSource: ({ id }: { id: string | null }) => + id != null && id === mockStackSource.current?.id + ? { data: mockStackSource.current, isSuccess: true, isLoading: false } + : { data: undefined }, +})); + +type HeaderProps = { bodyShownInTitle?: boolean; mainContent?: string }; +const mockHeaderProps: { current: HeaderProps } = { current: {} }; +jest.mock('../DBRowSidePanelHeader', () => ({ + __esModule: true, + default: (props: HeaderProps) => { + mockHeaderProps.current = props; + return null; + }, +})); + +// Records the attribute expressions the panel asks to highlight. +const mockHighlightedExpressions: { current: { sqlExpression: string }[] } = { + current: [], +}; +jest.mock('@/utils/highlightedAttributes', () => ({ + __esModule: true, + getHighlightedAttributesFromData: ( + _source: unknown, + expressions: { sqlExpression: string }[], + ) => { + mockHighlightedExpressions.current = expressions; + return []; + }, +})); + +jest.mock('@/TabBar', () => ({ __esModule: true, default: () => null })); +jest.mock('../TraceLogsPanel', () => ({ + __esModule: true, + default: () => null, +})); +jest.mock('../DBSessionPanel', () => ({ + __esModule: true, + useSessionId: () => ({ rumSessionId: undefined, rumServiceName: undefined }), + DBSessionPanel: () => null, +})); +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('../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 useSidePanelStack from '@/hooks/useSidePanelStack'; + +const TIMESTAMP_VALUE = '2024-05-01T10:00:00.123456789Z'; + +function rowResult(row: Record) { + return { + data: { + data: [ + { + __hdx_trace_id: '7316d5a2ab0dc2efa72258f64a98a405', + __hdx_span_id: 'e3748131832d6176', + __hdx_timestamp: TIMESTAMP_VALUE, + __hdx_timestamp_value_0: TIMESTAMP_VALUE, + __hdx_service_name: 'frontend', + ...row, + }, + ], + meta: [{ name: '__hdx_timestamp_value_0', type: 'DateTime64(9)' }], + }, + isLoading: false, + isSuccess: true, + isError: false, + error: null, + }; +} + +function renderPanel( + source: TSource, + { parentBreadcrumbs }: { parentBreadcrumbs?: { label: string }[] } = {}, +) { + function Harness() { + const sidePanelStack = useSidePanelStack({ initialRowId: 'row-1' }); + return ( + + ); + } + return render( + + + , + ); +} + +describe('DBRowSidePanelInner: compact header', () => { + beforeEach(() => { + localStorage.clear(); + Object.keys(mockQueryStore).forEach(k => delete mockQueryStore[k]); + mockStackSource.current = undefined; + mockHeaderProps.current = {}; + mockHighlightedExpressions.current = []; + mockUseRowData.mockReset(); + }); + + it('skips the body paper on a trace span whose name is the panel title', () => { + mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'GET /checkout' })); + renderPanel(makeTraceSource()); + + expect(mockHeaderProps.current.mainContent).toBe('GET /checkout'); + expect(mockHeaderProps.current.bodyShownInTitle).toBe(true); + }); + + it('keeps the body paper when the span name is too long for the title', () => { + mockUseRowData.mockReturnValue( + rowResult({ __hdx_body: `SELECT ${'column, '.repeat(30)}id FROM t` }), + ); + renderPanel(makeTraceSource()); + + expect(mockHeaderProps.current.bodyShownInTitle).toBe(false); + }); + + it('skips the body paper when a breadcrumb trail ends on the span name', () => { + mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'GET /checkout' })); + renderPanel(makeTraceSource(), { + parentBreadcrumbs: [{ label: 'Session' }], + }); + + expect(mockHeaderProps.current.bodyShownInTitle).toBe(true); + }); + + it('keeps the body paper when the trail truncates the span name', () => { + // Longer than the current crumb's limit in a trail, shorter than a lone + // title's limit. + mockUseRowData.mockReturnValue( + rowResult({ __hdx_body: `GET /api/${'segment/'.repeat(6)}id` }), + ); + renderPanel(makeTraceSource(), { + parentBreadcrumbs: [{ label: 'Session' }], + }); + + expect(mockHeaderProps.current.bodyShownInTitle).toBe(false); + }); + + it('keeps the body paper after View Trace, where the title is the log', () => { + const traceSource = makeTraceSource(); + mockStackSource.current = traceSource; + mockQueryStore.sidePanelStackRoot = 'row-1'; + mockQueryStore.sidePanelSourceStack = [ + { + sourceId: traceSource.id, + rowId: 'span-row', + aliasWith: [], + label: 'Checkout failed for user u_18342', + sourceKind: traceSource.kind, + }, + ]; + mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'GET /checkout' })); + renderPanel(makeLogSource()); + + expect(mockHeaderProps.current.mainContent).toBe('GET /checkout'); + expect(mockHeaderProps.current.bodyShownInTitle).toBe(false); + }); + + it('keeps the body paper on log rows', () => { + mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'a log line' })); + renderPanel(makeLogSource()); + + expect(mockHeaderProps.current.bodyShownInTitle).toBe(false); + }); + + it('does not repeat the service as a highlighted chip on trace spans', () => { + mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'GET /checkout' })); + renderPanel(makeTraceSource()); + + expect(mockHighlightedExpressions.current).not.toContainEqual({ + sqlExpression: 'ServiceName', + }); + }); + + it('still highlights the service on log rows', () => { + mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'a log line' })); + renderPanel(makeLogSource()); + + expect(mockHighlightedExpressions.current).toContainEqual({ + sqlExpression: 'ServiceName', + }); + }); + + it('keeps highlighted expressions configured on a trace source', () => { + mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'GET /checkout' })); + renderPanel( + makeTraceSource({ + highlightedRowAttributeExpressions: [ + { sqlExpression: "SpanAttributes['http.method']" }, + ], + }), + ); + + expect(mockHighlightedExpressions.current).toEqual([ + { sqlExpression: "SpanAttributes['http.method']" }, + ]); + }); +}); diff --git a/packages/app/src/components/__tests__/DBRowSidePanelHeader.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanelHeader.test.tsx index 0bd32b7f3c..9b49e56c3c 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanelHeader.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanelHeader.test.tsx @@ -1,7 +1,9 @@ import React from 'react'; import { screen } from '@testing-library/react'; +import type { HighlightedAttribute } from '@/components/DBHighlightedAttributesList'; import DBRowSidePanelHeader from '@/components/DBRowSidePanelHeader'; +import { makeTraceSource } from '@/llm/__fixtures__/sources'; // Stub modules that touch user preferences, AI APIs, or DOM observers; the // behavior under test is the body-paper visibility logic, not those deps. @@ -22,7 +24,9 @@ jest.mock('../AISummarizeButton', () => ({ })); jest.mock('../DBHighlightedAttributesList', () => ({ - DBHighlightedAttributesList: () => null, + DBHighlightedAttributesList: ({ attributes }: { attributes: unknown[] }) => ( +
{attributes.length}
+ ), })); jest.mock('../DBRowSidePanel', () => ({ @@ -84,4 +88,47 @@ describe('DBRowSidePanelHeader: body section (HDX-4373)', () => { renderWithMantine(); expect(screen.queryByText('No body for this event.')).toBeInTheDocument(); }); + + it('omits the body paper when the panel title already shows the body', () => { + renderWithMantine( + , + ); + expect(screen.queryByText('GET /checkout')).not.toBeInTheDocument(); + expect(screen.queryByText('SpanName')).not.toBeInTheDocument(); + expect( + screen.queryByText('No body for this event.'), + ).not.toBeInTheDocument(); + }); +}); + +describe('DBRowSidePanelHeader: highlighted attributes', () => { + const attribute: HighlightedAttribute = { + source: makeTraceSource(), + displayedKey: 'http.method', + value: 'GET', + sql: "SpanAttributes['http.method']", + }; + + it('renders the attribute list when there are attributes', () => { + renderWithMantine( + , + ); + expect(screen.getByTestId('highlighted-attributes')).toHaveTextContent('1'); + }); + + it('renders nothing for the attribute list when there are none', () => { + renderWithMantine( + , + ); + expect( + screen.queryByTestId('highlighted-attributes'), + ).not.toBeInTheDocument(); + }); }); diff --git a/packages/app/src/components/__tests__/SidePanelBreadcrumbs.test.tsx b/packages/app/src/components/__tests__/SidePanelBreadcrumbs.test.tsx index 7957ef9f37..85644e2626 100644 --- a/packages/app/src/components/__tests__/SidePanelBreadcrumbs.test.tsx +++ b/packages/app/src/components/__tests__/SidePanelBreadcrumbs.test.tsx @@ -2,6 +2,7 @@ import { SourceKind } from '@hyperdx/common-utils/dist/types'; import { MantineProvider } from '@mantine/core'; import { fireEvent, render, screen } from '@testing-library/react'; +import { getBreadcrumbMaxLabelLength } from '@/components/sidePanelBreadcrumbLimits'; import SidePanelBreadcrumbs, { BreadcrumbItem, } from '@/components/SidePanelBreadcrumbs'; @@ -69,4 +70,28 @@ describe('SidePanelBreadcrumbs', () => { const { container } = renderCrumbs([{ label: 'First', sourceKind: kind }]); expect(container.querySelector(`.${iconClass}`)).toBeInTheDocument(); }); + + it('shows a lone label in full up to the single-crumb limit', () => { + const max = getBreadcrumbMaxLabelLength(0, 1); + renderCrumbs([{ label: 'z'.repeat(max) }]); + expect(screen.getByText('z'.repeat(max))).toBeInTheDocument(); + }); + + it('truncates a lone label one character past the single-crumb limit', () => { + const max = getBreadcrumbMaxLabelLength(0, 1); + renderCrumbs([{ label: 'z'.repeat(max + 1) }]); + expect(screen.getByText(`${'z'.repeat(max)}…`)).toBeInTheDocument(); + }); +}); + +describe('getBreadcrumbMaxLabelLength', () => { + it('gives a lone crumb the most room', () => { + expect(getBreadcrumbMaxLabelLength(0, 1)).toBe(120); + }); + + it('gives the current crumb of a trail more room than its ancestors', () => { + expect(getBreadcrumbMaxLabelLength(2, 3)).toBe(40); + expect(getBreadcrumbMaxLabelLength(0, 3)).toBe(20); + expect(getBreadcrumbMaxLabelLength(1, 3)).toBe(20); + }); }); diff --git a/packages/app/src/components/sidePanelBreadcrumbLimits.ts b/packages/app/src/components/sidePanelBreadcrumbLimits.ts new file mode 100644 index 0000000000..8c9e0b8188 --- /dev/null +++ b/packages/app/src/components/sidePanelBreadcrumbLimits.ts @@ -0,0 +1,18 @@ +// Breadcrumb label truncation, kept apart from SidePanelBreadcrumbs so the side +// panel can check whether its title shows a label in full without depending on +// the component (which the side panel tests mock out). +const MAX_LABEL_LENGTH_SINGLE = 120; +const MAX_LABEL_LENGTH_CURRENT = 40; +const MAX_LABEL_LENGTH_PREVIOUS = 20; + +/** + * Longest label a crumb renders before truncating. A lone crumb is the panel + * title and gets the most room; in a trail the current crumb gets more room + * than its ancestors. + */ +export function getBreadcrumbMaxLabelLength(index: number, itemCount: number) { + if (itemCount === 1) return MAX_LABEL_LENGTH_SINGLE; + return index === itemCount - 1 + ? MAX_LABEL_LENGTH_CURRENT + : MAX_LABEL_LENGTH_PREVIOUS; +} From c8573dabaf68395ad7f4ca3c5e69e4a84492cd7b Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Wed, 4 Nov 2026 04:41:54 +0000 Subject: [PATCH 2/5] fix(trace-panel): only hide the span name box when the title fits it The breadcrumb title is ellipsized by CSS as well as by its character cap, so a span name under the 120 character cap could still be cut off in a narrow drawer while the body box was hidden. Hide the box only for names up to 40 characters, which the title shows in full at realistic drawer widths, and drop the breadcrumb limits module the old rule needed. Also give the first waterfall row a little more clearance from the hover time label, anchor the timeline test on a data-testid instead of SCSS class names, note that the attribute list leaves spacing to its callers, and cover a span with no body and a stack frame that lands on the span name. Co-Authored-By: Claude Opus 5.5 --- .../DBHighlightedAttributesList.tsx | 1 + .../app/src/components/DBRowSidePanel.tsx | 22 ++++--- .../src/components/SidePanelBreadcrumbs.tsx | 11 +++- .../TimelineChart/TimelineChart.tsx | 9 ++- .../__tests__/TimelineChart.test.tsx | 7 +-- .../DBRowSidePanel.compactHeader.test.tsx | 61 ++++++++++++++----- .../__tests__/SidePanelBreadcrumbs.test.tsx | 25 -------- .../components/sidePanelBreadcrumbLimits.ts | 18 ------ 8 files changed, 79 insertions(+), 75 deletions(-) delete mode 100644 packages/app/src/components/sidePanelBreadcrumbLimits.ts diff --git a/packages/app/src/components/DBHighlightedAttributesList.tsx b/packages/app/src/components/DBHighlightedAttributesList.tsx index a243e4f234..c5efbe0ae3 100644 --- a/packages/app/src/components/DBHighlightedAttributesList.tsx +++ b/packages/app/src/components/DBHighlightedAttributesList.tsx @@ -15,6 +15,7 @@ export type HighlightedAttribute = { lucene?: string; }; +// Renders without an outer margin; callers set the spacing around the list. export function DBHighlightedAttributesList({ attributes, }: { diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index 4b4482f383..d4f5998d17 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -82,7 +82,6 @@ import { } from './DrawerUtils'; import { eventRowWhereParser } from './eventRowWhere'; import LogLevel from './LogLevel'; -import { getBreadcrumbMaxLabelLength } from './sidePanelBreadcrumbLimits'; import SidePanelBreadcrumbs, { BreadcrumbItem } from './SidePanelBreadcrumbs'; import { SpanLinkData } from './SpanLinksSubpanel'; import TraceLogsPanel from './TraceLogsPanel'; @@ -240,6 +239,11 @@ const SPAN_KIND_LABELS: Record = { SPAN_KIND_CONSUMER: 'Consumer', }; +// Longest span name the header trusts the title breadcrumb to show in full. +// The crumb is ellipsized by CSS as well as by its character cap, so a longer +// name can be cut off in a narrow drawer; those keep the body paper. +const TITLE_FITS_MAX_LENGTH = 40; + type DBRowSidePanelProps = { source: TSource; rowId: string | undefined; @@ -869,16 +873,16 @@ export const DBRowSidePanelInner = ({ ]); // The current (last) breadcrumb is the panel title. On a trace panel it is - // usually the span name; when it shows that name without truncating, a body - // paper would only repeat the title, so the header skips it. Longer names, - // and hops where the current crumb carries another frame's label (View - // Trace from a log), keep the paper so the full span name stays readable. - const titleCrumbIndex = allBreadcrumbs.length - 1; + // usually the span name; when the name is short enough to show in full, a + // body paper would only repeat the title, so the header skips it. Longer + // names, and hops where the current crumb carries another frame's label + // (View Trace from a log), keep the paper so the full span name stays + // readable. const isBodyShownInTitle = sourceIsTrace && - allBreadcrumbs[titleCrumbIndex]?.label === mainContent && - mainContent.length <= - getBreadcrumbMaxLabelLength(titleCrumbIndex, allBreadcrumbs.length); + mainContent != null && + mainContent.length <= TITLE_FITS_MAX_LENGTH && + allBreadcrumbs.at(-1)?.label === mainContent; const availableTabs = useMemo(() => { const tabs: Tab[] = []; diff --git a/packages/app/src/components/SidePanelBreadcrumbs.tsx b/packages/app/src/components/SidePanelBreadcrumbs.tsx index b2a9792075..2c90d4724e 100644 --- a/packages/app/src/components/SidePanelBreadcrumbs.tsx +++ b/packages/app/src/components/SidePanelBreadcrumbs.tsx @@ -15,7 +15,9 @@ import { IconLogs, } from '@tabler/icons-react'; -import { getBreadcrumbMaxLabelLength } from './sidePanelBreadcrumbLimits'; +const MAX_LABEL_LENGTH_SINGLE = 120; +const MAX_LABEL_LENGTH_CURRENT = 40; +const MAX_LABEL_LENGTH_PREVIOUS = 20; export type BreadcrumbItem = { label: string; @@ -51,7 +53,12 @@ function SidePanelBreadcrumbs({ const breadcrumbElements = useMemo(() => { return items.map((item, i) => { const isLast = i === items.length - 1; - const maxLen = getBreadcrumbMaxLabelLength(i, items.length); + const isSingle = items.length === 1; + const maxLen = isSingle + ? MAX_LABEL_LENGTH_SINGLE + : isLast + ? MAX_LABEL_LENGTH_CURRENT + : MAX_LABEL_LENGTH_PREVIOUS; const truncatedLabel = truncate(item.label, maxLen); const needsTooltip = item.label.length > maxLen; diff --git a/packages/app/src/components/TimelineChart/TimelineChart.tsx b/packages/app/src/components/TimelineChart/TimelineChart.tsx index dea3911a44..c54e9ab2ce 100644 --- a/packages/app/src/components/TimelineChart/TimelineChart.tsx +++ b/packages/app/src/components/TimelineChart/TimelineChart.tsx @@ -78,9 +78,11 @@ const MIN_VIEWPORT_FRAC = 0.02; const axisHeight = 24; // Gap between the axis and the first row. The hover time label -// (TimelineMouseCursor) reaches about 37px below the top of the chart, so the -// first row starts at axisHeight + rowsMarginTop = 40px to stay clear of it. -const rowsMarginTop = 16; +// (TimelineMouseCursor, styled in TimelineChart.module.scss) reaches about 37px +// below the top of the chart; the first row starts at +// axisHeight + rowsMarginTop = 44px, which keeps it clear with some headroom. +// Revisit this if the label's padding or offset grows. +const rowsMarginTop = 20; const maxScale = 100; export const TimelineChart = memo(function (props: TimelineChartProps) { @@ -427,6 +429,7 @@ export const TimelineChart = memo(function (props: TimelineChartProps) { zoom hint lives here instead of in its own row above the chart. */}
{ const hint = screen.getByTestId('timeline-zoom-hint'); expect(hint).toHaveTextContent('⌘/Ctrl + scroll to zoom'); - // Lives in the sticky corner cell of the scroll container rather than in - // a row of its own above the chart. - expect(hint.closest('.timelineCorner')).not.toBeNull(); - expect(hint.closest('.timelineContainer')).not.toBeNull(); + // Lives in the corner cell above the span labels rather than in a row of + // its own above the chart. + expect(hint.closest('[data-testid="timeline-corner"]')).toBeInTheDocument(); }); }); diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.tsx index 6b3c80108e..f23597a345 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.tsx @@ -199,9 +199,28 @@ describe('DBRowSidePanelInner: compact header', () => { expect(mockHeaderProps.current.bodyShownInTitle).toBe(true); }); - it('keeps the body paper when the span name is too long for the title', () => { + it('skips the body paper for a 40 character span name', () => { + mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'x'.repeat(40) })); + renderPanel(makeTraceSource()); + + expect(mockHeaderProps.current.bodyShownInTitle).toBe(true); + }); + + it('keeps the body paper for a 41 character span name', () => { + mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'x'.repeat(41) })); + renderPanel(makeTraceSource()); + + expect(mockHeaderProps.current.bodyShownInTitle).toBe(false); + }); + + it('keeps the body paper for a name the title can cut off in a narrow drawer', () => { + // Under the breadcrumb's 120 character cap, but long enough that CSS can + // ellipsize it, so the paper stays to keep the full name readable. mockUseRowData.mockReturnValue( - rowResult({ __hdx_body: `SELECT ${'column, '.repeat(30)}id FROM t` }), + rowResult({ + __hdx_body: + 'GET /api/v1/inventory/warehouses/{warehouseId}/low-stock-items', + }), ); renderPanel(makeTraceSource()); @@ -217,20 +236,27 @@ describe('DBRowSidePanelInner: compact header', () => { expect(mockHeaderProps.current.bodyShownInTitle).toBe(true); }); - it('keeps the body paper when the trail truncates the span name', () => { - // Longer than the current crumb's limit in a trail, shorter than a lone - // title's limit. - mockUseRowData.mockReturnValue( - rowResult({ __hdx_body: `GET /api/${'segment/'.repeat(6)}id` }), - ); - renderPanel(makeTraceSource(), { - parentBreadcrumbs: [{ label: 'Session' }], - }); + it('keeps the body paper after View Trace, where the title is the log', () => { + const traceSource = makeTraceSource(); + mockStackSource.current = traceSource; + mockQueryStore.sidePanelStackRoot = 'row-1'; + mockQueryStore.sidePanelSourceStack = [ + { + sourceId: traceSource.id, + rowId: 'span-row', + aliasWith: [], + label: 'Checkout failed for user u_18342', + sourceKind: traceSource.kind, + }, + ]; + mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'GET /checkout' })); + renderPanel(makeLogSource()); + expect(mockHeaderProps.current.mainContent).toBe('GET /checkout'); expect(mockHeaderProps.current.bodyShownInTitle).toBe(false); }); - it('keeps the body paper after View Trace, where the title is the log', () => { + it('skips the body paper when a stack frame lands on the span name', () => { const traceSource = makeTraceSource(); mockStackSource.current = traceSource; mockQueryStore.sidePanelStackRoot = 'row-1'; @@ -239,14 +265,21 @@ describe('DBRowSidePanelInner: compact header', () => { sourceId: traceSource.id, rowId: 'span-row', aliasWith: [], - label: 'Checkout failed for user u_18342', + label: 'GET /checkout', sourceKind: traceSource.kind, }, ]; mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'GET /checkout' })); renderPanel(makeLogSource()); - expect(mockHeaderProps.current.mainContent).toBe('GET /checkout'); + expect(mockHeaderProps.current.bodyShownInTitle).toBe(true); + }); + + it('keeps the body paper when the span has no body value', () => { + mockUseRowData.mockReturnValue(rowResult({})); + renderPanel(makeTraceSource()); + + expect(mockHeaderProps.current.mainContent).toBeUndefined(); expect(mockHeaderProps.current.bodyShownInTitle).toBe(false); }); diff --git a/packages/app/src/components/__tests__/SidePanelBreadcrumbs.test.tsx b/packages/app/src/components/__tests__/SidePanelBreadcrumbs.test.tsx index 85644e2626..7957ef9f37 100644 --- a/packages/app/src/components/__tests__/SidePanelBreadcrumbs.test.tsx +++ b/packages/app/src/components/__tests__/SidePanelBreadcrumbs.test.tsx @@ -2,7 +2,6 @@ import { SourceKind } from '@hyperdx/common-utils/dist/types'; import { MantineProvider } from '@mantine/core'; import { fireEvent, render, screen } from '@testing-library/react'; -import { getBreadcrumbMaxLabelLength } from '@/components/sidePanelBreadcrumbLimits'; import SidePanelBreadcrumbs, { BreadcrumbItem, } from '@/components/SidePanelBreadcrumbs'; @@ -70,28 +69,4 @@ describe('SidePanelBreadcrumbs', () => { const { container } = renderCrumbs([{ label: 'First', sourceKind: kind }]); expect(container.querySelector(`.${iconClass}`)).toBeInTheDocument(); }); - - it('shows a lone label in full up to the single-crumb limit', () => { - const max = getBreadcrumbMaxLabelLength(0, 1); - renderCrumbs([{ label: 'z'.repeat(max) }]); - expect(screen.getByText('z'.repeat(max))).toBeInTheDocument(); - }); - - it('truncates a lone label one character past the single-crumb limit', () => { - const max = getBreadcrumbMaxLabelLength(0, 1); - renderCrumbs([{ label: 'z'.repeat(max + 1) }]); - expect(screen.getByText(`${'z'.repeat(max)}…`)).toBeInTheDocument(); - }); -}); - -describe('getBreadcrumbMaxLabelLength', () => { - it('gives a lone crumb the most room', () => { - expect(getBreadcrumbMaxLabelLength(0, 1)).toBe(120); - }); - - it('gives the current crumb of a trail more room than its ancestors', () => { - expect(getBreadcrumbMaxLabelLength(2, 3)).toBe(40); - expect(getBreadcrumbMaxLabelLength(0, 3)).toBe(20); - expect(getBreadcrumbMaxLabelLength(1, 3)).toBe(20); - }); }); diff --git a/packages/app/src/components/sidePanelBreadcrumbLimits.ts b/packages/app/src/components/sidePanelBreadcrumbLimits.ts deleted file mode 100644 index 8c9e0b8188..0000000000 --- a/packages/app/src/components/sidePanelBreadcrumbLimits.ts +++ /dev/null @@ -1,18 +0,0 @@ -// Breadcrumb label truncation, kept apart from SidePanelBreadcrumbs so the side -// panel can check whether its title shows a label in full without depending on -// the component (which the side panel tests mock out). -const MAX_LABEL_LENGTH_SINGLE = 120; -const MAX_LABEL_LENGTH_CURRENT = 40; -const MAX_LABEL_LENGTH_PREVIOUS = 20; - -/** - * Longest label a crumb renders before truncating. A lone crumb is the panel - * title and gets the most room; in a trail the current crumb gets more room - * than its ancestors. - */ -export function getBreadcrumbMaxLabelLength(index: number, itemCount: number) { - if (itemCount === 1) return MAX_LABEL_LENGTH_SINGLE; - return index === itemCount - 1 - ? MAX_LABEL_LENGTH_CURRENT - : MAX_LABEL_LENGTH_PREVIOUS; -} From 594efd56a9391f2c9087308bb2dbc80092a3901a Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:51:15 +0000 Subject: [PATCH 3/5] fix(trace-panel): style the zoom hint like the axis labels The hint used smaller text with Kbd chips, so it sat a few pixels below the axis ticks in a different font. Render it as plain text with the tick label class and the inherited axis font, dimmed, so it lines up with the ticks and reads as the label column's header. The shortcut is also listed in the side panel's keyboard shortcuts dialog. Co-Authored-By: Claude Opus 5.5 --- .../src/components/TimelineChart/TimelineChart.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/app/src/components/TimelineChart/TimelineChart.tsx b/packages/app/src/components/TimelineChart/TimelineChart.tsx index c54e9ab2ce..991f8f63b6 100644 --- a/packages/app/src/components/TimelineChart/TimelineChart.tsx +++ b/packages/app/src/components/TimelineChart/TimelineChart.tsx @@ -1,6 +1,6 @@ import { memo, useEffect, useLayoutEffect, useMemo, useRef } from 'react'; import cx from 'classnames'; -import { Flex, Kbd, Text } from '@mantine/core'; +import { Flex, Text } from '@mantine/core'; import { useVirtualizer } from '@tanstack/react-virtual'; import { @@ -426,7 +426,9 @@ export const TimelineChart = memo(function (props: TimelineChartProps) { /> {/* The corner above the label column is otherwise empty, so the - zoom hint lives here instead of in its own row above the chart. */} + zoom hint lives here instead of in its own row above the chart. + It reuses the tick label class and inherits the axis font so it + reads as part of the axis header. */}
- ⌘/Ctrl + scroll to - zoom + ⌘/Ctrl + scroll to zoom
From e8f8c14f99eb56d53a135511d44f81e221aaa9df Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Wed, 4 Nov 2026 16:25:39 +0000 Subject: [PATCH 4/5] refactor(trace-panel): test the header rules as plain functions Move the two header decisions (which attributes become chips, and whether the title already shows the span name) into small exported functions, and test them directly. This replaces a 325-line test that rendered the whole side panel behind a wall of mocks with about 60 lines of plain assertions. No behavior change. Co-Authored-By: Claude Opus 5.5 --- .../app/src/components/DBRowSidePanel.tsx | 78 +++-- .../DBRowSidePanel.compactHeader.test.ts | 62 ++++ .../DBRowSidePanel.compactHeader.test.tsx | 325 ------------------ 3 files changed, 108 insertions(+), 357 deletions(-) create mode 100644 packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.ts delete mode 100644 packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.tsx diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index d4f5998d17..6ab961d230 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -244,6 +244,46 @@ const SPAN_KIND_LABELS: Record = { // name can be cut off in a narrow drawer; those keep the body paper. const TITLE_FITS_MAX_LENGTH = 40; +/** + * Attribute expressions shown as chips in the side panel header. Log panels + * also get the service name, kept from before highlightedRowAttributeExpressions + * existed; trace panels skip it because the metadata row already shows it. + */ +export function getHeaderHighlightExpressions(source: TSource) { + const expressions: NonNullable< + (TLogSource | TTraceSource)['highlightedRowAttributeExpressions'] + > = []; + if ( + (source.kind === SourceKind.Trace || source.kind === SourceKind.Log) && + source.highlightedRowAttributeExpressions + ) { + expressions.push(...source.highlightedRowAttributeExpressions); + } + if (isLogSource(source) && source.serviceNameExpression) { + expressions.push({ sqlExpression: source.serviceNameExpression }); + } + return expressions; +} + +/** + * Whether the panel title (the current breadcrumb) already shows a trace + * span's name in full, so the header can skip the body paper that would only + * repeat it. Long names, and hops where the title is another frame's label + * (View Trace from a log), keep the paper. + */ +export function isSpanNameShownInTitle( + source: TSource, + spanName: string | undefined, + titleLabel: string | undefined, +) { + return ( + source.kind === SourceKind.Trace && + spanName != null && + spanName.length <= TITLE_FITS_MAX_LENGTH && + titleLabel === spanName + ); +} + type DBRowSidePanelProps = { source: TSource; rowId: string | undefined; @@ -503,30 +543,10 @@ export const DBRowSidePanelInner = ({ }, [activeRowId, mainContent, resolvedFrameLabels]); const highlightedAttributeValues = useMemo(() => { - const attributeExpressions: NonNullable< - (TLogSource | TTraceSource)['highlightedRowAttributeExpressions'] - > = []; - if ( - (source.kind === SourceKind.Trace || source.kind === SourceKind.Log) && - source.highlightedRowAttributeExpressions - ) { - attributeExpressions.push(...source.highlightedRowAttributeExpressions); - } - - // Add service name expression to log sources, to maintain compatibility - // with the behavior prior to the addition of - // highlightedRowAttributeExpressions. Trace panels skip it: the metadata - // row already shows the service, and the waterfall below needs the room. - if (isLogSource(source) && source.serviceNameExpression) { - attributeExpressions.push({ - sqlExpression: source.serviceNameExpression, - }); - } - return rowData ? getHighlightedAttributesFromData( source, - attributeExpressions, + getHeaderHighlightExpressions(source), rowData.data || [], rowData.meta || [], ) @@ -872,17 +892,11 @@ export const DBRowSidePanelInner = ({ clearSpanSelection, ]); - // The current (last) breadcrumb is the panel title. On a trace panel it is - // usually the span name; when the name is short enough to show in full, a - // body paper would only repeat the title, so the header skips it. Longer - // names, and hops where the current crumb carries another frame's label - // (View Trace from a log), keep the paper so the full span name stays - // readable. - const isBodyShownInTitle = - sourceIsTrace && - mainContent != null && - mainContent.length <= TITLE_FITS_MAX_LENGTH && - allBreadcrumbs.at(-1)?.label === mainContent; + const isBodyShownInTitle = isSpanNameShownInTitle( + source, + mainContent, + allBreadcrumbs.at(-1)?.label, + ); const availableTabs = useMemo(() => { const tabs: Tab[] = []; diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.ts b/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.ts new file mode 100644 index 0000000000..1670071ada --- /dev/null +++ b/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.ts @@ -0,0 +1,62 @@ +import { + getHeaderHighlightExpressions, + isSpanNameShownInTitle, +} from '@/components/DBRowSidePanel'; +import { makeLogSource, makeTraceSource } from '@/llm/__fixtures__/sources'; + +describe('isSpanNameShownInTitle', () => { + const trace = makeTraceSource(); + + it('is true when the title is a short span name', () => { + expect( + isSpanNameShownInTitle(trace, 'GET /checkout', 'GET /checkout'), + ).toBe(true); + }); + + it('allows names up to 40 characters', () => { + const name = 'x'.repeat(40); + expect(isSpanNameShownInTitle(trace, name, name)).toBe(true); + }); + + it('keeps the body for longer names the title can cut off', () => { + const name = 'x'.repeat(41); + expect(isSpanNameShownInTitle(trace, name, name)).toBe(false); + }); + + it('keeps the body when the title is another frame label (View Trace)', () => { + expect( + isSpanNameShownInTitle(trace, 'GET /checkout', 'Checkout failed'), + ).toBe(false); + }); + + it('keeps the body when the span has no body value', () => { + expect(isSpanNameShownInTitle(trace, undefined, 'Trace')).toBe(false); + }); + + it('keeps the body on log panels', () => { + expect( + isSpanNameShownInTitle(makeLogSource(), 'a log line', 'a log line'), + ).toBe(false); + }); +}); + +describe('getHeaderHighlightExpressions', () => { + it('does not repeat the service as a chip on trace panels', () => { + expect(getHeaderHighlightExpressions(makeTraceSource())).toEqual([]); + }); + + it('still highlights the service on log panels', () => { + expect(getHeaderHighlightExpressions(makeLogSource())).toEqual([ + { sqlExpression: 'ServiceName' }, + ]); + }); + + it('keeps highlights configured on the source', () => { + const highlight = { sqlExpression: "SpanAttributes['http.method']" }; + expect( + getHeaderHighlightExpressions( + makeTraceSource({ highlightedRowAttributeExpressions: [highlight] }), + ), + ).toEqual([highlight]); + }); +}); diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.tsx deleted file mode 100644 index f23597a345..0000000000 --- a/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.tsx +++ /dev/null @@ -1,325 +0,0 @@ -import React from 'react'; -import { TSource } from '@hyperdx/common-utils/dist/types'; -import { MantineProvider } from '@mantine/core'; -import { render } from '@testing-library/react'; - -import { makeLogSource, makeTraceSource } from '@/llm/__fixtures__/sources'; - -// In-memory nuqs so a test can seed the side panel stack. Prefixed with -// `mock` so jest.mock factories may reference it. -const mockQueryStore: Record = {}; -jest.mock('nuqs', () => { - const actual = jest.requireActual('nuqs'); - return { - ...actual, - useQueryState: (key: string, parser?: { defaultValue?: unknown }) => [ - key in mockQueryStore - ? mockQueryStore[key] - : parser && 'defaultValue' in parser - ? parser.defaultValue - : null, - jest.fn(), - ], - }; -}); - -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: () => [], -})); - -// Resolves the trace source a "View Trace" frame points at. -const mockStackSource: { current: TSource | undefined } = { - current: undefined, -}; -jest.mock('@/source', () => ({ - __esModule: true, - getEventBody: () => 'SpanName', - useSource: ({ id }: { id: string | null }) => - id != null && id === mockStackSource.current?.id - ? { data: mockStackSource.current, isSuccess: true, isLoading: false } - : { data: undefined }, -})); - -type HeaderProps = { bodyShownInTitle?: boolean; mainContent?: string }; -const mockHeaderProps: { current: HeaderProps } = { current: {} }; -jest.mock('../DBRowSidePanelHeader', () => ({ - __esModule: true, - default: (props: HeaderProps) => { - mockHeaderProps.current = props; - return null; - }, -})); - -// Records the attribute expressions the panel asks to highlight. -const mockHighlightedExpressions: { current: { sqlExpression: string }[] } = { - current: [], -}; -jest.mock('@/utils/highlightedAttributes', () => ({ - __esModule: true, - getHighlightedAttributesFromData: ( - _source: unknown, - expressions: { sqlExpression: string }[], - ) => { - mockHighlightedExpressions.current = expressions; - return []; - }, -})); - -jest.mock('@/TabBar', () => ({ __esModule: true, default: () => null })); -jest.mock('../TraceLogsPanel', () => ({ - __esModule: true, - default: () => null, -})); -jest.mock('../DBSessionPanel', () => ({ - __esModule: true, - useSessionId: () => ({ rumSessionId: undefined, rumServiceName: undefined }), - DBSessionPanel: () => null, -})); -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('../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 useSidePanelStack from '@/hooks/useSidePanelStack'; - -const TIMESTAMP_VALUE = '2024-05-01T10:00:00.123456789Z'; - -function rowResult(row: Record) { - return { - data: { - data: [ - { - __hdx_trace_id: '7316d5a2ab0dc2efa72258f64a98a405', - __hdx_span_id: 'e3748131832d6176', - __hdx_timestamp: TIMESTAMP_VALUE, - __hdx_timestamp_value_0: TIMESTAMP_VALUE, - __hdx_service_name: 'frontend', - ...row, - }, - ], - meta: [{ name: '__hdx_timestamp_value_0', type: 'DateTime64(9)' }], - }, - isLoading: false, - isSuccess: true, - isError: false, - error: null, - }; -} - -function renderPanel( - source: TSource, - { parentBreadcrumbs }: { parentBreadcrumbs?: { label: string }[] } = {}, -) { - function Harness() { - const sidePanelStack = useSidePanelStack({ initialRowId: 'row-1' }); - return ( - - ); - } - return render( - - - , - ); -} - -describe('DBRowSidePanelInner: compact header', () => { - beforeEach(() => { - localStorage.clear(); - Object.keys(mockQueryStore).forEach(k => delete mockQueryStore[k]); - mockStackSource.current = undefined; - mockHeaderProps.current = {}; - mockHighlightedExpressions.current = []; - mockUseRowData.mockReset(); - }); - - it('skips the body paper on a trace span whose name is the panel title', () => { - mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'GET /checkout' })); - renderPanel(makeTraceSource()); - - expect(mockHeaderProps.current.mainContent).toBe('GET /checkout'); - expect(mockHeaderProps.current.bodyShownInTitle).toBe(true); - }); - - it('skips the body paper for a 40 character span name', () => { - mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'x'.repeat(40) })); - renderPanel(makeTraceSource()); - - expect(mockHeaderProps.current.bodyShownInTitle).toBe(true); - }); - - it('keeps the body paper for a 41 character span name', () => { - mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'x'.repeat(41) })); - renderPanel(makeTraceSource()); - - expect(mockHeaderProps.current.bodyShownInTitle).toBe(false); - }); - - it('keeps the body paper for a name the title can cut off in a narrow drawer', () => { - // Under the breadcrumb's 120 character cap, but long enough that CSS can - // ellipsize it, so the paper stays to keep the full name readable. - mockUseRowData.mockReturnValue( - rowResult({ - __hdx_body: - 'GET /api/v1/inventory/warehouses/{warehouseId}/low-stock-items', - }), - ); - renderPanel(makeTraceSource()); - - expect(mockHeaderProps.current.bodyShownInTitle).toBe(false); - }); - - it('skips the body paper when a breadcrumb trail ends on the span name', () => { - mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'GET /checkout' })); - renderPanel(makeTraceSource(), { - parentBreadcrumbs: [{ label: 'Session' }], - }); - - expect(mockHeaderProps.current.bodyShownInTitle).toBe(true); - }); - - it('keeps the body paper after View Trace, where the title is the log', () => { - const traceSource = makeTraceSource(); - mockStackSource.current = traceSource; - mockQueryStore.sidePanelStackRoot = 'row-1'; - mockQueryStore.sidePanelSourceStack = [ - { - sourceId: traceSource.id, - rowId: 'span-row', - aliasWith: [], - label: 'Checkout failed for user u_18342', - sourceKind: traceSource.kind, - }, - ]; - mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'GET /checkout' })); - renderPanel(makeLogSource()); - - expect(mockHeaderProps.current.mainContent).toBe('GET /checkout'); - expect(mockHeaderProps.current.bodyShownInTitle).toBe(false); - }); - - it('skips the body paper when a stack frame lands on the span name', () => { - const traceSource = makeTraceSource(); - mockStackSource.current = traceSource; - mockQueryStore.sidePanelStackRoot = 'row-1'; - mockQueryStore.sidePanelSourceStack = [ - { - sourceId: traceSource.id, - rowId: 'span-row', - aliasWith: [], - label: 'GET /checkout', - sourceKind: traceSource.kind, - }, - ]; - mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'GET /checkout' })); - renderPanel(makeLogSource()); - - expect(mockHeaderProps.current.bodyShownInTitle).toBe(true); - }); - - it('keeps the body paper when the span has no body value', () => { - mockUseRowData.mockReturnValue(rowResult({})); - renderPanel(makeTraceSource()); - - expect(mockHeaderProps.current.mainContent).toBeUndefined(); - expect(mockHeaderProps.current.bodyShownInTitle).toBe(false); - }); - - it('keeps the body paper on log rows', () => { - mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'a log line' })); - renderPanel(makeLogSource()); - - expect(mockHeaderProps.current.bodyShownInTitle).toBe(false); - }); - - it('does not repeat the service as a highlighted chip on trace spans', () => { - mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'GET /checkout' })); - renderPanel(makeTraceSource()); - - expect(mockHighlightedExpressions.current).not.toContainEqual({ - sqlExpression: 'ServiceName', - }); - }); - - it('still highlights the service on log rows', () => { - mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'a log line' })); - renderPanel(makeLogSource()); - - expect(mockHighlightedExpressions.current).toContainEqual({ - sqlExpression: 'ServiceName', - }); - }); - - it('keeps highlighted expressions configured on a trace source', () => { - mockUseRowData.mockReturnValue(rowResult({ __hdx_body: 'GET /checkout' })); - renderPanel( - makeTraceSource({ - highlightedRowAttributeExpressions: [ - { sqlExpression: "SpanAttributes['http.method']" }, - ], - }), - ); - - expect(mockHighlightedExpressions.current).toEqual([ - { sqlExpression: "SpanAttributes['http.method']" }, - ]); - }); -}); From 9611b7f4eefef84cb14ba0978888dbe044026c8e Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Wed, 4 Nov 2026 16:41:30 +0000 Subject: [PATCH 5/5] fix(trace-panel): only skip the span name box under a lone title crumb In a breadcrumb trail (a span opened from a session, or a hop between spans) the crumbs share one row, so CSS can cut the span name off even when it is short, and the crumb only shows a tooltip past its character cap. Skip the body box only when the title is a single crumb, where the label gets the breadcrumb's 120 character cap and the 40 character limit is well inside it. The comment now points at that constant. Co-Authored-By: Claude Opus 5.5 --- .../app/src/components/DBRowSidePanel.tsx | 23 +++++++++------- .../DBRowSidePanel.compactHeader.test.ts | 26 +++++++++++++------ 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index 6ab961d230..161224a67e 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -239,9 +239,10 @@ const SPAN_KIND_LABELS: Record = { SPAN_KIND_CONSUMER: 'Consumer', }; -// Longest span name the header trusts the title breadcrumb to show in full. -// The crumb is ellipsized by CSS as well as by its character cap, so a longer -// name can be cut off in a narrow drawer; those keep the body paper. +// Longest span name the header trusts a lone title breadcrumb to show in full. +// Kept well under that crumb's 120 character cap (MAX_LABEL_LENGTH_SINGLE in +// SidePanelBreadcrumbs.tsx) because CSS can still ellipsize a long title in a +// narrow drawer; longer names keep the body paper. const TITLE_FITS_MAX_LENGTH = 40; /** @@ -266,21 +267,23 @@ export function getHeaderHighlightExpressions(source: TSource) { } /** - * Whether the panel title (the current breadcrumb) already shows a trace - * span's name in full, so the header can skip the body paper that would only - * repeat it. Long names, and hops where the title is another frame's label - * (View Trace from a log), keep the paper. + * Whether the panel title already shows a trace span's name in full, so the + * header can skip the body paper that would only repeat it. Only a lone + * breadcrumb counts: in a trail the crumbs share one row and can cut the name + * off, and after View Trace the title is the originating log. Long names keep + * the paper too. */ export function isSpanNameShownInTitle( source: TSource, spanName: string | undefined, - titleLabel: string | undefined, + breadcrumbLabels: string[], ) { return ( source.kind === SourceKind.Trace && spanName != null && spanName.length <= TITLE_FITS_MAX_LENGTH && - titleLabel === spanName + breadcrumbLabels.length === 1 && + breadcrumbLabels[0] === spanName ); } @@ -895,7 +898,7 @@ export const DBRowSidePanelInner = ({ const isBodyShownInTitle = isSpanNameShownInTitle( source, mainContent, - allBreadcrumbs.at(-1)?.label, + allBreadcrumbs.map(crumb => crumb.label), ); const availableTabs = useMemo(() => { diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.ts b/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.ts index 1670071ada..6f7b43f221 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.ts +++ b/packages/app/src/components/__tests__/DBRowSidePanel.compactHeader.test.ts @@ -7,35 +7,45 @@ import { makeLogSource, makeTraceSource } from '@/llm/__fixtures__/sources'; describe('isSpanNameShownInTitle', () => { const trace = makeTraceSource(); - it('is true when the title is a short span name', () => { + it('is true when the lone title crumb is a short span name', () => { expect( - isSpanNameShownInTitle(trace, 'GET /checkout', 'GET /checkout'), + isSpanNameShownInTitle(trace, 'GET /checkout', ['GET /checkout']), ).toBe(true); }); it('allows names up to 40 characters', () => { const name = 'x'.repeat(40); - expect(isSpanNameShownInTitle(trace, name, name)).toBe(true); + expect(isSpanNameShownInTitle(trace, name, [name])).toBe(true); }); it('keeps the body for longer names the title can cut off', () => { const name = 'x'.repeat(41); - expect(isSpanNameShownInTitle(trace, name, name)).toBe(false); + expect(isSpanNameShownInTitle(trace, name, [name])).toBe(false); }); - it('keeps the body when the title is another frame label (View Trace)', () => { + it('keeps the body in a breadcrumb trail, where crumbs share the row', () => { + // A hop between two spans with the same name is still a trail. expect( - isSpanNameShownInTitle(trace, 'GET /checkout', 'Checkout failed'), + isSpanNameShownInTitle(trace, 'GET /checkout', [ + 'GET /checkout', + 'GET /checkout', + ]), ).toBe(false); }); + it('keeps the body when the title is not the span name', () => { + expect(isSpanNameShownInTitle(trace, 'GET /checkout', ['Trace'])).toBe( + false, + ); + }); + it('keeps the body when the span has no body value', () => { - expect(isSpanNameShownInTitle(trace, undefined, 'Trace')).toBe(false); + expect(isSpanNameShownInTitle(trace, undefined, ['Trace'])).toBe(false); }); it('keeps the body on log panels', () => { expect( - isSpanNameShownInTitle(makeLogSource(), 'a log line', 'a log line'), + isSpanNameShownInTitle(makeLogSource(), 'a log line', ['a log line']), ).toBe(false); }); });