From 521750ffa2451d2e3505150be544a87f0a19ef41 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 12 Aug 2026 16:44:26 -0400 Subject: [PATCH 1/3] feat(dashboards): release markers on tile charts Correlating a latency or error spike with a release meant leaving the product. Dashboard tiles can now overlay the moment each version of a service first appeared, derived from the version expression on the tile's source, so no CI integration is needed. Markers reuse the annotation overlay built for alert firing/recovery lines, which was written source-agnostic for exactly this. They are off by default, toggled from the dashboard overflow menu, with the state in the URL as `releaseMarkers` so a shared link keeps it. Called release markers rather than deployment markers on purpose. What is detected is a new version value appearing in telemetry, which is not the same as a deployment: a deploy that doesn't change the version string produces no marker at all, and a service idle past the lookback draws one when it scales back up. The narrower name keeps the failure mode legible. Three rules keep them trustworthy rather than noisy. A marker only helps if the reader can attribute it, so what a tile shows depends on what it charts: - Filtered to one service: that service's releases. - Grouped by service: every charted service's releases, each tinted to match its own line. - An aggregate line over many services: none, since a marker naming a service with no visible line invites false attribution. The query runs against the tile's own source with the tile's own filters, which is what makes scoping possible. The version already running when the window opens is recognised and dropped rather than drawn as a release that never happened. Dense clusters collapse to "N releases", sized from the estimated label width; a cluster spanning several services goes neutral rather than wearing one of their colours and claiming the others' releases as its own. --- .changeset/release-markers-on-tile-charts.md | 10 + packages/app/src/ChartUtils.tsx | 27 + packages/app/src/DBDashboardPage.tsx | 70 ++- packages/app/src/HDXMultiSeriesTimeChart.tsx | 32 +- packages/app/src/__tests__/ChartUtils.test.ts | 55 ++ .../__tests__/chartAnnotations.test.tsx | 332 ++++++++++- .../components/charts/chartAnnotations.tsx | 247 +++++++- .../__tests__/useReleaseAnnotations.test.tsx | 537 ++++++++++++++++++ .../app/src/hooks/useReleaseAnnotations.tsx | 337 +++++++++++ .../e2e/features/release-markers.spec.ts | 169 ++++++ .../tests/e2e/page-objects/DashboardPage.ts | 38 ++ 11 files changed, 1826 insertions(+), 28 deletions(-) create mode 100644 .changeset/release-markers-on-tile-charts.md create mode 100644 packages/app/src/hooks/__tests__/useReleaseAnnotations.test.tsx create mode 100644 packages/app/src/hooks/useReleaseAnnotations.tsx create mode 100644 packages/app/tests/e2e/features/release-markers.spec.ts diff --git a/.changeset/release-markers-on-tile-charts.md b/.changeset/release-markers-on-tile-charts.md new file mode 100644 index 0000000000..1e615d2d9f --- /dev/null +++ b/.changeset/release-markers-on-tile-charts.md @@ -0,0 +1,10 @@ +--- +'@hyperdx/app': minor +--- + +Overlay release markers on dashboard tile charts, showing when each version of a +service first appeared so a deployment can be lined up against a change in the +data. Markers are scoped to the data each tile is charting and tinted to match +their service's series color, and are suppressed on charts where they can't be +tied to a visible line, so an aggregate line spanning many services isn't +annotated with releases you can't attribute to it. diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index 2d6587e37e..3e787dca1f 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -198,6 +198,33 @@ export function useTimeChartSettings( export const ChartKeyJoiner = ' · '; const PreviousPeriodSuffix = ' (previous)'; +/** + * Finds the color of the series a group value belongs to — e.g. the color of + * the `checkout-service` line on a chart grouped by service name. + * + * Series keys are the group values joined by `ChartKeyJoiner`, prefixed by the + * value column name when the chart has more than one, so the group value is + * matched against the key's components rather than the whole key. Previous + * period series are skipped; they mirror a current period series' color. + * + * Used to tint chart annotations (release markers) to match the series they + * describe, so a marker for one service can't be read as another's. + */ +export function getSeriesColorForGroup( + lineData: LineData[], + group: string, +): string | undefined { + for (const line of lineData) { + if (line.isDashed) { + continue; + } + if (line.currentPeriodKey.split(ChartKeyJoiner).includes(group)) { + return line.color; + } + } + return undefined; +} + // Note: roundToNearestMinutes is broken in date-fns currently // additionally it doesn't support seconds or > 30min // so we need to write our own :( diff --git a/packages/app/src/DBDashboardPage.tsx b/packages/app/src/DBDashboardPage.tsx index d4f00d45c4..ee7d2e3a59 100644 --- a/packages/app/src/DBDashboardPage.tsx +++ b/packages/app/src/DBDashboardPage.tsx @@ -108,6 +108,7 @@ import { IconPlus, IconPresentation, IconRefresh, + IconRocket, IconSearch, IconSquaresDiagonal, IconTags, @@ -119,6 +120,7 @@ import { } from '@tabler/icons-react'; import { IsolatedChartSyncProvider } from '@/chartSync'; +import { mergeAnnotations } from '@/components/charts/chartAnnotations'; import { ContactSupportText } from '@/components/ContactSupportText'; import SnapGridLayout from '@/components/dashboard/SnapGridLayout'; import DashboardContainer from '@/components/DashboardContainer'; @@ -155,6 +157,7 @@ import useDashboardContainers, { TabDeleteAction, } from '@/hooks/useDashboardContainers'; import { useDashboardKioskMode } from '@/hooks/useDashboardKioskMode'; +import { useReleaseAnnotations } from '@/hooks/useReleaseAnnotations'; import { calculateNextTilePosition, makeId } from '@/utils/tilePositioning'; import ChartContainer, { @@ -394,6 +397,7 @@ const Tile = forwardRef( filters, variables, showAlertAnnotations, + showReleaseAnnotations, isLive, readOnly, @@ -424,6 +428,8 @@ const Tile = forwardRef( variables?: ChartVariable[]; // When true, draw alert firing/recovery annotations on this tile's chart. showAlertAnnotations?: boolean; + // When true, draw release markers on this tile's chart. + showReleaseAnnotations?: boolean; isLive?: boolean; readOnly?: boolean; @@ -686,6 +692,30 @@ const Tile = forwardRef( showAlertAnnotations, ); + // Release markers, over the same visible window. Scoped to this tile: the + // query runs against the tile's own source with the tile's own predicates, + // so a chart filtered to one service isn't annotated with another's + // releases. Tiles sharing a source and filters share one query. + const releaseAnnotations = useReleaseAnnotations( + isFullscreen ? fullscreenDateRange : dateRange, + showReleaseAnnotations, + { + source, + where: isBuilderSavedChartConfig(chart.config) + ? chart.config.where + : undefined, + whereLanguage: isBuilderSavedChartConfig(chart.config) + ? chart.config.whereLanguage + : undefined, + filters, + }, + ); + + const annotations = useMemo( + () => mergeAnnotations(alertAnnotations, releaseAnnotations), + [alertAnnotations, releaseAnnotations], + ); + const filterWarning = useMemo(() => { const doFiltersExist = !!filters?.filter( f => (f.type === 'lucene' || f.type === 'sql') && f.condition.trim(), @@ -1175,7 +1205,7 @@ const Tile = forwardRef( showDisplaySwitcher={!readOnly} enabled={chartEnabled} config={effectiveQueriedConfig} - annotations={alertAnnotations} + annotations={annotations} onTimeRangeSelect={ readOnly ? undefined @@ -1387,7 +1417,7 @@ const Tile = forwardRef( isSourceMissing, isSourceUnset, hasBeenVisible, - alertAnnotations, + annotations, isLive, readOnly, ], @@ -1802,6 +1832,11 @@ function DBDashboardPage({ presetConfig }: { presetConfig?: Dashboard }) { 'alertAnnotations', parseAsBoolean.withDefault(false), ); + // Same for release markers, derived from `service.version` changes. + const [showReleaseAnnotations, setShowReleaseAnnotations] = useQueryState( + 'releaseMarkers', + parseAsBoolean.withDefault(false), + ); // Track if we've initialized query for this dashboard const initializedDashboardRef = useRef(undefined); @@ -2270,6 +2305,7 @@ function DBDashboardPage({ presetConfig }: { presetConfig?: Dashboard }) { variables={showFilterVariableOptions ? variables : undefined} onTimeRangeSelect={onTimeRangeSelect} showAlertAnnotations={showAlertAnnotations} + showReleaseAnnotations={showReleaseAnnotations} isHighlighted={highlightedTileId === chart.id} onUpdateChart={ isKioskMode @@ -2367,6 +2403,7 @@ function DBDashboardPage({ presetConfig }: { presetConfig?: Dashboard }) { whereLanguage, onTimeRangeSelect, showAlertAnnotations, + showReleaseAnnotations, getFilterQueriesForSource, showFilterVariableOptions, variables, @@ -2811,15 +2848,26 @@ function DBDashboardPage({ presetConfig }: { presetConfig?: Dashboard }) { {(hasTiles || containers.length > 0) && ( <> {hasTiles && ( - } - onClick={() => setShowAlertAnnotations(v => !v)} - data-testid="toggle-alert-annotations-menu-item" - > - {showAlertAnnotations - ? 'Hide alert annotations' - : 'Show alert annotations'} - + <> + } + onClick={() => setShowAlertAnnotations(v => !v)} + data-testid="toggle-alert-annotations-menu-item" + > + {showAlertAnnotations + ? 'Hide alert annotations' + : 'Show alert annotations'} + + } + onClick={() => setShowReleaseAnnotations(v => !v)} + data-testid="toggle-release-annotations-menu-item" + > + {showReleaseAnnotations + ? 'Hide release markers' + : 'Show release markers'} + + )} {containers.length > 0 && ( <> diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index d06e89792f..d829541f5a 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -36,6 +36,7 @@ import { COLORS, formatNumber, truncateMiddle } from '@/utils'; import { ChartAnnotation, getAnnotationElements, + resolveAnnotationSeries, } from './components/charts/chartAnnotations'; import { ChartOverlayControls } from './components/charts/ChartOverlayControls'; import { @@ -48,6 +49,7 @@ import { import { useChartSyncId } from './chartSync'; import { findNearestSeriesKey, + getSeriesColorForGroup, LineData, MAX_TIME_CHART_SERIES, toStartOfInterval, @@ -796,7 +798,7 @@ export const MemoChart = memo(function MemoChart({ lineData: LineData[]; referenceLines?: React.ReactNode; /** - * Event markers (alerts, deploys, …) drawn as dashed vertical lines with a + * Event markers (alerts, releases, …) 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). @@ -1395,7 +1397,9 @@ export const MemoChart = memo(function MemoChart({ [buildActivePayloadFromState, setIsClickActive], ); - const xAxisDomain: AxisDomain = useMemo(() => { + // Typed as the tuple it actually is (assignable to AxisDomain) so the + // annotation helpers can take it without an unsafe cast. + const xAxisDomain: [number, number] = useMemo(() => { let startTime = toStartOfInterval(dateRange[0], granularity); let endTime = toStartOfInterval(dateRange[1], granularity); const endTimeIsBoundaryAligned = isSameSecond(dateRange[1], endTime); @@ -1420,15 +1424,29 @@ export const MemoChart = memo(function MemoChart({ // 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(() => { + // Tint each marker to match the series it describes and drop the ones that + // can't be tied to anything on this chart — see `resolveAnnotationSeries`. + const coloredAnnotations = useMemo(() => { if (!annotations?.length) { + return annotations; + } + return resolveAnnotationSeries(annotations, group => + getSeriesColorForGroup(lineData, group), + ); + }, [annotations, lineData]); + + const annotationElements = useMemo(() => { + if (!coloredAnnotations?.length) { return null; } - // xAxisDomain is a [min, max] tuple at runtime (declared as AxisDomain). - return getAnnotationElements(annotations, { - domain: xAxisDomain as [number, number], + return getAnnotationElements(coloredAnnotations, { + domain: xAxisDomain, + // Drawable width, so markers too close together share one label. Zero on + // the first paint (before ResponsiveContainer measures), which the + // renderer treats as "label everything". + plotWidth: Math.max(0, containerWidth - Y_AXIS_WIDTH), }); - }, [annotations, xAxisDomain]); + }, [coloredAnnotations, xAxisDomain, containerWidth]); return (
{ }); }); }); + +describe('getSeriesColorForGroup', () => { + const line = ( + currentPeriodKey: string, + color: string, + isDashed = false, + ): LineData => ({ + dataKey: currentPeriodKey, + currentPeriodKey, + previousPeriodKey: `${currentPeriodKey} (previous)`, + displayName: currentPeriodKey, + valueColumnName: 'count()', + color, + isDashed, + }); + + // Single value column + group by: the series key is just the group value. + it('matches a group value that is the whole series key', () => { + const lineData = [line('checkout-service', '#blue'), line('api', '#gold')]; + + expect(getSeriesColorForGroup(lineData, 'api')).toBe('#gold'); + }); + + // Multiple value columns: the key is prefixed with the column name, joined + // by ChartKeyJoiner, so the group value is only one component of it. + it('matches a group value that is one component of a composite key', () => { + const lineData = [ + line(`count()${ChartKeyJoiner}checkout-service`, '#blue'), + line(`count()${ChartKeyJoiner}api`, '#gold'), + ]; + + expect(getSeriesColorForGroup(lineData, 'api')).toBe('#gold'); + }); + + it('returns undefined when no series covers the group', () => { + expect( + getSeriesColorForGroup([line('checkout-service', '#blue')], 'api'), + ).toBeUndefined(); + }); + + it('returns undefined for an empty series list', () => { + expect(getSeriesColorForGroup([], 'api')).toBeUndefined(); + }); + + // Previous-period series mirror a current-period series' color, so skipping + // them keeps the marker tied to the solid line the user sees. + it('skips previous-period series', () => { + const lineData = [line('api', '#dashed', true), line('api', '#solid')]; + + expect(getSeriesColorForGroup(lineData, 'api')).toBe('#solid'); + }); +}); diff --git a/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx b/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx index 8591bf3e8a..af5ff334d2 100644 --- a/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx +++ b/packages/app/src/components/charts/__tests__/chartAnnotations.test.tsx @@ -2,8 +2,12 @@ import { ReactElement } from 'react'; import { ReferenceLine } from 'recharts'; import { + type ChartAnnotation, getAnnotationElements, + labelSeparationPx, MAX_ANNOTATION_MARKERS, + mergeAnnotations, + resolveAnnotationSeries, } from '@/components/charts/chartAnnotations'; // ReferenceLine element props are typed loosely; narrow for assertions. @@ -11,10 +15,13 @@ const lineProps = (el: ReactElement) => el.props as { stroke: string; strokeDasharray?: string; + strokeOpacity?: number; x: number; - label?: unknown; + label?: { props: { value: string } }; }; +const labelOf = (el: ReactElement) => lineProps(el).label?.props.value; + // 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] }; @@ -97,4 +104,327 @@ describe('getAnnotationElements', () => { expect(lines[1].key).toEqual(expect.any(String)); expect(lines[0].key).not.toEqual(lines[1].key); }); + + it('drops an unparseable time instead of rendering a broken line', () => { + const lines = getAnnotationElements( + [{ time: 'not a date' }, { time: 1_000_300_000 }], + bounded, + ); + + expect(lines).toHaveLength(1); + expect(lineProps(lines[0]).x).toBe(1_000_300); + }); +}); + +describe('mergeAnnotations', () => { + const at = (ms: number, label: string): ChartAnnotation => ({ + time: ms, + label, + }); + + it('returns undefined when every list is empty or absent', () => { + expect(mergeAnnotations(undefined, [], undefined)).toBeUndefined(); + }); + + it('concatenates lists in ascending time order, ignoring absent ones', () => { + const merged = mergeAnnotations( + [at(3_000, 'c'), at(1_000, 'a')], + undefined, + [at(2_000, 'b')], + ); + + expect(merged?.map(a => a.label)).toEqual(['a', 'b', 'c']); + }); + + it('sorts mixed Date / ISO / epoch-ms times together', () => { + const merged = mergeAnnotations( + [{ time: new Date(3_000), label: 'c' }], + [{ time: '1970-01-01T00:00:01.000Z', label: 'a' }], + [{ time: 2_000, label: 'b' }], + ); + + expect(merged?.map(a => a.label)).toEqual(['a', 'b', 'c']); + }); + + // Recharts 3 keeps props in an Immer store and hands back frozen arrays, so + // merging must never sort a caller's list in place. + it('does not mutate or reorder the input lists', () => { + const input = Object.freeze([at(3_000, 'c'), at(1_000, 'a')]); + + expect(() => mergeAnnotations(input)).not.toThrow(); + expect(input.map(a => a.label)).toEqual(['c', 'a']); + }); +}); + +describe('labelSeparationPx', () => { + // Labels are centered on their marker, so the room two neighbours need is + // half of each width, plus a gap. + it('scales with the combined width of both labels', () => { + expect(labelSeparationPx('1.0.0', '2.0.0')).toBeLessThan( + labelSeparationPx('2026.08.04-abcdef12', '2026.08.04-abcdef34'), + ); + }); + + it('still separates unlabelled markers by a gap', () => { + expect(labelSeparationPx(undefined, undefined)).toBeGreaterThan(0); + }); +}); + +describe('getAnnotationElements label collapsing', () => { + // 1000px over a 1000s domain => 1px per second, so a marker's `time` in + // seconds is also its pixel offset. + const collapsing = { + domain: [0, 1000] as [number, number], + plotWidth: 1000, + }; + const deploy = (seconds: number, label: string): ChartAnnotation => ({ + time: seconds * 1000, + label, + kind: 'release', + groupNoun: 'releases', + }); + + it('leaves markers with room for both labels individually labelled', () => { + const lines = getAnnotationElements( + [deploy(0, '1.0.0'), deploy(60, '2.0.0')], + collapsing, + ); + + expect(lines.map(labelOf)).toEqual(['1.0.0', '2.0.0']); + }); + + it('collapses markers whose labels would overlap', () => { + const lines = getAnnotationElements( + [deploy(0, '1.0.0'), deploy(20, '2.0.0')], + collapsing, + ); + + expect(lines.map(labelOf)).toEqual(['2 releases', undefined]); + }); + + // Regression: a fixed separation let long version strings render on top of + // each other ("2.0.0" and "2 releases" colliding into "2.0.02 depl…"). + it('reserves more room for longer labels at the same spacing', () => { + const lines = getAnnotationElements( + [deploy(0, '2026.08.04-abcdef12'), deploy(60, '2026.08.04-abcdef34')], + collapsing, + ); + + // Same 60px gap that leaves short labels alone is not enough for these. + expect(lines.map(labelOf)).toEqual(['2 releases', undefined]); + }); + + // Anchoring on the group's first member (not the running last one) is what + // stops a chain of near-threshold gaps collapsing into one giant group. + it('groups against the first member, not the running last one', () => { + const lines = getAnnotationElements( + [ + deploy(0, '1.0.0-beta'), + deploy(40, '1.1.0-beta'), + deploy(100, '1.2.0-beta'), + ], + collapsing, + ); + + // 40 is inside the anchor's label footprint; 100 is clear of it and starts + // its own group. Anchoring on the last member would have swallowed it. + expect(lines).toHaveLength(3); + expect(labelOf(lines[0])).toBe('2 releases'); + expect(labelOf(lines[1])).toBeUndefined(); + expect(labelOf(lines[2])).toBe('1.2.0-beta'); + }); + + // Regression: a cluster spanning two services took the anchor's series color, + // so "2 releases" rendered in one service's color while counting the other's + // release as that service's. + it('drops the series color when a cluster spans several series', () => { + const [anchor] = getAnnotationElements( + [ + { ...deploy(0, '9.0.0'), group: 'semconv-api', color: '#blue' }, + { ...deploy(10, 'v8.0.0'), group: 'gitops-api', color: '#gold' }, + ], + collapsing, + ); + + expect(labelOf(anchor)).toBe('2 releases'); + // Neutral, and not either series' colour. + expect(lineProps(anchor).stroke).toBe('var(--color-text-muted)'); + }); + + it('keeps the series color when a cluster is all one series', () => { + const [anchor] = getAnnotationElements( + [ + { ...deploy(0, '9.0.0'), group: 'semconv-api', color: '#blue' }, + { ...deploy(10, '9.1.0'), group: 'semconv-api', color: '#blue' }, + ], + collapsing, + ); + + expect(labelOf(anchor)).toBe('2 releases'); + expect(lineProps(anchor).stroke).toBe('#blue'); + }); + + it('mutes the line of a marker whose label was collapsed away', () => { + const [anchor, absorbed] = getAnnotationElements( + [deploy(0, '1.0.0'), deploy(10, '2.0.0')], + collapsing, + ); + + expect(lineProps(anchor).strokeOpacity).toBe(0.9); + expect(lineProps(absorbed).strokeOpacity).toBeLessThan(0.9); + // The line is still drawn, so a dense cluster stays visible. + expect(lineProps(absorbed).x).toBe(10); + }); + + it('falls back to "events" when no group noun is supplied', () => { + const lines = getAnnotationElements( + [ + { time: 0, label: 'a', kind: 'alert' }, + { time: 10_000, label: 'b', kind: 'alert' }, + ], + collapsing, + ); + + expect(labelOf(lines[0])).toBe('2 events'); + }); + + it('never collapses markers of different kinds together', () => { + const lines = getAnnotationElements( + [deploy(0, '1.0.0'), { time: 5_000, label: 'Alert', kind: 'alert' }], + collapsing, + ); + + // Both are within each other's label footprint, but they are different + // kinds, so each keeps its own label rather than becoming "2 releases". + expect(lines.map(labelOf).sort()).toEqual(['1.0.0', 'Alert']); + }); + + it('labels everything when the plot width is not yet measured', () => { + const annotations = [deploy(0, '1.0.0'), deploy(10, '2.0.0')]; + + // plotWidth 0 is the pre-measure state; omitted is the legacy caller. + expect( + getAnnotationElements(annotations, { ...collapsing, plotWidth: 0 }).map( + labelOf, + ), + ).toEqual(['1.0.0', '2.0.0']); + expect( + getAnnotationElements(annotations, { + domain: collapsing.domain, + }).map(labelOf), + ).toEqual(['1.0.0', '2.0.0']); + }); + + it('labels everything when the domain collapses to a single point', () => { + const lines = getAnnotationElements([deploy(5, 'v1'), deploy(5, 'v2')], { + domain: [5, 5], + plotWidth: 1000, + }); + + expect(lines.map(labelOf)).toEqual(['v1', 'v2']); + }); + + it('still caps the rendered markers after collapsing', () => { + const many = Array.from({ length: MAX_ANNOTATION_MARKERS + 50 }, (_, i) => + deploy(i / 10, `v${i}`), + ); + + expect(getAnnotationElements(many, collapsing)).toHaveLength( + MAX_ANNOTATION_MARKERS, + ); + }); +}); + +describe('resolveAnnotationSeries', () => { + const deploy = (group: string, label: string): ChartAnnotation => ({ + time: 1_000, + label, + group, + color: '#fallback', + }); + // A chart grouped by service: each of these has its own line. + const charted = (group: string) => + ({ checkout: '#blue', payments: '#teal' })[group]; + const nothingCharted = () => undefined; + + it('tints a marker to match its series', () => { + const [resolved] = resolveAnnotationSeries( + [deploy('checkout', '1.0.0')], + charted, + ); + + expect(resolved.color).toBe('#blue'); + }); + + it('keeps markers for every service the chart breaks out', () => { + const resolved = resolveAnnotationSeries( + [deploy('checkout', '1.0.0'), deploy('payments', '2.0.0')], + charted, + ); + + expect(resolved.map(a => a.color)).toEqual(['#blue', '#teal']); + }); + + // A tile filtered to one service: there is no per-service line to match, but + // the whole chart is about that service, so the markers are unambiguous. + it('keeps markers when every one belongs to the same group', () => { + const resolved = resolveAnnotationSeries( + [deploy('checkout', '1.0.0'), deploy('checkout', '2.0.0')], + nothingCharted, + ); + + expect(resolved.map(a => a.label)).toEqual(['1.0.0', '2.0.0']); + // No series to borrow from, so the marker keeps its own color. + expect(resolved[0].color).toBe('#fallback'); + }); + + // The case this rule exists for: an aggregate line over several services. + // A marker naming a service the reader cannot locate invites false + // attribution, so it is dropped rather than drawn. + it('drops markers that span several groups none of which are charted', () => { + const resolved = resolveAnnotationSeries( + [deploy('checkout', '1.0.0'), deploy('payments', '2.0.0')], + nothingCharted, + ); + + expect(resolved).toEqual([]); + }); + + it('keeps only the charted services when a chart shows a subset', () => { + const resolved = resolveAnnotationSeries( + [ + deploy('checkout', '1.0.0'), + deploy('payments', '2.0.0'), + deploy('billing', '3.0.0'), + ], + charted, + ); + + expect(resolved.map(a => a.label)).toEqual(['1.0.0', '2.0.0']); + }); + + // Alert markers describe the whole chart, not one series. + it('always keeps markers that carry no group', () => { + const resolved = resolveAnnotationSeries( + [ + { time: 1_000, label: 'Alert', color: '#red' }, + deploy('checkout', '1.0.0'), + deploy('payments', '2.0.0'), + ], + nothingCharted, + ); + + expect(resolved.map(a => a.label)).toEqual(['Alert']); + }); + + it('returns an empty list for no annotations', () => { + expect(resolveAnnotationSeries([], charted)).toEqual([]); + }); + + it('does not mutate the input annotations', () => { + const input = deploy('checkout', '1.0.0'); + resolveAnnotationSeries([input], charted); + + expect(input.color).toBe('#fallback'); + }); }); diff --git a/packages/app/src/components/charts/chartAnnotations.tsx b/packages/app/src/components/charts/chartAnnotations.tsx index d2430c05b1..934de4c8e8 100644 --- a/packages/app/src/components/charts/chartAnnotations.tsx +++ b/packages/app/src/components/charts/chartAnnotations.tsx @@ -3,7 +3,7 @@ import { Label, ReferenceLine } from 'recharts'; /** * A single marker to overlay on a timeseries chart at a point in time. - * Source-agnostic: alerts, deployments, incidents, config changes, etc. all map + * Source-agnostic: alerts, releases, incidents, config changes, etc. all map * to this shape. */ export type ChartAnnotation = { @@ -15,6 +15,24 @@ export type ChartAnnotation = { color?: string; /** Stable React key; defaults to the resolved timestamp + index. */ key?: string; + /** + * Feature that produced the marker ('alert', 'release', …). Markers only + * ever share a collapsed label with others of the same kind, so a release is + * never counted as an alert. + */ + kind?: string; + /** + * Plural noun for the collapsed label when several markers of this kind sit + * too close to label individually ("3 releases"). Defaults to 'events'. + */ + groupNoun?: string; + /** + * Series this marker belongs to (e.g. a service name on a chart grouped by + * service). The chart tints the marker to match that series' color, so a + * marker for one service can't be read as another's. Ignored when the chart + * has no matching series — `color` is then used as the fallback. + */ + group?: string; }; // Safety valve: past this many markers the chart is unreadable anyway, and @@ -22,6 +40,204 @@ export type ChartAnnotation = { // flapping alert over a wide window). export const MAX_ANNOTATION_MARKERS = 1000; +// Labels are centered on their marker, so two neighbours collide once they are +// closer than half of each label's width plus a gap. Widths are estimated from +// the character count — a fixed separation would either let long version +// strings overlap or collapse short ones that had room to spare. +const LABEL_CHAR_WIDTH_PX = 6; // approximate advance at fontSize 10 +const LABEL_GAP_PX = 8; + +function estimateLabelWidthPx(label: string | undefined): number { + return (label?.length ?? 0) * LABEL_CHAR_WIDTH_PX; +} + +/** Minimum distance between two labelled markers before they overlap. */ +export function labelSeparationPx( + left: string | undefined, + right: string | undefined, +): number { + return ( + (estimateLabelWidthPx(left) + estimateLabelWidthPx(right)) / 2 + + LABEL_GAP_PX + ); +} + +// Colour for a cluster that covers several series. Legible, unlike the border +// default, and deliberately not a palette colour so it can't be mistaken for +// one of the series it summarises. +const MIXED_SERIES_COLOR = 'var(--color-text-muted)'; + +const STROKE_OPACITY = 0.9; +// Members of a collapsed group still get a line (so the density is visible), +// but a faint one, so the labelled anchor stays legible. +const MUTED_STROKE_OPACITY = 0.35; + +/** A marker resolved to its clamped x position, in unix seconds. */ +type PositionedAnnotation = ChartAnnotation & { + x: number; + /** Label suppressed — a neighbour carries the group label for this cluster. */ + muted?: boolean; +}; + +/** + * Merges annotation lists from independent features (alerts, releases, …) + * into the single array the chart takes. Returns `undefined` when nothing is + * left, matching the chart's "no annotations" prop state. + */ +export function mergeAnnotations( + ...lists: (readonly ChartAnnotation[] | undefined)[] +): ChartAnnotation[] | undefined { + // `flat` already produces a fresh array, so the sort below never touches the + // caller's lists — which matters because Recharts 3 keeps props in an Immer + // store and hands back frozen arrays. + const merged = lists.filter((list): list is readonly ChartAnnotation[] => + Boolean(list?.length), + ); + if (merged.length === 0) { + return undefined; + } + return merged + .flat() + .sort((a, b) => new Date(a.time).getTime() - new Date(b.time).getTime()); +} + +/** + * Ties markers to the chart's series, dropping the ones that can't be tied to + * anything visible. + * + * A marker only aids correlation if the reader can attribute it. Three cases: + * + * - The marker's group has its own series (a chart grouped by service): tint it + * to match that line, so a release of one service can't be read as another's. + * - Every grouped marker shares one group (a tile filtered to one service): the + * chart is entirely about that thing, so the markers are unambiguous even + * with no series to match. Kept with their own color. + * - Otherwise — several services' releases over a chart that doesn't break them + * out — the marker names something the reader cannot locate on the chart. A + * wall of those reads as noise and invites false attribution, so they are + * dropped. + * + * Markers with no group at all (alerts) are always kept; they describe the + * whole chart rather than one series. + */ +export function resolveAnnotationSeries( + annotations: ChartAnnotation[], + seriesColorFor: (group: string) => string | undefined, +): ChartAnnotation[] { + const groups = new Set(); + for (const annotation of annotations) { + if (annotation.group != null) { + groups.add(annotation.group); + } + } + const isSingleGroup = groups.size <= 1; + + const resolved: ChartAnnotation[] = []; + for (const annotation of annotations) { + if (annotation.group == null) { + resolved.push(annotation); + continue; + } + const seriesColor = seriesColorFor(annotation.group); + if (seriesColor != null) { + resolved.push({ ...annotation, color: seriesColor }); + } else if (isSingleGroup) { + resolved.push(annotation); + } + } + return resolved; +} + +function positionAnnotations( + annotations: ChartAnnotation[], + [minX, maxX]: [number, number], +): PositionedAnnotation[] { + const positioned: PositionedAnnotation[] = []; + for (const annotation of annotations) { + const seconds = new Date(annotation.time).getTime() / 1000; + if (!Number.isFinite(seconds)) { + // An unparseable time would render as a broken line and poison the + // collapse sort. Drop it rather than draw it. + continue; + } + // Clamp into the visible domain so edge markers snap to the edge. + positioned.push({ + ...annotation, + x: Math.min(Math.max(seconds, minX), maxX), + }); + } + return positioned; +} + +/** + * Groups markers that are too close together to label individually, keeping one + * labelled anchor per cluster and muting the rest. Grouping is per `kind`, so an + * alert marker is never folded into a release's count. + */ +function collapseLabels( + positioned: PositionedAnnotation[], + pxPerSecond: number, +): PositionedAnnotation[] { + const byKind = new Map(); + for (const annotation of positioned) { + const kind = annotation.kind ?? ''; + const bucket = byKind.get(kind); + if (bucket) { + bucket.push(annotation); + } else { + byKind.set(kind, [annotation]); + } + } + + const collapsed: PositionedAnnotation[] = []; + for (const bucket of byKind.values()) { + const sorted = [...bucket].sort((a, b) => a.x - b.x); + const noun = sorted[0].groupNoun ?? 'events'; + let start = 0; + while (start < sorted.length) { + const anchor = sorted[start]; + // Measure every candidate against the group's *first* member, not the + // running last one — otherwise a long run of markers each just under the + // threshold apart would chain into one arbitrarily wide group. The + // anchor's label widens to "N events" as it absorbs, so re-measure it. + let end = start + 1; + while (end < sorted.length) { + const groupSize = end - start + 1; + const anchorLabel = + groupSize > 1 ? `${groupSize} ${noun}` : anchor.label; + const required = labelSeparationPx(anchorLabel, sorted[end].label); + if ((sorted[end].x - anchor.x) * pxPerSecond >= required) { + break; + } + end++; + } + + const size = end - start; + if (size === 1) { + collapsed.push(anchor); + } else { + // A cluster covering several series can't wear one of their colors + // without claiming the others' events as that series'. Go neutral so + // the count stays true and the color stops implying an owner; the + // absorbed markers keep their own colors, so which series took part is + // still visible in the lines. + const spansSeries = + new Set(sorted.slice(start, end).map(a => a.group)).size > 1; + collapsed.push({ + ...anchor, + label: `${size} ${noun}`, + color: spansSeries ? MIXED_SERIES_COLOR : anchor.color, + }); + } + for (let i = start + 1; i < end; i++) { + collapsed.push({ ...sorted[i], label: undefined, muted: true }); + } + start = end; + } + } + return collapsed; +} + /** * 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 @@ -33,27 +249,40 @@ export const MAX_ANNOTATION_MARKERS = 1000; * already firing when the window opens, pinned to a coarser-quantized start * time — snaps to the visible edge instead of being dropped by Recharts. * + * `plotWidth` (the drawable width in pixels) enables label collapsing for dense + * clusters. It is optional because the chart cannot measure itself on the first + * paint; without it every marker is labelled, as before. + * * Generic over source — feature hooks map their events to `ChartAnnotation[]`. * Capped at `MAX_ANNOTATION_MARKERS` to protect against pathological inputs. */ export function getAnnotationElements( annotations: ChartAnnotation[], - opts: { domain: [number, number] }, + opts: { domain: [number, number]; plotWidth?: number }, ): ReactElement[] { const [minX, maxX] = opts.domain; + const positioned = positionAnnotations(annotations, [minX, maxX]); - return annotations.slice(0, MAX_ANNOTATION_MARKERS).map((annotation, i) => { - 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); + // Collapse only when the pixel geometry is known. `plotWidth` is 0 before + // ResponsiveContainer measures, and the domain collapses to a point on a + // single-bucket chart — both degrade to "label everything", never to + // "drop markers". + const spanSeconds = maxX - minX; + const plotWidth = opts.plotWidth ?? 0; + const laidOut = + plotWidth > 0 && spanSeconds > 0 + ? collapseLabels(positioned, plotWidth / spanSeconds) + : positioned; + + return laidOut.slice(0, MAX_ANNOTATION_MARKERS).map((annotation, i) => { const color = annotation.color ?? 'var(--color-border)'; return ( ({ + useQueriedChartConfig: jest.fn(), +})); +jest.mock('@mantine/notifications', () => ({ + notifications: { show: jest.fn() }, +})); + +// Untyped handle on the mocked hook. Going through the module object keeps the +// mock's argument and return types loose, so the fixtures below don't need +// wide casts to stand in for a full react-query result. +const chartConfigModule: { useQueriedChartConfig: jest.Mock } = + jest.requireMock('@/hooks/useChartConfig'); +const mockedUseQueriedChartConfig = chartConfigModule.useQueriedChartConfig; + +// Fully-formed sources, typed as their concrete kind rather than asserted, so +// a schema change surfaces here instead of being silently cast away. +const logSource: TLogSource = { + id: 'log-1', + name: 'Logs', + kind: SourceKind.Log, + connection: 'conn-1', + from: { databaseName: 'default', tableName: 'otel_logs' }, + timestampValueExpression: 'TimestampTime', + defaultTableSelectExpression: 'Timestamp, Body', + serviceNameExpression: 'ServiceName', + implicitColumnExpression: 'Body', +}; + +const traceSource: TTraceSource = { + id: 'trace-1', + name: 'Traces', + kind: SourceKind.Trace, + connection: 'conn-1', + from: { databaseName: 'default', tableName: 'otel_traces' }, + timestampValueExpression: 'Timestamp', + defaultTableSelectExpression: 'Timestamp, SpanName', + durationExpression: 'Duration', + durationPrecision: 9, + traceIdExpression: 'TraceId', + spanIdExpression: 'SpanId', + parentSpanIdExpression: 'ParentSpanId', + spanNameExpression: 'SpanName', + spanKindExpression: 'SpanKind', + serviceNameExpression: 'ServiceName', +}; + +const metricSource: TMetricSource = { + id: 'metric-1', + name: 'Metrics', + kind: SourceKind.Metric, + connection: 'conn-1', + from: { databaseName: 'default', tableName: '' }, + timestampValueExpression: 'TimeUnix', + resourceAttributesExpression: 'ResourceAttributes', + metricTables: { + gauge: 'otel_metrics_gauge', + histogram: 'otel_metrics_histogram', + sum: 'otel_metrics_sum', + summary: 'otel_metrics_summary', + 'exponential histogram': 'otel_metrics_exponential_histogram', + }, +}; + +const sessionSource: TSessionSource = { + id: 'session-1', + name: 'Sessions', + kind: SourceKind.Session, + connection: 'conn-1', + from: { databaseName: 'default', tableName: 'hyperdx_sessions' }, + timestampValueExpression: 'TimestampTime', + traceSourceId: 'trace-1', +}; + +describe('canDeriveReleases', () => { + it('accepts log and trace sources', () => { + expect(canDeriveReleases(logSource)).toBe(true); + expect(canDeriveReleases(traceSource)).toBe(true); + }); + + // The deployments query re-aggregates the tile's own table, which is what + // makes the tile's filters meaningful against it. Metric sources resolve + // their table per metric type, so there is no single table to re-aggregate. + it('rejects metric, session and disabled sources', () => { + expect(canDeriveReleases(metricSource)).toBe(false); + expect(canDeriveReleases(sessionSource)).toBe(false); + expect(canDeriveReleases({ ...logSource, disabled: true })).toBe(false); + }); + + it('rejects a missing source', () => { + expect(canDeriveReleases(undefined)).toBe(false); + }); +}); + +describe('resolveVersionExpression', () => { + it('falls back to the OTel service.version resource attribute', () => { + expect(resolveVersionExpression(logSource)).toBe( + DEFAULT_VERSION_EXPRESSION, + ); + expect(resolveVersionExpression(undefined)).toBe( + DEFAULT_VERSION_EXPRESSION, + ); + }); + + // The point of the setting: GitOps teams whose release identifier is a + // container image tag shouldn't have to change instrumentation. + it("uses the source's own expression when configured", () => { + expect( + resolveVersionExpression({ + ...logSource, + serviceVersionExpression: "ResourceAttributes['container.image.tag']", + }), + ).toBe("ResourceAttributes['container.image.tag']"); + }); + + it('works on trace sources too', () => { + expect( + resolveVersionExpression({ + ...traceSource, + serviceVersionExpression: "SpanAttributes['release']", + }), + ).toBe("SpanAttributes['release']"); + }); + + it('treats a blank expression as unset', () => { + expect( + resolveVersionExpression({ + ...logSource, + serviceVersionExpression: ' ', + }), + ).toBe(DEFAULT_VERSION_EXPRESSION); + }); + + // Metric and session sources have no such field; it must not throw. + it('falls back for sources that cannot carry the field', () => { + expect(resolveVersionExpression(metricSource)).toBe( + DEFAULT_VERSION_EXPRESSION, + ); + expect(resolveVersionExpression(sessionSource)).toBe( + DEFAULT_VERSION_EXPRESSION, + ); + }); +}); + +describe('buildReleaseChartConfig', () => { + const range: [Date, Date] = [new Date(1_000), new Date(2_000)]; + + it('selects the first timestamp each version was seen at, grouped by version and service', () => { + const config = buildReleaseChartConfig( + logSource, + DEFAULT_VERSION_EXPRESSION, + range, + ); + + expect(config.select).toBe( + "min(TimestampTime) AS firstSeen, ResourceAttributes['service.version'] AS version, ServiceName AS service", + ); + expect(config.groupBy).toBe( + "ResourceAttributes['service.version'], ServiceName", + ); + expect(config.where).toBe("ResourceAttributes['service.version'] != ''"); + expect(config.whereLanguage).toBe('sql'); + expect(config.orderBy).toBe('firstSeen ASC'); + expect(config.limit).toEqual({ limit: 500 }); + expect(config.dateRange).toBe(range); + expect(config.source).toBe('log-1'); + expect(config.from).toBe(logSource.from); + }); + + // The group-by columns are already named in `select`; leaving this unset + // makes renderChartConfig append them a second time. + it('disables automatic group-by projection', () => { + expect( + buildReleaseChartConfig(logSource, DEFAULT_VERSION_EXPRESSION, range) + .selectGroupBy, + ).toBe(false); + }); + + it('omits the service column when the source has no service expression', () => { + const noService = { ...logSource, serviceNameExpression: undefined }; + const config = buildReleaseChartConfig( + noService, + DEFAULT_VERSION_EXPRESSION, + range, + ); + + expect(config.select).toBe( + "min(TimestampTime) AS firstSeen, ResourceAttributes['service.version'] AS version", + ); + expect(config.groupBy).toBe("ResourceAttributes['service.version']"); + }); + + it('uses only the first expression of a multi-column timestamp', () => { + const multi = { + ...logSource, + timestampValueExpression: 'TimestampTime, Timestamp', + }; + + expect( + buildReleaseChartConfig(multi, DEFAULT_VERSION_EXPRESSION, range).select, + ).toContain('min(TimestampTime) AS firstSeen'); + }); + + it('honors a custom version expression', () => { + const config = buildReleaseChartConfig( + logSource, + "LogAttributes['release']", + range, + ); + + expect(config.select).toContain("LogAttributes['release'] AS version"); + expect(config.groupBy).toContain("LogAttributes['release']"); + expect(config.where).toBe("LogAttributes['release'] != ''"); + }); + + describe('tile scoping', () => { + it('sends no filters when the tile is unfiltered', () => { + const config = buildReleaseChartConfig( + logSource, + DEFAULT_VERSION_EXPRESSION, + range, + { where: ' ', filters: [] }, + ); + + expect(config.filters).toBeUndefined(); + }); + + // The whole point: a tile filtered to one service must not be annotated + // with another service's releases. + it("carries the tile's own where clause as a filter", () => { + const config = buildReleaseChartConfig( + logSource, + DEFAULT_VERSION_EXPRESSION, + range, + { where: 'ServiceName:"checkout"', whereLanguage: 'lucene' }, + ); + + expect(config.filters).toEqual([ + { type: 'lucene', condition: 'ServiceName:"checkout"' }, + ]); + // The config's own `where` stays reserved for the SQL version predicate. + expect(config.where).toBe("ResourceAttributes['service.version'] != ''"); + }); + + it('preserves a SQL tile where clause as a SQL filter', () => { + const config = buildReleaseChartConfig( + logSource, + DEFAULT_VERSION_EXPRESSION, + range, + { where: "ServiceName = 'checkout'", whereLanguage: 'sql' }, + ); + + expect(config.filters).toEqual([ + { type: 'sql', condition: "ServiceName = 'checkout'" }, + ]); + }); + + it('appends dashboard filters and drops empty ones', () => { + const config = buildReleaseChartConfig( + logSource, + DEFAULT_VERSION_EXPRESSION, + range, + { + where: 'ServiceName:"checkout"', + whereLanguage: 'lucene', + filters: [ + { type: 'lucene', condition: '' }, + { type: 'sql', condition: "Env = 'prod'" }, + ], + }, + ); + + expect(config.filters).toEqual([ + { type: 'lucene', condition: 'ServiceName:"checkout"' }, + { type: 'sql', condition: "Env = 'prod'" }, + ]); + }); + + // Lucene bare terms need the source's implicit column to render. + it('carries the implicit column expression for Lucene filters', () => { + const config = buildReleaseChartConfig( + logSource, + DEFAULT_VERSION_EXPRESSION, + range, + ); + + expect(config.implicitColumnExpression).toBe('Body'); + }); + }); +}); + +describe('releaseRowsToAnnotations', () => { + const windowStart = new Date('2026-07-01T00:00:00.000Z'); + const inside = '2026-07-01T00:30:00.000Z'; + + it('maps a version first seen inside the window to a marker', () => { + const [annotation] = releaseRowsToAnnotations( + [{ firstSeen: inside, version: '1.43.1', service: 'checkout' }], + { windowStart }, + ); + + expect(annotation).toMatchObject({ + time: new Date(inside).getTime(), + label: '1.43.1', + color: getChartColorInfo(), + kind: 'release', + groupNoun: 'releases', + }); + }); + + // The query range is widened backwards so the version that was already + // running shows up; it must not be drawn as a deploy at the left edge. + it('drops the version that was already running when the window opened', () => { + const annotations = releaseRowsToAnnotations( + [ + { firstSeen: '2026-06-30T23:00:00.000Z', version: '1.43.0' }, + { firstSeen: inside, version: '1.43.1' }, + ], + { windowStart }, + ); + + expect(annotations.map(a => a.label)).toEqual(['1.43.1']); + }); + + it('keeps a version first seen exactly at the window start', () => { + const annotations = releaseRowsToAnnotations( + [{ firstSeen: windowStart.toISOString(), version: '1.43.1' }], + { windowStart }, + ); + + expect(annotations).toHaveLength(1); + }); + + it('drops rows with a missing or unparseable timestamp or version', () => { + const annotations = releaseRowsToAnnotations( + [ + { firstSeen: null, version: '1.0.0' }, + { firstSeen: 'not a date', version: '1.0.0' }, + { firstSeen: inside, version: '' }, + { firstSeen: inside, version: null }, + { firstSeen: inside, version: '1.43.1' }, + ], + { windowStart }, + ); + + expect(annotations.map(a => a.label)).toEqual(['1.43.1']); + }); + + it('gives each marker a distinct key so React can reconcile them', () => { + const annotations = releaseRowsToAnnotations( + [ + { firstSeen: inside, version: '1.43.1', service: 'checkout' }, + { firstSeen: inside, version: '1.43.1', service: 'payments' }, + ], + { windowStart }, + ); + + expect(annotations[0].key).not.toEqual(annotations[1].key); + }); +}); + +describe('useReleaseAnnotations', () => { + const range: [Date, Date] = [ + new Date('2026-07-01T00:00:00.000Z'), + new Date('2026-07-01T01:00:00.000Z'), + ]; + + const mockQuery = (rows: unknown[] | undefined, isFetching = false) => + mockedUseQueriedChartConfig.mockReturnValue({ + data: rows ? { data: rows } : undefined, + isFetching, + }); + + /** The config and options the hook last handed to `useQueriedChartConfig`. */ + const lastCall = (): [ + BuilderChartConfigWithDateRange, + { enabled: boolean }, + ] => + mockedUseQueriedChartConfig.mock.calls[ + mockedUseQueriedChartConfig.mock.calls.length - 1 + ]; + + beforeEach(() => mockQuery([])); + afterEach(() => jest.clearAllMocks()); + + it('returns undefined and keeps the query idle when disabled', () => { + mockQuery([{ firstSeen: '2026-07-01T00:30:00.000Z', version: '1.0.0' }]); + + const { result } = renderHook(() => + useReleaseAnnotations(range, false, { source: logSource }), + ); + + expect(result.current).toBeUndefined(); + expect(lastCall()[1]).toMatchObject({ enabled: false }); + }); + + it('keeps the query idle when the tile has no source', () => { + renderHook(() => useReleaseAnnotations(range, true)); + + expect(lastCall()[1]).toMatchObject({ enabled: false }); + }); + + it('keeps the query idle for a source that cannot provide releases', () => { + renderHook(() => + useReleaseAnnotations(range, true, { source: metricSource }), + ); + + expect(lastCall()[1]).toMatchObject({ enabled: false }); + }); + + it('returns markers for versions first seen inside the window', () => { + mockQuery([ + { firstSeen: '2026-07-01T00:30:00.000Z', version: '1.43.1' }, + { firstSeen: '2026-06-30T20:00:00.000Z', version: '1.43.0' }, + ]); + + const { result } = renderHook(() => + useReleaseAnnotations(range, true, { source: logSource }), + ); + + expect(result.current).toHaveLength(1); + expect(result.current?.[0]).toMatchObject({ + label: '1.43.1', + kind: 'release', + }); + }); + + it('returns undefined rather than an empty array when nothing is found', () => { + mockQuery([{ firstSeen: '2026-06-30T20:00:00.000Z', version: '1.43.0' }]); + + const { result } = renderHook(() => + useReleaseAnnotations(range, true, { source: logSource }), + ); + + expect(result.current).toBeUndefined(); + }); + + it("uses the source's configured version expression", () => { + renderHook(() => + useReleaseAnnotations(range, true, { + source: { + ...logSource, + serviceVersionExpression: "ResourceAttributes['container.image.tag']", + }, + }), + ); + + expect(lastCall()[0].select).toContain( + "ResourceAttributes['container.image.tag'] AS version", + ); + }); + + it("scopes the query with the tile's filters", () => { + renderHook(() => + useReleaseAnnotations(range, true, { + source: logSource, + where: 'ServiceName:"checkout"', + whereLanguage: 'lucene', + }), + ); + + expect(lastCall()[0].filters).toEqual([ + { type: 'lucene', condition: 'ServiceName:"checkout"' }, + ]); + }); + + it('widens the query range backwards to spot the already-running version', () => { + renderHook(() => useReleaseAnnotations(range, true, { source: logSource })); + + const [queryStart, queryEnd] = lastCall()[0].dateRange; + + // 10% of a one-hour window is under the 30 minute floor, so the floor wins. + expect(range[0].getTime() - queryStart.getTime()).toBe(30 * 60_000); + expect(queryEnd.getTime()).toBe(range[1].getTime()); + }); + + // A live window ending mid-minute; both bounds quantize to the same bucket + // as it slides forward a few seconds. + const liveRange: [Date, Date] = [ + new Date('2026-07-01T00:00:00.000Z'), + new Date('2026-07-01T01:00:30.000Z'), + ]; + const slid = (ms: number): [Date, Date] => [ + liveRange[0], + new Date(liveRange[1].getTime() + ms), + ]; + + // Callers rebuild the filters array every render. If that churned the config, + // react-query would issue one request per tile instead of sharing one. + it('reuses one config object as a live window slides within the minute', () => { + const { rerender } = renderHook( + ({ dateRange }) => + useReleaseAnnotations(dateRange, true, { + source: logSource, + filters: [{ type: 'sql', condition: "Env = 'prod'" }], + }), + { initialProps: { dateRange: liveRange } }, + ); + const first = mockedUseQueriedChartConfig.mock.calls[0][0]; + + rerender({ dateRange: slid(5_000) }); + + expect(lastCall()[0]).toBe(first); + }); + + it('rebuilds the config once the window crosses a minute boundary', () => { + const { rerender } = renderHook( + ({ dateRange }) => + useReleaseAnnotations(dateRange, true, { source: logSource }), + { initialProps: { dateRange: liveRange } }, + ); + const first = mockedUseQueriedChartConfig.mock.calls[0][0]; + + rerender({ dateRange: slid(120_000) }); + + expect(lastCall()[0]).not.toBe(first); + }); +}); diff --git a/packages/app/src/hooks/useReleaseAnnotations.tsx b/packages/app/src/hooks/useReleaseAnnotations.tsx new file mode 100644 index 0000000000..d51aa096c9 --- /dev/null +++ b/packages/app/src/hooks/useReleaseAnnotations.tsx @@ -0,0 +1,337 @@ +import { useEffect, useMemo, useRef } from 'react'; +import { + BuilderChartConfigWithDateRange, + Filter, + SearchConditionLanguage, + SourceKind, + TSource, +} from '@hyperdx/common-utils/dist/types'; +import { notifications } from '@mantine/notifications'; + +import { ChartAnnotation } from '@/components/charts/chartAnnotations'; +import { useQueriedChartConfig } from '@/hooks/useChartConfig'; +import { getFirstTimestampValueExpression } from '@/source'; +import { getChartColorInfo } from '@/utils'; + +/** + * Resource attribute carrying the running release. `service.version` is OTel + * resource semconv, so this works out of the box for instrumented services. + */ +export const DEFAULT_VERSION_EXPRESSION = + "ResourceAttributes['service.version']"; + +const RELEASE_EMPTY_NOTIFICATION_ID = 'release-markers-empty'; + +/** Distinct versions fetched per window. Far above any real release cadence. */ +const MAX_RELEASE_ROWS = 500; + +/** + * Query bounds are floored/ceiled to this, so a live-tailing dashboard reuses + * one cached result instead of issuing a fresh query every tick. Matches the + * bucketing `api.useAlertHistory` applies for the same reason. + */ +const BUCKET_MS = 60_000; + +// How far back before the visible window we look for the already-running +// version. See `releaseRowsToAnnotations` for why. +const MIN_LOOKBACK_MS = 30 * 60_000; +const LOOKBACK_RATIO = 0.1; + +/** A row of the releases query. Values arrive as strings from ClickHouse. */ +export type ReleaseRow = { + firstSeen?: string | number | null; + version?: string | null; + service?: string | null; +}; + +/** Narrows the tile's filters to the ones worth sending. */ +type ReleaseScope = { + where?: string; + whereLanguage?: SearchConditionLanguage; + filters?: Filter[]; +}; + +/** + * Whether releases can be derived from this source. + * + * Only log and trace sources qualify: the releases query runs against the + * *same table* the tile charts, which is what makes the tile's own filters + * meaningful against it. Metric sources resolve their table from `metricTables` + * per metric type, so there is no single table to re-aggregate, and a tile + * filter written against metric columns would not apply to a log table. + */ +export function canDeriveReleases( + source: TSource | undefined, +): source is TSource { + return ( + source != null && + !source.disabled && + (source.kind === SourceKind.Log || source.kind === SourceKind.Trace) + ); +} + +/** + * The expression that yields a service's running release for this source. + * + * Falls back to the OpenTelemetry `service.version` resource attribute, which + * covers teams following resource semconv. Teams whose version lives elsewhere + * — a container image tag under GitOps, a custom attribute — set + * `serviceVersionExpression` on the source instead of changing instrumentation. + */ +export function resolveVersionExpression(source: TSource | undefined): string { + const configured = + source != null && 'serviceVersionExpression' in source + ? source.serviceVersionExpression + : undefined; + return configured?.trim() || DEFAULT_VERSION_EXPRESSION; +} + +/** + * Builds the "when did each release first appear" query: one row per version + * (per service), carrying the earliest timestamp it was seen at. + * + * `scope` carries the tile's own predicates so the markers describe the slice + * the chart is actually showing — a tile filtered to one service must not be + * annotated with another service's releases. + * + * Uses string `select`/`groupBy` rather than the structured builder form. This + * is a fixed one-off aggregate rather than a user-editable series, and + * `SelectListSchema` accepts a raw string for exactly that case — which also + * keeps `min()` over a `DateTime64` out of the aggregate-function machinery. + */ +export function buildReleaseChartConfig( + source: TSource, + versionExpression: string, + dateRange: [Date, Date], + scope: ReleaseScope = {}, +): BuilderChartConfigWithDateRange { + const timestampExpression = getFirstTimestampValueExpression( + source.timestampValueExpression, + ); + const serviceExpression = + 'serviceNameExpression' in source + ? source.serviceNameExpression + : undefined; + + // The tile's own `where` and the dashboard filters both go through `filters`, + // which carries a language per entry — the config's own `where` is reserved + // for the SQL version predicate below. + const scopeFilters: Filter[] = [ + ...(scope.where?.trim() + ? [ + { + type: scope.whereLanguage === 'sql' ? 'sql' : 'lucene', + condition: scope.where, + } as Filter, + ] + : []), + ...(scope.filters ?? []).filter( + filter => !('condition' in filter) || filter.condition?.trim(), + ), + ]; + + return { + connection: source.connection, + source: source.id, + from: source.from, + timestampValueExpression: source.timestampValueExpression, + // Needed for Lucene scope filters to resolve bare terms. + implicitColumnExpression: + 'implicitColumnExpression' in source + ? source.implicitColumnExpression + : undefined, + useTextIndexForImplicitColumn: + 'useTextIndexForImplicitColumn' in source + ? source.useTextIndexForImplicitColumn + : undefined, + bodyExpression: + 'bodyExpression' in source ? source.bodyExpression : undefined, + select: [ + `min(${timestampExpression}) AS firstSeen`, + `${versionExpression} AS version`, + ...(serviceExpression ? [`${serviceExpression} AS service`] : []), + ].join(', '), + where: `${versionExpression} != ''`, + whereLanguage: 'sql', + ...(scopeFilters.length ? { filters: scopeFilters } : {}), + groupBy: [ + versionExpression, + ...(serviceExpression ? [serviceExpression] : []), + ].join(', '), + // The group-by columns are already spelled out in `select`; without this + // the renderer appends them a second time. + selectGroupBy: false, + orderBy: 'firstSeen ASC', + limit: { limit: MAX_RELEASE_ROWS }, + dateRange, + }; +} + +/** + * Placeholder so the hook can call `useQueriedChartConfig` unconditionally when + * the tile's source can't provide releases. The query is disabled in that case + * and never runs; the module-level identity keeps the query key stable. + */ +const NO_SOURCE_CONFIG: BuilderChartConfigWithDateRange = { + connection: '', + from: { databaseName: '', tableName: '' }, + select: '', + where: '', + whereLanguage: 'sql', + timestampValueExpression: '', + dateRange: [new Date(0), new Date(0)], +}; + +/** + * Maps query rows to markers, keeping only versions whose first appearance + * lands inside the visible window. + * + * The query range is widened backwards (see `useReleaseAnnotations`) so the + * version that was *already running* when the window opened also comes back — + * its `min(timestamp)` would otherwise sit at the left edge and read as a + * release that never happened. Anything first seen before `windowStart` is that + * incumbent, so it is dropped. + * + * Residual artifact: a service idle for longer than the lookback has no rows in + * the widened prefix, so its first post-idle row still reads as a release at the + * left edge. + */ +export function releaseRowsToAnnotations( + rows: ReleaseRow[], + { windowStart }: { windowStart: Date }, +): ChartAnnotation[] { + // Resolve the theme color once (it reads computed styles). + const color = getChartColorInfo(); + const windowStartMs = windowStart.getTime(); + const annotations: ChartAnnotation[] = []; + + for (const row of rows) { + if (row.firstSeen == null || !row.version) { + continue; + } + const firstSeenMs = new Date(row.firstSeen).getTime(); + if (!Number.isFinite(firstSeenMs) || firstSeenMs < windowStartMs) { + continue; + } + annotations.push({ + time: firstSeenMs, + label: row.version, + // Fallback only: the chart re-tints the marker to its service's series + // color when that service is charted (see `getSeriesColorForGroup`). + color, + kind: 'release', + groupNoun: 'releases', + group: row.service ?? undefined, + key: `release-annotation-${firstSeenMs}-${row.version}-${row.service ?? ''}`, + }); + } + + return annotations; +} + +/** + * Returns release markers for a tile, derived from changes in the + * `service.version` resource attribute. + * + * Scoped to the tile: the query runs against the tile's own source with the + * tile's own filters, so the markers describe the data the chart is showing. An + * unfiltered tile spanning every service therefore does show every service's + * releases — that is consistent, not noise. + * + * Returns annotation *data*; the chart renders it (clamping and label + * collapsing need the chart's x-axis domain). The query stays idle unless + * `enabled` is true and the source can provide releases. + */ +export function useReleaseAnnotations( + dateRange: [Date, Date], + enabled: boolean = false, + options?: ReleaseScope & { + source?: TSource; + /** Overrides the source's own expression. Mainly a testing seam. */ + versionExpression?: string; + }, +): ChartAnnotation[] | undefined { + const source = options?.source; + const isSupported = canDeriveReleases(source); + const versionExpression = + options?.versionExpression || + resolveVersionExpression(isSupported ? source : undefined); + + // Quantize before memoizing so a sliding "last 15 minutes" window produces a + // stable config for a whole minute. + const windowStartMs = + Math.floor(dateRange[0].getTime() / BUCKET_MS) * BUCKET_MS; + const windowEndMs = Math.ceil(dateRange[1].getTime() / BUCKET_MS) * BUCKET_MS; + + // Callers rebuild the filter array every render, so key the memo on its + // content. Tiles sharing a source and filters then share one query. + const scopeKey = JSON.stringify({ + where: options?.where, + whereLanguage: options?.whereLanguage, + filters: options?.filters, + }); + + const config = useMemo(() => { + if (!isSupported || !source) { + return NO_SOURCE_CONFIG; + } + const lookbackMs = Math.max( + MIN_LOOKBACK_MS, + (windowEndMs - windowStartMs) * LOOKBACK_RATIO, + ); + return buildReleaseChartConfig( + source, + versionExpression, + [new Date(windowStartMs - lookbackMs), new Date(windowEndMs)], + JSON.parse(scopeKey), + ); + }, [ + isSupported, + source, + versionExpression, + windowStartMs, + windowEndMs, + scopeKey, + ]); + + const { data, isFetching } = useQueriedChartConfig(config, { + enabled: enabled && isSupported, + }); + + const annotations = useMemo(() => { + if (!enabled || !data?.data?.length) { + return undefined; + } + const mapped = releaseRowsToAnnotations(data.data, { + windowStart: new Date(windowStartMs), + }); + return mapped.length ? mapped : undefined; + }, [enabled, data, windowStartMs]); + + // Toggling markers on and seeing nothing reads as broken, and the common + // cause is simply that the service does not emit `service.version`. Say so + // once (Mantine dedupes concurrent tiles by notification id). + const hasWarnedRef = useRef(false); + useEffect(() => { + if (!enabled) { + hasWarnedRef.current = false; + return; + } + if (isFetching || data == null || annotations != null) { + return; + } + if (hasWarnedRef.current) { + return; + } + hasWarnedRef.current = true; + notifications.show({ + id: RELEASE_EMPTY_NOTIFICATION_ID, + color: 'yellow', + title: 'No releases found', + message: + 'Release markers come from changes in the version attribute configured on this source. No version changes were found in this time range - if you deploy without changing the version, there is nothing to mark.', + }); + }, [enabled, isFetching, data, annotations]); + + return annotations; +} diff --git a/packages/app/tests/e2e/features/release-markers.spec.ts b/packages/app/tests/e2e/features/release-markers.spec.ts new file mode 100644 index 0000000000..8962793e84 --- /dev/null +++ b/packages/app/tests/e2e/features/release-markers.spec.ts @@ -0,0 +1,169 @@ +/** + * Release markers: releases show up as dashed vertical lines on dashboard + * tile charts, labelled with the version. + * + * Markers are derived from `ResourceAttributes['service.version']` — one marker + * per version whose first appearance falls inside the visible window. Nothing + * in the global seed sets that attribute, so this spec seeds its own rows: two + * versions, each under its own unique service name so parallel specs and the + * global seed can't contribute markers of their own. + */ +import { DashboardPage } from '../page-objects/DashboardPage'; +import { expect, test } from '../utils/base-test'; +import { + DEFAULT_LOGS_SOURCE_NAME, + E2E_CLICKHOUSE_DATABASE, + E2E_LOGS_TABLE, +} from '../utils/constants'; + +const CLICKHOUSE_HOST = + process.env.CLICKHOUSE_HOST || + `http://localhost:${process.env.HDX_E2E_CH_PORT || '20500'}`; +const CLICKHOUSE_USER = process.env.CLICKHOUSE_USER || 'default'; +const CLICKHOUSE_PASSWORD = process.env.CLICKHOUSE_PASSWORD || ''; + +async function clickhouseQuery(sql: string): Promise { + const url = new URL(CLICKHOUSE_HOST); + url.searchParams.set('user', CLICKHOUSE_USER); + if (CLICKHOUSE_PASSWORD) { + url.searchParams.set('password', CLICKHOUSE_PASSWORD); + } + + const response = await fetch(url.toString(), { + method: 'POST', + body: sql, + headers: { 'Content-Type': 'text/plain' }, + }); + + if (!response.ok) { + const errorText = await response.text(); + throw new Error( + `ClickHouse query failed (${response.status}): ${errorText}`, + ); + } +} + +// Spread across the dashboard's default "Past 1h" window with ~20 minutes +// between each. Labels collapse into "N releases" once neighbours are closer +// than roughly half their combined width, and a dashboard tile's plot is only +// a few hundred pixels wide — keep these generously spaced (and the version +// strings short) so the assertions below aren't sitting on that threshold. +const OLD_VERSION_AGO_MS = 50 * 60 * 1000; +const NEW_VERSION_AGO_MS = 8 * 60 * 1000; + +// Versions are fixed rather than per-run unique, which makes seeding +// idempotent: a Playwright retry would otherwise add more versions a few +// seconds from the first attempt's — close enough to collapse into a +// "2 releases" label and break the assertions. Re-inserting the same versions +// still groups to the same markers, since the query is +// `min(Timestamp) GROUP BY version, service`. The global seed truncates the +// table between suite runs, so nothing accumulates. +const OLD_VERSION = 'e2e-1.0.0'; +const NEW_VERSION = 'e2e-2.0.0'; +const RELEASE_SERVICE = 'release_markers_e2e'; + +// A second service releasing in the same window. Markers are scoped to the data +// a tile is charting, so filtering to RELEASE_SERVICE must hide this one. +const OTHER_VERSION = 'oth-9.9.9'; +const OTHER_SERVICE = 'release_markers_other_e2e'; +const OTHER_VERSION_AGO_MS = 29 * 60 * 1000; + +/** Seed releases for two services into the shared logs table. */ +async function seedReleases(): Promise { + const row = (agoMs: number, version: string, service: string) => { + // Timestamp is written in nanoseconds; TimestampTime is a DEFAULT column. + const timestampNs = (Date.now() - agoMs) * 1_000_000; + return ( + `('${timestampNs}', '', '', 0, 'info', 0, ` + + `'${service}', 'release ${version}', '', ` + + `{'service.name':'${service}','service.version':'${version}'}, ` + + `'', '', '', {}, {})` + ); + }; + + const values = [ + row(OLD_VERSION_AGO_MS, OLD_VERSION, RELEASE_SERVICE), + row(NEW_VERSION_AGO_MS, NEW_VERSION, RELEASE_SERVICE), + row(OTHER_VERSION_AGO_MS, OTHER_VERSION, OTHER_SERVICE), + ].join(', '); + + await clickhouseQuery(` + INSERT INTO ${E2E_CLICKHOUSE_DATABASE}.${E2E_LOGS_TABLE} ( + Timestamp, TraceId, SpanId, TraceFlags, SeverityText, SeverityNumber, + ServiceName, Body, ResourceSchemaUrl, ResourceAttributes, ScopeSchemaUrl, + ScopeName, ScopeVersion, ScopeAttributes, LogAttributes + ) VALUES ${values} + `); +} + +test.describe('Release markers', { tag: ['@full-stack', '@dashboard'] }, () => { + let dashboardPage: DashboardPage; + + test.beforeEach(async ({ page }) => { + dashboardPage = new DashboardPage(page); + await dashboardPage.goto(); + }); + + test('overlays a labelled marker per release and clears them when toggled off', async ({ + page, + }) => { + await seedReleases(); + + await dashboardPage.createNewDashboard(); + await dashboardPage.addTileWithSource( + 'Release markers chart', + DEFAULT_LOGS_SOURCE_NAME, + ); + // Scope the chart to one service so its releases are attributable — see + // the suppression test below for why that matters. + await dashboardPage.setGlobalFilter(`ServiceName:"${RELEASE_SERVICE}"`); + + const markers = dashboardPage.getAnnotationMarkers(); + const labels = dashboardPage.getAnnotationLabels(); + await expect(markers).toHaveCount(0); + + await dashboardPage.toggleReleaseAnnotations(); + + // Ephemeral view state, carried in the URL so a shared link keeps it. + await expect(page).toHaveURL(/releaseMarkers=true/); + + // Both releases first appear inside the window and are far enough apart + // to stay individually labelled rather than collapsing. + await expect(markers).toHaveCount(2); + await expect(labels.filter({ hasText: OLD_VERSION })).toBeVisible(); + await expect(labels.filter({ hasText: NEW_VERSION })).toBeVisible(); + + // Regression: markers used to be read from the source globally, so a + // chart filtered to one service was still annotated with every other + // service's releases. + await expect(labels.filter({ hasText: OTHER_VERSION })).toHaveCount(0); + + await dashboardPage.toggleReleaseAnnotations(); + + await expect(markers).toHaveCount(0); + await expect(page).not.toHaveURL(/releaseMarkers=true/); + }); + + // 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, + // none are drawn at all. + test('suppresses markers it cannot attribute to the chart', async ({ + page, + }) => { + await seedReleases(); + + await dashboardPage.createNewDashboard(); + // No filter and no group by: one line covering both seeded services. + await dashboardPage.addTileWithSource( + 'Unattributable markers', + DEFAULT_LOGS_SOURCE_NAME, + ); + await dashboardPage.toggleReleaseAnnotations(); + + await expect(page).toHaveURL(/releaseMarkers=true/); + // The releases are found by the query — they just aren't drawn, because + // neither service has its own line here. + await expect(dashboardPage.getAnnotationMarkers()).toHaveCount(0); + }); +}); diff --git a/packages/app/tests/e2e/page-objects/DashboardPage.ts b/packages/app/tests/e2e/page-objects/DashboardPage.ts index d095839ead..c5f7182ee6 100644 --- a/packages/app/tests/e2e/page-objects/DashboardPage.ts +++ b/packages/app/tests/e2e/page-objects/DashboardPage.ts @@ -102,6 +102,7 @@ export class DashboardPage { private readonly removeDefaultQueryAndFiltersMenuItem: Locator; private readonly exportDashboardMenuItem: Locator; private readonly enterKioskModeMenuItem: Locator; + private readonly toggleReleaseAnnotationsMenuItem: Locator; private readonly exitKioskModeBtn: Locator; private readonly kioskHeaderContainer: Locator; private readonly kioskLiveStatusBadge: Locator; @@ -178,6 +179,9 @@ export class DashboardPage { this.enterKioskModeMenuItem = page.getByTestId( 'enter-kiosk-mode-menu-item', ); + this.toggleReleaseAnnotationsMenuItem = page.getByTestId( + 'toggle-release-annotations-menu-item', + ); this.exitKioskModeBtn = page.getByTestId('exit-kiosk-mode-button'); this.kioskHeaderContainer = page.getByTestId('kiosk-header'); this.kioskLiveStatusBadge = page.getByTestId('kiosk-live-status'); @@ -675,6 +679,10 @@ export class DashboardPage { */ async setGlobalFilter(filter: string) { await this.searchInput.fill(filter); + // Dismiss the suggestion popover, which otherwise overlays the submit + // button and fails its actionability check. Blur (not Escape) so it can't + // close a surrounding modal — same reasoning as `dismissSqlAutocomplete`. + await this.searchInput.blur(); await this.searchSubmitButton.click(); } @@ -1403,6 +1411,36 @@ export class DashboardPage { .click(); } + // ---- Chart annotation helpers ---- + + /** + * Open the dashboard overflow menu and toggle deployment markers on tile + * charts. Expects the menu item with + * data-testid="toggle-release-annotations-menu-item", which only renders once + * the dashboard has at least one tile. + */ + async toggleReleaseAnnotations() { + await this.dashboardMenuButton.click(); + await this.toggleReleaseAnnotationsMenuItem.click(); + } + + /** + * Annotation markers (deployments, alerts) drawn on a tile's time chart as + * dashed vertical Recharts reference lines. + */ + getAnnotationMarkers(tileIndex = 0): Locator { + return this.getTile(tileIndex).locator('.recharts-reference-line'); + } + + /** + * Text labels for the annotation markers. Recharts hoists reference-line + * labels into a separate z-index layer, so they are NOT descendants of the + * `.recharts-reference-line` group and cannot be matched by filtering it. + */ + getAnnotationLabels(tileIndex = 0): Locator { + return this.getTile(tileIndex).locator('text.recharts-label'); + } + // ---- Kiosk mode helpers ---- /** From 944eb6163bd8b613ed77102bef436e1a236a8684 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Thu, 13 Aug 2026 15:29:38 -0400 Subject: [PATCH 2/3] fix(dashboards): skip annotation queries on tiles that can't draw them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only DBTimeChart consumes the `annotations` prop, so a tile rendering as a table, number, pie, bar, heatmap or search discarded whatever the annotation hooks fetched. With release markers toggled on, that meant a `min(timestamp) ... GROUP BY version, service` aggregation against ClickHouse for every log- or trace-backed tile on the dashboard, non-time-series ones included — and on a dashboard of only such tiles, a spurious "No releases found" notification about markers that were never going to be drawn. Gate both annotation hooks on the tile's display type via the existing shared `isTimeSeriesDisplayType` predicate. The alert-history fetch had the same latent waste (cheaper, but a Number tile with an alert still queried history nothing rendered); gating both off one boolean keeps the two call sites consistent. --- packages/app/src/DBDashboardPage.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/app/src/DBDashboardPage.tsx b/packages/app/src/DBDashboardPage.tsx index ee7d2e3a59..9b253cabe8 100644 --- a/packages/app/src/DBDashboardPage.tsx +++ b/packages/app/src/DBDashboardPage.tsx @@ -33,6 +33,7 @@ import { displayTypeSupportsPromQLAlerts, displayTypeSupportsRawSqlAlerts, Granularity, + isTimeSeriesDisplayType, } from '@hyperdx/common-utils/dist/core/utils'; import { displayTypeRequiresSource, @@ -683,13 +684,20 @@ const Tile = forwardRef( return tooltip; }, [alert]); + // Only DBTimeChart draws annotations (see the render below) — a tile + // showing a table, number, pie, ... discards them. Both annotation queries + // stay idle for those rather than paying for a result nothing can show. + const tileCanDrawAnnotations = isTimeSeriesDisplayType( + chart.config.displayType, + ); + // 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 alertAnnotations = useAlertAnnotations( alert?.id, isFullscreen ? fullscreenDateRange : dateRange, - showAlertAnnotations, + showAlertAnnotations && tileCanDrawAnnotations, ); // Release markers, over the same visible window. Scoped to this tile: the @@ -698,7 +706,7 @@ const Tile = forwardRef( // releases. Tiles sharing a source and filters share one query. const releaseAnnotations = useReleaseAnnotations( isFullscreen ? fullscreenDateRange : dateRange, - showReleaseAnnotations, + showReleaseAnnotations && tileCanDrawAnnotations, { source, where: isBuilderSavedChartConfig(chart.config) From e30ecc3b346d4541b20819022e66349cd77a94a0 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Thu, 13 Aug 2026 16:45:45 -0400 Subject: [PATCH 3/3] fix(dashboards): scope release markers by the tile's series conditions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A builder time chart doesn't use the statement-level `where` — the editor clears it and each series carries its own `aggCondition` instead. Reading only `where` meant the release query was effectively unscoped for every time chart, which is the only display type that draws markers: exactly the case the scoping exists for. Confirmed against the shipped dashboard templates, where every time-series tile has `where: ''` and its filter in `aggCondition`. The visible symptom isn't just over-fetching. A tile filtered to one service would pull releases for every service in the source, so the markers span several groups, none of which matches a series on an ungrouped chart — `resolveAnnotationSeries` then drops all of them. The user filters to one service and sees no markers at all. Fold the series conditions into one predicate the same way renderChartConfig does: OR them, and only when every series is filtered (leave one unfiltered and the tile scans every row anyway). Identical conditions collapse rather than OR with themselves, and series in different condition languages can't be combined into one predicate, so those tiles stay unscoped rather than scoped wrongly. Only the folded condition keys the config memo, so a cosmetic series edit (color, number format) doesn't refire the query. The existing E2E spec missed this because it scopes via a dashboard-global filter, which travels through `filters` and was already applied. --- packages/app/src/DBDashboardPage.tsx | 17 +- .../__tests__/useReleaseAnnotations.test.tsx | 187 ++++++++++++++++++ .../app/src/hooks/useReleaseAnnotations.tsx | 92 ++++++++- 3 files changed, 283 insertions(+), 13 deletions(-) diff --git a/packages/app/src/DBDashboardPage.tsx b/packages/app/src/DBDashboardPage.tsx index 9b253cabe8..02ffe8a631 100644 --- a/packages/app/src/DBDashboardPage.tsx +++ b/packages/app/src/DBDashboardPage.tsx @@ -704,17 +704,22 @@ const Tile = forwardRef( // query runs against the tile's own source with the tile's own predicates, // so a chart filtered to one service isn't annotated with another's // releases. Tiles sharing a source and filters share one query. + // + // A time chart's filter lives in each series' `aggCondition`, not in the + // statement-level `where` (the editor clears that one), so `select` has to + // come along for the scoping to mean anything — `where` is carried for the + // configs that do set it, e.g. an imported dashboard. + const builderConfig = isBuilderSavedChartConfig(chart.config) + ? chart.config + : undefined; const releaseAnnotations = useReleaseAnnotations( isFullscreen ? fullscreenDateRange : dateRange, showReleaseAnnotations && tileCanDrawAnnotations, { source, - where: isBuilderSavedChartConfig(chart.config) - ? chart.config.where - : undefined, - whereLanguage: isBuilderSavedChartConfig(chart.config) - ? chart.config.whereLanguage - : undefined, + where: builderConfig?.where, + whereLanguage: builderConfig?.whereLanguage, + select: builderConfig?.select, filters, }, ); diff --git a/packages/app/src/hooks/__tests__/useReleaseAnnotations.test.tsx b/packages/app/src/hooks/__tests__/useReleaseAnnotations.test.tsx index 4717fd5570..ea77b461e9 100644 --- a/packages/app/src/hooks/__tests__/useReleaseAnnotations.test.tsx +++ b/packages/app/src/hooks/__tests__/useReleaseAnnotations.test.tsx @@ -1,5 +1,7 @@ import { BuilderChartConfigWithDateRange, + DerivedColumn, + SearchConditionLanguage, SourceKind, TLogSource, TMetricSource, @@ -9,6 +11,7 @@ import { import { renderHook } from '@testing-library/react'; import { + aggConditionScopeFilter, buildReleaseChartConfig, canDeriveReleases, DEFAULT_VERSION_EXPRESSION, @@ -295,6 +298,44 @@ describe('buildReleaseChartConfig', () => { ]); }); + it("carries the tile's folded series condition", () => { + const config = buildReleaseChartConfig( + logSource, + DEFAULT_VERSION_EXPRESSION, + range, + { + seriesCondition: { type: 'lucene', condition: 'ServiceName:"cart"' }, + }, + ); + + expect(config.filters).toEqual([ + { type: 'lucene', condition: 'ServiceName:"cart"' }, + ]); + }); + + // The tile ANDs its own where with its series conditions and the dashboard + // filters, so the releases query has to as well. + it('ANDs the where clause, the series condition and the dashboard filters', () => { + const config = buildReleaseChartConfig( + logSource, + DEFAULT_VERSION_EXPRESSION, + range, + { + where: 'Env:"prod"', + whereLanguage: 'lucene', + seriesCondition: { type: 'lucene', condition: 'ServiceName:"cart"' }, + filters: [{ type: 'sql', condition: "Region = 'us-east-1'" }], + }, + ); + + expect(config.filters).toEqual([ + { type: 'lucene', condition: 'Env:"prod"' }, + { type: 'lucene', condition: 'ServiceName:"cart"' }, + { type: 'sql', condition: "Region = 'us-east-1'" }, + ]); + expect(config.filtersLogicalOperator).toBeUndefined(); + }); + // Lucene bare terms need the source's implicit column to render. it('carries the implicit column expression for Lucene filters', () => { const config = buildReleaseChartConfig( @@ -308,6 +349,99 @@ describe('buildReleaseChartConfig', () => { }); }); +describe('aggConditionScopeFilter', () => { + /** A series carrying just the fields the fold reads. */ + const series = ( + aggCondition: string, + aggConditionLanguage?: SearchConditionLanguage, + ): DerivedColumn => ({ + aggFn: 'count', + valueExpression: '', + aggCondition, + aggConditionLanguage, + }); + + it("folds a single series' condition into a filter", () => { + expect( + aggConditionScopeFilter([series('ServiceName:"cart"', 'lucene')]), + ).toEqual({ type: 'lucene', condition: 'ServiceName:"cart"' }); + }); + + it('keeps a SQL series condition in SQL', () => { + expect( + aggConditionScopeFilter([series("ServiceName = 'cart'", 'sql')]), + ).toEqual({ type: 'sql', condition: "ServiceName = 'cart'" }); + }); + + // Matches how the per-series aggregate renders an unset language. + it('treats an unset language as Lucene', () => { + expect(aggConditionScopeFilter([series('ServiceName:"cart"')])).toEqual({ + type: 'lucene', + condition: 'ServiceName:"cart"', + }); + }); + + // renderChartConfig ORs the series conditions into the scan predicate, so the + // union is the set of rows the tile reads. + it('ORs several series conditions together', () => { + expect( + aggConditionScopeFilter([ + series('SeverityText:"error"', 'lucene'), + series('SeverityText:"warn"', 'lucene'), + ]), + ).toEqual({ + type: 'lucene', + condition: '(SeverityText:"error") OR (SeverityText:"warn")', + }); + }); + + // The common multi-series tile: several aggregates over one filter, as in the + // shipped browser-rum template. Collapse rather than OR the filter with itself. + it('collapses series that share one condition', () => { + expect( + aggConditionScopeFilter([ + series('SpanName:"documentLoad"', 'lucene'), + series('SpanName:"documentLoad"', 'lucene'), + series('SpanName:"documentLoad"', 'lucene'), + ]), + ).toEqual({ type: 'lucene', condition: 'SpanName:"documentLoad"' }); + }); + + // With one series unfiltered the tile scans every row, so there is nothing to + // narrow the releases query by. + it('yields nothing when any series is unfiltered', () => { + expect( + aggConditionScopeFilter([ + series('SeverityText:"error"', 'lucene'), + series(' ', 'lucene'), + ]), + ).toBeUndefined(); + expect(aggConditionScopeFilter([series('', 'lucene')])).toBeUndefined(); + }); + + // Conditions in different languages can't be combined into one predicate; + // going unscoped beats scoping wrongly. + it('yields nothing for series in mixed languages', () => { + expect( + aggConditionScopeFilter([ + series('SeverityText:"error"', 'lucene'), + series("SeverityText = 'warn'", 'sql'), + ]), + ).toBeUndefined(); + }); + + // `Filter` only speaks SQL and Lucene. + it('yields nothing for a PromQL series condition', () => { + expect(aggConditionScopeFilter([series('up', 'promql')])).toBeUndefined(); + }); + + it('yields nothing for a raw select string or an empty series list', () => { + expect(aggConditionScopeFilter('count() AS value')).toBeUndefined(); + expect(aggConditionScopeFilter([])).toBeUndefined(); + expect(aggConditionScopeFilter(undefined)).toBeUndefined(); + }); +}); + describe('releaseRowsToAnnotations', () => { const windowStart = new Date('2026-07-01T00:00:00.000Z'); const inside = '2026-07-01T00:30:00.000Z'; @@ -483,6 +617,59 @@ describe('useReleaseAnnotations', () => { ]); }); + // Regression: a time chart's filter lives in each series' `aggCondition`, not + // in the statement-level `where`, so reading only `where` left every time + // chart's markers unscoped. + it("scopes the query with the tile's per-series conditions", () => { + renderHook(() => + useReleaseAnnotations(range, true, { + source: logSource, + where: '', + select: [ + { + aggFn: 'count', + valueExpression: '', + aggCondition: 'ServiceName:"checkout"', + aggConditionLanguage: 'lucene', + }, + ], + }), + ); + + expect(lastCall()[0].filters).toEqual([ + { type: 'lucene', condition: 'ServiceName:"checkout"' }, + ]); + }); + + // Only the folded condition keys the memo, so restyling a series must not + // rebuild the config and refire the query. + it('reuses one config object when a cosmetic series field changes', () => { + type SeriesColor = DerivedColumn['color']; + const select = (color: SeriesColor): DerivedColumn[] => [ + { + aggFn: 'count', + valueExpression: '', + aggCondition: 'ServiceName:"checkout"', + aggConditionLanguage: 'lucene', + color, + }, + ]; + + const { result, rerender } = renderHook( + ({ color }: { color: SeriesColor }) => + useReleaseAnnotations(range, true, { + source: logSource, + select: select(color), + }), + { initialProps: { color: 'chart-blue' as SeriesColor } }, + ); + const first = lastCall()[0]; + rerender({ color: 'chart-red' }); + + expect(lastCall()[0]).toBe(first); + expect(result.current).toBeUndefined(); + }); + it('widens the query range backwards to spot the already-running version', () => { renderHook(() => useReleaseAnnotations(range, true, { source: logSource })); diff --git a/packages/app/src/hooks/useReleaseAnnotations.tsx b/packages/app/src/hooks/useReleaseAnnotations.tsx index d51aa096c9..3f885a6b43 100644 --- a/packages/app/src/hooks/useReleaseAnnotations.tsx +++ b/packages/app/src/hooks/useReleaseAnnotations.tsx @@ -3,6 +3,7 @@ import { BuilderChartConfigWithDateRange, Filter, SearchConditionLanguage, + SelectList, SourceKind, TSource, } from '@hyperdx/common-utils/dist/types'; @@ -48,9 +49,78 @@ export type ReleaseRow = { type ReleaseScope = { where?: string; whereLanguage?: SearchConditionLanguage; + /** The tile's series conditions, already folded by `aggConditionScopeFilter`. */ + seriesCondition?: Filter; filters?: Filter[]; }; +/** What the hook reads off the tile to build that scope. */ +type ReleaseTileScope = Omit & { + /** + * The tile's series. A time chart keeps its filter in each series' + * `aggCondition` rather than in `where` — see `aggConditionScopeFilter`. + */ + select?: SelectList; +}; + +/** + * The predicate describing which rows a tile's chart reads, taken from its + * series. + * + * A builder time chart does not use the statement-level `where` — the editor + * clears it and each series carries its own `aggCondition` instead, applied + * inside that series' aggregate. `renderChartConfig` additionally pre-filters + * the scan with the OR of those conditions, and only when *every* series has + * one: leave any series unfiltered and the tile reads every row anyway. That + * union is exactly "the rows this tile is about", so mirror the same rule here + * rather than letting the releases query span data the chart never saw. + * + * Series each choose their own condition language. Conditions in different + * languages can't be combined into one predicate, so those tiles go unscoped + * rather than scoped wrongly. + */ +export function aggConditionScopeFilter( + select: SelectList | undefined, +): Filter | undefined { + // A raw select string carries no per-series conditions to read. + if (!Array.isArray(select) || select.length === 0) { + return undefined; + } + + // Deduped: series sharing one filter (the usual multi-series tile) should + // collapse to that filter rather than OR it with itself. + const conditions = new Set(); + const languages = new Set(); + for (const series of select) { + const condition = series.aggCondition?.trim(); + if (!condition) { + return undefined; + } + conditions.add(condition); + // Undefined matches how the per-series aggregate renders it. + languages.add(series.aggConditionLanguage ?? 'lucene'); + } + + if (languages.size !== 1) { + return undefined; + } + const [language] = languages; + // `Filter` only speaks SQL and Lucene. PromQL series are a separate config + // type that never reaches here, but bail rather than mislabel one. + if (language !== undefined && language !== 'sql' && language !== 'lucene') { + return undefined; + } + + const unique = [...conditions]; + return { + type: language === 'sql' ? 'sql' : 'lucene', + condition: + unique.length === 1 + ? unique[0] + : unique.map(condition => `(${condition})`).join(' OR '), + }; +} + /** * Whether releases can be derived from this source. * @@ -113,9 +183,11 @@ export function buildReleaseChartConfig( ? source.serviceNameExpression : undefined; - // The tile's own `where` and the dashboard filters both go through `filters`, - // which carries a language per entry — the config's own `where` is reserved - // for the SQL version predicate below. + // The tile's own predicates and the dashboard filters all go through + // `filters`, which carries a language per entry — the config's own `where` is + // reserved for the SQL version predicate below. Entries are ANDed, matching + // how the tile's query combines its `where`, its series conditions and the + // dashboard filters. const scopeFilters: Filter[] = [ ...(scope.where?.trim() ? [ @@ -125,6 +197,7 @@ export function buildReleaseChartConfig( } as Filter, ] : []), + ...(scope.seriesCondition ? [scope.seriesCondition] : []), ...(scope.filters ?? []).filter( filter => !('condition' in filter) || filter.condition?.trim(), ), @@ -234,7 +307,8 @@ export function releaseRowsToAnnotations( * `service.version` resource attribute. * * Scoped to the tile: the query runs against the tile's own source with the - * tile's own filters, so the markers describe the data the chart is showing. An + * tile's own predicates — its `where`, its per-series conditions and the + * dashboard filters — so the markers describe the data the chart is showing. An * unfiltered tile spanning every service therefore does show every service's * releases — that is consistent, not noise. * @@ -245,7 +319,7 @@ export function releaseRowsToAnnotations( export function useReleaseAnnotations( dateRange: [Date, Date], enabled: boolean = false, - options?: ReleaseScope & { + options?: ReleaseTileScope & { source?: TSource; /** Overrides the source's own expression. Mainly a testing seam. */ versionExpression?: string; @@ -264,12 +338,16 @@ export function useReleaseAnnotations( const windowEndMs = Math.ceil(dateRange[1].getTime() / BUCKET_MS) * BUCKET_MS; // Callers rebuild the filter array every render, so key the memo on its - // content. Tiles sharing a source and filters then share one query. + // content. Tiles sharing a source and filters then share one query. Only the + // *folded* series condition goes in, not the raw series list — its display-only + // fields (number format, colors) would otherwise refire the query on a purely + // cosmetic tile edit. const scopeKey = JSON.stringify({ where: options?.where, whereLanguage: options?.whereLanguage, + seriesCondition: aggConditionScopeFilter(options?.select), filters: options?.filters, - }); + } satisfies ReleaseScope); const config = useMemo(() => { if (!isSupported || !source) {