From ef5308fefd9f38da0826a7a8546939dff26af3fb Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Tue, 1 Sep 2026 10:34:59 -0400 Subject: [PATCH 1/6] fix(app): explain unmatched metric names and stop the notice shifting the field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The metric picker's truncation and load-failure notices rendered above the input, so the input dropped ~20px when one appeared while the browse-metrics button beside it stayed put. Both notices now render below the input, and are short enough to fit the tile editor's column without wrapping. A search that matched nothing was silently invisible: without `nothingFoundMessage` Mantine sets `hiddenWhenEmpty`, so the whole dropdown was display:none. It now says why, and offers the searched name as a selectable option — the catalog covers only the most recent 3 days of the chart's range, so a metric that stopped reporting could not be charted by name at all. `metricNamesQueryArgs` and `useMetricNames` move to their own hook file to keep the component under the 300-line guideline. --- .changeset/metric-name-select-not-found.md | 5 + .../app/src/components/MetricNameSelect.tsx | 148 +++++------------- .../__tests__/MetricNameSelectSearch.test.tsx | 103 +++++++++++- packages/app/src/hooks/useMetricNames.ts | 122 +++++++++++++++ 4 files changed, 261 insertions(+), 117 deletions(-) create mode 100644 .changeset/metric-name-select-not-found.md create mode 100644 packages/app/src/hooks/useMetricNames.ts 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/MetricNameSelect.tsx b/packages/app/src/components/MetricNameSelect.tsx index ddf31cb617..4f59f45a93 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,118 +7,12 @@ import { import { Select } from '@mantine/core'; import { useDebouncedValue } from '@mantine/hooks'; -import { useGetMetricNames } from '@/hooks/useMetadata'; +import { useMetricNames } from '@/hooks/useMetricNames'; import { capitalizeFirstLetter } from '@/utils'; 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, - ), - }; -} - export function getMetricOptions( gaugeMetrics: string[] | undefined, histogramMetrics: string[] | undefined, @@ -214,10 +107,12 @@ export function MetricNameSelect({ exponentialHistogramMetrics, isTruncated, hasError, + hasNoMatches, + isLoading: isSearching, } = useMetricNames(metricSource, dateRange, debouncedSearch); const options = useMemo(() => { - return getMetricOptions( + const metricOptions = getMetricOptions( gaugeMetrics, histogramMetrics, sumMetrics, @@ -225,6 +120,21 @@ export function MetricNameSelect({ 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 the 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 this one. + if (debouncedSearch && hasNoMatches) { + metricOptions.push({ + value: `${debouncedSearch}${SEPARATOR}${metricType}`, + label: `Use "${debouncedSearch}" (no recent data)`, + }); + } + return metricOptions; }, [ gaugeMetrics, histogramMetrics, @@ -232,6 +142,8 @@ export function MetricNameSelect({ exponentialHistogramMetrics, metricName, metricType, + debouncedSearch, + hasNoMatches, ]); const currentValue = @@ -257,13 +169,25 @@ 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={ + isSearching + ? 'Searching…' + : hasError + ? 'Some metrics failed to load' + : '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__/MetricNameSelectSearch.test.tsx b/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx index 2a2aac2642..78d710a249 100644 --- a/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx +++ b/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx @@ -155,9 +155,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 +165,103 @@ 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( + 'Use "chi_clickhouse_metric_SystemErrors" (no recent data)', + ); + await userEvent.click(offer); + + expect(setMetricName).toHaveBeenCalledWith( + 'chi_clickhouse_metric_SystemErrors', + ); + expect(setMetricType).toHaveBeenCalledWith(MetricsDataType.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 loading or broken + // 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('Use "zzz" (no recent data)'), + ).not.toBeInTheDocument(); + }); + + it('offers nothing when a 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'); + await new Promise(resolve => setTimeout(resolve, DEBOUNCE_SETTLE_MS)); + + expect( + screen.queryByText('Use "zzz" (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('Use "up" (no recent data)'), + ).not.toBeInTheDocument(); }); }); diff --git a/packages/app/src/hooks/useMetricNames.ts b/packages/app/src/hooks/useMetricNames.ts new file mode 100644 index 0000000000..90e0dde237 --- /dev/null +++ b/packages/app/src/hooks/useMetricNames.ts @@ -0,0 +1,122 @@ +import { useMemo } from 'react'; +import { addDays, differenceInDays, subDays } from 'date-fns'; +import { + DateRange, + MetricsDataType, + TMetricSource, +} from '@hyperdx/common-utils/dist/types'; + +import { useGetMetricNames } from '@/hooks/useMetadata'; + +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. + */ +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]; + + return { + gaugeMetrics: gauge.data?.names, + histogramMetrics: histogram.data?.names, + sumMetrics: sum.data?.names, + exponentialHistogramMetrics: exponentialHistogram.data?.names, + 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), + isLoading: queries.some(query => query.isLoading), + // Only when every enabled kind has actually answered with nothing. Loading + // and error must not read as "no such metric", or a slow or broken query + // would invite the caller to commit a name that does exist. `isFetching` is + // part of that guard because these queries keep the previous page's data + // while a new pattern is in flight. + hasNoMatches: + queries.some(query => query.data !== undefined) && + queries.every(query => (query.data?.names?.length ?? 0) === 0) && + !queries.some( + query => query.isLoading || query.isFetching || query.isError, + ), + }; +} From 61e5ed681897c4ac24249e502c02dab6c32170b3 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Tue, 1 Sep 2026 11:03:44 -0400 Subject: [PATCH 2/6] fix(app): stop the typed metric-name option colliding with the saved one Committing the "use this name" offer crashed the chart editor. The offer and `getMetricOptions`' existing "assume it exists" branch build the same `name:::::::type` value, and for one debounce interval after the click both fire: `metricName` already holds the typed name while the debounced search still does too. Mantine's `validateOptions` throws on a duplicate option value and it is not gated on NODE_ENV, so the error boundary swallowed the whole editor. Skip the offer when its value is already an option. The existing test missed this because `setMetricName` was a bare mock, so no committed name ever came back as a prop. Added a stateful harness that holds `metricName` the way the form does, which reproduces the throw. --- .../app/src/components/MetricNameSelect.tsx | 15 ++++++- .../__tests__/MetricNameSelectSearch.test.tsx | 44 +++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/packages/app/src/components/MetricNameSelect.tsx b/packages/app/src/components/MetricNameSelect.tsx index 4f59f45a93..110ab43fad 100644 --- a/packages/app/src/components/MetricNameSelect.tsx +++ b/packages/app/src/components/MetricNameSelect.tsx @@ -128,9 +128,20 @@ export function MetricNameSelect({ // search, so the 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 this one. - if (debouncedSearch && hasNoMatches) { + const typedValue = `${debouncedSearch}${SEPARATOR}${metricType}`; + // Skipped when that value is already an option, or Mantine throws on the + // duplicate and takes the whole editor down. Committing this offer puts the + // name in `metricName`, which `getMetricOptions` then synthesizes an option + // for, while the debounced search still holds the same name for one more + // interval — so the collision is the normal path through here, not an edge + // case. + if ( + debouncedSearch && + hasNoMatches && + !metricOptions.some(option => option.value === typedValue) + ) { metricOptions.push({ - value: `${debouncedSearch}${SEPARATOR}${metricType}`, + value: typedValue, label: `Use "${debouncedSearch}" (no recent data)`, }); } diff --git a/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx b/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx index 78d710a249..54896ca723 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) => ({ @@ -207,6 +228,29 @@ describe('MetricNameSelect', () => { expect(setMetricType).toHaveBeenCalledWith(MetricsDataType.Gauge); }); + // 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('Use "chc_" (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(); From 4a4f716343e37bd1353ace9aa980ed8fc8b5c3d8 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Tue, 1 Sep 2026 13:57:20 -0400 Subject: [PATCH 3/6] fix(app): left-align the gauge Delta checkbox with the metric field Right-aligning it in the metric column parked it under the browse button, attached to nothing. Moving the truncation notice below the input pushed it further down and made the isolation obvious. It now shares a left edge with the metric select and its notice, so it reads as a modifier of that field. The `justify="end"` came from a bulk refactor (#2026), not a layout decision. --- .../DBEditTimeChartForm/ChartSeriesEditor.tsx | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) 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' && ( - - - + )} )} From 1d8af03ab16a4ff4c9db5867464dd6646bd3a76e Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Tue, 1 Sep 2026 15:06:09 -0400 Subject: [PATCH 4/6] fix(app): correct the in-flight and partial-failure states of the metric picker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings on this PR. The `Searching…` branch was dead after first mount. It hung off the hook's `isLoading`, but these queries set `placeholderData: keepPreviousData`, so a key change resolves to `success` with the previous pattern's names and `isPending` is false. Those retained names are then filtered out by the new search text, so the dropdown confidently claimed `No metrics reported recently` for the whole debounce plus round trip. Return `isFetching` and drive the branch off that. One permanently broken kind table disabled the "use this name" offer outright: `hasNoMatches` required no query to be in error, and a misconfigured table errors on every pattern and is never retried. That restored the dead end this PR set out to remove. An errored kind no longer disqualifies the offer; the case where nothing answered at all is already covered by the `data !== undefined` check, and the description still carries the partial-failure warning. The relocated date clamp was a line-for-line copy of the exported, unit-tested `clampCatalogDateRange`. Call it instead. --- .../app/src/components/MetricNameSelect.tsx | 2 +- .../__tests__/MetricNameSelectSearch.test.tsx | 42 +++++++++++++++++-- packages/app/src/hooks/useMetricNames.ts | 40 ++++++------------ 3 files changed, 53 insertions(+), 31 deletions(-) diff --git a/packages/app/src/components/MetricNameSelect.tsx b/packages/app/src/components/MetricNameSelect.tsx index 110ab43fad..90e8ec8212 100644 --- a/packages/app/src/components/MetricNameSelect.tsx +++ b/packages/app/src/components/MetricNameSelect.tsx @@ -108,7 +108,7 @@ export function MetricNameSelect({ isTruncated, hasError, hasNoMatches, - isLoading: isSearching, + isFetching: isSearching, } = useMetricNames(metricSource, dateRange, debouncedSearch); const options = useMemo(() => { diff --git a/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx b/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx index 54896ca723..9e0042612b 100644 --- a/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx +++ b/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx @@ -262,8 +262,8 @@ describe('MetricNameSelect', () => { ).toBeInTheDocument(); }); - // Claiming a name does not exist while a kind is still loading or broken - // would invite the user to commit a metric that is actually available. + // 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' @@ -280,7 +280,29 @@ describe('MetricNameSelect', () => { ).not.toBeInTheDocument(); }); - it('offers nothing when a kind failed to load', async () => { + // 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(); + }); + + // 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 } @@ -288,6 +310,20 @@ describe('MetricNameSelect', () => { ); renderSelect(); + await userEvent.type(screen.getByTestId('metric-name-selector'), 'zzz'); + + expect( + await screen.findByText('Use "zzz" (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)); diff --git a/packages/app/src/hooks/useMetricNames.ts b/packages/app/src/hooks/useMetricNames.ts index 90e0dde237..8e320e8cf3 100644 --- a/packages/app/src/hooks/useMetricNames.ts +++ b/packages/app/src/hooks/useMetricNames.ts @@ -1,5 +1,4 @@ import { useMemo } from 'react'; -import { addDays, differenceInDays, subDays } from 'date-fns'; import { DateRange, MetricsDataType, @@ -7,6 +6,7 @@ import { } from '@hyperdx/common-utils/dist/types'; import { useGetMetricNames } from '@/hooks/useMetadata'; +import { clampCatalogDateRange } from '@/hooks/useMetricCatalog'; const metricNamesQueryArgs = ({ dateRange, @@ -19,22 +19,6 @@ const metricNamesQueryArgs = ({ }) => { // 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, @@ -43,7 +27,7 @@ const metricNamesQueryArgs = ({ tableName: metricSource.metricTables?.[metricType] ?? '', connectionId: metricSource.connection, timestampValueExpression: metricSource.timestampValueExpression ?? '', - dateRange: _dateRange, + dateRange: clampCatalogDateRange(dateRange, now), }; }; @@ -106,17 +90,19 @@ export function useMetricNames( // 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), - isLoading: queries.some(query => query.isLoading), - // Only when every enabled kind has actually answered with nothing. Loading - // and error must not read as "no such metric", or a slow or broken query - // would invite the caller to commit a name that does exist. `isFetching` is - // part of that guard because these queries keep the previous page's data - // while a new pattern is in flight. + // 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.isLoading || query.isFetching || query.isError, - ), + !queries.some(query => query.isFetching), }; } From 16511935787493e536f7ad0aaf075e0d809d0410 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 2 Sep 2026 09:23:28 -0400 Subject: [PATCH 5/6] feat(app): let the user pick the kind when committing an unmatched metric name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The kind decides which table a series is read from, and a pasted name carries none. The single offer filed the name under the series' current metricType — whatever the last found metric happened to be, or gauge by default — so pasting a Sum metric queried the gauge table and charted nothing, with no way to correct it. Offer one option per kind the source has a table for instead: nanana (Gauge, no recent data) nanana (Sum, no recent data) Kinds with no table are skipped, since committing the name under one could never resolve. Summary is not offered at all — this select has no query behind it. The duplicate-value guard now runs per option, so the kind already held in `metricName` drops out rather than colliding. Kind display names moved into one table, replacing a third hardcoding of the same four strings. --- .../app/src/components/MetricNameSelect.tsx | 105 +++++++++++------- .../__tests__/MetricNameSelectSearch.test.tsx | 54 ++++++++- 2 files changed, 114 insertions(+), 45 deletions(-) diff --git a/packages/app/src/components/MetricNameSelect.tsx b/packages/app/src/components/MetricNameSelect.tsx index 90e8ec8212..7af52f7ba4 100644 --- a/packages/app/src/components/MetricNameSelect.tsx +++ b/packages/app/src/components/MetricNameSelect.tsx @@ -13,6 +13,28 @@ import { capitalizeFirstLetter } from '@/utils'; const SEPARATOR = ':::::::'; const SEARCH_DEBOUNCE_MS = 300; +/** The kinds this select queries, in the order they are offered. Summary is + * absent because there is no query behind it. */ +const QUERYABLE_METRIC_TYPES = [ + MetricsDataType.Gauge, + MetricsDataType.Histogram, + MetricsDataType.Sum, + MetricsDataType.ExponentialHistogram, +] as const; + +const METRIC_TYPE_LABELS: Partial> = { + [MetricsDataType.Gauge]: 'Gauge', + [MetricsDataType.Histogram]: 'Histogram', + [MetricsDataType.Sum]: 'Sum', + [MetricsDataType.ExponentialHistogram]: 'Exponential Histogram', +}; + +const metricTypeLabel = (metricType: MetricsDataType) => + METRIC_TYPE_LABELS[metricType] ?? capitalizeFirstLetter(metricType); + +const metricOptionValue = (metricName: string, metricType: MetricsDataType) => + `${metricName}${SEPARATOR}${metricType}`; + export function getMetricOptions( gaugeMetrics: string[] | undefined, histogramMetrics: string[] | undefined, @@ -21,35 +43,30 @@ export function getMetricOptions( 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 namesByType: Partial> = { + [MetricsDataType.Gauge]: gaugeMetrics, + [MetricsDataType.Histogram]: histogramMetrics, + [MetricsDataType.Sum]: sumMetrics, + [MetricsDataType.ExponentialHistogram]: exponentialHistogramMetrics, + }; + const metricsFromQuery = QUERYABLE_METRIC_TYPES.flatMap( + type => + namesByType[type]?.map(metric => ({ + value: metricOptionValue(metric, type), + label: `${metric} (${metricTypeLabel(type)})`, + })) ?? [], + ); // 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} (${metricTypeLabel(metricType)})`, }); } return metricsFromQuery; @@ -125,25 +142,34 @@ export function MetricNameSelect({ // 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 the offer appears only once the query behind it has answered, + // 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 this one. - const typedValue = `${debouncedSearch}${SEPARATOR}${metricType}`; - // Skipped when that value is already an option, or Mantine throws on the - // duplicate and takes the whole editor down. Committing this offer puts the - // name in `metricName`, which `getMetricOptions` then synthesizes an option - // for, while the debounced search still holds the same name for one more - // interval — so the collision is the normal path through here, not an edge - // case. - if ( - debouncedSearch && - hasNoMatches && - !metricOptions.some(option => option.value === typedValue) - ) { - metricOptions.push({ - value: typedValue, - label: `Use "${debouncedSearch}" (no recent data)`, - }); + // 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 type of QUERYABLE_METRIC_TYPES) { + // A kind with no table is not queryable on this source, so committing + // the name under it could never resolve. + if (!metricSource.metricTables?.[type]) { + continue; + } + const value = metricOptionValue(debouncedSearch, type); + // 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} (${metricTypeLabel(type)}, no recent data)`, + }); + } } return metricOptions; }, [ @@ -155,6 +181,7 @@ export function MetricNameSelect({ metricType, debouncedSearch, hasNoMatches, + metricSource, ]); const currentValue = diff --git a/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx b/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx index 9e0042612b..fd67a86de0 100644 --- a/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx +++ b/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx @@ -218,7 +218,7 @@ describe('MetricNameSelect', () => { ); const offer = await screen.findByText( - 'Use "chi_clickhouse_metric_SystemErrors" (no recent data)', + 'chi_clickhouse_metric_SystemErrors (Gauge, no recent data)', ); await userEvent.click(offer); @@ -228,6 +228,48 @@ describe('MetricNameSelect', () => { 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 @@ -240,7 +282,7 @@ describe('MetricNameSelect', () => { const input = screen.getByTestId('metric-name-selector'); await userEvent.type(input, 'chc_'); await userEvent.click( - await screen.findByText('Use "chc_" (no recent data)'), + await screen.findByText('chc_ (Gauge, no recent data)'), ); await waitFor(() => expect(input).toHaveValue('chc_ (Gauge)')); @@ -276,7 +318,7 @@ describe('MetricNameSelect', () => { await new Promise(resolve => setTimeout(resolve, DEBOUNCE_SETTLE_MS)); expect( - screen.queryByText('Use "zzz" (no recent data)'), + screen.queryByText('zzz (Gauge, no recent data)'), ).not.toBeInTheDocument(); }); @@ -313,7 +355,7 @@ describe('MetricNameSelect', () => { await userEvent.type(screen.getByTestId('metric-name-selector'), 'zzz'); expect( - await screen.findByText('Use "zzz" (no recent data)'), + await screen.findByText('zzz (Gauge, no recent data)'), ).toBeInTheDocument(); }); @@ -328,7 +370,7 @@ describe('MetricNameSelect', () => { await new Promise(resolve => setTimeout(resolve, DEBOUNCE_SETTLE_MS)); expect( - screen.queryByText('Use "zzz" (no recent data)'), + screen.queryByText('zzz (Gauge, no recent data)'), ).not.toBeInTheDocument(); }); }); @@ -341,7 +383,7 @@ describe('MetricNameSelect', () => { await new Promise(resolve => setTimeout(resolve, DEBOUNCE_SETTLE_MS)); expect( - screen.queryByText('Use "up" (no recent data)'), + screen.queryByText('up (Gauge, no recent data)'), ).not.toBeInTheDocument(); }); }); From 72faf3c49b911b3360ca1109335d0f089f0e0161 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 2 Sep 2026 09:46:11 -0400 Subject: [PATCH 6/6] refactor(app): share the queryable metric kind list and its labels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings. `QUERYABLE_METRIC_TYPES` duplicated the list `useMetricCatalog` already had one import away, and only drove rendering — the four `useGetMetricNames` calls behind it were hardcoded, so a fifth kind would have produced labels and offers for something never queried. `QUERYABLE_KINDS` moves to `utils/metricKinds` beside the labels, and `useMetricNames` now returns names keyed by `QueryableMetricKind`. That type is a union of exactly the listed kinds, so adding one stops the record literal compiling until its query is wired up. `getMetricOptions` takes that record rather than four positional lists. Kind labels now come from `metricKindLabel`, the documented single source of truth, replacing a third hand-maintained copy. Exponential histogram renders as `Exp. histogram`, which also fixes the Title Case the previous label broke. `selectedLabel` routes through the same helper, so a selection no longer renders under one spelling and gets rewritten to another on dropdown close. Grouping order follows `QUERYABLE_KINDS`: gauge, sum, histogram, exponential histogram. Sum moves ahead of histogram. `nothingFoundMessage` claimed "No metrics reported recently" during the ~300ms before the debounce fires, when nothing is in flight and the visible options still answer the previous keystroke. A pending debounce now counts as searching, and an unmatched search says so rather than making a claim about the source. --- .../app/src/components/MetricNameSelect.tsx | 92 +++++-------- .../__tests__/MetricNameSelect.test.ts | 126 +++++++++--------- .../__tests__/MetricNameSelectSearch.test.tsx | 48 +++++++ packages/app/src/hooks/useMetricCatalog.ts | 10 +- packages/app/src/hooks/useMetricNames.ts | 16 ++- packages/app/src/utils/metricKinds.ts | 18 +++ 6 files changed, 169 insertions(+), 141 deletions(-) diff --git a/packages/app/src/components/MetricNameSelect.tsx b/packages/app/src/components/MetricNameSelect.tsx index 7af52f7ba4..e5626c3380 100644 --- a/packages/app/src/components/MetricNameSelect.tsx +++ b/packages/app/src/components/MetricNameSelect.tsx @@ -8,52 +8,28 @@ import { Select } from '@mantine/core'; import { useDebouncedValue } from '@mantine/hooks'; import { useMetricNames } from '@/hooks/useMetricNames'; -import { capitalizeFirstLetter } from '@/utils'; +import { + metricKindLabel, + QUERYABLE_KINDS, + type QueryableMetricKind, +} from '@/utils/metricKinds'; const SEPARATOR = ':::::::'; const SEARCH_DEBOUNCE_MS = 300; -/** The kinds this select queries, in the order they are offered. Summary is - * absent because there is no query behind it. */ -const QUERYABLE_METRIC_TYPES = [ - MetricsDataType.Gauge, - MetricsDataType.Histogram, - MetricsDataType.Sum, - MetricsDataType.ExponentialHistogram, -] as const; - -const METRIC_TYPE_LABELS: Partial> = { - [MetricsDataType.Gauge]: 'Gauge', - [MetricsDataType.Histogram]: 'Histogram', - [MetricsDataType.Sum]: 'Sum', - [MetricsDataType.ExponentialHistogram]: 'Exponential Histogram', -}; - -const metricTypeLabel = (metricType: MetricsDataType) => - METRIC_TYPE_LABELS[metricType] ?? capitalizeFirstLetter(metricType); - 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 namesByType: Partial> = { - [MetricsDataType.Gauge]: gaugeMetrics, - [MetricsDataType.Histogram]: histogramMetrics, - [MetricsDataType.Sum]: sumMetrics, - [MetricsDataType.ExponentialHistogram]: exponentialHistogramMetrics, - }; - const metricsFromQuery = QUERYABLE_METRIC_TYPES.flatMap( - type => - namesByType[type]?.map(metric => ({ - value: metricOptionValue(metric, type), - label: `${metric} (${metricTypeLabel(type)})`, + 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 @@ -66,7 +42,7 @@ export function getMetricOptions( ) { metricsFromQuery.push({ value: metricOptionValue(metricName, metricType), - label: `${metricName} (${metricTypeLabel(metricType)})`, + label: `${metricName} (${metricKindLabel(metricType)})`, }); } return metricsFromQuery; @@ -103,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 = @@ -116,12 +92,12 @@ 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, @@ -129,14 +105,7 @@ export function MetricNameSelect({ } = useMetricNames(metricSource, dateRange, debouncedSearch); const options = useMemo(() => { - const metricOptions = 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 @@ -151,13 +120,13 @@ export function MetricNameSelect({ // last selection happened to use would query the wrong table and chart // nothing, with no way to correct it. if (debouncedSearch && hasNoMatches) { - for (const type of QUERYABLE_METRIC_TYPES) { + 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?.[type]) { + if (!metricSource.metricTables?.[kind]) { continue; } - const value = metricOptionValue(debouncedSearch, type); + 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 @@ -167,16 +136,13 @@ export function MetricNameSelect({ } metricOptions.push({ value, - label: `${debouncedSearch} (${metricTypeLabel(type)}, no recent data)`, + label: `${debouncedSearch} (${metricKindLabel(kind)}, no recent data)`, }); } } return metricOptions; }, [ - gaugeMetrics, - histogramMetrics, - sumMetrics, - exponentialHistogramMetrics, + namesByKind, metricName, metricType, debouncedSearch, @@ -210,11 +176,13 @@ export function MetricNameSelect({ // Without a message Mantine sets `hiddenWhenEmpty`, so a search matching // nothing hides the whole dropdown and the field just looks broken. nothingFoundMessage={ - isSearching + isSearchPending || isSearching ? 'Searching…' : hasError ? 'Some metrics failed to load' - : 'No metrics reported recently' + : 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. Kept below the input: it appears 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 fd67a86de0..e12dac5cb0 100644 --- a/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx +++ b/packages/app/src/components/__tests__/MetricNameSelectSearch.test.tsx @@ -341,6 +341,54 @@ describe('MetricNameSelect', () => { ).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. 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 index 8e320e8cf3..19db3204e7 100644 --- a/packages/app/src/hooks/useMetricNames.ts +++ b/packages/app/src/hooks/useMetricNames.ts @@ -7,6 +7,7 @@ import { import { useGetMetricNames } from '@/hooks/useMetadata'; import { clampCatalogDateRange } from '@/hooks/useMetricCatalog'; +import type { QueryableMetricKind } from '@/utils/metricKinds'; const metricNamesQueryArgs = ({ dateRange, @@ -80,11 +81,18 @@ export function useMetricNames( 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 { - gaugeMetrics: gauge.data?.names, - histogramMetrics: histogram.data?.names, - sumMetrics: sum.data?.names, - exponentialHistogramMetrics: exponentialHistogram.data?.names, + 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 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