From 9b88264046b50ca958408d07dc20cd206876ef1b Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Thu, 9 Jul 2026 16:14:16 -0400 Subject: [PATCH] fix(dashboards): improve alert annotation rendering on tile charts - Clamp firing/recovery markers to the chart's x-axis domain so an alert already firing when the window opens stays visible at sub-minute granularity (previously dropped when the domain started off the minute). - Draw annotations as dashed lines with the label floated in reserved top headroom (added only when annotations are shown) instead of an inside-top label, so they stay legible against dense series and stacked bars. - Pass annotation data (not pre-rendered elements) to the chart via a new `annotations` prop so it can clamp/size them against its own domain. --- ...dashboard-alert-annotation-improvements.md | 13 +++ packages/app/src/DBDashboardPage.tsx | 6 +- packages/app/src/HDXMultiSeriesTimeChart.tsx | 35 ++++++++ packages/app/src/components/DBTimeChart.tsx | 5 ++ .../__tests__/chartAnnotations.test.tsx | 83 ++++++++++++++----- .../components/charts/chartAnnotations.tsx | 40 +++++---- .../__tests__/useAlertAnnotations.test.tsx | 13 ++- .../app/src/hooks/useAlertAnnotations.tsx | 20 ++--- 8 files changed, 161 insertions(+), 54 deletions(-) create mode 100644 .changeset/dashboard-alert-annotation-improvements.md diff --git a/.changeset/dashboard-alert-annotation-improvements.md b/.changeset/dashboard-alert-annotation-improvements.md new file mode 100644 index 0000000000..71e06c838a --- /dev/null +++ b/.changeset/dashboard-alert-annotation-improvements.md @@ -0,0 +1,13 @@ +--- +"@hyperdx/app": patch +--- + +fix(dashboards): make alert annotations easier to read and keep the "already firing" marker on-screen at sub-minute granularity + +Alert firing/recovery annotations on dashboard tiles now float their "Alert" / +"OK" labels in reserved headroom above the marker line — added only on tiles +that are showing annotations — so the labels stay clear of dense series and +stacked bars. Also fixes a case where the marker for an alert that was already +firing when the window opened could be dropped on tiles using a sub-minute +granularity (non-minute-aligned start): the marker now snaps to the chart's +visible left edge instead of falling outside the plot. diff --git a/packages/app/src/DBDashboardPage.tsx b/packages/app/src/DBDashboardPage.tsx index 6190366b97..1926cca3be 100644 --- a/packages/app/src/DBDashboardPage.tsx +++ b/packages/app/src/DBDashboardPage.tsx @@ -636,7 +636,7 @@ const Tile = forwardRef( // Firing/recovery markers for this tile's alert, scoped to the *visible* // window — the fullscreen range while the fullscreen view is open, else the // dashboard range (off unless the dashboard toggle is on). - const alertAnnotationReferenceLines = useAlertAnnotations( + const alertAnnotations = useAlertAnnotations( alert?.id, isFullscreen ? fullscreenDateRange : dateRange, showAlertAnnotations, @@ -1077,7 +1077,7 @@ const Tile = forwardRef( showDisplaySwitcher={true} enabled={chartEnabled} config={effectiveQueriedConfig} - referenceLines={alertAnnotationReferenceLines} + annotations={alertAnnotations} onTimeRangeSelect={ isFullscreenView ? (start, end) => setFullscreenDateRange([start, end]) @@ -1273,7 +1273,7 @@ const Tile = forwardRef( isSourceMissing, isSourceUnset, hasBeenVisible, - alertAnnotationReferenceLines, + alertAnnotations, ], ); diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 5e576b7061..5353efef94 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -34,6 +34,10 @@ import { IconZoomReset } from '@tabler/icons-react'; import type { NumberFormat } from '@/types'; import { COLORS, formatNumber, truncateMiddle } from '@/utils'; +import { + ChartAnnotation, + getAnnotationElements, +} from './components/charts/chartAnnotations'; import { ChartTooltipContainer, ChartTooltipItem, @@ -58,6 +62,10 @@ const NEAREST_SERIES_MAX_DISTANCE_PX = 30; const Y_AXIS_WIDTH = 40; const SINGLE_POINT_BAR_RIGHT_PADDING = 10; const SINGLE_POINT_BAR_WIDTH_RATIO = 0.8; +// Top margin (px) reserved above the plot for annotation labels ("Alert"/"OK"), +// added only when a chart is showing annotations so other charts keep their +// tighter default headroom. +const ANNOTATION_LABEL_HEADROOM = 18; type TooltipPayload = { dataKey: string; @@ -470,6 +478,7 @@ export const MemoChart = memo(function MemoChart({ dateRange, lineData, referenceLines, + annotations, logReferenceTimestamp, displayType = DisplayType.Line, axisNumberFormat, @@ -492,6 +501,13 @@ export const MemoChart = memo(function MemoChart({ dateRange: [Date, Date] | Readonly<[Date, Date]>; lineData: LineData[]; referenceLines?: React.ReactNode; + /** + * Event markers (alerts, deploys, …) drawn as dashed vertical lines with a + * label above. Passed as data rather than pre-rendered elements so the chart + * can clamp them to its own x-axis domain. Distinct from `referenceLines` + * (threshold lines). + */ + annotations?: ChartAnnotation[]; displayType?: DisplayType; axisNumberFormat?: NumberFormat; fallbackNumberFormat?: NumberFormat; @@ -792,6 +808,19 @@ export const MemoChart = memo(function MemoChart({ return [startTime.getTime() / 1000, endTime.getTime() / 1000]; }, [dateRange, granularity, dateRangeEndInclusive, displayType]); + // Alert/event markers as dashed lines, clamped to the chart's x-axis domain so + // an edge marker (e.g. an alert already firing at window open) stays visible + // instead of being dropped. Labels float in the reserved top headroom. + const annotationElements = useMemo(() => { + if (!annotations?.length) { + return null; + } + // xAxisDomain is a [min, max] tuple at runtime (declared as AxisDomain). + return getAnnotationElements(annotations, { + domain: xAxisDomain as [number, number], + }); + }, [annotations, xAxisDomain]); + return (
{onTimeRangeSelect != null && zoomOrigin != null ? ( @@ -827,6 +856,11 @@ export const MemoChart = memo(function MemoChart({ width={500} height={300} data={graphResults} + margin={ + annotationElements != null + ? { top: ANNOTATION_LABEL_HEADROOM, right: 5, bottom: 5, left: 5 } + : undefined + } syncId="hdx" syncMethod="value" barSize={singlePointBarSize} @@ -1022,6 +1056,7 @@ export const MemoChart = memo(function MemoChart({ /> )} {referenceLines} + {annotationElements} {highlightStart && highlightEnd ? ( void; queryKeyPrefix?: string; referenceLines?: React.ReactNode; + /** Event markers (e.g. alert firing/recovery) drawn as dashed lines with labels. */ + annotations?: ChartAnnotation[]; setDisplayType?: (type: DisplayType) => void; showDisplaySwitcher?: boolean; showLegend?: boolean; @@ -233,6 +236,7 @@ function DBTimeChartComponent({ onTimeRangeSelect, queryKeyPrefix, referenceLines, + annotations, setDisplayType, showDisplaySwitcher = true, showLegend = true, @@ -733,6 +737,7 @@ function DBTimeChartComponent({ tooltipNumberFormatsByKey={formatByColumn} onTimeRangeSelect={onTimeRangeSelect} referenceLines={referenceLines} + annotations={annotations} setIsClickActive={setActiveClickPayloadIfSourceAvailable} showLegend={showLegend} timestampKey={timestampColumn?.name} diff --git a/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx b/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx index 90b58e2525..8591bf3e8a 100644 --- a/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx +++ b/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx @@ -1,56 +1,97 @@ import { ReactElement } from 'react'; +import { ReferenceLine } from 'recharts'; import { - getAnnotationReferenceLines, + getAnnotationElements, MAX_ANNOTATION_MARKERS, } from '@/components/charts/chartAnnotations'; -// ReferenceLine element props are typed as `unknown`; narrow for assertions. +// ReferenceLine element props are typed loosely; narrow for assertions. const lineProps = (el: ReactElement) => - el.props as { stroke: string; x: number; label?: unknown }; + el.props as { + stroke: string; + strokeDasharray?: string; + x: number; + label?: unknown; + }; -describe('getAnnotationReferenceLines', () => { +// A domain wide enough that no marker clamps, and one bounded window for the +// clamp cases. +const wide = { domain: [0, 2_000_000_000] as [number, number] }; +const bounded = { domain: [1_000_000, 1_000_600] as [number, number] }; + +describe('getAnnotationElements', () => { it('returns nothing for an empty list', () => { - expect(getAnnotationReferenceLines([])).toEqual([]); + expect(getAnnotationElements([], wide)).toEqual([]); }); - it('renders a line per annotation at the unix-second x, with color and label', () => { - const time = '2026-07-01T00:05:00.000Z'; - const [line] = getAnnotationReferenceLines([ - { time, label: 'Deploy', color: '#123456' }, - ]); + it('renders a dashed reference line at the unix-second x, with color and label', () => { + const ms = 1_000_300_000; // 1_000_300s, inside `bounded` + const [line] = getAnnotationElements( + [{ time: ms, label: 'Alert', color: '#123456' }], + bounded, + ); - expect(lineProps(line).x).toBe(new Date(time).getTime() / 1000); + expect(line.type).toBe(ReferenceLine); + expect(lineProps(line).x).toBe(1_000_300); + expect(lineProps(line).strokeDasharray).toBe('3 3'); expect(lineProps(line).stroke).toBe('#123456'); expect(lineProps(line).label).toBeTruthy(); }); it('accepts Date / epoch-ms times, defaults the color, and omits an absent label', () => { - const ms = Date.UTC(2026, 6, 1, 0, 5, 0); - const [fromMs] = getAnnotationReferenceLines([{ time: ms }]); - const [fromDate] = getAnnotationReferenceLines([{ time: new Date(ms) }]); + const ms = 1_000_300_000; + const [fromMs] = getAnnotationElements([{ time: ms }], bounded); + const [fromDate] = getAnnotationElements([{ time: new Date(ms) }], bounded); - expect(lineProps(fromMs).x).toBe(ms / 1000); - expect(lineProps(fromDate).x).toBe(ms / 1000); + expect(lineProps(fromMs).x).toBe(1_000_300); + expect(lineProps(fromDate).x).toBe(1_000_300); expect(lineProps(fromMs).stroke).toBe('var(--color-border)'); expect(lineProps(fromMs).label).toBeUndefined(); }); + // Regression: an "already firing at window open" marker pinned to a coarser + // (minute-floored) start time used to fall left of a sub-minute chart domain + // and get dropped by Recharts. It must now snap to the left edge instead. + it('clamps a marker before the domain to the left edge', () => { + const beforeStart = (bounded.domain[0] - 100) * 1000; + const [line] = getAnnotationElements([{ time: beforeStart }], bounded); + + expect(lineProps(line).x).toBe(bounded.domain[0]); + }); + + it('clamps a marker after the domain to the right edge', () => { + const afterEnd = (bounded.domain[1] + 100) * 1000; + const [line] = getAnnotationElements([{ time: afterEnd }], bounded); + + expect(lineProps(line).x).toBe(bounded.domain[1]); + }); + + it('leaves an in-range marker unclamped', () => { + const inRange = (bounded.domain[0] + 300) * 1000; + const [line] = getAnnotationElements([{ time: inRange }], bounded); + + expect(lineProps(line).x).toBe(bounded.domain[0] + 300); + }); + it('caps the number of rendered markers', () => { const many = Array.from( { length: MAX_ANNOTATION_MARKERS + 50 }, (_, i) => ({ time: 1_700_000_000_000 + i * 60_000 }), ); - expect(getAnnotationReferenceLines(many)).toHaveLength( + expect(getAnnotationElements(many, wide)).toHaveLength( MAX_ANNOTATION_MARKERS, ); }); it('uses a provided key and falls back to a generated one', () => { - const lines = getAnnotationReferenceLines([ - { time: '2026-07-01T00:00:00.000Z', key: 'custom' }, - { time: '2026-07-01T00:00:00.000Z' }, - ]); + const lines = getAnnotationElements( + [ + { time: '2026-07-01T00:00:00.000Z', key: 'custom' }, + { time: '2026-07-01T00:00:00.000Z' }, + ], + wide, + ); expect(lines[0].key).toBe('custom'); expect(lines[1].key).toEqual(expect.any(String)); diff --git a/packages/app/src/components/charts/chartAnnotations.tsx b/packages/app/src/components/charts/chartAnnotations.tsx index 62837fb612..d2430c05b1 100644 --- a/packages/app/src/components/charts/chartAnnotations.tsx +++ b/packages/app/src/components/charts/chartAnnotations.tsx @@ -2,14 +2,14 @@ import { type ReactElement } from 'react'; import { Label, ReferenceLine } from 'recharts'; /** - * A single vertical marker to overlay on a timeseries chart at a point in time. + * A single marker to overlay on a timeseries chart at a point in time. * Source-agnostic: alerts, deployments, incidents, config changes, etc. all map * to this shape. */ export type ChartAnnotation = { /** Event time. Accepts a Date, ISO string, or epoch milliseconds. */ time: Date | string | number; - /** Optional short label drawn at the marker. */ + /** Optional short label drawn above the marker. */ label?: string; /** Line/label color (any CSS value). Defaults to the chart border color. */ color?: string; @@ -23,32 +23,44 @@ export type ChartAnnotation = { export const MAX_ANNOTATION_MARKERS = 1000; /** - * Renders vertical annotation lines for a chart's `referenceLines` prop. `x` is - * in unix seconds to match the timeseries x-axis (`ts_bucket`); markers outside - * the domain are dropped by Recharts. Generic over source — feature hooks map - * their events to `ChartAnnotation[]` and call this. Capped at - * `MAX_ANNOTATION_MARKERS` to protect against pathological inputs. + * Renders annotation markers as dashed vertical reference lines, with the label + * floated in the chart's top headroom (above the line) so it stays legible and + * clear of the series. The chart reserves that headroom only while annotations + * are shown — see `ANNOTATION_LABEL_HEADROOM` in `HDXMultiSeriesTimeChart`. + * + * `domain` is the chart's x-axis domain in unix seconds (matching `ts_bucket`). + * Each marker is clamped into that domain so an edge marker — e.g. an alert + * already firing when the window opens, pinned to a coarser-quantized start + * time — snaps to the visible edge instead of being dropped by Recharts. + * + * Generic over source — feature hooks map their events to `ChartAnnotation[]`. + * Capped at `MAX_ANNOTATION_MARKERS` to protect against pathological inputs. */ -export function getAnnotationReferenceLines( +export function getAnnotationElements( annotations: ChartAnnotation[], + opts: { domain: [number, number] }, ): ReactElement[] { + const [minX, maxX] = opts.domain; + return annotations.slice(0, MAX_ANNOTATION_MARKERS).map((annotation, i) => { - const seconds = new Date(annotation.time).getTime() / 1000; + const rawSeconds = new Date(annotation.time).getTime() / 1000; + // Clamp into the visible domain so edge markers snap to the edge. + const x = Math.min(Math.max(rawSeconds, minX), maxX); const color = annotation.color ?? 'var(--color-border)'; return ( { expect(result.current).toBeUndefined(); }); - it('returns reference lines when transitions exist', () => { + it('returns annotation data when transitions exist', () => { + const firedAt = '2026-07-01T00:30:00.000Z'; mockHistory({ - data: [ - { createdAt: '2026-07-01T00:30:00.000Z', state: AlertState.ALERT }, - ], + data: [{ createdAt: firedAt, state: AlertState.ALERT }], }); const { result } = renderHook(() => useAlertAnnotations('alert-1', range, true), ); expect(result.current).toHaveLength(1); + // Returns annotation data (not rendered elements); the chart renders it. + expect(result.current?.[0]).toMatchObject({ + time: firedAt, + label: 'Alert', + color: getChartColorError(), + }); }); }); diff --git a/packages/app/src/hooks/useAlertAnnotations.tsx b/packages/app/src/hooks/useAlertAnnotations.tsx index b04246890e..c1577927fb 100644 --- a/packages/app/src/hooks/useAlertAnnotations.tsx +++ b/packages/app/src/hooks/useAlertAnnotations.tsx @@ -1,11 +1,8 @@ -import { type ReactElement, useMemo } from 'react'; +import { useMemo } from 'react'; import { AlertState, AlertTransition } from '@hyperdx/common-utils/dist/types'; import api from '@/api'; -import { - ChartAnnotation, - getAnnotationReferenceLines, -} from '@/components/charts/chartAnnotations'; +import { ChartAnnotation } from '@/components/charts/chartAnnotations'; import { getChartColorError, getChartColorSuccess } from '@/utils'; /** @@ -31,23 +28,22 @@ export function alertTransitionsToAnnotations( } /** - * Returns alert firing/recovery annotation lines for a dashboard tile, scoped - * to the given `dateRange` (the tile's visible window). The query stays idle - * unless `enabled` is true and an `alertId` is present. + * Returns alert firing/recovery annotations for a dashboard tile, scoped to the + * given `dateRange` (the tile's visible window). Returns annotation *data*; the + * chart renders it (clamping/band geometry needs the chart's x-axis domain). The + * query stays idle unless `enabled` is true and an `alertId` is present. */ export function useAlertAnnotations( alertId: string | undefined, dateRange: [Date, Date], enabled: boolean = false, -): ReactElement[] | undefined { +): ChartAnnotation[] | undefined { const { data } = api.useAlertHistory(alertId, dateRange, { enabled }); return useMemo(() => { if (!enabled || !data?.data.length) { return undefined; } - return getAnnotationReferenceLines( - alertTransitionsToAnnotations(data.data), - ); + return alertTransitionsToAnnotations(data.data); }, [enabled, data]); }