From 4ef63cc434b1957a2b8ac1f9da8554ac169db3fe Mon Sep 17 00:00:00 2001 From: Mike Shi Date: Fri, 10 Apr 2026 04:51:43 -0700 Subject: [PATCH] feat: Auto-detect trace duration and render in adaptive time units (HDX-3909) (#2046) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary When a chart uses a trace source with a Duration Expression, the chart now automatically defaults to adaptive time unit formatting (e.g., `120.41s`, `45ms`, `3µs`) instead of requiring users to manually select a format. Users can still override the format through the existing display settings. **Key changes:** 1. **New `duration` output type** in `NumberFormatSchema` — renders values adaptively as `µs`, `ms`, `s`, `min`, or `h` based on magnitude, instead of the clock-style `hh:mm:ss` format 2. **Auto-detection via exact match** — `getTraceDurationNumberFormat()` checks if any chart select `valueExpression` exactly equals the trace source's `durationExpression`. Only applies for unit-preserving aggregate functions (`avg`, `min`, `max`, `sum`, `quantile`, `any`, `last_value`, etc.) — skips `count` and `count_distinct` 3. **`useResolvedNumberFormat()` hook** — resolves the effective `numberFormat` for a chart: returns the user's explicit format if set, otherwise auto-detects duration format for trace sources 4. **UI form update** — Added "Duration" option to the number format selector with input unit picker (seconds/ms/µs/ns) 5. **Display settings drawer** — Shows the auto-detected format by default so users can see what's being applied 6. **Heatmap support** — Updated `DBHeatmapChart` tick formatter to use `formatDurationMs` for duration-formatted values **Components updated:** `DBTimeChart`, `DBNumberChart`, `DBListBarChart`, `DBPieChart`, `DBTableChart`, `DBHeatmapChart`, `DBSearchHeatmapChart`, `DBEditTimeChartForm`, `ChartDisplaySettingsDrawer` ### How to test locally or on Vercel 1. Create a chart from a trace source with a unit-preserving aggFn (e.g., avg/p95/p99/min/max of the Duration column) 2. Verify the chart y-axis and tooltips now show values like `120.41s` or `45ms` instead of raw numbers 3. Open the chart display settings and verify the "Duration" output format is shown as the default 4. Change the aggFn to `count` or `count_distinct` — verify duration formatting is NOT applied 5. Change the format to something else (e.g., "Number") and verify the override persists 6. Switch back to "Duration" and pick different input units (seconds, ms, µs, ns) — the preview should update correctly 7. Check that non-trace-source charts are unaffected (no auto-detection triggers) 8. Verify the search heatmap chart for traces still shows proper duration labels 9. Reset to defaults in the display settings drawer and verify it returns to the auto-detected duration format ### References - Linear Issue: HDX-3909 Linear Issue: [HDX-3909](https://linear.app/clickhouse/issue/HDX-3909/trace-duration-should-render-in-time-unit-by-default)
Open in Web Open in Cursor 
Co-authored-by: Cursor Agent <199161495+cursoragent@users.noreply.github.com> --- .../__tests__/DBSearchPageQueryKey.test.tsx | 1 + packages/app/src/__tests__/source.test.ts | 201 +++++++++++++++--- packages/app/src/__tests__/utils.test.ts | 106 +++++++++ .../components/ChartDisplaySettingsDrawer.tsx | 47 ++-- .../DBEditTimeChartForm/EditTimeChartForm.tsx | 12 +- .../__tests__/DBEditTimeChartForm.test.tsx | 1 + .../app/src/components/DBHeatmapChart.tsx | 26 +-- .../app/src/components/DBListBarChart.tsx | 8 +- packages/app/src/components/DBNumberChart.tsx | 6 +- packages/app/src/components/DBPieChart.tsx | 8 +- packages/app/src/components/DBTableChart.tsx | 8 +- packages/app/src/components/DBTimeChart.tsx | 6 +- packages/app/src/components/NumberFormat.tsx | 9 +- .../Search/DBSearchHeatmapChart.tsx | 7 +- .../__tests__/DBListBarChart.test.tsx | 1 + .../__tests__/DBNumberChart.test.tsx | 3 + .../components/__tests__/DBPieChart.test.tsx | 1 + .../__tests__/DBTableChart.test.tsx | 1 + .../components/__tests__/DBTimeChart.test.tsx | 1 + packages/app/src/source.ts | 94 +++++++- packages/app/src/utils.ts | 49 +++++ packages/common-utils/src/types.ts | 1 + 22 files changed, 519 insertions(+), 78 deletions(-) diff --git a/packages/app/src/__tests__/DBSearchPageQueryKey.test.tsx b/packages/app/src/__tests__/DBSearchPageQueryKey.test.tsx index c0b6309a..6b1b47ae 100644 --- a/packages/app/src/__tests__/DBSearchPageQueryKey.test.tsx +++ b/packages/app/src/__tests__/DBSearchPageQueryKey.test.tsx @@ -39,6 +39,7 @@ jest.mock('@/hooks/useChartConfig', () => ({ jest.mock('@/source', () => ({ useSource: () => ({ data: null, isLoading: false }), + useResolvedNumberFormat: () => undefined, })); jest.mock('@/ChartUtils', () => ({ diff --git a/packages/app/src/__tests__/source.test.ts b/packages/app/src/__tests__/source.test.ts index dccf3475..73ccd8b0 100644 --- a/packages/app/src/__tests__/source.test.ts +++ b/packages/app/src/__tests__/source.test.ts @@ -1,31 +1,184 @@ -import { SourceKind, TTraceSource } from '@hyperdx/common-utils/dist/types'; +import { + SourceKind, + TLogSource, + TTraceSource, +} from '@hyperdx/common-utils/dist/types'; -import { getEventBody } from '../source'; +import { getEventBody, getTraceDurationNumberFormat } from '../source'; + +const TRACE_SOURCE: TTraceSource = { + kind: SourceKind.Trace, + from: { + databaseName: 'default', + tableName: 'otel_traces', + }, + timestampValueExpression: 'Timestamp', + connection: 'test-connection', + name: 'Traces', + id: 'test-source-id', + spanNameExpression: 'SpanName', + durationExpression: 'Duration', + durationPrecision: 9, + traceIdExpression: 'TraceId', + spanIdExpression: 'SpanId', + parentSpanIdExpression: 'ParentSpanId', + spanKindExpression: 'SpanKind', + defaultTableSelectExpression: 'Timestamp, ServiceName', +} as TTraceSource; describe('getEventBody', () => { - // Added to prevent regression back to HDX-3361 it('returns spanNameExpression for trace kind source when both bodyExpression and spanNameExpression are present', () => { - const source = { - kind: SourceKind.Trace, - from: { - databaseName: 'default', - tableName: 'otel_traces', - }, - timestampValueExpression: 'Timestamp', - connection: 'test-connection', - name: 'Traces', - id: 'test-source-id', - spanNameExpression: 'SpanName', - durationExpression: 'Duration', - durationPrecision: 9, - traceIdExpression: 'TraceId', - spanIdExpression: 'SpanId', - parentSpanIdExpression: 'ParentSpanId', - spanKindExpression: 'SpanKind', - } as TTraceSource; - - const result = getEventBody(source); - + const result = getEventBody(TRACE_SOURCE); expect(result).toBe('SpanName'); }); }); + +describe('getTraceDurationNumberFormat', () => { + it('returns undefined for non-trace sources', () => { + const logSource = { + kind: SourceKind.Log, + id: 'log-source', + } as TLogSource; + const result = getTraceDurationNumberFormat(logSource, [ + { valueExpression: 'count()' }, + ]); + expect(result).toBeUndefined(); + }); + + it('returns undefined when source is undefined', () => { + const result = getTraceDurationNumberFormat(undefined, [ + { valueExpression: 'count()' }, + ]); + expect(result).toBeUndefined(); + }); + + it('returns undefined when select expressions do not reference duration', () => { + const result = getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'count()' }, + ]); + expect(result).toBeUndefined(); + }); + + // --- exact match --- + + it('matches when valueExpression exactly equals durationExpression', () => { + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'Duration', aggFn: 'avg' }, + ]), + ).toEqual({ output: 'duration', factor: 1e-9 }); + }); + + it('matches without aggFn (raw expression passed through)', () => { + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'Duration' }, + ]), + ).toEqual({ output: 'duration', factor: 1e-9 }); + }); + + // --- non-matching expressions --- + + it('does not match expressions that only contain the duration name', () => { + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'avg(Duration)' }, + ]), + ).toBeUndefined(); + }); + + it('does not match division expressions', () => { + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'Duration/1e6' }, + ]), + ).toBeUndefined(); + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: '(Duration)/1e6' }, + ]), + ).toBeUndefined(); + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'Duration / 1e9' }, + ]), + ).toBeUndefined(); + }); + + it('does not match modified or similar-named expressions', () => { + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'Duration * 2' }, + ]), + ).toBeUndefined(); + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'LongerDuration' }, + ]), + ).toBeUndefined(); + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'round(Duration / 1e6, 2)' }, + ]), + ).toBeUndefined(); + }); + + // --- aggFn filtering --- + + it('returns undefined for count aggFn', () => { + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'Duration', aggFn: 'count' }, + ]), + ).toBeUndefined(); + }); + + it('returns undefined for count_distinct aggFn', () => { + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'Duration', aggFn: 'count_distinct' }, + ]), + ).toBeUndefined(); + }); + + it.each(['sum', 'min', 'max', 'quantile', 'avg', 'any', 'last_value'])( + 'detects duration with %s aggFn', + aggFn => { + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'Duration', aggFn }, + ]), + ).toEqual({ output: 'duration', factor: 1e-9 }); + }, + ); + + it('detects duration with combinator aggFn like avgIf', () => { + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'Duration', aggFn: 'avgIf' }, + ]), + ).toEqual({ output: 'duration', factor: 1e-9 }); + }); + + it('skips non-preserving aggFn and detects preserving one in mixed selects', () => { + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'Duration', aggFn: 'count' }, + { valueExpression: 'Duration', aggFn: 'avg' }, + ]), + ).toEqual({ output: 'duration', factor: 1e-9 }); + }); + + it('returns undefined when only non-preserving aggFns reference duration', () => { + expect( + getTraceDurationNumberFormat(TRACE_SOURCE, [ + { valueExpression: 'Duration', aggFn: 'count' }, + { valueExpression: 'Duration', aggFn: 'count_distinct' }, + ]), + ).toBeUndefined(); + }); + + it('returns undefined when select is empty', () => { + expect(getTraceDurationNumberFormat(TRACE_SOURCE, [])).toBeUndefined(); + }); +}); diff --git a/packages/app/src/__tests__/utils.test.ts b/packages/app/src/__tests__/utils.test.ts index 26ac4277..5f9e3344 100644 --- a/packages/app/src/__tests__/utils.test.ts +++ b/packages/app/src/__tests__/utils.test.ts @@ -6,6 +6,7 @@ import { MetricsDataType, NumberFormat } from '../types'; import * as utils from '../utils'; import { formatAttributeClause, + formatDurationMs, formatNumber, getAllMetricTables, getMetricTableName, @@ -357,6 +358,68 @@ describe('formatNumber', () => { }); }); + describe('duration format', () => { + it('formats seconds input as adaptive duration', () => { + const format: NumberFormat = { + output: 'duration', + factor: 1, + }; + expect(formatNumber(30.41, format)).toBe('30.41s'); + expect(formatNumber(0.045, format)).toBe('45ms'); + expect(formatNumber(3661, format)).toBe('1.02h'); + }); + + it('formats milliseconds input as adaptive duration', () => { + const format: NumberFormat = { + output: 'duration', + factor: 0.001, + }; + expect(formatNumber(30410, format)).toBe('30.41s'); + expect(formatNumber(45, format)).toBe('45ms'); + }); + + it('formats nanoseconds input as adaptive duration', () => { + const format: NumberFormat = { + output: 'duration', + factor: 0.000000001, + }; + expect(formatNumber(30410000000, format)).toBe('30.41s'); + expect(formatNumber(45000000, format)).toBe('45ms'); + expect(formatNumber(500, format)).toBe('0.5µs'); + }); + + it('handles zero value', () => { + const format: NumberFormat = { + output: 'duration', + factor: 1, + }; + expect(formatNumber(0, format)).toBe('0ms'); + }); + + it('defaults factor to 1 (seconds) when not specified', () => { + const format: NumberFormat = { + output: 'duration', + }; + expect(formatNumber(1.5, format)).toBe('1.5s'); + }); + + it('formats sub-millisecond values as microseconds', () => { + const format: NumberFormat = { + output: 'duration', + factor: 1, + }; + expect(formatNumber(0.0003, format)).toBe('300µs'); + }); + + it('formats large values as hours', () => { + const format: NumberFormat = { + output: 'duration', + factor: 1, + }; + expect(formatNumber(7200, format)).toBe('2h'); + }); + }); + describe('unit handling', () => { it('appends unit to formatted number', () => { const format: NumberFormat = { @@ -596,6 +659,49 @@ describe('formatNumber', () => { }); }); +describe('formatDurationMs', () => { + it('formats zero', () => { + expect(formatDurationMs(0)).toBe('0ms'); + }); + + it('formats microseconds', () => { + expect(formatDurationMs(0.5)).toBe('500µs'); + expect(formatDurationMs(0.003)).toBe('3µs'); + expect(formatDurationMs(0.01)).toBe('10µs'); + }); + + it('formats milliseconds', () => { + expect(formatDurationMs(1)).toBe('1ms'); + expect(formatDurationMs(45)).toBe('45ms'); + expect(formatDurationMs(999)).toBe('999ms'); + expect(formatDurationMs(5.5)).toBe('5.5ms'); + }); + + it('formats seconds', () => { + expect(formatDurationMs(1000)).toBe('1s'); + expect(formatDurationMs(1500)).toBe('1.5s'); + expect(formatDurationMs(30410)).toBe('30.41s'); + }); + + it('formats minutes', () => { + expect(formatDurationMs(60000)).toBe('1min'); + expect(formatDurationMs(90000)).toBe('1.5min'); + }); + + it('formats hours', () => { + expect(formatDurationMs(3600000)).toBe('1h'); + expect(formatDurationMs(7200000)).toBe('2h'); + }); + + it('handles negative values', () => { + expect(formatDurationMs(-1500)).toBe('-1.5s'); + }); + + it('handles sub-microsecond precision', () => { + expect(formatDurationMs(0.0005)).toBe('0.5µs'); + }); +}); + describe('useLocalStorage', () => { // Create a mock for localStorage let localStorageMock: jest.Mocked; diff --git a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx index 3614740c..ff0ba5e0 100644 --- a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx +++ b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx @@ -1,8 +1,9 @@ -import { useCallback } from 'react'; +import { useCallback, useEffect, useMemo } from 'react'; import { useForm, useWatch } from 'react-hook-form'; import { ChartConfigWithDateRange, DisplayType, + NumberFormat, } from '@hyperdx/common-utils/dist/types'; import { Box, @@ -30,24 +31,28 @@ export type ChartConfigDisplaySettings = Pick< interface ChartDisplaySettingsDrawerProps { opened: boolean; settings: ChartConfigDisplaySettings; + /** Auto-detected number format (e.g. duration for trace sources). + * Used as the default when no explicit numberFormat is set. */ + defaultNumberFormat?: NumberFormat; displayType: DisplayType; previousDateRange?: [Date, Date]; onChange: (settings: ChartConfigDisplaySettings) => void; onClose: () => void; } -function applyDefaultSettings({ - numberFormat, - alignDateRangeToGranularity, - compareToPreviousPeriod, - fillNulls, -}: ChartConfigDisplaySettings): ChartConfigDisplaySettings { +function applyDefaultSettings( + settings: ChartConfigDisplaySettings, + fallbackNumberFormat?: NumberFormat, +): ChartConfigDisplaySettings { return { - numberFormat: numberFormat ?? DEFAULT_NUMBER_FORMAT, + numberFormat: + settings.numberFormat ?? fallbackNumberFormat ?? DEFAULT_NUMBER_FORMAT, alignDateRangeToGranularity: - alignDateRangeToGranularity == null ? true : alignDateRangeToGranularity, - fillNulls: fillNulls ?? 0, - compareToPreviousPeriod: compareToPreviousPeriod ?? false, + settings.alignDateRangeToGranularity == null + ? true + : settings.alignDateRangeToGranularity, + fillNulls: settings.fillNulls ?? 0, + compareToPreviousPeriod: settings.compareToPreviousPeriod ?? false, }; } @@ -55,22 +60,32 @@ export default function ChartDisplaySettingsDrawer({ settings, opened, displayType, + defaultNumberFormat, onChange, onClose, previousDateRange, }: ChartDisplaySettingsDrawerProps) { + const appliedDefaults = useMemo( + () => applyDefaultSettings(settings, defaultNumberFormat), + [settings, defaultNumberFormat], + ); + const { control, handleSubmit, register, reset, setValue } = useForm({ - defaultValues: applyDefaultSettings(settings), + defaultValues: appliedDefaults, }); + useEffect(() => { + reset(appliedDefaults); + }, [appliedDefaults, reset]); + const fillNulls = useWatch({ control, name: 'fillNulls' }); const isFillNullsEnabled = shouldFillNullsWithZero(fillNulls); const handleClose = useCallback(() => { - reset(applyDefaultSettings(settings)); // Reset to default values, without saving + reset(appliedDefaults); onClose(); - }, [onClose, reset, settings]); + }, [onClose, reset, appliedDefaults]); const applyChanges = useCallback(() => { handleSubmit(onChange)(); @@ -78,8 +93,8 @@ export default function ChartDisplaySettingsDrawer({ }, [onChange, handleSubmit, onClose]); const resetToDefaults = useCallback(() => { - reset(applyDefaultSettings({})); - }, [reset]); + reset(applyDefaultSettings({}, defaultNumberFormat)); + }, [reset, defaultNumberFormat]); const isTimeChart = displayType === DisplayType.Line || displayType === DisplayType.StackedBar; diff --git a/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx b/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx index 78fc7e0b..10a13b27 100644 --- a/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx @@ -51,7 +51,7 @@ import { InputControlled } from '@/components/InputControlled'; import SaveToDashboardModal from '@/components/SaveToDashboardModal'; import { getStoredLanguage } from '@/components/SearchInput/SearchWhereInput'; import HDXMarkdownChart from '@/HDXMarkdownChart'; -import { useSource } from '@/source'; +import { getTraceDurationNumberFormat, useSource } from '@/source'; import { normalizeNoOpAlertScheduleFields } from '@/utils/alerts'; import { ChartActionBar } from './ChartActionBar'; @@ -189,6 +189,15 @@ export default function EditTimeChartForm({ ], }); + const autoDetectedNumberFormat = useMemo( + () => + getTraceDurationNumberFormat( + tableSource, + Array.isArray(select) ? select : undefined, + ), + [tableSource, select], + ); + const displaySettings: ChartConfigDisplaySettings = useMemo( () => ({ alignDateRangeToGranularity, @@ -642,6 +651,7 @@ export default function EditTimeChartForm({ ({ return { data: undefined }; }), getFirstTimestampValueExpression: jest.fn().mockReturnValue('Timestamp'), + getTraceDurationNumberFormat: jest.fn().mockReturnValue(undefined), })); jest.mock('../../MetricNameSelect', () => ({ diff --git a/packages/app/src/components/DBHeatmapChart.tsx b/packages/app/src/components/DBHeatmapChart.tsx index f548a3b7..a5c0986d 100644 --- a/packages/app/src/components/DBHeatmapChart.tsx +++ b/packages/app/src/components/DBHeatmapChart.tsx @@ -28,7 +28,7 @@ import { isAggregateFunction, timeBucketByGranularity } from '@/ChartUtils'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; import { NumberFormat } from '@/types'; import { FormatTime } from '@/useFormatTime'; -import { formatNumber } from '@/utils'; +import { formatDurationMs, formatNumber } from '@/utils'; import ChartContainer from './charts/ChartContainer'; import { SQLPreview } from './ChartSQLPreview'; @@ -843,24 +843,12 @@ function Heatmap({ // to the actual value before formatting. const actualValue = scaleType === 'log' ? Math.exp(value) : value; - if (numberFormat?.unit === 'ms') { - // Auto-scale duration: ms → s → min, picking the most compact unit - const abs = Math.abs(actualValue); - if (abs >= 60_000) { - const v = actualValue / 60_000; - return `${Number.isInteger(v) ? v : v.toFixed(1)}m`; - } - if (abs >= 1_000) { - const v = actualValue / 1_000; - return `${Number.isInteger(v) ? v : v.toFixed(1)}s`; - } - if (abs >= 1) { - return `${Math.round(actualValue)}ms`; - } - if (abs >= 0.001) { - return `${+(actualValue * 1_000).toPrecision(2)}µs`; - } - return `${actualValue.toPrecision(2)}ms`; + if (numberFormat?.unit === 'ms' || numberFormat?.output === 'duration') { + const msValue = + numberFormat?.output === 'duration' + ? actualValue * (numberFormat?.factor ?? 1) * 1000 + : actualValue; + return formatDurationMs(msValue); } return numberFormat diff --git a/packages/app/src/components/DBListBarChart.tsx b/packages/app/src/components/DBListBarChart.tsx index 6c477d71..d541ef40 100644 --- a/packages/app/src/components/DBListBarChart.tsx +++ b/packages/app/src/components/DBListBarChart.tsx @@ -8,7 +8,7 @@ import { Box, Code, Flex, HoverCard, Text } from '@mantine/core'; import { buildMVDateRangeIndicator } from '@/ChartUtils'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; import { useMVOptimizationExplanation } from '@/hooks/useMVOptimizationExplanation'; -import { useSource } from '@/source'; +import { useResolvedNumberFormat, useSource } from '@/source'; import type { NumberFormat } from '@/types'; import { omit } from '@/utils'; import { formatNumber, semanticKeyedColor } from '@/utils'; @@ -213,6 +213,8 @@ export default function DBListBarChart({ const { data: source } = useSource({ id: config.source }); + const resolvedNumberFormat = useResolvedNumberFormat(config); + const columns = useMemo(() => { const rows = data?.data ?? []; if (rows.length === 0) { @@ -224,9 +226,9 @@ export default function DBListBarChart({ .map(key => ({ dataKey: key, displayName: key, - numberFormat: config.numberFormat, + numberFormat: resolvedNumberFormat, })); - }, [config.numberFormat, data, hiddenSeries]); + }, [resolvedNumberFormat, data, hiddenSeries]); const toolbarItemsMemo = useMemo(() => { const allToolbarItems = []; diff --git a/packages/app/src/components/DBNumberChart.tsx b/packages/app/src/components/DBNumberChart.tsx index 5d46aed1..4e3f6f42 100644 --- a/packages/app/src/components/DBNumberChart.tsx +++ b/packages/app/src/components/DBNumberChart.tsx @@ -19,7 +19,7 @@ import { } from '@/ChartUtils'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; import { useMVOptimizationExplanation } from '@/hooks/useMVOptimizationExplanation'; -import { useSource } from '@/source'; +import { useResolvedNumberFormat, useSource } from '@/source'; import { formatNumber } from '@/utils'; import ChartContainer from './charts/ChartContainer'; @@ -81,10 +81,12 @@ export default function DBNumberChart({ ) : error; + const resolvedNumberFormat = useResolvedNumberFormat(config); + const value = valueColumn ? data?.data?.[0]?.[valueColumn.name] : (Object.values(data?.data?.[0] ?? {})?.[0] ?? Number.NaN); - const formattedValue = formatNumber(value as number, config.numberFormat); + const formattedValue = formatNumber(value as number, resolvedNumberFormat); const { data: source } = useSource({ id: config.source, diff --git a/packages/app/src/components/DBPieChart.tsx b/packages/app/src/components/DBPieChart.tsx index 54e0be42..e2821cfe 100644 --- a/packages/app/src/components/DBPieChart.tsx +++ b/packages/app/src/components/DBPieChart.tsx @@ -14,7 +14,7 @@ import { } from '@/ChartUtils'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; import { useMVOptimizationExplanation } from '@/hooks/useMVOptimizationExplanation'; -import { useSource } from '@/source'; +import { useResolvedNumberFormat, useSource } from '@/source'; import type { NumberFormat } from '@/types'; import { getColorProps } from '@/utils'; @@ -74,6 +74,8 @@ export const DBPieChart = ({ id: config.source, }); + const resolvedNumberFormat = useResolvedNumberFormat(config); + const queriedConfig = useMemo(() => { return isBuilderChartConfig(config) ? convertToPieChartConfig(config) @@ -188,7 +190,9 @@ export const DBPieChart = ({ ))} } + content={ + + } /> diff --git a/packages/app/src/components/DBTableChart.tsx b/packages/app/src/components/DBTableChart.tsx index 286918ba..cdbade2f 100644 --- a/packages/app/src/components/DBTableChart.tsx +++ b/packages/app/src/components/DBTableChart.tsx @@ -15,7 +15,7 @@ import { import { Table, TableVariant } from '@/HDXMultiSeriesTableChart'; import { useMVOptimizationExplanation } from '@/hooks/useMVOptimizationExplanation'; import useOffsetPaginatedQuery from '@/hooks/useOffsetPaginatedQuery'; -import { useSource } from '@/source'; +import { useResolvedNumberFormat, useSource } from '@/source'; import { useIntersectionObserver } from '@/utils'; import ChartContainer from './charts/ChartContainer'; @@ -57,6 +57,8 @@ export default function DBTableChart({ id: config.source, }); + const resolvedNumberFormat = useResolvedNumberFormat(config); + const effectiveSort = useMemo( () => controlledSort || sort, [controlledSort, sort], @@ -143,10 +145,10 @@ export default function DBTableChart({ displayName: key, numberFormat: groupByKeys.includes(key) ? undefined - : config.numberFormat, + : resolvedNumberFormat, sortingFn: getClientSideSortingFn(data?.meta, key), })); - }, [config.numberFormat, aliasMap, queriedConfig, data, hiddenColumns]); + }, [resolvedNumberFormat, aliasMap, queriedConfig, data, hiddenColumns]); const toolbarItemsMemo = useMemo(() => { const allToolbarItems = []; diff --git a/packages/app/src/components/DBTimeChart.tsx b/packages/app/src/components/DBTimeChart.tsx index d4f81738..ab1a5de7 100644 --- a/packages/app/src/components/DBTimeChart.tsx +++ b/packages/app/src/components/DBTimeChart.tsx @@ -40,7 +40,7 @@ import { import { MemoChart } from '@/HDXMultiSeriesTimeChart'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; import { useMVOptimizationExplanation } from '@/hooks/useMVOptimizationExplanation'; -import { useSource } from '@/source'; +import { useResolvedNumberFormat, useSource } from '@/source'; import ChartContainer from './charts/ChartContainer'; import ChartErrorState, { @@ -365,6 +365,8 @@ function DBTimeChartComponent({ id: sourceId || config.source, }); + const resolvedNumberFormat = useResolvedNumberFormat(config); + const { error: resultFormattingError, graphResults, @@ -722,7 +724,7 @@ function DBTimeChartComponent({ lineData={lineData} isLoading={isLoadingOrPlaceholder} logReferenceTimestamp={logReferenceTimestamp} - numberFormat={config.numberFormat} + numberFormat={resolvedNumberFormat} onTimeRangeSelect={onTimeRangeSelect} referenceLines={referenceLines} setIsClickActive={setActiveClickPayloadIfSourceAvailable} diff --git a/packages/app/src/components/NumberFormat.tsx b/packages/app/src/components/NumberFormat.tsx index b9b55686..8901c190 100644 --- a/packages/app/src/components/NumberFormat.tsx +++ b/packages/app/src/components/NumberFormat.tsx @@ -19,6 +19,7 @@ import { IconClock, IconCurrencyDollar, IconDatabase, + IconHourglass, IconNumbers, IconPercentage, } from '@tabler/icons-react'; @@ -33,6 +34,7 @@ const FORMAT_ICONS: Record = { percent: , byte: , time: , + duration: , data_rate: , throughput: , }; @@ -131,7 +133,8 @@ const OUTPUT_CATEGORY_OPTIONS: OutputGroup[] = [ { value: 'number', label: 'Number' }, { value: 'currency', label: 'Currency' }, { value: 'percent', label: 'Percentage' }, - { value: 'time', label: 'Time' }, + { value: 'duration', label: 'Duration' }, + { value: 'time', label: 'Time (clock)' }, ], }, { @@ -249,7 +252,7 @@ export const NumberFormatForm: React.FC<{ - {format.output !== 'time' && ( + {format.output !== 'time' && format.output !== 'duration' && (
Decimals
- ) : format.output === 'time' ? ( + ) : format.output === 'time' || format.output === 'duration' ? ( ({ jest.mock('@/source', () => ({ useSource: jest.fn().mockReturnValue({ data: null }), + useResolvedNumberFormat: jest.fn().mockReturnValue(undefined), })); jest.mock('../MaterializedViews/MVOptimizationIndicator', () => diff --git a/packages/app/src/components/__tests__/DBNumberChart.test.tsx b/packages/app/src/components/__tests__/DBNumberChart.test.tsx index 621d7918..789175ad 100644 --- a/packages/app/src/components/__tests__/DBNumberChart.test.tsx +++ b/packages/app/src/components/__tests__/DBNumberChart.test.tsx @@ -26,6 +26,9 @@ jest.mock('@/hooks/useMVOptimizationExplanation', () => ({ jest.mock('@/source', () => ({ useSource: jest.fn().mockReturnValue({ data: null }), + useResolvedNumberFormat: jest + .fn() + .mockImplementation((config: any) => config.numberFormat), })); jest.mock('@/utils', () => ({ diff --git a/packages/app/src/components/__tests__/DBPieChart.test.tsx b/packages/app/src/components/__tests__/DBPieChart.test.tsx index f4fab9ec..cb46c534 100644 --- a/packages/app/src/components/__tests__/DBPieChart.test.tsx +++ b/packages/app/src/components/__tests__/DBPieChart.test.tsx @@ -23,6 +23,7 @@ jest.mock('@/hooks/useMVOptimizationExplanation', () => ({ jest.mock('@/source', () => ({ useSource: jest.fn().mockReturnValue({ data: null }), + useResolvedNumberFormat: jest.fn().mockReturnValue(undefined), })); jest.mock('../MaterializedViews/MVOptimizationIndicator', () => diff --git a/packages/app/src/components/__tests__/DBTableChart.test.tsx b/packages/app/src/components/__tests__/DBTableChart.test.tsx index ee39a61e..34e979da 100644 --- a/packages/app/src/components/__tests__/DBTableChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTableChart.test.tsx @@ -24,6 +24,7 @@ jest.mock('@/hooks/useMVOptimizationExplanation', () => ({ jest.mock('@/source', () => ({ useSource: jest.fn().mockReturnValue({ data: null }), + useResolvedNumberFormat: jest.fn().mockReturnValue(undefined), })); jest.mock('../MaterializedViews/MVOptimizationIndicator', () => diff --git a/packages/app/src/components/__tests__/DBTimeChart.test.tsx b/packages/app/src/components/__tests__/DBTimeChart.test.tsx index 39c66e03..5f1ce5a1 100644 --- a/packages/app/src/components/__tests__/DBTimeChart.test.tsx +++ b/packages/app/src/components/__tests__/DBTimeChart.test.tsx @@ -31,6 +31,7 @@ jest.mock('@/api', () => ({ jest.mock('@/source', () => ({ useSource: jest.fn(), + useResolvedNumberFormat: jest.fn().mockReturnValue(undefined), })); jest.mock('../MaterializedViews/MVOptimizationIndicator', () => diff --git a/packages/app/src/source.ts b/packages/app/src/source.ts index c062af1e..1f0faf17 100644 --- a/packages/app/src/source.ts +++ b/packages/app/src/source.ts @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useMemo } from 'react'; import pick from 'lodash/pick'; import objectHash from 'object-hash'; import { @@ -9,8 +9,11 @@ import { } from '@hyperdx/common-utils/dist/clickhouse'; import { Metadata } from '@hyperdx/common-utils/dist/core/metadata'; import { splitAndTrimWithBracket } from '@hyperdx/common-utils/dist/core/utils'; +import { isBuilderChartConfig } from '@hyperdx/common-utils/dist/guards'; import { + ChartConfigWithOptDateRange, MetricsDataType, + NumberFormat, SourceKind, SourceSchema, TLogSource, @@ -392,6 +395,95 @@ export function getDurationSecondsExpression(source: TTraceSource) { return `(${source.durationExpression})/1e${source.durationPrecision ?? 9}`; } +// Aggregate functions whose output preserves the unit of the input value. +// count and count_distinct produce dimensionless counts and should not +// inherit the duration format. +const DURATION_PRESERVING_AGG_FNS = new Set([ + 'avg', + 'min', + 'max', + 'sum', + 'any', + 'last_value', + 'quantile', + 'quantileMerge', + 'p50', + 'p90', + 'p95', + 'p99', + 'heatmap', + 'histogram', + 'histogramMerge', +]); + +function isDurationPreservingAggFn(aggFn: string | undefined): boolean { + if (!aggFn) return true; // no aggFn means raw expression — preserve unit + // Handle combinator forms like "avgIf", "quantileIfState" + const baseFn = aggFn.replace(/If(State|Merge)?$/, ''); + return DURATION_PRESERVING_AGG_FNS.has(baseFn); +} + +/** + * Returns a NumberFormat for duration display if the chart config's select + * expressions exactly match a trace source's durationExpression. Returns + * undefined if no match is detected. + * + * Only applies when the aggregate function preserves the unit of the input + * (e.g. avg, min, max, sum, p95). Functions like count and count_distinct + * produce dimensionless values and are skipped. + * + * Uses exact match only — the duration expression can be arbitrary SQL, + * so substring or regex matching would be fragile. + */ +export function getTraceDurationNumberFormat( + source: TSource | undefined, + selectExpressions: + | Array<{ valueExpression?: string; aggFn?: string }> + | undefined, +): NumberFormat | undefined { + if (!source || source.kind !== SourceKind.Trace || !source.durationExpression) + return undefined; + if (!selectExpressions || selectExpressions.length === 0) return undefined; + + const durationExpr = source.durationExpression; + const precision = source.durationPrecision ?? 9; + + for (const sel of selectExpressions) { + if (!sel.valueExpression) continue; + if (!isDurationPreservingAggFn(sel.aggFn)) continue; + + if (sel.valueExpression === durationExpr) { + return { + output: 'duration', + factor: Math.pow(10, -precision), + }; + } + } + + return undefined; +} + +/** + * Hook that resolves the effective numberFormat for a chart config. + * If the config already has an explicit numberFormat, it's returned as-is. + * Otherwise, auto-detects duration format when the chart uses a trace source + * with duration expressions. + */ +export function useResolvedNumberFormat( + config: ChartConfigWithOptDateRange, +): NumberFormat | undefined { + const { data: source } = useSource({ id: config.source }); + + return useMemo(() => { + if (config.numberFormat) return config.numberFormat; + + if (!isBuilderChartConfig(config)) return undefined; + + const select = Array.isArray(config.select) ? config.select : undefined; + return getTraceDurationNumberFormat(source, select); + }, [config, source]); +} + // defined in https://github.com/open-telemetry/opentelemetry-proto/blob/cfbf9357c03bf4ac150a3ab3bcbe4cc4ed087362/opentelemetry/proto/metrics/v1/metrics.proto // NOTE: We don't follow the standard perfectly, we enforce the required fields + a few more (ServiceName, MetricName, and ResourceAttributes primarily) const ReqMetricTableColumns = { diff --git a/packages/app/src/utils.ts b/packages/app/src/utils.ts index e20b5b1d..a0112344 100644 --- a/packages/app/src/utils.ts +++ b/packages/app/src/utils.ts @@ -839,6 +839,12 @@ export const formatNumber = ( return value.toFixed(mantissa); } + if (options.output === 'duration') { + const factor = options.factor ?? 1; + const ms = value * factor * 1000; + return formatDurationMs(ms); + } + const numbroFormat: numbro.Format = { output: options.output || 'number', mantissa: mantissa, @@ -863,6 +869,49 @@ export const formatNumber = ( ); }; +/** + * Formats a duration value given in milliseconds into a human-readable + * adaptive string (e.g. "120.41s", "45ms", "3µs"). Mirrors the trace + * waterfall rendering style. + */ +export function formatDurationMs(ms: number): string { + if (ms < 0) { + return `-${formatDurationMs(-ms)}`; + } + + if (ms === 0) { + return '0ms'; + } + + if (ms < 1) { + const µs = ms * 1000; + if (µs < 10) { + return `${parseFloat(µs.toPrecision(2))}µs`; + } + const µsRounded = Math.round(µs); + if (µsRounded < 1000) { + return `${µsRounded}µs`; + } + } + + if (ms < 1000) { + if (ms < 10) { + return `${parseFloat(ms.toPrecision(3))}ms`; + } + return `${parseFloat(ms.toFixed(1))}ms`; + } + + if (ms < 60_000) { + return `${parseFloat((ms / 1000).toFixed(2))}s`; + } + + if (ms < 3_600_000) { + return `${parseFloat((ms / 60_000).toFixed(2))}min`; + } + + return `${parseFloat((ms / 3_600_000).toFixed(2))}h`; +} + // format uptime as days, hours, minutes or seconds export const formatUptime = (seconds: number) => { if (seconds < 60) { diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 6151234c..ad965637 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -577,6 +577,7 @@ export const NumberFormatSchema = z.object({ 'percent', 'byte', // legacy, treated as data/bytes_iec 'time', + 'duration', 'number', 'data_rate', 'throughput',