Skip to content

Commit 513b237

Browse files
fix(trace-panel): start the waterfall higher in the trace side panel (#3536)
Trims the fixed chrome above the trace waterfall so the first span lands about a third of the way down a 1080p screen instead of past the middle. Nothing the panel shows is dropped: the repeats go, and the zoom hint moves into space the chart already had. ## Summary Opening a span from search stacked several things above the waterfall that either repeated information or spent a full row on a one-line hint: - a bordered **SpanName** box repeating the span name that the panel title (the breadcrumb) already shows in bold - a lone `ServiceName` chip repeating the service from the metadata row right above it - a full-width row for the "⌘/Ctrl + scroll to zoom" hint - loose gaps after the minimap, below the trace attributes, and between the axis and the first span Changes: - **Trace header:** skip the body box when the panel title is a single breadcrumb holding the span name, and the name is at most 40 characters, so the title shows it in full even in a narrower drawer. Longer names keep the box, and so do breadcrumb trails (a span opened from a session, hops between spans, **View Trace** from a log), where crumbs share one row and can cut the name off. Log panels keep their body box. - **Service chip:** trace panels no longer add the implicit `serviceNameExpression` chip. The metadata row shows the service, and the span detail pane next to the waterfall still offers **Add to Filters**, **Search for this value only**, and **Copy value** on `ServiceName`. Highlighted attributes configured on the source still render, and log panels keep the chip. - **Zoom hint:** moves from its own row into the timeline's top-left corner, the empty cell above the span labels next to the axis ticks. It's plain dimmed text in the axis label font, on the same baseline as the ticks, so it reads as the label column's header. The same shortcut is listed in the side panel's keyboard shortcuts dialog. - **Spacing:** drops the minimap's bottom margin (the controls row below already spaces it), moves the bottom margin of `DBHighlightedAttributesList` to its two callers so the header doesn't stack it on top of its own padding (log panels get the same tighter gap below their chips), and cuts the gap between the axis and the first span from 32px to 20px. The hover time label reaches 37px below the top of the chart, so the first span (now at 44px) still clears it. Measured on the same trace and data at 1920x945 (a 1080p screen minus browser chrome), identical in light and dark: | | `main` | This branch | | :-- | --: | --: | | Tabs start at | 218px | 82px | | First waterfall span | 506px (53.6% down) | 307px (32.4% down) | | Spans visible without scrolling | 18 | 26 | | First span, long span name or View Trace | 506px | 386px (box kept) | ### Screenshots or video Same trace, data, viewport (1920x945), and theme on both sides: | Before | After | | :----- | :---- | | ![before light](https://raw.githubusercontent.com/alex-fedotyev/hdx-pr-assets/main/compact-trace-side-panel-before-light.png) | ![after light](https://raw.githubusercontent.com/alex-fedotyev/hdx-pr-assets/main/compact-trace-side-panel-after-light.png) | | ![before dark](https://raw.githubusercontent.com/alex-fedotyev/hdx-pr-assets/main/compact-trace-side-panel-before-dark.png) | ![after dark](https://raw.githubusercontent.com/alex-fedotyev/hdx-pr-assets/main/compact-trace-side-panel-after-dark.png) | A span name longer than 40 characters keeps the box, so the full statement stays readable: ![long span name light](https://raw.githubusercontent.com/alex-fedotyev/hdx-pr-assets/main/compact-trace-side-panel-long-span-name-light.png) The zoom hint in the chart corner, light and dark: ![zoom hint corner light](https://raw.githubusercontent.com/alex-fedotyev/hdx-pr-assets/main/compact-trace-side-panel-zoom-hint-corner-light.png) ![zoom hint corner dark](https://raw.githubusercontent.com/alex-fedotyev/hdx-pr-assets/main/compact-trace-side-panel-zoom-hint-corner-dark.png) ### How to reproduce **Starting point:** `yarn app:dev:local` connected to the demo server (onboarding, then "Connect to Demo Server"), or the Vercel preview on this PR. **On `main`:** 1. Open **Search** and pick the **Demo Traces** source. 2. Click any span in the results table. 3. The waterfall starts below the **SpanName** box, the `ServiceName` chip, the tabs, the minimap, the controls, and a separate "⌘/Ctrl + scroll to zoom" row. **On this branch:** 1. Same steps. The tabs sit right under the metadata row, the hint sits in the chart's top-left corner, and the first span starts noticeably higher. ### How to test on Vercel preview **Preview routes:** /search **Steps:** 1. Open /search. If the **Welcome to ClickStack** dialog appears, click **Connect to Demo Server**. Then select the **Demo Traces** source from the source picker. 2. Click the first row in the results table. 3. Verify the side panel opens on the **Trace** tab with no box labeled "SpanName" between the metadata row and the tabs. 4. Verify `[data-testid="timeline-zoom-hint"]` is visible inside the waterfall and reads "⌘/Ctrl + scroll to zoom". 5. Click a different span in the waterfall and confirm its details open next to the waterfall. ### Test plan - [x] unit: `isSpanNameShownInTitle` and `getHeaderHighlightExpressions` are tested directly in `DBRowSidePanel.compactHeader.test.ts`: the 40/41 character boundary, breadcrumb trails, a title that isn't the span name, no body value, log panels, the service chip on trace and log panels, and configured highlights - [x] unit: `DBRowSidePanelHeader` tests for `bodyShownInTitle` and the empty attribute list, and a `TimelineChart` test for the hint placement; each new assertion fails when its guarded line is reverted - [x] full app unit suite, app `ci:lint` (tsc, eslint within the warning budget, stylelint), ratchet, knip - [x] manual, local stack with a seeded 25-span checkout trace and a long SQL span name: before and after at 1920x945 in light and dark, hover time label clearance on the first span, View Trace from a log, log panel keeps its body box and chip - [x] viewport: 1920x945 and 1366x768; the corner hint fits the default label column at both, and truncates with an ellipsis if the column is dragged narrower - States: the waterfall's loading, empty, and error branches render exactly as before; only the header and chart chrome around them changed ### What's not in this PR - The waterfall controls row wraps when the span detail pane is open on the side (two lines at 1920, three at 1366), same as on `main`. Moving the **Correlated logs** source picker behind **Show Filters** would keep it to one line; that felt like a separate call. - Log panels also show the service twice (metadata row and chip). I left them alone to keep this change to the trace view. ### References - Related PRs: #2541 (single drawer header and metadata row), #2552 (timeline minimap)
1 parent 0f659fd commit 513b237

9 files changed

Lines changed: 268 additions & 45 deletions
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@hyperdx/app': patch
3+
---
4+
5+
fix: start the trace waterfall higher in the side panel
6+
7+
Trace spans no longer repeat their name in a box under the panel title, or the
8+
service as a chip next to the metadata row that already shows it. The
9+
"⌘/Ctrl + scroll to zoom" hint moves into the timeline's top-left corner
10+
instead of taking its own row. Long span names that the title truncates keep
11+
the box. Log panels keep their body box and service chip.

‎packages/app/src/components/DBHighlightedAttributesList.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export type HighlightedAttribute = {
1515
lucene?: string;
1616
};
1717

18+
// Renders without an outer margin; callers set the spacing around the list.
1819
export function DBHighlightedAttributesList({
1920
attributes,
2021
}: {
@@ -41,7 +42,7 @@ export function DBHighlightedAttributesList({
4142
const hiddenAttributesCount = attributes.length - sortedAttributes.length;
4243

4344
return (
44-
<Flex wrap="wrap" gap="2px" mb="md" align="baseline">
45+
<Flex wrap="wrap" gap="2px" align="baseline">
4546
{sortedAttributes.map(({ displayedKey, value, sql, lucene, source }) => (
4647
<EventTag
4748
displayedKey={displayedKey}

‎packages/app/src/components/DBRowSidePanel.tsx‎

Lines changed: 56 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,54 @@ const SPAN_KIND_LABELS: Record<string, string> = {
239239
SPAN_KIND_CONSUMER: 'Consumer',
240240
};
241241

242+
// Longest span name the header trusts a lone title breadcrumb to show in full.
243+
// Kept well under that crumb's 120 character cap (MAX_LABEL_LENGTH_SINGLE in
244+
// SidePanelBreadcrumbs.tsx) because CSS can still ellipsize a long title in a
245+
// narrow drawer; longer names keep the body paper.
246+
const TITLE_FITS_MAX_LENGTH = 40;
247+
248+
/**
249+
* Attribute expressions shown as chips in the side panel header. Log panels
250+
* also get the service name, kept from before highlightedRowAttributeExpressions
251+
* existed; trace panels skip it because the metadata row already shows it.
252+
*/
253+
export function getHeaderHighlightExpressions(source: TSource) {
254+
const expressions: NonNullable<
255+
(TLogSource | TTraceSource)['highlightedRowAttributeExpressions']
256+
> = [];
257+
if (
258+
(source.kind === SourceKind.Trace || source.kind === SourceKind.Log) &&
259+
source.highlightedRowAttributeExpressions
260+
) {
261+
expressions.push(...source.highlightedRowAttributeExpressions);
262+
}
263+
if (isLogSource(source) && source.serviceNameExpression) {
264+
expressions.push({ sqlExpression: source.serviceNameExpression });
265+
}
266+
return expressions;
267+
}
268+
269+
/**
270+
* Whether the panel title already shows a trace span's name in full, so the
271+
* header can skip the body paper that would only repeat it. Only a lone
272+
* breadcrumb counts: in a trail the crumbs share one row and can cut the name
273+
* off, and after View Trace the title is the originating log. Long names keep
274+
* the paper too.
275+
*/
276+
export function isSpanNameShownInTitle(
277+
source: TSource,
278+
spanName: string | undefined,
279+
breadcrumbLabels: string[],
280+
) {
281+
return (
282+
source.kind === SourceKind.Trace &&
283+
spanName != null &&
284+
spanName.length <= TITLE_FITS_MAX_LENGTH &&
285+
breadcrumbLabels.length === 1 &&
286+
breadcrumbLabels[0] === spanName
287+
);
288+
}
289+
242290
type DBRowSidePanelProps = {
243291
source: TSource;
244292
rowId: string | undefined;
@@ -498,31 +546,10 @@ export const DBRowSidePanelInner = ({
498546
}, [activeRowId, mainContent, resolvedFrameLabels]);
499547

500548
const highlightedAttributeValues = useMemo(() => {
501-
const attributeExpressions: NonNullable<
502-
(TLogSource | TTraceSource)['highlightedRowAttributeExpressions']
503-
> = [];
504-
if (
505-
(source.kind === SourceKind.Trace || source.kind === SourceKind.Log) &&
506-
source.highlightedRowAttributeExpressions
507-
) {
508-
attributeExpressions.push(...source.highlightedRowAttributeExpressions);
509-
}
510-
511-
// Add service name expression to all sources, to maintain compatibility with
512-
// the behavior prior to the addition of highlightedRowAttributeExpressions
513-
if (
514-
(isLogSource(source) || isTraceSource(source)) &&
515-
source.serviceNameExpression
516-
) {
517-
attributeExpressions.push({
518-
sqlExpression: source.serviceNameExpression,
519-
});
520-
}
521-
522549
return rowData
523550
? getHighlightedAttributesFromData(
524551
source,
525-
attributeExpressions,
552+
getHeaderHighlightExpressions(source),
526553
rowData.data || [],
527554
rowData.meta || [],
528555
)
@@ -868,6 +895,12 @@ export const DBRowSidePanelInner = ({
868895
clearSpanSelection,
869896
]);
870897

898+
const isBodyShownInTitle = isSpanNameShownInTitle(
899+
source,
900+
mainContent,
901+
allBreadcrumbs.map(crumb => crumb.label),
902+
);
903+
871904
const availableTabs = useMemo<Tab[]>(() => {
872905
const tabs: Tab[] = [];
873906
if (hasOverviewPanel && !sourceIsTrace) tabs.push(Tab.Overview);
@@ -1075,6 +1108,7 @@ export const DBRowSidePanelInner = ({
10751108
attributes={highlightedAttributeValues}
10761109
mainContent={mainContent}
10771110
mainContentHeader={mainContentColumn}
1111+
bodyShownInTitle={isBodyShownInTitle}
10781112
severityText={severityText}
10791113
rowData={normalizedRow}
10801114
/>

‎packages/app/src/components/DBRowSidePanelHeader.tsx‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,17 @@ export default function DBRowSidePanelHeader({
2424
// source has neither body nor implicit column configured), the body
2525
// paper is suppressed entirely; the highlighted attributes still render.
2626
bodyConfigured = true,
27+
// When `true`, the panel title already shows `mainContent` in full (a
28+
// trace span's name, for example), so the body paper is omitted rather
29+
// than repeating the title directly below it.
30+
bodyShownInTitle = false,
2731
severityText,
2832
rowData,
2933
}: {
3034
mainContent?: string;
3135
mainContentHeader?: string;
3236
bodyConfigured?: boolean;
37+
bodyShownInTitle?: boolean;
3338
attributes?: HighlightedAttribute[];
3439
severityText?: string;
3540
rowData?: Record<string, any>;
@@ -85,7 +90,7 @@ export default function DBRowSidePanelHeader({
8590

8691
return (
8792
<>
88-
{!bodyConfigured ? null : mainContent ? (
93+
{!bodyConfigured || bodyShownInTitle ? null : mainContent ? (
8994
<Paper
9095
p="xs"
9196
mt="sm"
@@ -135,9 +140,11 @@ export default function DBRowSidePanelHeader({
135140
</Paper>
136141
)}
137142
<AISummarizeButton rowData={rowData} severityText={severityText} />
138-
<Box mt="xs">
139-
<DBHighlightedAttributesList attributes={attributesWithDefault} />
140-
</Box>
143+
{attributesWithDefault.length > 0 && (
144+
<Box mt="xs">
145+
<DBHighlightedAttributesList attributes={attributesWithDefault} />
146+
</Box>
147+
)}
141148
</>
142149
);
143150
}

‎packages/app/src/components/DBTraceWaterfallChart.tsx‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1394,12 +1394,7 @@ export function DBTraceWaterfallChartContainer({
13941394
return (
13951395
<>
13961396
{showMinimap && (
1397-
<Box mb="md">
1398-
<TimelineMinimap
1399-
rows={timelineRows}
1400-
controller={viewportController}
1401-
/>
1402-
</Box>
1397+
<TimelineMinimap rows={timelineRows} controller={viewportController} />
14031398
)}
14041399
{isFilterExpanded && (
14051400
<form onSubmit={handleSubmit(onSubmitFilters)}>
@@ -1618,7 +1613,11 @@ export function DBTraceWaterfallChartContainer({
16181613
</Group>
16191614
</Group>
16201615
{!isFetching && !error && highlightedAttributeValues?.length > 0 && (
1621-
<DBHighlightedAttributesList attributes={highlightedAttributeValues} />
1616+
<Box mb="xs">
1617+
<DBHighlightedAttributesList
1618+
attributes={highlightedAttributeValues}
1619+
/>
1620+
</Box>
16221621
)}
16231622
<div
16241623
ref={timelineWrapperRef}

‎packages/app/src/components/TimelineChart/TimelineChart.tsx‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { memo, useEffect, useLayoutEffect, useMemo, useRef } from 'react';
22
import cx from 'classnames';
3-
import { Flex, Kbd, Text } from '@mantine/core';
3+
import { Flex, Text } from '@mantine/core';
44
import { useVirtualizer } from '@tanstack/react-virtual';
55

66
import {
@@ -77,7 +77,12 @@ type TimelineChartProps = {
7777
const MIN_VIEWPORT_FRAC = 0.02;
7878

7979
const axisHeight = 24;
80-
const rowsMarginTop = 32;
80+
// Gap between the axis and the first row. The hover time label
81+
// (TimelineMouseCursor, styled in TimelineChart.module.scss) reaches about 37px
82+
// below the top of the chart; the first row starts at
83+
// axisHeight + rowsMarginTop = 44px, which keeps it clear with some headroom.
84+
// Revisit this if the label's padding or offset grows.
85+
const rowsMarginTop = 20;
8186
const maxScale = 100;
8287

8388
export const TimelineChart = memo(function (props: TimelineChartProps) {
@@ -404,12 +409,6 @@ export const TimelineChart = memo(function (props: TimelineChartProps) {
404409

405410
return (
406411
<Flex h={maxHeight} mah={maxHeight} direction="column">
407-
<Flex justify="end" mb="sm">
408-
<Text>
409-
<Kbd>⌘/Ctrl</Kbd> + <Kbd>scroll</Kbd> to zoom
410-
</Text>
411-
</Flex>
412-
413412
<div className={styles.timelineViewport}>
414413
<div className={styles.timelineContainer} ref={timelineRef}>
415414
<div
@@ -426,15 +425,29 @@ export const TimelineChart = memo(function (props: TimelineChartProps) {
426425
labelWidth={labelWidth}
427426
/>
428427

428+
{/* The corner above the label column is otherwise empty, so the
429+
zoom hint lives here instead of in its own row above the chart.
430+
It reuses the tick label class and inherits the axis font so it
431+
reads as part of the axis header. */}
429432
<div
430-
aria-hidden
431433
className={styles.timelineCorner}
434+
data-testid="timeline-corner"
432435
style={{
433436
width: labelWidth,
434437
height: `${axisHeight + rowsMarginTop}px`,
435438
marginTop: `-${axisHeight + rowsMarginTop}px`,
436439
}}
437-
/>
440+
>
441+
<Text
442+
inherit
443+
c="dimmed"
444+
truncate="end"
445+
className={styles.xAxisTickLabel}
446+
data-testid="timeline-zoom-hint"
447+
>
448+
⌘/Ctrl + scroll to zoom
449+
</Text>
450+
</div>
438451

439452
{rowVirtualizer.getVirtualItems().map(virtualRow => {
440453
const row = rows[virtualRow.index];
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { screen } from '@testing-library/react';
2+
3+
import { TimelineChart } from '@/components/TimelineChart/TimelineChart';
4+
import type { TTimelineEvent } from '@/components/TimelineChart/TimelineChartRowEvents';
5+
6+
function makeRow(id: string, start: number, end: number) {
7+
const event: TTimelineEvent = {
8+
id,
9+
start,
10+
end,
11+
tooltip: id,
12+
color: 'white',
13+
backgroundColor: 'gray',
14+
body: <span>{id}</span>,
15+
minWidthPx: 1,
16+
};
17+
return { id, label: <span>{id}</span>, events: [event] };
18+
}
19+
20+
describe('TimelineChart', () => {
21+
it('shows the zoom hint in the corner above the labels, inside the chart', () => {
22+
renderWithMantine(
23+
<TimelineChart
24+
initialScrollRowIndex={-1}
25+
labelWidth={300}
26+
maxHeight={400}
27+
rowHeight={22}
28+
rows={[makeRow('root', 0, 100), makeRow('child', 10, 40)]}
29+
onEventClick={jest.fn()}
30+
/>,
31+
);
32+
33+
const hint = screen.getByTestId('timeline-zoom-hint');
34+
expect(hint).toHaveTextContent('⌘/Ctrl + scroll to zoom');
35+
// Lives in the corner cell above the span labels rather than in a row of
36+
// its own above the chart.
37+
expect(hint.closest('[data-testid="timeline-corner"]')).toBeInTheDocument();
38+
});
39+
});
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import {
2+
getHeaderHighlightExpressions,
3+
isSpanNameShownInTitle,
4+
} from '@/components/DBRowSidePanel';
5+
import { makeLogSource, makeTraceSource } from '@/llm/__fixtures__/sources';
6+
7+
describe('isSpanNameShownInTitle', () => {
8+
const trace = makeTraceSource();
9+
10+
it('is true when the lone title crumb is a short span name', () => {
11+
expect(
12+
isSpanNameShownInTitle(trace, 'GET /checkout', ['GET /checkout']),
13+
).toBe(true);
14+
});
15+
16+
it('allows names up to 40 characters', () => {
17+
const name = 'x'.repeat(40);
18+
expect(isSpanNameShownInTitle(trace, name, [name])).toBe(true);
19+
});
20+
21+
it('keeps the body for longer names the title can cut off', () => {
22+
const name = 'x'.repeat(41);
23+
expect(isSpanNameShownInTitle(trace, name, [name])).toBe(false);
24+
});
25+
26+
it('keeps the body in a breadcrumb trail, where crumbs share the row', () => {
27+
// A hop between two spans with the same name is still a trail.
28+
expect(
29+
isSpanNameShownInTitle(trace, 'GET /checkout', [
30+
'GET /checkout',
31+
'GET /checkout',
32+
]),
33+
).toBe(false);
34+
});
35+
36+
it('keeps the body when the title is not the span name', () => {
37+
expect(isSpanNameShownInTitle(trace, 'GET /checkout', ['Trace'])).toBe(
38+
false,
39+
);
40+
});
41+
42+
it('keeps the body when the span has no body value', () => {
43+
expect(isSpanNameShownInTitle(trace, undefined, ['Trace'])).toBe(false);
44+
});
45+
46+
it('keeps the body on log panels', () => {
47+
expect(
48+
isSpanNameShownInTitle(makeLogSource(), 'a log line', ['a log line']),
49+
).toBe(false);
50+
});
51+
});
52+
53+
describe('getHeaderHighlightExpressions', () => {
54+
it('does not repeat the service as a chip on trace panels', () => {
55+
expect(getHeaderHighlightExpressions(makeTraceSource())).toEqual([]);
56+
});
57+
58+
it('still highlights the service on log panels', () => {
59+
expect(getHeaderHighlightExpressions(makeLogSource())).toEqual([
60+
{ sqlExpression: 'ServiceName' },
61+
]);
62+
});
63+
64+
it('keeps highlights configured on the source', () => {
65+
const highlight = { sqlExpression: "SpanAttributes['http.method']" };
66+
expect(
67+
getHeaderHighlightExpressions(
68+
makeTraceSource({ highlightedRowAttributeExpressions: [highlight] }),
69+
),
70+
).toEqual([highlight]);
71+
});
72+
});

0 commit comments

Comments
 (0)