diff --git a/.changeset/warm-berries-wash.md b/.changeset/warm-berries-wash.md new file mode 100644 index 000000000..ac85854fb --- /dev/null +++ b/.changeset/warm-berries-wash.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/api': patch +'@hyperdx/app': patch +--- + +refactor + perf: decouple and performance opt metrics tags endpoints diff --git a/packages/api/src/clickhouse/index.ts b/packages/api/src/clickhouse/index.ts index 4de5c9bf0..408532117 100644 --- a/packages/api/src/clickhouse/index.ts +++ b/packages/api/src/clickhouse/index.ts @@ -593,7 +593,8 @@ export const getCHServerMetrics = async () => { }, {}); }; -export const getMetricsTags = async ({ +// ONLY USED IN EXTERNAL API +export const getMetricsTagsDEPRECATED = async ({ teamId, startTime, endTime, @@ -644,6 +645,150 @@ export const getMetricsTags = async ({ unit: string; }> >(); + logger.info({ + message: 'getMetricsTagsDEPRECATED', + query, + took: Date.now() - ts, + }); + return result; +}; + +export const getMetricsNames = async ({ + teamId, + startTime, + endTime, +}: { + teamId: string; + startTime: number; // unix in ms + endTime: number; // unix in ms +}) => { + const tableName = `default.${TableName.Metric}`; + // WARNING: _created_at is ciritcal for the query to work efficiently (by using partitioning) + // TODO: remove 'data_type' in the name field + const query = SqlString.format( + ` + SELECT + any(is_delta) as is_delta, + any(is_monotonic) as is_monotonic, + any(unit) as unit, + data_type, + format('{} - {}', name, data_type) as name + FROM ?? + WHERE (_timestamp_sort_key >= ? AND _timestamp_sort_key < ?) + AND (_created_at >= fromUnixTimestamp64Milli(?) AND _created_at < fromUnixTimestamp64Milli(?)) + GROUP BY name, data_type + ORDER BY name + `, + [ + tableName, + msToBigIntNs(startTime), + msToBigIntNs(endTime), + startTime, + endTime, + ], + ); + const ts = Date.now(); + const rows = await client.query({ + query, + format: 'JSON', + clickhouse_settings: { + additional_table_filters: buildMetricStreamAdditionalFilters( + null, + teamId, + ), + }, + }); + const result = await rows.json< + ResponseJSON<{ + data_type: string; + is_delta: boolean; + is_monotonic: boolean; + name: string; + unit: string; + }> + >(); + logger.info({ + message: 'getMetricsNames', + query, + took: Date.now() - ts, + }); + return result; +}; + +export const getMetricsTags = async ({ + endTime, + metrics, + startTime, + teamId, +}: { + endTime: number; // unix in ms + metrics: { + name: string; + dataType: MetricsDataType; + }[]; + startTime: number; // unix in ms + teamId: string; +}) => { + const tableName = `default.${TableName.Metric}`; + // TODO: theoretically, we should be able to traverse each tag's keys and values + // and intersect them to get the final result. Currently this is done on the client side. + const unions = metrics + .map(m => + SqlString.format( + ` + SELECT + groupUniqArray(_string_attributes) AS tags, + data_type, + format('{} - {}', name, data_type) AS combined_name + FROM ?? + WHERE name = ? + AND data_type = ? + AND (_timestamp_sort_key >= ? AND _timestamp_sort_key < ?) + AND (_created_at >= fromUnixTimestamp64Milli(?) AND _created_at < fromUnixTimestamp64Milli(?)) + GROUP BY name, data_type + `, + [ + tableName, + m.name, + m.dataType, + msToBigIntNs(startTime), + msToBigIntNs(endTime), + startTime, + endTime, + ], + ), + ) + .join(' UNION DISTINCT '); + const query = SqlString.format( + ` + SELECT + combined_name AS name, + data_type, + tags + FROM (?) + ORDER BY name + `, + [SqlString.raw(unions)], + ); + + const ts = Date.now(); + const rows = await client.query({ + query, + format: 'JSON', + clickhouse_settings: { + additional_table_filters: buildMetricStreamAdditionalFilters( + null, + teamId, + ), + }, + }); + const result = await rows.json< + ResponseJSON<{ + name: string; + data_type: string; + tags: Record[]; + }> + >(); logger.info({ message: 'getMetricsTags', query, diff --git a/packages/api/src/routers/api/__tests__/metrics.test.ts b/packages/api/src/routers/api/__tests__/metrics.test.ts index 8019c1249..efeaaa058 100644 --- a/packages/api/src/routers/api/__tests__/metrics.test.ts +++ b/packages/api/src/routers/api/__tests__/metrics.test.ts @@ -1,7 +1,13 @@ +import ms from 'ms'; + import * as clickhouse from '@/clickhouse'; import { buildMetricSeries, getLoggedInAgent, getServer } from '@/fixtures'; describe('metrics router', () => { + const now = Date.now(); + let agent; + let teamId; + const server = getServer(); beforeAll(async () => { @@ -16,10 +22,10 @@ describe('metrics router', () => { await server.stop(); }); - it('GET /metrics/tags', async () => { - const { agent, team } = await getLoggedInAgent(server); - - const now = Date.now(); + beforeEach(async () => { + const { agent: _agent, team } = await getLoggedInAgent(server); + agent = _agent; + teamId = team.id; await clickhouse.bulkInsertTeamMetricStream( buildMetricSeries({ name: 'test.cpu', @@ -28,7 +34,7 @@ describe('metrics router', () => { is_monotonic: false, is_delta: false, unit: 'Percent', - points: [{ value: 1, timestamp: now }], + points: [{ value: 1, timestamp: now - ms('1d') }], team_id: team.id, }), ); @@ -40,30 +46,121 @@ describe('metrics router', () => { is_monotonic: false, is_delta: false, unit: 'Percent', - points: [{ value: 1, timestamp: now }], + points: [{ value: 1, timestamp: now - ms('1d') }], team_id: team.id, }), ); - - const results = await agent.get('/metrics/tags').expect(200); - expect(results.body.data).toEqual([ - { - is_delta: false, + await clickhouse.bulkInsertTeamMetricStream( + buildMetricSeries({ + name: 'test.cpu2', + tags: { host: 'host2', foo2: 'bar2' }, + data_type: clickhouse.MetricsDataType.Gauge, is_monotonic: false, + is_delta: false, unit: 'Percent', - data_type: 'Gauge', - name: 'test.cpu - Gauge', - tags: [ + points: [{ value: 1, timestamp: now - ms('1d') }], + team_id: team.id, + }), + ); + }); + + it('GET /metrics/names', async () => { + const names = await agent.get('/metrics/names').expect(200); + expect(names.body.data).toMatchInlineSnapshot(` +Array [ + Object { + "data_type": "Gauge", + "is_delta": false, + "is_monotonic": false, + "name": "test.cpu - Gauge", + "unit": "Percent", + }, + Object { + "data_type": "Gauge", + "is_delta": false, + "is_monotonic": false, + "name": "test.cpu2 - Gauge", + "unit": "Percent", + }, +] +`); + }); + + it('GET /metrics/tags - single metric', async () => { + const tags = await agent + .post('/metrics/tags') + .send({ + metrics: [ { - foo2: 'bar2', - host: 'host2', + name: 'test.cpu', + dataType: clickhouse.MetricsDataType.Gauge, + }, + ], + }) + .expect(200); + expect(tags.body.data).toMatchInlineSnapshot(` +Array [ + Object { + "data_type": "Gauge", + "name": "test.cpu - Gauge", + "tags": Array [ + Object { + "foo2": "bar2", + "host": "host2", + }, + Object { + "foo": "bar", + "host": "host1", + }, + ], + }, +] +`); + }); + + it('GET /metrics/tags - multi metrics', async () => { + const tags = await agent + .post('/metrics/tags') + .send({ + metrics: [ + { + name: 'test.cpu', + dataType: clickhouse.MetricsDataType.Gauge, }, { - foo: 'bar', - host: 'host1', + name: 'test.cpu2', + dataType: clickhouse.MetricsDataType.Gauge, }, ], + }) + .expect(200); + expect(tags.body.data).toMatchInlineSnapshot(` +Array [ + Object { + "data_type": "Gauge", + "name": "test.cpu - Gauge", + "tags": Array [ + Object { + "foo2": "bar2", + "host": "host2", + }, + Object { + "foo": "bar", + "host": "host1", + }, + ], + }, + Object { + "data_type": "Gauge", + "name": "test.cpu2 - Gauge", + "tags": Array [ + Object { + "foo2": "bar2", + "host": "host2", }, - ]); + ], + }, +] +`); }); }); diff --git a/packages/api/src/routers/api/metrics.ts b/packages/api/src/routers/api/metrics.ts index 9060cbf17..ddaa25bbd 100644 --- a/packages/api/src/routers/api/metrics.ts +++ b/packages/api/src/routers/api/metrics.ts @@ -9,7 +9,7 @@ import { SimpleCache } from '@/utils/redis'; const router = express.Router(); -router.get('/tags', async (req, res, next) => { +router.get('/names', async (req, res, next) => { try { const teamId = req.user?.team; if (teamId == null) { @@ -18,12 +18,12 @@ router.get('/tags', async (req, res, next) => { const nowInMs = Date.now(); const simpleCache = new SimpleCache< - Awaited> + Awaited> >( - `metrics-tags-${teamId}`, + `metrics-names-${teamId}`, ms('10m'), () => - clickhouse.getMetricsTags({ + clickhouse.getMetricsNames({ // FIXME: fix it 5 days ago for now startTime: nowInMs - ms('5d'), endTime: nowInMs, @@ -45,6 +45,47 @@ router.get('/tags', async (req, res, next) => { } }); +router.post( + '/tags', + validateRequest({ + body: z.object({ + metrics: z + .array( + z.object({ + name: z.string().min(1).max(1024), + dataType: z.nativeEnum(clickhouse.MetricsDataType), + }), + ) + .max(32), + }), + }), + async (req, res, next) => { + try { + const teamId = req.user?.team; + if (teamId == null) { + return res.sendStatus(403); + } + const { metrics } = req.body; + + const nowInMs = Date.now(); + res.json( + await clickhouse.getMetricsTags({ + metrics, + // FIXME: fix it 5 days ago for now + startTime: nowInMs - ms('5d'), + endTime: nowInMs, + teamId: teamId.toString(), + }), + ); + } catch (e) { + const span = opentelemetry.trace.getActiveSpan(); + span?.recordException(e as Error); + span?.setStatus({ code: SpanStatusCode.ERROR }); + next(e); + } + }, +); + router.post( '/chart', validateRequest({ diff --git a/packages/api/src/routers/external-api/__tests__/v1.test.ts b/packages/api/src/routers/external-api/__tests__/v1.test.ts index 644f9379e..d524ae5c4 100644 --- a/packages/api/src/routers/external-api/__tests__/v1.test.ts +++ b/packages/api/src/routers/external-api/__tests__/v1.test.ts @@ -28,7 +28,7 @@ describe('external api v1', () => { }); it('GET /api/v1/metrics/tags', async () => { - jest.spyOn(clickhouse, 'getMetricsTags').mockResolvedValueOnce({ + jest.spyOn(clickhouse, 'getMetricsTagsDEPRECATED').mockResolvedValueOnce({ data: [ { name: 'system.filesystem.usage - Sum', @@ -67,7 +67,7 @@ describe('external api v1', () => { .set('Authorization', `Bearer ${user?.accessKey}`) .expect(200); - expect(clickhouse.getMetricsTags).toBeCalledTimes(1); + expect(clickhouse.getMetricsTagsDEPRECATED).toBeCalledTimes(1); expect(resp.body).toEqual({ data: [ { diff --git a/packages/api/src/routers/external-api/v1/index.ts b/packages/api/src/routers/external-api/v1/index.ts index b815253dd..e1e087bec 100644 --- a/packages/api/src/routers/external-api/v1/index.ts +++ b/packages/api/src/routers/external-api/v1/index.ts @@ -194,12 +194,12 @@ router.get( const nowInMs = Date.now(); const simpleCache = new SimpleCache< - Awaited> + Awaited> >( - `metrics-tags-${teamId}`, + `v1-api-metrics-tags-${teamId}`, ms('10m'), () => - clickhouse.getMetricsTags({ + clickhouse.getMetricsTagsDEPRECATED({ // FIXME: fix it 5 days ago for now startTime: nowInMs - ms('5d'), endTime: nowInMs, diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index f722f8c4f..14709fd63 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -11,6 +11,7 @@ import MetricTagFilterInput from './MetricTagFilterInput'; import SearchInput from './SearchInput'; import type { AggFn, ChartSeries, SourceTable } from './types'; import { NumberFormat } from './types'; +import { legacyMetricNameToNameAndDataType } from './utils'; export const SORT_ORDER = [ { value: 'asc' as const, label: 'Ascending' }, @@ -247,7 +248,10 @@ export function usePropertyOptions(types: ('number' | 'string' | 'bool')[]) { } function useMetricTagOptions({ metricNames }: { metricNames?: string[] }) { - const { data: metricTagsData } = api.useMetricsTags(); + const metrics = (metricNames ?? []).map(m => ({ + ...legacyMetricNameToNameAndDataType(m), + })); + const { data: metricTagsData } = api.useMetricsTags(metrics); const options = useMemo(() => { let tagNameSet = new Set(); @@ -286,7 +290,7 @@ function useMetricTagOptions({ metricNames }: { metricNames?: string[] }) { label: tagName, })), ]; - }, [metricTagsData, metricNames]); + }, [metricTagsData]); return options; } @@ -342,7 +346,7 @@ export function MetricSelect({ setMetricName: (value: string | undefined) => void; }) { // TODO: Dedup with metric rate checkbox - const { data: metricTagsData, isLoading, isError } = api.useMetricsTags(); + const { data: metricNamesData, isLoading, isError } = api.useMetricsNames(); const aggFnWithMaybeRate = (aggFn: AggFn, isRate: boolean) => { if (isRate) { @@ -368,7 +372,7 @@ export function MetricSelect({ isError={isError} value={metricName} setValue={name => { - const metricType = metricTagsData?.data?.find( + const metricType = metricNamesData?.data?.find( metric => metric.name === name, )?.data_type; @@ -400,12 +404,12 @@ export function MetricRateSelect({ setIsRate: (isRate: boolean) => void; metricName: string | undefined | null; }) { - const { data: metricTagsData } = api.useMetricsTags(); + const { data: metricNamesData } = api.useMetricsNames(); const metricType = useMemo(() => { - return metricTagsData?.data?.find(metric => metric.name === metricName) + return metricNamesData?.data?.find(metric => metric.name === metricName) ?.data_type; - }, [metricTagsData, metricName]); + }, [metricNamesData, metricName]); return ( <> @@ -435,16 +439,16 @@ export function MetricNameSelect({ isLoading?: boolean; isError?: boolean; }) { - const { data: metricTagsData } = api.useMetricsTags(); + const { data: metricNamesData } = api.useMetricsNames(); const options = useMemo(() => { return ( - metricTagsData?.data?.map(entry => ({ + metricNamesData?.data?.map(entry => ({ value: entry.name, label: entry.name, })) ?? [] ); - }, [metricTagsData]); + }, [metricNamesData]); return ( { const tags = metricTagsData?.data?.filter(metric => metric.name === metricName)?.[0] ?.tags ?? []; - const tagNameValueSet = new Set(); - tags.forEach(tag => { Object.entries(tag).forEach(([name, value]) => tagNameValueSet.add(`${name}:"${value}"`), ); }); - return Array.from(tagNameValueSet).map(tagName => ({ value: tagName, label: tagName, })); - }, [metricTagsData, metricName]); + }, [metricTagsData]); return ( ); } diff --git a/packages/app/src/MetricTagValueSelect.tsx b/packages/app/src/MetricTagValueSelect.tsx index 6ad55a6e2..95432ad46 100644 --- a/packages/app/src/MetricTagValueSelect.tsx +++ b/packages/app/src/MetricTagValueSelect.tsx @@ -2,6 +2,7 @@ import React, { useMemo } from 'react'; import { Select } from '@mantine/core'; import api from './api'; +import { legacyMetricNameToNameAndDataType } from './utils'; export default function MetricTagValueSelect({ metricName, @@ -19,25 +20,30 @@ export default function MetricTagValueSelect({ dropdownClosedWidth?: number; onChange: (value: string) => void; } & Partial>) { + const { name: mName, dataType: mDataType } = + legacyMetricNameToNameAndDataType(metricName); const { data: metricTagsData, isLoading: isMetricTagsLoading } = - api.useMetricsTags(); + api.useMetricsTags([ + { + name: mName, + dataType: mDataType, + }, + ]); const options = useMemo(() => { const tags = metricTagsData?.data?.filter(metric => metric.name === metricName)?.[0] ?.tags ?? []; - - const valueSet = new Set(); - + const tagNameValueSet = new Set(); tags.forEach(tag => { - Object.entries(tag).forEach(([name, value]) => { - if (name === metricAttribute) { - valueSet.add(value); - } - }); + Object.entries(tag).forEach(([name, value]) => + tagNameValueSet.add(`${name}:"${value}"`), + ); }); - - return Array.from(valueSet); + return Array.from(tagNameValueSet).map(tagName => ({ + value: tagName, + label: tagName, + })); }, [metricTagsData, metricName]); const [dropdownOpen, setDropdownOpen] = React.useState(false); diff --git a/packages/app/src/api.ts b/packages/app/src/api.ts index 77d63e070..cc3cb1c1b 100644 --- a/packages/app/src/api.ts +++ b/packages/app/src/api.ts @@ -18,6 +18,7 @@ import type { ChartSeries, Dashboard, LogView, + MetricsDataType, Session, } from './types'; @@ -89,7 +90,33 @@ const api = { }, ); }, - useMetricsTags() { + useMetricsNames() { + return useQuery< + { + data: { + data_type: string; + is_delta: boolean; + is_monotonic: boolean; + name: string; + unit: string; + }[]; + }, + Error + >({ + refetchOnWindowFocus: false, + queryKey: ['metrics/names'], + queryFn: () => + server('metrics/names', { + method: 'GET', + }).json(), + }); + }, + useMetricsTags( + metrics: { + name: string; + dataType: MetricsDataType; + }[], + ) { return useQuery< { data: { @@ -101,10 +128,13 @@ const api = { Error >({ refetchOnWindowFocus: false, - queryKey: ['metrics/tags'], + queryKey: ['metrics/tags', metrics], queryFn: () => server('metrics/tags', { - method: 'GET', + method: 'POST', + json: { + metrics, + }, }).json(), }); }, diff --git a/packages/app/src/types.ts b/packages/app/src/types.ts index 7a716a0b3..f903e1444 100644 --- a/packages/app/src/types.ts +++ b/packages/app/src/types.ts @@ -198,6 +198,12 @@ export type AggFn = export type SourceTable = 'logs' | 'rrweb' | 'metrics'; +export enum MetricsDataType { + Gauge = 'Gauge', + Histogram = 'Histogram', + Sum = 'Sum', +} + export type TimeChartSeries = { displayName?: string; table: SourceTable; diff --git a/packages/app/src/utils.tsx b/packages/app/src/utils.tsx index c53ad5439..01fa335a8 100644 --- a/packages/app/src/utils.tsx +++ b/packages/app/src/utils.tsx @@ -5,7 +5,7 @@ import numbro from 'numbro'; import type { MutableRefObject } from 'react'; import { dateRangeToString } from './timeQuery'; -import { NumberFormat } from './types'; +import { MetricsDataType, NumberFormat } from './types'; export function generateSearchUrl({ query, @@ -444,3 +444,13 @@ export const formatUptime = (seconds: number) => { return `${Math.floor(seconds / 60 / 60 / 24)}d`; } }; + +// FIXME: eventually we want to separate metric name into two fields +export const legacyMetricNameToNameAndDataType = (metricName?: string) => { + const [mName, mDataType] = (metricName ?? '').split(' - '); + + return { + name: mName, + dataType: mDataType as MetricsDataType, + }; +};