Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/deterministic-metric-name-listing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@hyperdx/app': patch
'@hyperdx/common-utils': patch
---

fix: metric names in the chart editor are now listed deterministically instead of sampled. The dropdown discovered names with `groupUniqArray(3000)(MetricName)`, which keeps an arbitrary subset once a metrics table holds more than 3000 distinct names — the survivors follow hash order, not name order — so metrics that exist and are actively reporting could be unselectable, with no warning and no way to search for what had been dropped. Names are now fetched with an ordered, paginated query and matched server-side, ranked so an exact match is always on the first page, and the dropdown says when the list is incomplete. Also fixes the metric list ignoring the chart's selected time range, which pinned it to the last 24 hours.
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,7 @@ export function ChartSeriesEditor({
setValue(`${namePrefix}metricType`, value)
}
metricSource={tableSource}
dateRange={dateRange}
data-testid="metric-name-selector"
error={errors?.metricName?.message}
onFocus={() => clearErrors(`${namePrefix}metricName`)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ jest.mock('@/source', () => ({
useSources: jest.fn().mockReturnValue({ data: [] }),
}));

// Records every render's props so tests can assert what the form passes down.
const metricNameSelectProps: any[] = [];

// What the stubbed explorer reports as staged when a metric is applied. The
// `mock` prefix is required for a jest.mock factory to close over it.
const mockStagedWhere: string[] = [];
Expand Down Expand Up @@ -133,6 +136,7 @@ jest.mock('@/components/MetricExplorer/MetricExplorerModal', () => ({

jest.mock('../../MetricNameSelect', () => ({
MetricNameSelect: (props: any) => {
metricNameSelectProps.push(props);
const { error, onFocus, setMetricName, metricName } = props;
const testId = props['data-testid'];
return (
Expand Down Expand Up @@ -446,6 +450,39 @@ describe('DBEditTimeChartForm - Metric Name Validation', () => {
});
});

describe('DBEditTimeChartForm - Metric name date range wiring', () => {
beforeEach(() => {
jest.clearAllMocks();
metricNameSelectProps.length = 0;
});

// Regression guard: MetricNameSelect only lists metrics that reported inside
// the range it is given, and falls back to the last 24h when the prop is
// missing. That pass-through was silently dropped once already when
// ChartSeriesEditor was split out of this file, which made any metric last
// seen over a day ago unselectable regardless of the chart's own range.
it('passes the chart date range down to the metric name select', () => {
renderComponent();

expect(metricNameSelectProps.length).toBeGreaterThan(0);
expect(metricNameSelectProps.at(-1)?.dateRange).toEqual([
new Date('2024-01-01'),
new Date('2024-01-02'),
]);
});

it('forwards an updated date range', () => {
renderComponent({
dateRange: [new Date('2024-06-01'), new Date('2024-06-08')],
});

expect(metricNameSelectProps.at(-1)?.dateRange).toEqual([
new Date('2024-06-01'),
new Date('2024-06-08'),
]);
});
});

describe('DBEditTimeChartForm - Save Button Metric Name Validation', () => {
beforeEach(() => {
jest.clearAllMocks();
Expand Down
183 changes: 99 additions & 84 deletions packages/app/src/components/MetricNameSelect.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
import { useMemo } from 'react';
import { useMemo, useState } from 'react';
import { addDays, differenceInDays, subDays } from 'date-fns';
import {
DateRange,
MetricsDataType,
TMetricSource,
} from '@hyperdx/common-utils/dist/types';
import { Select } from '@mantine/core';
import { useDebouncedValue } from '@mantine/hooks';

import { useGetKeyValues } from '@/hooks/useMetadata';
import { useGetMetricNames } from '@/hooks/useMetadata';
import { capitalizeFirstLetter } from '@/utils';

const MAX_METRIC_NAME_OPTIONS = 3000;
const SEPARATOR = ':::::::';
const SEARCH_DEBOUNCE_MS = 300;

const chartConfigByMetricType = ({
const metricNamesQueryArgs = ({
dateRange,
metricSource,
metricType,
Expand Down Expand Up @@ -42,16 +43,12 @@ const chartConfigByMetricType = ({
}

return {
// metricSource,
from: {
databaseName: metricSource.from.databaseName,
tableName: metricSource.metricTables?.[metricType] ?? '',
},
where: '',
whereLanguage: 'sql' as const,
select: '',
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 ?? '',
connection: metricSource.connection,
dateRange: _dateRange,
};
};
Expand All @@ -66,82 +63,60 @@ const chartConfigByMetricType = ({
function useMetricNames(
metricSource: TMetricSource,
dateRange?: DateRange['dateRange'],
namePattern?: string,
) {
const {
gaugeConfig,
histogramConfig,
sumConfig,
exponentialHistogramConfig,
} = useMemo(() => {
return {
gaugeConfig: chartConfigByMetricType({
dateRange,
metricSource,
metricType: MetricsDataType.Gauge,
}),
histogramConfig: chartConfigByMetricType({
dateRange,
metricSource,
metricType: MetricsDataType.Histogram,
}),
sumConfig: chartConfigByMetricType({
dateRange,
metricSource,
metricType: MetricsDataType.Sum,
}),
exponentialHistogramConfig: chartConfigByMetricType({
dateRange,
metricSource,
metricType: MetricsDataType.ExponentialHistogram,
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]);
[metricSource, dateRange],
);

const { data: gaugeMetrics, isLoading: isGaugeLoading } = useGetKeyValues({
chartConfig: gaugeConfig,
keys: ['MetricName'],
limit: MAX_METRIC_NAME_OPTIONS,
disableRowLimit: true,
});
const { data: histogramMetrics, isLoading: isHistogramLoading } =
useGetKeyValues({
chartConfig: histogramConfig,
keys: ['MetricName'],
limit: MAX_METRIC_NAME_OPTIONS,
disableRowLimit: true,
});
const { data: sumMetrics, isLoading: isSumLoading } = useGetKeyValues({
chartConfig: sumConfig,
keys: ['MetricName'],
limit: MAX_METRIC_NAME_OPTIONS,
disableRowLimit: true,
const gauge = useGetMetricNames({ ...gaugeArgs, namePattern });
const histogram = useGetMetricNames({ ...histogramArgs, namePattern });
const sum = useGetMetricNames({ ...sumArgs, namePattern });
const exponentialHistogram = useGetMetricNames({
...exponentialHistogramArgs,
Comment thread
greptile-apps[bot] marked this conversation as resolved.
namePattern,
});
const {
data: exponentialHistogramMetrics,
isLoading: isExponentialHistogramLoading,
} = useGetKeyValues(
{
chartConfig: exponentialHistogramConfig,
keys: ['MetricName'],
limit: MAX_METRIC_NAME_OPTIONS,
disableRowLimit: true,
},
{
enabled:
!!metricSource.metricTables?.[MetricsDataType.ExponentialHistogram],
},
);

return {
gaugeMetrics: gaugeMetrics?.[0].value,
histogramMetrics: histogramMetrics?.[0].value,
sumMetrics: sumMetrics?.[0].value,
exponentialHistogramMetrics: exponentialHistogramMetrics?.[0].value,
isLoading:
isGaugeLoading ||
isHistogramLoading ||
isSumLoading ||
isExponentialHistogramLoading,
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,
),
};
}

Expand Down Expand Up @@ -195,6 +170,7 @@ export function MetricNameSelect({
isLoading,
isError,
metricSource,
dateRange,
error,
onFocus,
'data-testid': dataTestId,
Expand All @@ -206,16 +182,39 @@ export function MetricNameSelect({
isLoading?: boolean;
isError?: boolean;
metricSource: TMetricSource;
dateRange?: DateRange['dateRange'];
error?: string;
onFocus?: () => void;
'data-testid'?: string;
}) {
const [searchValue, setSearchValue] = useState('');

// Mantine mirrors the selected option's *label* into a searchable Select's
// 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.
const selectedLabel = metricName
? `${metricName} (${capitalizeFirstLetter(metricType)})`
: '';
const trimmedSearch = searchValue.trim();
const activeSearch =
trimmedSearch.toLowerCase() === selectedLabel.toLowerCase()
? ''
: trimmedSearch;

const [debouncedSearch] = useDebouncedValue(activeSearch, SEARCH_DEBOUNCE_MS);

const {
gaugeMetrics,
histogramMetrics,
sumMetrics,
exponentialHistogramMetrics,
} = useMetricNames(metricSource);
isTruncated,
hasError,
} = useMetricNames(metricSource, dateRange, debouncedSearch);

const options = useMemo(() => {
return getMetricOptions(
Expand Down Expand Up @@ -251,6 +250,22 @@ export function MetricNameSelect({
}
data={options}
limit={100}
searchValue={searchValue}
onSearchChange={setSearchValue}
// Start each browse from an empty input so the full list is offered and the
// mirrored label cannot be partially deleted and searched for; restore it
// on close so a collapsed control still shows its selection.
onDropdownOpen={() => setSearchValue('')}
onDropdownClose={() => setSearchValue(selectedLabel)}
// Reported in the description rather than the `error` slot, which belongs
// to form validation for this field.
description={
hasError
? 'Some metrics could not be loaded'
: isTruncated
? 'Too many metrics to list — type to search'
: undefined
}
comboboxProps={{
position: 'bottom-start',
width: 'auto',
Expand Down
Loading
Loading