diff --git a/.changeset/metric-name-select-not-found.md b/.changeset/metric-name-select-not-found.md new file mode 100644 index 0000000000..0e8251095e --- /dev/null +++ b/.changeset/metric-name-select-not-found.md @@ -0,0 +1,5 @@ +--- +'@hyperdx/app': patch +--- + +Fix two problems with the chart editor's metric name picker. The truncation and load-failure notices now render below the input instead of above it, so they no longer push the field down out of alignment with the browse-metrics button beside it, and both were shortened to fit the tile editor's column without wrapping. A search that matches nothing now says so rather than silently hiding the dropdown, and offers the searched name as a selectable option — the catalog only covers the most recent three days of the chart's time range, so a metric that stopped reporting was previously impossible to chart by name. diff --git a/packages/app/src/components/DBEditTimeChartForm/ChartSeriesEditor.tsx b/packages/app/src/components/DBEditTimeChartForm/ChartSeriesEditor.tsx index 0a96bb8d1a..a87bb3efdc 100644 --- a/packages/app/src/components/DBEditTimeChartForm/ChartSeriesEditor.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/ChartSeriesEditor.tsx @@ -482,15 +482,13 @@ export function ChartSeriesEditor({ onApply={applyExplorerMetric} /> {metricType === 'gauge' && ( - - - + )} )} diff --git a/packages/app/src/components/MetricNameSelect.tsx b/packages/app/src/components/MetricNameSelect.tsx index ddf31cb617..e5626c3380 100644 --- a/packages/app/src/components/MetricNameSelect.tsx +++ b/packages/app/src/components/MetricNameSelect.tsx @@ -1,5 +1,4 @@ import { useMemo, useState } from 'react'; -import { addDays, differenceInDays, subDays } from 'date-fns'; import { DateRange, MetricsDataType, @@ -8,155 +7,42 @@ import { import { Select } from '@mantine/core'; import { useDebouncedValue } from '@mantine/hooks'; -import { useGetMetricNames } from '@/hooks/useMetadata'; -import { capitalizeFirstLetter } from '@/utils'; +import { useMetricNames } from '@/hooks/useMetricNames'; +import { + metricKindLabel, + QUERYABLE_KINDS, + type QueryableMetricKind, +} from '@/utils/metricKinds'; const SEPARATOR = ':::::::'; const SEARCH_DEBOUNCE_MS = 300; -const metricNamesQueryArgs = ({ - dateRange, - metricSource, - metricType, -}: { - dateRange?: DateRange['dateRange']; - metricSource: TMetricSource; - metricType: MetricsDataType; -}) => { - // eslint-disable-next-line no-restricted-syntax - const now = new Date(); - let _dateRange: DateRange['dateRange'] = dateRange - ? dateRange - : [subDays(now, 1), now]; - const diffInDays = differenceInDays(_dateRange[1], _dateRange[0]); - - if (diffInDays < 1) { - const nextDay = addDays(_dateRange[0], 1); - if (nextDay > now) { - _dateRange = [subDays(_dateRange[1], 1), _dateRange[1]]; - } else { - _dateRange = [_dateRange[0], nextDay]; - } - } else if (diffInDays > 3) { - // most recent 3 days - _dateRange = [subDays(_dateRange[1], 3), _dateRange[1]]; - } - - return { - databaseName: metricSource.from.databaseName, - // Empty when this source has no table for the kind, which disables the query - // rather than emitting `FROM db.``` and failing on every render. - tableName: metricSource.metricTables?.[metricType] ?? '', - connectionId: metricSource.connection, - timestampValueExpression: metricSource.timestampValueExpression ?? '', - dateRange: _dateRange, - }; -}; - -/** - * Metric names available on a metric source, one list per queryable kind. - * - * Names only, and deliberately so: it backs the always-mounted select, where a - * grouped scan for unit/description would be too heavy. The explorer pays for - * that richer catalog itself via `useMetricCatalog`, only while it is open. - */ -function useMetricNames( - metricSource: TMetricSource, - dateRange?: DateRange['dateRange'], - namePattern?: string, -) { - const { gaugeArgs, histogramArgs, sumArgs, exponentialHistogramArgs } = - useMemo( - () => ({ - gaugeArgs: metricNamesQueryArgs({ - dateRange, - metricSource, - metricType: MetricsDataType.Gauge, - }), - histogramArgs: metricNamesQueryArgs({ - dateRange, - metricSource, - metricType: MetricsDataType.Histogram, - }), - sumArgs: metricNamesQueryArgs({ - dateRange, - metricSource, - metricType: MetricsDataType.Sum, - }), - exponentialHistogramArgs: metricNamesQueryArgs({ - dateRange, - metricSource, - metricType: MetricsDataType.ExponentialHistogram, - }), - }), - [metricSource, dateRange], - ); - - const gauge = useGetMetricNames({ ...gaugeArgs, namePattern }); - const histogram = useGetMetricNames({ ...histogramArgs, namePattern }); - const sum = useGetMetricNames({ ...sumArgs, namePattern }); - const exponentialHistogram = useGetMetricNames({ - ...exponentialHistogramArgs, - namePattern, - }); - - return { - gaugeMetrics: gauge.data?.names, - histogramMetrics: histogram.data?.names, - sumMetrics: sum.data?.names, - exponentialHistogramMetrics: exponentialHistogram.data?.names, - isTruncated: [gauge, histogram, sum, exponentialHistogram].some( - query => query.data?.truncated, - ), - // Surfaced because a failed kind otherwise just vanishes from the list: the - // query is not retried, so a transient error or a query too slow to finish - // would silently omit those metrics from an apparently healthy dropdown. - hasError: [gauge, histogram, sum, exponentialHistogram].some( - query => query.isError, - ), - isLoading: [gauge, histogram, sum, exponentialHistogram].some( - query => query.isLoading, - ), - }; -} +const metricOptionValue = (metricName: string, metricType: MetricsDataType) => + `${metricName}${SEPARATOR}${metricType}`; export function getMetricOptions( - gaugeMetrics: string[] | undefined, - histogramMetrics: string[] | undefined, - sumMetrics: string[] | undefined, - exponentialHistogramMetrics: string[] | undefined, + namesByKind: Record, metricName: string | null | undefined, metricType: MetricsDataType, ) { - const metricsFromQuery = [ - ...(gaugeMetrics?.map(metric => ({ - value: `${metric}${SEPARATOR}${MetricsDataType.Gauge}`, - label: `${metric} (Gauge)`, - })) ?? []), - ...(histogramMetrics?.map(metric => ({ - value: `${metric}${SEPARATOR}${MetricsDataType.Histogram}`, - label: `${metric} (Histogram)`, - })) ?? []), - ...(sumMetrics?.map(metric => ({ - value: `${metric}${SEPARATOR}${MetricsDataType.Sum}`, - label: `${metric} (Sum)`, - })) ?? []), - ...(exponentialHistogramMetrics?.map(metric => ({ - value: `${metric}${SEPARATOR}${MetricsDataType.ExponentialHistogram}`, - label: `${metric} (Exponential Histogram)`, - })) ?? []), - ]; + const metricsFromQuery = QUERYABLE_KINDS.flatMap( + kind => + namesByKind[kind]?.map(metric => ({ + value: metricOptionValue(metric, kind), + label: `${metric} (${metricKindLabel(kind)})`, + })) ?? [], + ); // if saved metric does not exist in the available options, assume it exists // and add it to options if ( metricName && !metricsFromQuery.find( - metric => metric.value === `${metricName}${SEPARATOR}${metricType}`, + metric => metric.value === metricOptionValue(metricName, metricType), ) ) { metricsFromQuery.push({ - value: `${metricName}${SEPARATOR}${metricType}`, - label: `${metricName} (${capitalizeFirstLetter(metricType)})`, + value: metricOptionValue(metricName, metricType), + label: `${metricName} (${metricKindLabel(metricType)})`, }); } return metricsFromQuery; @@ -193,11 +79,11 @@ export function MetricNameSelect({ // input when the selection changes, and reports it through `onSearchChange` // exactly like typed text. Passing that on would search ClickHouse for // "up (Gauge)", which matches nothing, so an already-configured chart would - // open to an empty list. Compared case-insensitively because the label for a - // saved exponential-histogram metric differs only in case from the one built - // for a discovered metric. + // open to an empty list. Built from the same label helper as the options, so + // the two always agree; compared case-insensitively as a cheap guard against + // that drifting again. const selectedLabel = metricName - ? `${metricName} (${capitalizeFirstLetter(metricType)})` + ? `${metricName} (${metricKindLabel(metricType)})` : ''; const trimmedSearch = searchValue.trim(); const activeSearch = @@ -206,32 +92,62 @@ export function MetricNameSelect({ : trimmedSearch; const [debouncedSearch] = useDebouncedValue(activeSearch, SEARCH_DEBOUNCE_MS); + // The debounce has not fired yet, so the options on screen answer the + // previous text and no query is in flight for the current one. + const isSearchPending = activeSearch !== debouncedSearch; const { - gaugeMetrics, - histogramMetrics, - sumMetrics, - exponentialHistogramMetrics, + namesByKind, isTruncated, hasError, + hasNoMatches, + isFetching: isSearching, } = useMetricNames(metricSource, dateRange, debouncedSearch); const options = useMemo(() => { - return getMetricOptions( - gaugeMetrics, - histogramMetrics, - sumMetrics, - exponentialHistogramMetrics, - metricName, - metricType, - ); + const metricOptions = getMetricOptions(namesByKind, metricName, metricType); + // A name missing from the picker is not necessarily missing from the data: + // the catalog query covers only the most recent 3 days of the chart's range, + // and a kind with no table configured is never queried. Offer the searched + // name so a pasted metric is not stranded — the select has no other way to + // commit a value that is not already an option. Built from the debounced + // search, so an offer appears only once the query behind it has answered, + // and the name is embedded in the label because Mantine filters options by + // substring against the label and would otherwise drop these. + // + // One offer per kind, because the kind decides which table the series is + // read from and a pasted name carries no kind. Filing it under whatever the + // last selection happened to use would query the wrong table and chart + // nothing, with no way to correct it. + if (debouncedSearch && hasNoMatches) { + for (const kind of QUERYABLE_KINDS) { + // A kind with no table is not queryable on this source, so committing + // the name under it could never resolve. + if (!metricSource.metricTables?.[kind]) { + continue; + } + const value = metricOptionValue(debouncedSearch, kind); + // Committing an offer puts the name in `metricName`, which + // `getMetricOptions` then synthesizes its own option for while the + // debounced search still holds the same name — and Mantine throws on a + // duplicate option value, taking the editor down with it. + if (metricOptions.some(option => option.value === value)) { + continue; + } + metricOptions.push({ + value, + label: `${debouncedSearch} (${metricKindLabel(kind)}, no recent data)`, + }); + } + } + return metricOptions; }, [ - gaugeMetrics, - histogramMetrics, - sumMetrics, - exponentialHistogramMetrics, + namesByKind, metricName, metricType, + debouncedSearch, + hasNoMatches, + metricSource, ]); const currentValue = @@ -257,13 +173,27 @@ export function MetricNameSelect({ // on close so a collapsed control still shows its selection. onDropdownOpen={() => setSearchValue('')} onDropdownClose={() => setSearchValue(selectedLabel)} + // Without a message Mantine sets `hiddenWhenEmpty`, so a search matching + // nothing hides the whole dropdown and the field just looks broken. + nothingFoundMessage={ + isSearchPending || isSearching + ? 'Searching…' + : hasError + ? 'Some metrics failed to load' + : activeSearch + ? 'No matching metrics' + : 'No metrics reported recently' + } // Reported in the description rather than the `error` slot, which belongs - // to form validation for this field. + // to form validation for this field. Kept below the input: it appears + // mid-session, and above the input it pushes the field down out of + // alignment with the browse-metrics button beside it. + inputWrapperOrder={['label', 'input', 'description', 'error']} description={ hasError - ? 'Some metrics could not be loaded' + ? 'Some metrics failed to load' : isTruncated - ? 'Too many metrics to list — type to search' + ? 'Type to search all metrics' : undefined } comboboxProps={{ diff --git a/packages/app/src/components/__tests__/MetricNameSelect.test.ts b/packages/app/src/components/__tests__/MetricNameSelect.test.ts index 0a6ccd09a2..bb3f5ea6bd 100644 --- a/packages/app/src/components/__tests__/MetricNameSelect.test.ts +++ b/packages/app/src/components/__tests__/MetricNameSelect.test.ts @@ -1,29 +1,38 @@ import { MetricsDataType } from '@hyperdx/common-utils/dist/types'; import { getMetricOptions } from '@/components/MetricNameSelect'; +import type { QueryableMetricKind } from '@/utils/metricKinds'; const SEPARATOR = ':::::::'; +/** Builds the per-kind name map `getMetricOptions` takes, defaulting to none. */ +const kinds = ({ + gauge, + sum, + histogram, + exponentialHistogram, +}: { + gauge?: string[]; + sum?: string[]; + histogram?: string[]; + exponentialHistogram?: string[]; +} = {}): Record => ({ + [MetricsDataType.Gauge]: gauge, + [MetricsDataType.Sum]: sum, + [MetricsDataType.Histogram]: histogram, + [MetricsDataType.ExponentialHistogram]: exponentialHistogram, +}); + describe('getMetricOptions', () => { describe('no metrics provided', () => { it('returns empty array when all metric lists are undefined', () => { - const result = getMetricOptions( - undefined, - undefined, - undefined, - undefined, - null, - MetricsDataType.Gauge, - ); + const result = getMetricOptions(kinds(), null, MetricsDataType.Gauge); expect(result).toEqual([]); }); it('returns empty array when all metric lists are empty', () => { const result = getMetricOptions( - [], - [], - [], - [], + kinds({ gauge: [], sum: [], histogram: [], exponentialHistogram: [] }), null, MetricsDataType.Gauge, ); @@ -32,10 +41,7 @@ describe('getMetricOptions', () => { it('adds saved metricName when it is not in empty results', () => { const result = getMetricOptions( - undefined, - undefined, - undefined, - undefined, + kinds(), 'my.metric', MetricsDataType.Sum, ); @@ -51,10 +57,7 @@ describe('getMetricOptions', () => { describe('one metric in a single argument list', () => { it('returns a single gauge option', () => { const result = getMetricOptions( - ['cpu.usage'], - undefined, - undefined, - undefined, + kinds({ gauge: ['cpu.usage'] }), null, MetricsDataType.Gauge, ); @@ -65,10 +68,7 @@ describe('getMetricOptions', () => { it('returns a single histogram option', () => { const result = getMetricOptions( - undefined, - ['request.duration'], - undefined, - undefined, + kinds({ histogram: ['request.duration'] }), null, MetricsDataType.Histogram, ); @@ -82,10 +82,7 @@ describe('getMetricOptions', () => { it('returns a single sum option', () => { const result = getMetricOptions( - undefined, - undefined, - ['bytes.sent'], - undefined, + kinds({ sum: ['bytes.sent'] }), null, MetricsDataType.Sum, ); @@ -96,27 +93,21 @@ describe('getMetricOptions', () => { it('returns a single exponential histogram option', () => { const result = getMetricOptions( - undefined, - undefined, - undefined, - ['request.duration'], + kinds({ exponentialHistogram: ['request.duration'] }), null, MetricsDataType.ExponentialHistogram, ); expect(result).toEqual([ { value: `request.duration${SEPARATOR}${MetricsDataType.ExponentialHistogram}`, - label: 'request.duration (Exponential Histogram)', + label: 'request.duration (Exp. histogram)', }, ]); }); it('does not duplicate a saved metricName already present in results', () => { const result = getMetricOptions( - ['cpu.usage'], - undefined, - undefined, - undefined, + kinds({ gauge: ['cpu.usage'] }), 'cpu.usage', MetricsDataType.Gauge, ); @@ -129,10 +120,7 @@ describe('getMetricOptions', () => { it('appends saved metricName when it is not in single-entry results', () => { const result = getMetricOptions( - ['cpu.usage'], - undefined, - undefined, - undefined, + kinds({ gauge: ['cpu.usage'] }), 'missing.metric', MetricsDataType.Gauge, ); @@ -145,23 +133,16 @@ describe('getMetricOptions', () => { }); describe('multiple metrics in each argument list', () => { - const gaugeMetrics = ['cpu.usage', 'mem.usage', 'disk.usage']; - const histogramMetrics = ['request.duration', 'db.query.duration']; - const sumMetrics = ['bytes.sent', 'bytes.received', 'requests.total']; - const exponentialHistogramMetrics = [ - 'http.request.duration', - 'rpc.server.duration', - ]; + const all = () => + kinds({ + gauge: ['cpu.usage', 'mem.usage', 'disk.usage'], + histogram: ['request.duration', 'db.query.duration'], + sum: ['bytes.sent', 'bytes.received', 'requests.total'], + exponentialHistogram: ['http.request.duration', 'rpc.server.duration'], + }); it('returns all options for all metric types', () => { - const result = getMetricOptions( - gaugeMetrics, - histogramMetrics, - sumMetrics, - exponentialHistogramMetrics, - null, - MetricsDataType.Gauge, - ); + const result = getMetricOptions(all(), null, MetricsDataType.Gauge); expect(result).toHaveLength(10); @@ -199,20 +180,36 @@ describe('getMetricOptions', () => { }); expect(result).toContainEqual({ value: `http.request.duration${SEPARATOR}${MetricsDataType.ExponentialHistogram}`, - label: 'http.request.duration (Exponential Histogram)', + label: 'http.request.duration (Exp. histogram)', }); expect(result).toContainEqual({ value: `rpc.server.duration${SEPARATOR}${MetricsDataType.ExponentialHistogram}`, - label: 'rpc.server.duration (Exponential Histogram)', + label: 'rpc.server.duration (Exp. histogram)', }); }); + // Grouping order comes from QUERYABLE_KINDS, which is what the dropdown + // renders top to bottom. + it('groups the options gauge, sum, histogram, exponential histogram', () => { + const result = getMetricOptions(all(), null, MetricsDataType.Gauge); + + expect(result.map(option => option.value.split(SEPARATOR)[1])).toEqual([ + 'gauge', + 'gauge', + 'gauge', + 'sum', + 'sum', + 'sum', + 'histogram', + 'histogram', + MetricsDataType.ExponentialHistogram, + MetricsDataType.ExponentialHistogram, + ]); + }); + it('does not duplicate a saved metricName already present among multiple options', () => { const result = getMetricOptions( - gaugeMetrics, - histogramMetrics, - sumMetrics, - exponentialHistogramMetrics, + all(), 'mem.usage', MetricsDataType.Gauge, ); @@ -225,10 +222,7 @@ describe('getMetricOptions', () => { it('appends saved metricName when absent from multiple-option results', () => { const result = getMetricOptions( - gaugeMetrics, - histogramMetrics, - sumMetrics, - exponentialHistogramMetrics, + all(), 'absent.metric', MetricsDataType.Histogram, ); diff --git a/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx b/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx index 2a2aac2642..e12dac5cb0 100644 --- a/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx +++ b/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx @@ -1,3 +1,4 @@ +import { useState } from 'react'; import { MetricsDataType, SourceKind, @@ -57,6 +58,26 @@ const renderSelect = ( />, ); +/** + * Holds `metricName` the way the chart form does. The mock-callback harness + * above never feeds a committed name back in, which hides every bug that only + * appears on the render after a selection. + */ +const StatefulSelect = () => { + const [metricName, setMetricName] = useState(null); + const [metricType, setMetricType] = useState(MetricsDataType.Gauge); + return ( + + ); +}; + beforeEach(() => { useGetMetricNames.mockReset(); useGetMetricNames.mockImplementation(({ tableName }: any) => ({ @@ -155,9 +176,7 @@ describe('MetricNameSelect', () => { renderSelect(); - expect( - screen.getByText('Some metrics could not be loaded'), - ).toBeInTheDocument(); + expect(screen.getByText('Some metrics failed to load')).toBeInTheDocument(); }); it('tells the user to search when the catalog is truncated', () => { @@ -167,8 +186,252 @@ describe('MetricNameSelect', () => { renderSelect(); + const input = screen.getByTestId('metric-name-selector'); + expect(input).toHaveAccessibleDescription('Type to search all metrics'); + // Under the input, so appearing mid-session cannot push the input down out + // of alignment with the browse-metrics button beside it. + const notice = screen.getByText('Type to search all metrics'); + expect( + input.compareDocumentPosition(notice) & Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + }); + + describe('a search that matches nothing', () => { + const noMatches = () => + useGetMetricNames.mockImplementation(({ tableName }: any) => ({ + data: tableName ? { names: [], truncated: false } : undefined, + })); + + // The catalog only covers the most recent 3 days of the chart's range, so a + // metric that stopped reporting is absent from the picker while its data is + // still chartable. Mantine cannot commit a value that is not an option, so + // without this offer the pasted name is unusable. + it('offers the typed name, and commits it verbatim', async () => { + noMatches(); + const setMetricName = jest.fn(); + const setMetricType = jest.fn(); + renderSelect({ setMetricName, setMetricType }); + + await userEvent.type( + screen.getByTestId('metric-name-selector'), + 'chi_clickhouse_metric_SystemErrors', + ); + + const offer = await screen.findByText( + 'chi_clickhouse_metric_SystemErrors (Gauge, no recent data)', + ); + await userEvent.click(offer); + + expect(setMetricName).toHaveBeenCalledWith( + 'chi_clickhouse_metric_SystemErrors', + ); + expect(setMetricType).toHaveBeenCalledWith(MetricsDataType.Gauge); + }); + + // The kind decides which table the series reads from, and a pasted name + // carries none. One offer per kind lets the user say which, instead of + // inheriting whatever the previous selection happened to be. + it('offers one kind per configured table, and no others', async () => { + noMatches(); + renderSelect(); + + await userEvent.type(screen.getByTestId('metric-name-selector'), 'zzz'); + + expect( + await screen.findByText('zzz (Gauge, no recent data)'), + ).toBeInTheDocument(); + expect(screen.getByText('zzz (Sum, no recent data)')).toBeInTheDocument(); + // The fixture source has no histogram or exponential-histogram table, so + // committing the name under either could never resolve. + expect( + screen.queryByText('zzz (Histogram, no recent data)'), + ).not.toBeInTheDocument(); + expect( + screen.queryByText('zzz (Exponential Histogram, no recent data)'), + ).not.toBeInTheDocument(); + }); + + it('commits the kind the user picked, not the current one', async () => { + noMatches(); + const setMetricName = jest.fn(); + const setMetricType = jest.fn(); + renderSelect({ + setMetricName, + setMetricType, + metricType: MetricsDataType.Gauge, + }); + + await userEvent.type(screen.getByTestId('metric-name-selector'), 'zzz'); + await userEvent.click( + await screen.findByText('zzz (Sum, no recent data)'), + ); + + expect(setMetricName).toHaveBeenCalledWith('zzz'); + expect(setMetricType).toHaveBeenCalledWith(MetricsDataType.Sum); + }); + + // Committing the offer puts the name in `metricName`, which + // `getMetricOptions` synthesizes its own option for, while the debounced + // search still holds the same name for one more interval. Both build the + // same `name:::::::type` value, and Mantine throws on a duplicate option + // value — taking the whole chart editor down rather than degrading. + it('survives committing the typed name', async () => { + noMatches(); + renderWithMantine(); + + const input = screen.getByTestId('metric-name-selector'); + await userEvent.type(input, 'chc_'); + await userEvent.click( + await screen.findByText('chc_ (Gauge, no recent data)'), + ); + + await waitFor(() => expect(input).toHaveValue('chc_ (Gauge)')); + + // Past the debounce, where the committed name and the searched name are + // both still in play. + await new Promise(resolve => setTimeout(resolve, DEBOUNCE_SETTLE_MS)); + expect(input).toHaveValue('chc_ (Gauge)'); + }); + + it('explains itself rather than hiding the dropdown', async () => { + noMatches(); + renderSelect(); + + await userEvent.click(screen.getByTestId('metric-name-selector')); + + expect( + await screen.findByText('No metrics reported recently'), + ).toBeInTheDocument(); + }); + + // Claiming a name does not exist while a kind is still answering would + // invite the user to commit a metric that is actually available. + it('offers nothing while a kind is in flight', async () => { + useGetMetricNames.mockImplementation(({ tableName }: any) => + tableName === 'otel_metrics_sum' + ? { data: undefined, isFetching: true } + : { data: tableName ? { names: [], truncated: false } : undefined }, + ); + renderSelect(); + + await userEvent.type(screen.getByTestId('metric-name-selector'), 'zzz'); + await new Promise(resolve => setTimeout(resolve, DEBOUNCE_SETTLE_MS)); + + expect( + screen.queryByText('zzz (Gauge, no recent data)'), + ).not.toBeInTheDocument(); + }); + + // These queries keep the previous pattern's data while a new one is in + // flight, so TanStack reports `success` and `isLoading` stays false. Off + // `isLoading` the dropdown would confidently deny the metric exists for the + // whole round trip. + it('says it is searching rather than denying the metric exists', async () => { + useGetMetricNames.mockImplementation(({ tableName }: any) => ({ + data: tableName ? { names: [], truncated: false } : undefined, + isFetching: true, + })); + renderSelect(); + + await userEvent.click(screen.getByTestId('metric-name-selector')); + + expect(await screen.findByText('Searching…')).toBeInTheDocument(); + expect( + screen.queryByText('No metrics reported recently'), + ).not.toBeInTheDocument(); + }); + + // Nothing is in flight during the 300ms before the debounce fires, so an + // `isFetching`-only check reads as settled while the options on screen still + // answer the previous keystroke. + it('says it is searching before the debounce has even fired', async () => { + noMatches(); + renderSelect(); + + await userEvent.type(screen.getByTestId('metric-name-selector'), 'ab'); + + expect(await screen.findByText('Searching…')).toBeInTheDocument(); + expect( + screen.queryByText('No metrics reported recently'), + ).not.toBeInTheDocument(); + }); + + // "No metrics reported recently" is a claim about the source, which is the + // wrong thing to say about a search that simply did not match. + it('distinguishes an unmatched search from an empty source', async () => { + useGetMetricNames.mockImplementation(({ tableName }: any) => ({ + data: tableName ? { names: [], truncated: false } : undefined, + })); + // No table for any kind, so no offer is appended and the message shows. + renderSelect({ + metricSource: { + ...metricSource, + metricTables: { + gauge: '', + sum: '', + histogram: '', + summary: '', + 'exponential histogram': '', + }, + }, + }); + + await userEvent.click(screen.getByTestId('metric-name-selector')); + expect( + await screen.findByText('No metrics reported recently'), + ).toBeInTheDocument(); + + await userEvent.type(screen.getByTestId('metric-name-selector'), 'zzz'); + await new Promise(resolve => setTimeout(resolve, DEBOUNCE_SETTLE_MS)); + + expect( + await screen.findByText('No matching metrics'), + ).toBeInTheDocument(); + }); + + // A kind whose table is misconfigured errors on every pattern and is never + // retried, so blocking the offer on it would permanently strand this source + // at the dead end the offer exists to remove. + it('still offers the typed name when one kind failed to load', async () => { + useGetMetricNames.mockImplementation(({ tableName }: any) => + tableName === 'otel_metrics_sum' + ? { data: undefined, isError: true } + : { data: tableName ? { names: [], truncated: false } : undefined }, + ); + renderSelect(); + + await userEvent.type(screen.getByTestId('metric-name-selector'), 'zzz'); + + expect( + await screen.findByText('zzz (Gauge, no recent data)'), + ).toBeInTheDocument(); + }); + + it('offers nothing when no kind answered at all', async () => { + useGetMetricNames.mockImplementation(() => ({ + data: undefined, + isError: true, + })); + renderSelect(); + + await userEvent.type(screen.getByTestId('metric-name-selector'), 'zzz'); + await new Promise(resolve => setTimeout(resolve, DEBOUNCE_SETTLE_MS)); + + expect( + screen.queryByText('zzz (Gauge, no recent data)'), + ).not.toBeInTheDocument(); + }); + }); + + // Otherwise the escape hatch would clutter every partial search. + it('does not offer the typed name when a metric matches', async () => { + renderSelect(); + + await userEvent.type(screen.getByTestId('metric-name-selector'), 'up'); + await new Promise(resolve => setTimeout(resolve, DEBOUNCE_SETTLE_MS)); + expect( - screen.getByText('Too many metrics to list — type to search'), - ).toBeInTheDocument(); + screen.queryByText('up (Gauge, no recent data)'), + ).not.toBeInTheDocument(); }); }); diff --git a/packages/app/src/hooks/useMetricCatalog.ts b/packages/app/src/hooks/useMetricCatalog.ts index ec950e7782..01cd71898e 100644 --- a/packages/app/src/hooks/useMetricCatalog.ts +++ b/packages/app/src/hooks/useMetricCatalog.ts @@ -8,7 +8,6 @@ import { } from '@hyperdx/common-utils/dist/clickhouse'; import { DateRange, - MetricsDataType, SourceKind, TMetricSource, } from '@hyperdx/common-utils/dist/types'; @@ -16,19 +15,12 @@ import { useQuery } from '@tanstack/react-query'; import { getClickhouseClient } from '@/clickhouse'; import { useMetadataWithSettings } from '@/hooks/useMetadata'; +import { QUERYABLE_KINDS } from '@/utils/metricKinds'; import { mergeMetricCatalog, type MetricCatalogEntry, } from '@/utils/metricNameTree'; -/** Metric kinds the query renderer can chart. `summary` is not one of them. */ -const QUERYABLE_KINDS: MetricsDataType[] = [ - MetricsDataType.Gauge, - MetricsDataType.Sum, - MetricsDataType.Histogram, - MetricsDataType.ExponentialHistogram, -]; - const MAX_METRICS_PER_KIND = 3000; /** diff --git a/packages/app/src/hooks/useMetricNames.ts b/packages/app/src/hooks/useMetricNames.ts new file mode 100644 index 0000000000..19db3204e7 --- /dev/null +++ b/packages/app/src/hooks/useMetricNames.ts @@ -0,0 +1,116 @@ +import { useMemo } from 'react'; +import { + DateRange, + MetricsDataType, + TMetricSource, +} from '@hyperdx/common-utils/dist/types'; + +import { useGetMetricNames } from '@/hooks/useMetadata'; +import { clampCatalogDateRange } from '@/hooks/useMetricCatalog'; +import type { QueryableMetricKind } from '@/utils/metricKinds'; + +const metricNamesQueryArgs = ({ + dateRange, + metricSource, + metricType, +}: { + dateRange?: DateRange['dateRange']; + metricSource: TMetricSource; + metricType: MetricsDataType; +}) => { + // eslint-disable-next-line no-restricted-syntax + const now = new Date(); + + return { + databaseName: metricSource.from.databaseName, + // Empty when this source has no table for the kind, which disables the query + // rather than emitting `FROM db.``` and failing on every render. + tableName: metricSource.metricTables?.[metricType] ?? '', + connectionId: metricSource.connection, + timestampValueExpression: metricSource.timestampValueExpression ?? '', + dateRange: clampCatalogDateRange(dateRange, now), + }; +}; + +/** + * Metric names available on a metric source, one list per queryable kind. + * + * Names only, and deliberately so: it backs the always-mounted select, where a + * grouped scan for unit/description would be too heavy. The explorer pays for + * that richer catalog itself via `useMetricCatalog`, only while it is open. + */ +export function useMetricNames( + metricSource: TMetricSource, + dateRange?: DateRange['dateRange'], + namePattern?: string, +) { + const { gaugeArgs, histogramArgs, sumArgs, exponentialHistogramArgs } = + useMemo( + () => ({ + gaugeArgs: metricNamesQueryArgs({ + dateRange, + metricSource, + metricType: MetricsDataType.Gauge, + }), + histogramArgs: metricNamesQueryArgs({ + dateRange, + metricSource, + metricType: MetricsDataType.Histogram, + }), + sumArgs: metricNamesQueryArgs({ + dateRange, + metricSource, + metricType: MetricsDataType.Sum, + }), + exponentialHistogramArgs: metricNamesQueryArgs({ + dateRange, + metricSource, + metricType: MetricsDataType.ExponentialHistogram, + }), + }), + [metricSource, dateRange], + ); + + const gauge = useGetMetricNames({ ...gaugeArgs, namePattern }); + const histogram = useGetMetricNames({ ...histogramArgs, namePattern }); + const sum = useGetMetricNames({ ...sumArgs, namePattern }); + const exponentialHistogram = useGetMetricNames({ + ...exponentialHistogramArgs, + namePattern, + }); + + const queries = [gauge, histogram, sum, exponentialHistogram]; + + // Keyed by `QueryableMetricKind`, so adding a kind to `QUERYABLE_KINDS` + // fails to compile here until a query for it is wired up above — rather than + // quietly offering that kind in the dropdown with nothing behind it. + const namesByKind: Record = { + [MetricsDataType.Gauge]: gauge.data?.names, + [MetricsDataType.Sum]: sum.data?.names, + [MetricsDataType.Histogram]: histogram.data?.names, + [MetricsDataType.ExponentialHistogram]: exponentialHistogram.data?.names, + }; + + return { + namesByKind, + isTruncated: queries.some(query => query.data?.truncated), + // Surfaced because a failed kind otherwise just vanishes from the list: the + // query is not retried, so a transient error or a query too slow to finish + // would silently omit those metrics from an apparently healthy dropdown. + hasError: queries.some(query => query.isError), + // Not `isLoading`: these queries keep the previous pattern's data while a + // new one is in flight, so TanStack reports `success` and `isLoading` is + // false for every search after the first mount. + isFetching: queries.some(query => query.isFetching), + // Only when every kind that answered returned nothing. A kind still in + // flight must not read as "no such metric". A kind that *errored* is + // deliberately not disqualifying: its table can be misconfigured + // indefinitely, and blocking on it would restore the dead end this exists + // to remove. When no kind answered at all, the `data !== undefined` check + // already holds the offer back. + hasNoMatches: + queries.some(query => query.data !== undefined) && + queries.every(query => (query.data?.names?.length ?? 0) === 0) && + !queries.some(query => query.isFetching), + }; +} diff --git a/packages/app/src/utils/metricKinds.ts b/packages/app/src/utils/metricKinds.ts index bbcd12023d..8eb2c6db58 100644 --- a/packages/app/src/utils/metricKinds.ts +++ b/packages/app/src/utils/metricKinds.ts @@ -1,5 +1,23 @@ import { MetricsDataType } from '@hyperdx/common-utils/dist/types'; +/** + * Metric kinds the query renderer can chart, in the order they are listed to + * the user. `summary` is not one of them. + * + * A readonly tuple rather than an array so `QueryableMetricKind` is a union of + * exactly these four: anything keyed by that type stops compiling when a kind + * is added here, which is what forces the per-kind queries behind it to be + * wired up rather than silently skipped. + */ +export const QUERYABLE_KINDS = [ + MetricsDataType.Gauge, + MetricsDataType.Sum, + MetricsDataType.Histogram, + MetricsDataType.ExponentialHistogram, +] as const; + +export type QueryableMetricKind = (typeof QUERYABLE_KINDS)[number]; + /** * Display labels for OTel metric kinds. Single source of truth so the metric * explorer's tree, its detail pane, and the chart editor's inline attribute