Repository navigation
feat(app): add a duration heatmap view to trace search #3184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: alex/red-metrics-trace-search-v2
Are you sure you want to change the base?
Changes from all commits
a690c51
7de8c84
76e4dca
3c58c91
2d62337
8bb3d02
46b3d9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@hyperdx/app': patch | ||
| --- | ||
|
|
||
| Count spans recorded with the legacy `STATUS_CODE_ERROR` status as errors on the Services and LLM dashboards. The errored-span definition now lives in one place and matches both that value and the short `Error` form, so those dashboards no longer undercount errors from collectors that emit the long form. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@hyperdx/app': minor | ||
| --- | ||
|
|
||
| Show RED metrics (Throughput, Errors, Duration) above the trace search results instead of the single count histogram. The three charts share a synced hover cursor, Errors toggles between rate and volume, and clicking a status series on the volume chart filters the results. Logs and other sources keep the existing histogram. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@hyperdx/app': minor | ||
| --- | ||
|
|
||
| Add a RED/Heatmap switch to the trace search results view. Heatmap shows the duration distribution over time as the same tile the dashboard renders, and the choice is kept in the URL so a reload or a shared link reopens the same view. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,6 +60,7 @@ import { | |
| Group, | ||
| Modal, | ||
| Paper, | ||
| SegmentedControl, | ||
| Select, | ||
| Stack, | ||
| Text, | ||
|
|
@@ -140,6 +141,10 @@ import DBSqlRowTableWithSideBar from './components/DBSqlRowTableWithSidebar'; | |
| import PatternTable from './components/PatternTable'; | ||
| import { DBSearchHeatmapChart } from './components/Search/DBSearchHeatmapChart'; | ||
| import DirectTraceSidePanel from './components/Search/DirectTraceSidePanel'; | ||
| import { | ||
| type TraceChartMode, | ||
| TraceRedMetricsChart, | ||
| } from './components/Search/TraceRedMetricsChart'; | ||
| import SourceSchemaPreview, { | ||
| isSourceSchemaPreviewEnabled, | ||
| } from './components/SourceSchemaPreview'; | ||
|
|
@@ -206,6 +211,13 @@ const SearchConfigSchema = z.object({ | |
| type SearchConfigFromSchema = z.infer<typeof SearchConfigSchema>; | ||
|
|
||
| const QUERY_KEY_PREFIX = 'search'; | ||
| // The RED tiles are auxiliary viz, not the results query, so they key their | ||
| // queries under a separate prefix to stay out of the `search executed` | ||
| // telemetry (which matches [QUERY_KEY_PREFIX] only). Otherwise the slow | ||
| // avg/p95/p99 duration query would become what the reported search latency | ||
| // measures. The live-tail pause, on the other hand, does wait on this prefix | ||
| // (see the pause signal below) so live ticks don't stack fresh quantile scans. | ||
| const RED_QUERY_KEY_PREFIX = `${QUERY_KEY_PREFIX}-red`; | ||
|
|
||
| // Clicks inside the results panel keep the row side panel open (so users can | ||
| // scroll the table or select a different row); clicks anywhere else on the page | ||
|
|
@@ -1061,6 +1073,14 @@ export function DBSearchPage() { | |
| ]).withDefault('results'), | ||
| ); | ||
|
|
||
| // RED metrics vs heatmap for the trace results chart area. URL state (like | ||
| // the other view toggles on this page) so a reload or shared link keeps the | ||
| // chosen view; the switch lives inline in the search stats row. | ||
| const [traceChartMode, setTraceChartMode] = useQueryState( | ||
| 'traceChartMode', | ||
| parseAsStringEnum<TraceChartMode>(['red', 'heatmap']).withDefault('red'), | ||
| ); | ||
|
|
||
| const [patternColumn, setPatternColumn] = useQueryState( | ||
| 'patternColumn', | ||
| parseAsString, | ||
|
|
@@ -1611,6 +1631,16 @@ export function DBSearchPage() { | |
| queryKey: [QUERY_KEY_PREFIX], | ||
| }) > 0; | ||
|
|
||
| // Live tail also waits on the RED metric queries. They live under a separate | ||
| // prefix (kept out of the search-latency telemetry above), so without this | ||
| // each ~10s tick would fire a fresh avg/p95/p99 scan without waiting for the | ||
| // previous one, stacking slow quantile queries while live tail runs. Resolves | ||
| // to 0 when the RED tiles aren't mounted, so it's a no-op off the trace view. | ||
| const isRedQueryFetching = | ||
| useIsFetching({ | ||
| queryKey: [RED_QUERY_KEY_PREFIX], | ||
| }) > 0; | ||
|
|
||
| const { searchElapsedMs } = useSearchTelemetry({ | ||
| isAnyQueryFetching, | ||
| isLive: isLive ?? false, | ||
|
|
@@ -1665,7 +1695,8 @@ export function DBSearchPage() { | |
| interval, | ||
| refreshFrequency, | ||
| onTimeRangeSelect, | ||
| pause: isAnyQueryFetching || !queryReady || !isTabVisible, | ||
| pause: | ||
| isAnyQueryFetching || isRedQueryFetching || !queryReady || !isTabVisible, | ||
| }); | ||
|
|
||
| // This ensures we only render this conditionally on the client | ||
|
|
@@ -1864,6 +1895,53 @@ export function DBSearchPage() { | |
|
|
||
| const aliasWith = useMemo(() => aliasMapToWithClauses(aliasMap), [aliasMap]); | ||
|
|
||
| // The trace results chart shows RED metrics (and a heatmap) only for a trace | ||
| // source that exposes a duration column; otherwise the single histogram | ||
| // stays. Derived once, as the narrowed source or null, so the stats-row | ||
| // toggle and the chart branch can't drift apart on which one renders. | ||
| const traceRedMetricsSource = | ||
| searchedSource != null && | ||
| isTraceSource(searchedSource) && | ||
| searchedSource.durationExpression | ||
| ? searchedSource | ||
| : null; | ||
|
|
||
| // Reset to the default RED view when the source actually changes, so | ||
| // returning to a trace source after viewing another one doesn't silently | ||
| // reopen in Heatmap. The ref starts undefined and is only compared once a | ||
| // real id has been seen, so the async source resolution on first load | ||
| // (undefined -> id) doesn't count as a change and a shared link that pins | ||
| // ?traceChartMode=heatmap survives. | ||
| const prevSourceIdRef = useRef<string | undefined>(undefined); | ||
| useEffect(() => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — Reset effect discards the user's Heatmap choice even when the new source can render it The effect fires on any Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond |
||
| if ( | ||
| prevSourceIdRef.current !== undefined && | ||
| prevSourceIdRef.current !== searchedSource?.id | ||
| ) { | ||
| setTraceChartMode('red'); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — Every source switch adds The app uses nuqs 1.17.0, where Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond |
||
| } | ||
| prevSourceIdRef.current = searchedSource?.id; | ||
| }, [searchedSource?.id, setTraceChartMode]); | ||
|
|
||
| // Pin live tail off while the heatmap is the visible chart. A live tick | ||
| // moves the absolute time range, and the heatmap keys its uPlot instance on | ||
| // the chart config, so every refresh would tear the chart down and rebuild | ||
| // it with a recomputed y-axis instead of updating in place. Delta and | ||
| // pattern modes already pin it off (see the analysisMode effect above), so | ||
| // the delta-view heatmap never hit this; results mode defaults to live, so | ||
| // the view has to opt out for itself. Only pinned while the heatmap is | ||
| // actually rendered, so a stale ?traceChartMode=heatmap on a log source | ||
| // can't disable live tail for a search that shows the histogram. | ||
| const isHeatmapChartVisible = | ||
| analysisMode === 'results' && | ||
| traceRedMetricsSource != null && | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 major — Resume live tail button undoes the heatmap's live-tail pin The effect only calls Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — Switching back to RED leaves live tail off The pin writes Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond |
||
| traceChartMode === 'heatmap'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 major — Resume Live Tail gets around the heatmap live-tail pin The pin effect only runs when Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond |
||
| useEffect(() => { | ||
| if (isHeatmapChartVisible) { | ||
| setIsLive(false); | ||
| } | ||
| }, [isHeatmapChartVisible, setIsLive]); | ||
|
|
||
| const histogramTimeChartConfig = useMemo(() => { | ||
| if (chartConfig == null) { | ||
| return undefined; | ||
|
|
@@ -2491,11 +2569,7 @@ export function DBSearchPage() { | |
| setAnalysisMode={setAnalysisMode} | ||
| chartConfig={filtersChartConfig} | ||
| sourceId={inputSourceObj?.id} | ||
| showDelta={ | ||
| !!(searchedSource?.kind === SourceKind.Trace | ||
| ? searchedSource.durationExpression | ||
| : undefined) | ||
| } | ||
| showDelta={traceRedMetricsSource != null} | ||
| onColumnToggle={toggleColumn} | ||
| displayedColumns={displayedColumns} | ||
| onCollapse={() => setIsFilterSidebarCollapsed(true)} | ||
|
|
@@ -2612,6 +2686,21 @@ export function DBSearchPage() { | |
| enableParallelQueries | ||
| /> | ||
| <Group gap="sm" align="center"> | ||
| {traceRedMetricsSource != null && ( | ||
| <SegmentedControl | ||
| size="xs" | ||
| value={traceChartMode} | ||
| onChange={v => | ||
| setTraceChartMode( | ||
| v === 'heatmap' ? 'heatmap' : 'red', | ||
| ) | ||
| } | ||
| data={[ | ||
| { label: 'RED', value: 'red' }, | ||
| { label: 'Heatmap', value: 'heatmap' }, | ||
| ]} | ||
| /> | ||
| )} | ||
| {shouldShowLiveModeHint && | ||
| denoiseResults != true && ( | ||
| <ResumeLiveTailButton | ||
|
|
@@ -2632,26 +2721,50 @@ export function DBSearchPage() { | |
| </Group> | ||
| </Group> | ||
| </Box> | ||
| {!hasQueryError && ( | ||
| <Box | ||
| className={searchPageStyles.timeChartContainer} | ||
| mih="0" | ||
| > | ||
| <DBTimeChart | ||
| sourceId={searchedConfig.source ?? undefined} | ||
| showLegend={false} | ||
| config={histogramTimeChartConfig} | ||
| enabled={isReady} | ||
| showDisplaySwitcher={false} | ||
| showMVOptimizationIndicator={false} | ||
| showDateRangeIndicator={false} | ||
| queryKeyPrefix={QUERY_KEY_PREFIX} | ||
| onTimeRangeSelect={handleTimeRangeSelect} | ||
| onFocusSeries={handleFocusSeries} | ||
| enableParallelQueries | ||
| /> | ||
| </Box> | ||
| )} | ||
| {!hasQueryError && | ||
| (traceRedMetricsSource != null ? ( | ||
| <Box | ||
| className={searchPageStyles.timeChartContainer} | ||
| mih="0" | ||
| h={240} | ||
| > | ||
| <TraceRedMetricsChart | ||
| mode={traceChartMode} | ||
| histogramTimeChartConfig={ | ||
| histogramTimeChartConfig | ||
| } | ||
| heatmapChartConfig={{ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — The inline Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — The inline
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — Inline
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond |
||
| ...chartConfig, | ||
| dateRange: searchedTimeRange, | ||
| with: aliasWith, | ||
| }} | ||
| source={traceRedMetricsSource} | ||
| isReady={isReady} | ||
| queryKeyPrefix={RED_QUERY_KEY_PREFIX} | ||
| onTimeRangeSelect={handleTimeRangeSelect} | ||
| onFocusSeries={handleFocusSeries} | ||
| /> | ||
| </Box> | ||
| ) : ( | ||
| <Box | ||
| className={searchPageStyles.timeChartContainer} | ||
| mih="0" | ||
| > | ||
| <DBTimeChart | ||
| sourceId={searchedConfig.source ?? undefined} | ||
| showLegend={false} | ||
| config={histogramTimeChartConfig} | ||
| enabled={isReady} | ||
| showDisplaySwitcher={false} | ||
| showMVOptimizationIndicator={false} | ||
| showDateRangeIndicator={false} | ||
| queryKeyPrefix={QUERY_KEY_PREFIX} | ||
| onTimeRangeSelect={handleTimeRangeSelect} | ||
| onFocusSeries={handleFocusSeries} | ||
| enableParallelQueries | ||
| /> | ||
| </Box> | ||
| ))} | ||
| </> | ||
| )} | ||
| {hasQueryError && queryError ? ( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔵 minor — New mode-switch and reset logic has no tests
No test renders
TraceRedMetricsChartin either mode. None checks that the heatmap queries carryqueryKeyPrefix, which is what lets live tail wait on them, and none covers the source-change reset effect (a first-load undefined→id transition must keepheatmap; a change from one id to another must reset). Add component tests, for example next tocomponents/Search/__tests__/traceRedMetrics.test.ts, that assert the rendered chart and query keys for each mode, plus a hook-level test for the reset effect.Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond