diff --git a/.changeset/release-marker-hover-tooltip.md b/.changeset/release-marker-hover-tooltip.md new file mode 100644 index 0000000000..6567c44199 --- /dev/null +++ b/.changeset/release-marker-hover-tooltip.md @@ -0,0 +1,8 @@ +--- +'@hyperdx/app': minor +--- + +Hovering a release marker now lists every release in its cluster with the +service that shipped it, its version, and the time. Colour alone could not +identify a service once a chart had more series than the legend shows, and a +collapsed "N releases" cluster named none of them. diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index d829541f5a..922f07ccc9 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -17,6 +17,7 @@ import { BarChart, BarProps, CartesianGrid, + Customized, Legend, ReferenceArea, ReferenceLine, @@ -33,9 +34,15 @@ import { Popover } from '@mantine/core'; import type { NumberFormat } from '@/types'; import { COLORS, formatNumber, truncateMiddle } from '@/utils'; +import { + AnnotationHitLayer, + type HoveredAnnotation, +} from './components/charts/AnnotationHitLayer'; +import { AnnotationTooltip } from './components/charts/AnnotationTooltip'; import { ChartAnnotation, getAnnotationElements, + layoutAnnotations, resolveAnnotationSeries, } from './components/charts/chartAnnotations'; import { ChartOverlayControls } from './components/charts/ChartOverlayControls'; @@ -1435,6 +1442,21 @@ export const MemoChart = memo(function MemoChart({ ); }, [annotations, lineData]); + // Same geometry the hit layer positions against, so the hover bands can't + // drift from the lines they belong to. + const laidOutAnnotations = useMemo(() => { + if (!coloredAnnotations?.length) { + return null; + } + return layoutAnnotations(coloredAnnotations, { + domain: xAxisDomain, + plotWidth: Math.max(0, containerWidth - Y_AXIS_WIDTH), + }); + }, [coloredAnnotations, xAxisDomain, containerWidth]); + + const [hoveredAnnotation, setHoveredAnnotation] = + useState(null); + const annotationElements = useMemo(() => { if (!coloredAnnotations?.length) { return null; @@ -1469,6 +1491,9 @@ export const MemoChart = memo(function MemoChart({ style={{ position: 'relative', width: '100%', height: '100%' }} > {nearestSeriesStyle} + {hoveredAnnotation != null && ( + + )} + } + /> + )} {highlightStart && highlightEnd ? ( ` so it can read the + * real plot geometry (`usePlotArea`, `useXAxisScale`) instead of re-deriving it + * from the container width. Handlers deliberately do not stop propagation: + * events still bubble to the chart wrapper, which is what keeps drag-to-zoom + * and the series tooltip working underneath the bands. + */ +export function AnnotationHitLayer({ + annotations, + onHover, +}: { + annotations: PositionedAnnotation[]; + onHover: (hovered: HoveredAnnotation | null) => void; +}) { + const plot = usePlotArea(); + const scale = useXAxisScale(); + + if (plot == null || scale == null) { + return null; + } + + // One target per cluster, not per line. Anchors carry `members`; the muted + // lines inside a cluster are already covered by their anchor's band, and + // giving them their own rects would just have them intercept each other. + const anchors = annotations.filter(annotation => annotation.members != null); + + return ( + // Above every other chart layer, otherwise the series areas (zIndex 100) + // and the marker lines (400) receive the pointer instead of these bands. + + + {anchors.map((annotation, i) => { + const members = annotation.members ?? [annotation]; + // Span the whole cluster so hovering any of its lines — including the + // muted ones, which carry no label — surfaces the same tooltip. + const xs = members + .map(member => scale(member.x)) + .filter((px): px is number => px != null && Number.isFinite(px)); + if (xs.length === 0) { + return null; + } + const left = Math.min(...xs); + const right = Math.max(...xs); + + const width = Math.max(right - left, MIN_HIT_WIDTH_PX); + const x = left - (width - (right - left)) / 2; + + return ( + { + // Measured here rather than passed down from the chart + // container: reading the DOM in an event handler is fine, + // whereas reading a ref during render is not. + const band = event.currentTarget.getBoundingClientRect(); + onHover({ + annotation, + point: { x: band.left + band.width / 2, y: band.bottom }, + }); + }} + onMouseLeave={() => onHover(null)} + /> + ); + })} + + + ); +} diff --git a/packages/app/src/components/charts/AnnotationTooltip.tsx b/packages/app/src/components/charts/AnnotationTooltip.tsx new file mode 100644 index 0000000000..21c858b37a --- /dev/null +++ b/packages/app/src/components/charts/AnnotationTooltip.tsx @@ -0,0 +1,80 @@ +import { createPortal } from 'react-dom'; +import { Group, Stack, Text } from '@mantine/core'; + +import type { HoveredAnnotation } from './AnnotationHitLayer'; +import { + ChartTooltipContainer, + ChartTooltipHeader, + useChartTooltipZIndex, +} from './ChartTooltip'; + +/** Gap below the marker so the tooltip does not sit on top of its own label. */ +const TOOLTIP_OFFSET_PX = 6; + +/** + * Hover tooltip for a release marker: one row per release in the hovered + * cluster, each naming the service it belongs to. + * + * The service name is the whole point. On the chart the only carrier of that is + * the marker's colour, which stops being resolvable once the legend overflows + * past `MAX_LEGEND_ITEMS` and the matching entry hides behind "+N more". + * + * Portaled to the body and positioned fixed, matching the series tooltip: a + * dashboard tile clips its overflow, so an absolutely-positioned tooltip gets + * cut off at the tile edge. `hovered.point` is already in viewport pixels, + * measured from the hit band when the pointer entered it. + */ +export function AnnotationTooltip({ hovered }: { hovered: HoveredAnnotation }) { + const zIndex = useChartTooltipZIndex(); + const members = hovered.annotation.members ?? [hovered.annotation]; + + if (typeof document === 'undefined') { + return null; + } + + return createPortal( +
+ } + > + + {members.map((member, i) => ( + +
+ {member.group != null && ( + + {member.group} + + )} + + {member.label} + + + ))} + + +
, + document.body, + ); +} diff --git a/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx b/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx index af5ff334d2..01c5210c10 100644 --- a/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx +++ b/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx @@ -5,6 +5,7 @@ import { type ChartAnnotation, getAnnotationElements, labelSeparationPx, + layoutAnnotations, MAX_ANNOTATION_MARKERS, mergeAnnotations, resolveAnnotationSeries, @@ -428,3 +429,62 @@ describe('resolveAnnotationSeries', () => { expect(input.color).toBe('#fallback'); }); }); + +describe('layoutAnnotations', () => { + const collapsing = { + domain: [0, 1000] as [number, number], + plotWidth: 1000, + }; + const release = (seconds: number, label: string, group: string) => ({ + time: seconds * 1000, + label, + group, + kind: 'release', + groupNoun: 'releases', + }); + + // The hover tooltip reads `members` so a cluster shown as "N releases" can + // still name every service inside it — which colour alone cannot do once the + // legend overflows. + it('gives a collapsed cluster the members it swallowed', () => { + const laidOut = layoutAnnotations( + [ + release(0, '1.0.0', 'checkout'), + release(10, '2.0.0', 'payments'), + release(600, '3.0.0', 'search'), + ], + collapsing, + ); + + const anchors = laidOut.filter(a => a.members != null); + expect(anchors).toHaveLength(2); + expect(anchors[0].label).toBe('2 releases'); + expect(anchors[0].members?.map(m => m.group)).toEqual([ + 'checkout', + 'payments', + ]); + // Members keep their own labels, so the tooltip shows real versions. + expect(anchors[0].members?.map(m => m.label)).toEqual(['1.0.0', '2.0.0']); + expect(anchors[1].members?.map(m => m.group)).toEqual(['search']); + }); + + it('gives a lone marker itself as its only member', () => { + const [only] = layoutAnnotations( + [release(500, '1.0.0', 'checkout')], + collapsing, + ); + + expect(only.members?.map(m => m.label)).toEqual(['1.0.0']); + }); + + // Before the plot is measured there is no collapsing, but hover still needs + // members to render a row. + it('populates members even when the geometry is unknown', () => { + const laidOut = layoutAnnotations( + [release(0, '1.0.0', 'checkout'), release(10, '2.0.0', 'payments')], + { domain: collapsing.domain }, + ); + + expect(laidOut.every(a => a.members?.length === 1)).toBe(true); + }); +}); diff --git a/packages/app/src/components/charts/chartAnnotations.tsx b/packages/app/src/components/charts/chartAnnotations.tsx index 934de4c8e8..67edbf9165 100644 --- a/packages/app/src/components/charts/chartAnnotations.tsx +++ b/packages/app/src/components/charts/chartAnnotations.tsx @@ -73,10 +73,16 @@ const STROKE_OPACITY = 0.9; const MUTED_STROKE_OPACITY = 0.35; /** A marker resolved to its clamped x position, in unix seconds. */ -type PositionedAnnotation = ChartAnnotation & { +export type PositionedAnnotation = ChartAnnotation & { x: number; /** Label suppressed — a neighbour carries the group label for this cluster. */ muted?: boolean; + /** + * Every marker in this one's cluster, itself included, in time order. Only + * set on the labelled anchor. Hover surfaces read this so a cluster shown as + * "3 releases" can name all three. + */ + members?: PositionedAnnotation[]; }; /** @@ -213,8 +219,9 @@ function collapseLabels( } const size = end - start; + const members = sorted.slice(start, end); if (size === 1) { - collapsed.push(anchor); + collapsed.push({ ...anchor, members }); } else { // A cluster covering several series can't wear one of their colors // without claiming the others' events as that series'. Go neutral so @@ -227,6 +234,7 @@ function collapseLabels( ...anchor, label: `${size} ${noun}`, color: spansSeries ? MIXED_SERIES_COLOR : anchor.color, + members, }); } for (let i = start + 1; i < end; i++) { @@ -256,10 +264,10 @@ function collapseLabels( * Generic over source — feature hooks map their events to `ChartAnnotation[]`. * Capped at `MAX_ANNOTATION_MARKERS` to protect against pathological inputs. */ -export function getAnnotationElements( +export function layoutAnnotations( annotations: ChartAnnotation[], opts: { domain: [number, number]; plotWidth?: number }, -): ReactElement[] { +): PositionedAnnotation[] { const [minX, maxX] = opts.domain; const positioned = positionAnnotations(annotations, [minX, maxX]); @@ -272,9 +280,24 @@ export function getAnnotationElements( const laidOut = plotWidth > 0 && spanSeconds > 0 ? collapseLabels(positioned, plotWidth / spanSeconds) - : positioned; + : positioned.map(annotation => ({ + ...annotation, + members: [annotation], + })); + + return laidOut.slice(0, MAX_ANNOTATION_MARKERS); +} - return laidOut.slice(0, MAX_ANNOTATION_MARKERS).map((annotation, i) => { +/** + * Renders the laid-out markers as dashed vertical reference lines. Takes the + * output of `layoutAnnotations` so the hover hit layer can position itself + * against exactly the same geometry. + */ +export function getAnnotationElements( + annotations: ChartAnnotation[], + opts: { domain: [number, number]; plotWidth?: number }, +): ReactElement[] { + return layoutAnnotations(annotations, opts).map((annotation, i) => { const color = annotation.color ?? 'var(--color-border)'; return ( ) : undefined diff --git a/packages/app/tests/e2e/features/release-markers.spec.ts b/packages/app/tests/e2e/features/release-markers.spec.ts index 8962793e84..01e1450fe7 100644 --- a/packages/app/tests/e2e/features/release-markers.spec.ts +++ b/packages/app/tests/e2e/features/release-markers.spec.ts @@ -144,6 +144,31 @@ test.describe('Release markers', { tag: ['@full-stack', '@dashboard'] }, () => { await expect(page).not.toHaveURL(/releaseMarkers=true/); }); + // Colour alone can't identify a service once the legend overflows its 4-item + // cap and the matching entry hides behind "+N more". Hover has to name the + // service outright. + test('names the service on hover', async ({ page }) => { + await seedReleases(); + + await dashboardPage.createNewDashboard(); + await dashboardPage.addTileWithSource( + 'Release marker hover', + DEFAULT_LOGS_SOURCE_NAME, + ); + await dashboardPage.setGlobalFilter(`ServiceName:"${RELEASE_SERVICE}"`); + await dashboardPage.toggleReleaseAnnotations(); + + await expect(dashboardPage.getAnnotationMarkers()).toHaveCount(2); + + const targets = dashboardPage.getAnnotationHitTargets(); + await expect(targets).toHaveCount(2); + await targets.first().hover(); + + // Service name and version together, so no colour lookup is needed. + await expect(page.getByText(RELEASE_SERVICE).last()).toBeVisible(); + await expect(page.getByText(OLD_VERSION).last()).toBeVisible(); + }); + // A marker only aids correlation if the reader can tie it to something on // the chart. An aggregate line over several services can't do that, so // rather than draw a wall of markers naming services with no visible line, diff --git a/packages/app/tests/e2e/page-objects/DashboardPage.ts b/packages/app/tests/e2e/page-objects/DashboardPage.ts index c5f7182ee6..31284350ee 100644 --- a/packages/app/tests/e2e/page-objects/DashboardPage.ts +++ b/packages/app/tests/e2e/page-objects/DashboardPage.ts @@ -1432,6 +1432,16 @@ export class DashboardPage { return this.getTile(tileIndex).locator('.recharts-reference-line'); } + /** + * Transparent hover targets over the annotation markers, one per cluster. + * Hovering one opens a tooltip naming the services that released. + */ + getAnnotationHitTargets(tileIndex = 0): Locator { + return this.getTile(tileIndex).locator( + '.recharts-annotation-hit-layer rect', + ); + } + /** * Text labels for the annotation markers. Recharts hoists reference-line * labels into a separate z-index layer, so they are NOT descendants of the