diff --git a/.changeset/clean-jeans-complain.md b/.changeset/clean-jeans-complain.md new file mode 100644 index 00000000..e4a0101c --- /dev/null +++ b/.changeset/clean-jeans-complain.md @@ -0,0 +1,5 @@ +--- +"@hyperdx/app": patch +--- + +feat: Allow customizing zero-fill behavior diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index d0d4d2f8..f8f81378 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -19,6 +19,7 @@ import { import { AggregateFunction as AggFnV2, ChartConfigWithDateRange, + ChartConfigWithOptDateRange, ChartConfigWithOptTimestamp, DisplayType, Filter, @@ -1157,3 +1158,10 @@ export function buildMVDateRangeIndicator({ /> ); } + +export function shouldFillNullsWithZero( + fillNulls: ChartConfigWithOptDateRange['fillNulls'], +): boolean { + // To match legacy behavior, fill nulls with 0 unless explicitly disabled + return fillNulls !== false; +} diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index f2d11105..33d20bd8 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -500,6 +500,7 @@ export const MemoChart = memo(function MemoChart({ })} name={seriesName} isAnimationActive={false} + connectNulls /> ); }); diff --git a/packages/app/src/__tests__/ChartUtils.test.ts b/packages/app/src/__tests__/ChartUtils.test.ts index 53301f62..35d239eb 100644 --- a/packages/app/src/__tests__/ChartUtils.test.ts +++ b/packages/app/src/__tests__/ChartUtils.test.ts @@ -326,7 +326,7 @@ describe('ChartUtils', () => { ]); }); - it('should zero-fill missing time buckets', () => { + it('should zero-fill missing time buckets when generateEmptyBuckets is undefined', () => { const res = { data: [ { @@ -372,7 +372,6 @@ describe('ChartUtils', () => { currentPeriodResponse: res, dateRange: [new Date(1764159780000), new Date(1764159900000)], granularity: '1 minute', - generateEmptyBuckets: true, }); expect(actual.graphResults).toEqual([ @@ -442,6 +441,97 @@ describe('ChartUtils', () => { ]); }); + it('should fill missing time buckets with zero when generateEmptyBuckets is true', () => { + const res = { + data: [ + { + 'count()': 10, + __hdx_time_bucket: '2025-11-26T12:23:00Z', + }, + { + 'count()': 20, + __hdx_time_bucket: '2025-11-26T12:25:00Z', + }, + ], + meta: [ + { + name: 'count()', + type: 'UInt64', + }, + { + name: '__hdx_time_bucket', + type: 'DateTime', + }, + ], + }; + + const actual = formatResponseForTimeChart({ + currentPeriodResponse: res, + dateRange: [new Date(1764159780000), new Date(1764159900000)], + granularity: '1 minute', + generateEmptyBuckets: true, + }); + + expect(actual.graphResults).toEqual([ + { + __hdx_time_bucket: 1764159780, + 'count()': 10, + }, + { + __hdx_time_bucket: 1764159840, + 'count()': 0, + }, + { + __hdx_time_bucket: 1764159900, + 'count()': 20, + }, + ]); + }); + + it('should not fill missing time buckets when generateEmptyBuckets is false', () => { + const res = { + data: [ + { + 'count()': 10, + __hdx_time_bucket: '2025-11-26T12:23:00Z', + }, + { + 'count()': 20, + __hdx_time_bucket: '2025-11-26T12:25:00Z', + }, + ], + meta: [ + { + name: 'count()', + type: 'UInt64', + }, + { + name: '__hdx_time_bucket', + type: 'DateTime', + }, + ], + }; + + const actual = formatResponseForTimeChart({ + currentPeriodResponse: res, + dateRange: [new Date(1764159780000), new Date(1764159900000)], + granularity: '1 minute', + generateEmptyBuckets: false, + }); + + // Should only have the two data points, no filled buckets + expect(actual.graphResults).toEqual([ + { + __hdx_time_bucket: 1764159780, + 'count()': 10, + }, + { + __hdx_time_bucket: 1764159900, + 'count()': 20, + }, + ]); + }); + it('should plot previous period data when provided, shifted to align with current period', () => { const currentPeriodResponse = { data: [ diff --git a/packages/app/src/__tests__/utils.test.ts b/packages/app/src/__tests__/utils.test.ts index e130adca..b729bb23 100644 --- a/packages/app/src/__tests__/utils.test.ts +++ b/packages/app/src/__tests__/utils.test.ts @@ -149,12 +149,12 @@ describe('formatNumber', () => { expect(formatNumber(1234567, format)).toBe('1,234,567'); }); - it('applies factor multiplication', () => { + it('does not apply factor multiplication', () => { const format: NumberFormat = { output: 'number', factor: 0.001, // Convert to milliseconds }; - expect(formatNumber(1000, format)).toBe('1'); + expect(formatNumber(1000, format)).toBe('1000'); }); }); @@ -211,6 +211,40 @@ describe('formatNumber', () => { }); }); + describe('time format', () => { + it('formats seconds input', () => { + const format: NumberFormat = { + output: 'time', + factor: 1, // seconds + }; + expect(formatNumber(3661, format)).toBe('1:01:01'); + }); + + it('formats milliseconds input', () => { + const format: NumberFormat = { + output: 'time', + factor: 0.001, // milliseconds + }; + expect(formatNumber(61000, format)).toBe('0:01:01'); + }); + + it('formats microseconds input', () => { + const format: NumberFormat = { + output: 'time', + factor: 0.000001, // microseconds + }; + expect(formatNumber(1000000, format)).toBe('0:00:01'); + }); + + it('formats nanoseconds input', () => { + const format: NumberFormat = { + output: 'time', + factor: 0.000000001, // nanoseconds + }; + expect(formatNumber(1000000001, format)).toBe('0:00:01'); + }); + }); + describe('unit handling', () => { it('appends unit to formatted number', () => { const format: NumberFormat = { diff --git a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx new file mode 100644 index 00000000..f10a7f2d --- /dev/null +++ b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx @@ -0,0 +1,144 @@ +import { useCallback } from 'react'; +import { useForm, useWatch } from 'react-hook-form'; +import { + ChartConfigWithDateRange, + DisplayType, +} from '@hyperdx/common-utils/dist/types'; +import { + Box, + Button, + Checkbox, + Divider, + Drawer, + Group, + Stack, +} from '@mantine/core'; + +import { shouldFillNullsWithZero } from '@/ChartUtils'; +import { FormatTime } from '@/useFormatTime'; + +import { DEFAULT_NUMBER_FORMAT, NumberFormatForm } from './NumberFormat'; + +export type ChartConfigDisplaySettings = Pick< + ChartConfigWithDateRange, + | 'numberFormat' + | 'alignDateRangeToGranularity' + | 'fillNulls' + | 'compareToPreviousPeriod' +>; + +interface ChartDisplaySettingsDrawerProps { + opened: boolean; + settings: ChartConfigDisplaySettings; + displayType: DisplayType; + previousDateRange?: [Date, Date]; + onChange: (settings: ChartConfigDisplaySettings) => void; + onClose: () => void; +} + +function applyDefaultSettings({ + numberFormat, + alignDateRangeToGranularity, + compareToPreviousPeriod, + fillNulls, +}: ChartConfigDisplaySettings): ChartConfigDisplaySettings { + return { + numberFormat: numberFormat ?? DEFAULT_NUMBER_FORMAT, + alignDateRangeToGranularity: + alignDateRangeToGranularity == null ? true : alignDateRangeToGranularity, + fillNulls: fillNulls ?? 0, + compareToPreviousPeriod: compareToPreviousPeriod ?? false, + }; +} + +export default function ChartDisplaySettingsDrawer({ + settings, + opened, + displayType, + onChange, + onClose, + previousDateRange, +}: ChartDisplaySettingsDrawerProps) { + const { control, handleSubmit, register, reset, setValue } = + useForm({ + defaultValues: applyDefaultSettings(settings), + }); + + const fillNulls = useWatch({ control, name: 'fillNulls' }); + const isFillNullsEnabled = shouldFillNullsWithZero(fillNulls); + + const handleClose = useCallback(() => { + reset(applyDefaultSettings(settings)); // Reset to default values, without saving + onClose(); + }, [onClose, reset, settings]); + + const applyChanges = useCallback(() => { + handleSubmit(onChange)(); + onClose(); + }, [onChange, handleSubmit, onClose]); + + const resetToDefaults = useCallback(() => { + reset(applyDefaultSettings({})); + }, [reset]); + + const isTimeChart = + displayType === DisplayType.Line || displayType === DisplayType.StackedBar; + + return ( + + + {isTimeChart && ( + <> + + + { + setValue('fillNulls', e.currentTarget.checked ? 0 : false); + }} + /> + + + ( + + {' - '} + ) + + ) + } + {...register('compareToPreviousPeriod')} + /> + + + )} + + + + + + + + + + ); +} diff --git a/packages/app/src/components/DBEditTimeChartForm.tsx b/packages/app/src/components/DBEditTimeChartForm.tsx index cfadcdd1..dd469d04 100644 --- a/packages/app/src/components/DBEditTimeChartForm.tsx +++ b/packages/app/src/components/DBEditTimeChartForm.tsx @@ -45,6 +45,7 @@ import { Text, Textarea, } from '@mantine/core'; +import { useDisclosure } from '@mantine/hooks'; import { IconArrowDown, IconArrowUp, @@ -82,7 +83,6 @@ import { GranularityPickerControlled } from '@/GranularityPicker'; import { useFetchMetricResourceAttrs } from '@/hooks/useFetchMetricResourceAttrs'; import SearchInputV2 from '@/SearchInputV2'; import { getFirstTimestampValueExpression, useSource } from '@/source'; -import { FormatTime } from '@/useFormatTime'; import { getMetricTableName, optionsToSelectData, @@ -99,20 +99,20 @@ import { } from '@/utils/alerts'; import HDXMarkdownChart from '../HDXMarkdownChart'; -import type { NumberFormat } from '../types'; import MVOptimizationIndicator from './MaterializedViews/MVOptimizationIndicator'; import { AggFnSelectControlled } from './AggFnSelect'; +import ChartDisplaySettingsDrawer, { + ChartConfigDisplaySettings, +} from './ChartDisplaySettingsDrawer'; import DBNumberChart from './DBNumberChart'; import DBSqlRowTableWithSideBar from './DBSqlRowTableWithSidebar'; import { CheckBoxControlled, InputControlled, - SwitchControlled, TextInputControlled, } from './InputControlled'; import { MetricNameSelect } from './MetricNameSelect'; -import { NumberFormatInput } from './NumberFormat'; import SaveToDashboardModal from './SaveToDashboardModal'; import SourceSchemaPreview from './SourceSchemaPreview'; import { SourceSelectControlled } from './SourceSelect'; @@ -160,30 +160,6 @@ const validateMetricNames = ( return false; }; -const NumberFormatInputControlled = ({ - control, - onSubmit, -}: { - control: Control; - onSubmit: () => void; -}) => { - return ( - ( - { - onChange(newValue); - onSubmit(); - }} - value={value} - /> - )} - /> - ); -}; - type SeriesItem = NonNullable< SavedChartConfigWithSelectArray['select'] >[number]; @@ -537,14 +513,6 @@ export default function EditTimeChartForm({ const whereLanguage = useWatch({ control, name: 'whereLanguage' }); const alert = useWatch({ control, name: 'alert' }); const seriesReturnType = useWatch({ control, name: 'seriesReturnType' }); - const compareToPreviousPeriod = useWatch({ - control, - name: 'compareToPreviousPeriod', - }); - const alignDateRangeToGranularity = useWatch({ - control, - name: 'alignDateRangeToGranularity', - }); const groupBy = useWatch({ control, name: 'groupBy' }); const displayType = useWatch({ control, name: 'displayType' }) ?? DisplayType.Line; @@ -580,6 +548,41 @@ export default function EditTimeChartForm({ const showGeneratedSql = ['table', 'time', 'number'].includes(activeTab); // Whether to show the generated SQL preview const showSampleEvents = tableSource?.kind !== SourceKind.Metric; + const [ + alignDateRangeToGranularity, + fillNulls, + compareToPreviousPeriod, + numberFormat, + ] = useWatch({ + control, + name: [ + 'alignDateRangeToGranularity', + 'fillNulls', + 'compareToPreviousPeriod', + 'numberFormat', + ], + }); + + const displaySettings: ChartConfigDisplaySettings = useMemo( + () => ({ + alignDateRangeToGranularity, + fillNulls, + compareToPreviousPeriod, + numberFormat, + }), + [ + alignDateRangeToGranularity, + fillNulls, + compareToPreviousPeriod, + numberFormat, + ], + ); + + const [ + displaySettingsOpened, + { open: openDisplaySettings, close: closeDisplaySettings }, + ] = useDisclosure(false); + // Only update this on submit, otherwise we'll have issues // with using the source value from the last submit // (ex. ignoring local custom source updates) @@ -762,34 +765,6 @@ export default function EditTimeChartForm({ }); }, [dateRange]); - // Trigger a search when "Show Complete Intervals" changes - useEffect(() => { - setQueriedConfig((config: ChartConfigWithDateRange | undefined) => { - if (config == null) { - return config; - } - - return { - ...config, - alignDateRangeToGranularity, - }; - }); - }, [alignDateRangeToGranularity]); - - // Trigger a search when "compare to previous period" changes - useEffect(() => { - setQueriedConfig((config: ChartConfigWithDateRange | undefined) => { - if (config == null) { - return config; - } - - return { - ...config, - compareToPreviousPeriod, - }; - }); - }, [compareToPreviousPeriod]); - const queryReady = isQueryReady(queriedConfig); // The chart config to use when showing the user the generated SQL @@ -877,6 +852,22 @@ export default function EditTimeChartForm({ // Need to force a rerender on change as the modal will not be mounted when initially rendered const [parentRef, setParentRef] = useState(null); + const handleUpdateDisplaySettings = useCallback( + ({ + numberFormat, + alignDateRangeToGranularity, + fillNulls, + compareToPreviousPeriod, + }: ChartConfigDisplaySettings) => { + setValue('numberFormat', numberFormat); + setValue('alignDateRangeToGranularity', alignDateRangeToGranularity); + setValue('fillNulls', fillNulls); + setValue('compareToPreviousPeriod', compareToPreviousPeriod); + onSubmit(); + }, + [setValue, onSubmit], + ); + return (
)} - + ) : ( @@ -1288,33 +1282,6 @@ export default function EditTimeChartForm({ )} - {activeTab === 'time' && ( - - - - Compare to Previous Period{' '} - {!dashboardId && ( - <> - ( - - {' - '} - - ) - - )} - - } - /> - - )} {!queryReady && activeTab !== 'markdown' ? (
@@ -1462,6 +1429,14 @@ export default function EditTimeChartForm({ opened={saveToDashboardModalOpen} onClose={() => setSaveToDashboardModalOpen(false)} /> +
); } diff --git a/packages/app/src/components/DBTimeChart.tsx b/packages/app/src/components/DBTimeChart.tsx index fb909851..d5f6ef4a 100644 --- a/packages/app/src/components/DBTimeChart.tsx +++ b/packages/app/src/components/DBTimeChart.tsx @@ -36,6 +36,7 @@ import { formatResponseForTimeChart, getPreviousDateRange, PreviousPeriodSuffix, + shouldFillNullsWithZero, useTimeChartSettings, } from '@/ChartUtils'; import { MemoChart } from '@/HDXMultiSeriesTimeChart'; @@ -395,7 +396,7 @@ function DBTimeChartComponent({ : undefined, dateRange, granularity, - generateEmptyBuckets: fillNulls !== false, + generateEmptyBuckets: shouldFillNullsWithZero(fillNulls), source, hiddenSeries, previousPeriodOffsetSeconds, diff --git a/packages/app/src/components/NumberFormat.tsx b/packages/app/src/components/NumberFormat.tsx index 55d7745b..26b44efe 100644 --- a/packages/app/src/components/NumberFormat.tsx +++ b/packages/app/src/components/NumberFormat.tsx @@ -1,36 +1,26 @@ import * as React from 'react'; -import { useForm, useWatch } from 'react-hook-form'; +import { useMemo } from 'react'; +import { Control, Controller, useWatch } from 'react-hook-form'; +import { NumberFormat } from '@hyperdx/common-utils/dist/types'; import { - ActionIcon, - Button, Checkbox as MCheckbox, - Drawer, NativeSelect, Paper, Slider, Stack, TextInput, } from '@mantine/core'; -import { useDisclosure } from '@mantine/hooks'; import { IconClock, IconCurrencyDollar, IconDatabase, IconNumbers, IconPercentage, - IconX, } from '@tabler/icons-react'; -import { NumberFormat } from '../types'; import { formatNumber } from '../utils'; -const FORMAT_NAMES: Record = { - number: 'Number', - currency: 'Currency', - percent: 'Percentage', - byte: 'Bytes', - time: 'Time', -}; +import { ChartConfigDisplaySettings } from './ChartDisplaySettingsDrawer'; const FORMAT_ICONS: Record = { number: , @@ -40,48 +30,26 @@ const FORMAT_ICONS: Record = { time: , }; -const DEFAULT_NUMBER_FORMAT: NumberFormat = { +const TEST_NUMBER = 1234; + +export const DEFAULT_NUMBER_FORMAT: NumberFormat = { factor: 1, - output: 'number', + output: 'number' as const, mantissa: 2, thousandSeparated: true, average: false, decimalBytes: false, }; -const TEST_NUMBER = 1234; - export const NumberFormatForm: React.FC<{ - value?: NumberFormat; - onApply: (value: NumberFormat) => void; - onClose: () => void; -}> = ({ value, onApply, onClose }) => { - const { register, handleSubmit, control, setValue } = useForm({ - defaultValues: value ?? DEFAULT_NUMBER_FORMAT, - }); - - const format = useWatch({ control }); + control: Control; +}> = ({ control }) => { + const format = + useWatch({ control, name: 'numberFormat' }) ?? DEFAULT_NUMBER_FORMAT; return ( <> - {/* setValue('factor', 1)} - > - Reset - - } - /> */}
- ( + + )} /> {format.output === 'currency' && ( - ( + + )} /> )}
@@ -123,124 +100,121 @@ export const NumberFormatForm: React.FC<{ > Example - {formatNumber(TEST_NUMBER, format as NumberFormat)} + {formatNumber(TEST_NUMBER || 0, format)} {format.output !== 'time' && (
Decimals
- `Decimals: ${value}`} - marks={[ - { value: 0, label: '0' }, - { value: 10, label: '10' }, - ]} - value={format.mantissa} - onChange={value => { - setValue('mantissa', value); - }} + ( + `Decimals: ${val}`} + marks={[ + { value: 0, label: '0' }, + { value: 10, label: '10' }, + ]} + value={value ?? 2} + onChange={onChange} + /> + )} />
)} {format.output === 'byte' ? ( - { + return ( + + ); + }} /> ) : format.output === 'time' ? ( - parseFloat(value), - })} - data={[ - { value: '1', label: 'Seconds' }, - { value: '0.001', label: 'Milliseconds' }, - ]} + { + const options = useMemo( + () => [ + { value: '1', label: 'Seconds' }, + { value: '0.001', label: 'Milliseconds' }, + { value: '0.000001', label: 'Microseconds' }, + { value: '0.000000001', label: 'Nanoseconds' }, + ], + [], + ); + + const stringValue = + options.find(option => parseFloat(option.value) === value) + ?.value ?? '1'; + + return ( + onChange(parseFloat(e.target.value))} + data={options} + /> + ); + }} /> ) : ( <> - ( + + )} /> - ( + + )} /> )} - - - -
); }; - -export const NumberFormatInput: React.FC<{ - value?: NumberFormat; - onChange: (value?: NumberFormat) => void; -}> = ({ value, onChange }) => { - const [opened, { open, close }] = useDisclosure(false); - - const handleApply = React.useCallback( - (value?: NumberFormat) => { - onChange(value); - close(); - }, - [onChange, close], - ); - - return ( - <> - - - - - - {value?.output && ( - handleApply(undefined)} - > - - - )} - - - ); -}; diff --git a/packages/app/src/utils.ts b/packages/app/src/utils.ts index be98a679..6a1dfc64 100644 --- a/packages/app/src/utils.ts +++ b/packages/app/src/utils.ts @@ -630,8 +630,12 @@ export const formatNumber = ( currencySymbol: options.currencySymbol || '$', }), }; + + // Factor is only currently available for the time output + const factor = options.output === 'time' ? (options.factor ?? 1) : 1; + return ( - numbro(value * (options.factor ?? 1)).format(numbroFormat) + + numbro(value * factor).format(numbroFormat) + (options.unit ? ` ${options.unit}` : '') ); };