From 76745fd40b264223f07037295a0fa533e4fc1523 Mon Sep 17 00:00:00 2001 From: Warren <5959690+wrn14897@users.noreply.github.com> Date: Tue, 23 Jul 2024 23:10:00 -0700 Subject: [PATCH 01/12] fix: parse histogram metric name + tags --- packages/api/src/clickhouse/index.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/api/src/clickhouse/index.ts b/packages/api/src/clickhouse/index.ts index 28bfe1116..13ed489f9 100644 --- a/packages/api/src/clickhouse/index.ts +++ b/packages/api/src/clickhouse/index.ts @@ -675,7 +675,7 @@ export const getMetricsNames = async ({ any(unit) AS unit, data_type, if( - data_type = ?, + data_type IN (?), format( '{} - {}', replaceRegexpOne(name, '_[^_]+$', ''), @@ -691,7 +691,7 @@ export const getMetricsNames = async ({ ORDER BY name `, [ - MetricsDataType.Summary, + [MetricsDataType.Summary, MetricsDataType.Histogram], tableName, Object.values(MetricsDataType), msToBigIntNs(startTime), @@ -753,7 +753,7 @@ export const getMetricsTags = async ({ groupUniqArray(_string_attributes) AS tags, data_type, if( - data_type = ?, + data_type IN (?), format( '{} - {}', replaceRegexpOne(name, '_[^_]+$', ''), @@ -769,10 +769,12 @@ export const getMetricsTags = async ({ GROUP BY name, data_type `, [ - MetricsDataType.Summary, + [MetricsDataType.Summary, MetricsDataType.Histogram], tableName, SqlString.raw( - m.dataType === MetricsDataType.Summary + [MetricsDataType.Summary, MetricsDataType.Histogram].includes( + m.dataType, + ) ? // FIXME: since the summary data type doesn't include the postfix SqlString.format('name LIKE ?', [`${m.name}_%`]) : SqlString.format('name = ?', [m.name]), From 3c4d138c1716338b76cef955fc8b996b85cb88db Mon Sep 17 00:00:00 2001 From: Warren <5959690+wrn14897@users.noreply.github.com> Date: Tue, 23 Jul 2024 23:17:14 -0700 Subject: [PATCH 02/12] feat: query histogram sum and count metrics --- packages/api/src/clickhouse/index.ts | 69 +++++++++++++++++++--------- packages/app/src/ChartUtils.tsx | 2 + 2 files changed, 50 insertions(+), 21 deletions(-) diff --git a/packages/api/src/clickhouse/index.ts b/packages/api/src/clickhouse/index.ts index 13ed489f9..26f5dfce6 100644 --- a/packages/api/src/clickhouse/index.ts +++ b/packages/api/src/clickhouse/index.ts @@ -1061,6 +1061,7 @@ export const buildMetricSeriesQuery = async ({ const tableName = `default.${TableName.Metric}`; const isRate = isRateAggFn(aggFn); + let isHistogram = false; const groupByColumnNames = groupBy.map(g => { return SqlString.format(`_string_attributes[?]`, [g]); @@ -1142,8 +1143,35 @@ export const buildMetricSeriesQuery = async ({ })(${isRate ? 'rate' : 'value'}) as data`, ); } else if (dataType === MetricsDataType.Histogram) { - if (!['p50', 'p90', 'p95', 'p99'].includes(aggFn)) { - throw new Error(`Unsupported aggFn for Histogram: ${aggFn}`); + switch (aggFn) { + case AggFn.Sum: + name = `${name}_sum`; + gaugeMetricSelectClause.push('SUM(value) as value'); + selectClause.push('SUM(value) as data'); + break; + case AggFn.Count: + name = `${name}_count`; + gaugeMetricSelectClause.push('SUM(value) as value'); + selectClause.push('SUM(value) as data'); + break; + case AggFn.P50: + isHistogram = true; + name = `${name}_bucket`; + break; + case AggFn.P90: + isHistogram = true; + name = `${name}_bucket`; + break; + case AggFn.P95: + isHistogram = true; + name = `${name}_bucket`; + break; + case AggFn.P99: + isHistogram = true; + name = `${name}_bucket`; + break; + default: + throw new Error(`Unsupported aggFn for Histogram: ${aggFn}`); } } else if (dataType === MetricsDataType.Summary) { switch (aggFn) { @@ -1384,11 +1412,10 @@ export const buildMetricSeriesQuery = async ({ // TODO: merge this with the histogram query const source = isRate ? rateMetricSource : gaugeMetricSource; - const query = - dataType === MetricsDataType.Histogram - ? histogramQuery - : SqlString.format( - ` + const query = isHistogram + ? histogramQuery + : SqlString.format( + ` WITH metrics AS (?) SELECT ? FROM metrics @@ -1403,20 +1430,20 @@ export const buildMetricSeriesQuery = async ({ : '' } `, - [ - SqlString.raw(source), - SqlString.raw(selectClause.join(',')), - ...(granularity != null - ? [ - startTime / 1000, - granularity, - endTime / 1000, - granularity, - ms(granularity) / 1000, - ] - : []), - ], - ); + [ + SqlString.raw(source), + SqlString.raw(selectClause.join(',')), + ...(granularity != null + ? [ + startTime / 1000, + granularity, + endTime / 1000, + granularity, + ms(granularity) / 1000, + ] + : []), + ], + ); return { query, diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index 8205acc72..bda8d14b6 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -49,10 +49,12 @@ export const getMetricAggFns = ( ): { value: AggFn; label: string }[] => { if (dataType === MetricsDataType.Histogram) { return [ + { value: 'sum', label: 'Sum' }, { value: 'p99', label: '99th Percentile' }, { value: 'p95', label: '95th Percentile' }, { value: 'p90', label: '90th Percentile' }, { value: 'p50', label: 'Median' }, + { value: 'count', label: 'Sample Count' }, ]; } else if (dataType === MetricsDataType.Summary) { return [ From 18053a5ed8424f1b54019a347913ef547615b38d Mon Sep 17 00:00:00 2001 From: Warren <5959690+wrn14897@users.noreply.github.com> Date: Tue, 23 Jul 2024 23:32:52 -0700 Subject: [PATCH 03/12] docs: add changeset --- .changeset/chatty-maps-march.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/chatty-maps-march.md diff --git a/.changeset/chatty-maps-march.md b/.changeset/chatty-maps-march.md new file mode 100644 index 000000000..a7d40da09 --- /dev/null +++ b/.changeset/chatty-maps-march.md @@ -0,0 +1,7 @@ +--- +'@hyperdx/api': major +'@hyperdx/app': major +--- + +feat: support sum + count aggregation for histogram metrics + merge histogram +metric names From 76e3c012be32022c67b409fddaaf332614e75378 Mon Sep 17 00:00:00 2001 From: Warren <5959690+wrn14897@users.noreply.github.com> Date: Tue, 6 Aug 2024 23:14:22 -0700 Subject: [PATCH 04/12] fix: histogram metric name in tests --- .../clickhouseGetMultiSeriesChart.test.ts | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts b/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts index d41cec227..3a2855178 100644 --- a/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts +++ b/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts @@ -117,7 +117,7 @@ describe('clickhouse - getMultiSeriesChart', () => { ), clickhouse.bulkInsertTeamMetricStream( buildMetricSeries({ - name: 'test.two_timestamps_lower_bound', + name: 'test.two_timestamps_lower_bound_bucket', tags: { host: 'test2', runId }, data_type: clickhouse.MetricsDataType.Histogram, is_monotonic: false, @@ -137,7 +137,7 @@ describe('clickhouse - getMultiSeriesChart', () => { ), clickhouse.bulkInsertTeamMetricStream( buildMetricSeries({ - name: 'test.two_timestamps_lower_bound_inf', + name: 'test.two_timestamps_lower_bound_inf_bucket', tags: { host: 'test2', runId }, data_type: clickhouse.MetricsDataType.Histogram, is_monotonic: false, @@ -157,7 +157,7 @@ describe('clickhouse - getMultiSeriesChart', () => { ), clickhouse.bulkInsertTeamMetricStream( buildMetricSeries({ - name: 'test.two_timestamps_higher_bound', + name: 'test.two_timestamps_higher_bound_bucket', tags: { host: 'test2', runId }, data_type: clickhouse.MetricsDataType.Histogram, is_monotonic: false, @@ -177,7 +177,7 @@ describe('clickhouse - getMultiSeriesChart', () => { ), clickhouse.bulkInsertTeamMetricStream( buildMetricSeries({ - name: 'test.two_timestamps_higher_bound_inf', + name: 'test.two_timestamps_higher_bound_inf_bucket', tags: { host: 'test2', runId }, data_type: clickhouse.MetricsDataType.Histogram, is_monotonic: false, @@ -197,7 +197,7 @@ describe('clickhouse - getMultiSeriesChart', () => { ), clickhouse.bulkInsertTeamMetricStream( buildMetricSeries({ - name: 'test.two_timestamps_zero_offset', + name: 'test.two_timestamps_zero_offset_bucket', tags: { host: 'test2', runId }, data_type: clickhouse.MetricsDataType.Histogram, is_monotonic: false, @@ -217,7 +217,7 @@ describe('clickhouse - getMultiSeriesChart', () => { ), clickhouse.bulkInsertTeamMetricStream( buildMetricSeries({ - name: 'test.two_timestamps_non_zero_offset', + name: 'test.two_timestamps_non_zero_offset_bucket', tags: { host: 'test2', runId }, data_type: clickhouse.MetricsDataType.Histogram, is_monotonic: false, @@ -237,7 +237,7 @@ describe('clickhouse - getMultiSeriesChart', () => { ), clickhouse.bulkInsertTeamMetricStream( buildMetricSeries({ - name: 'test.three_timestamps', + name: 'test.three_timestamps_bucket', tags: { host: 'test2', runId }, data_type: clickhouse.MetricsDataType.Histogram, is_monotonic: false, @@ -261,7 +261,7 @@ describe('clickhouse - getMultiSeriesChart', () => { ), clickhouse.bulkInsertTeamMetricStream( buildMetricSeries({ - name: 'test.three_timestamps_group_by', + name: 'test.three_timestamps_group_by_bucket', tags: { host: 'host-a', region: 'region-a' }, data_type: clickhouse.MetricsDataType.Histogram, is_monotonic: false, @@ -285,7 +285,7 @@ describe('clickhouse - getMultiSeriesChart', () => { ), clickhouse.bulkInsertTeamMetricStream( buildMetricSeries({ - name: 'test.three_timestamps_group_by', + name: 'test.three_timestamps_group_by_bucket', tags: { host: 'host-b', region: 'region-a' }, data_type: clickhouse.MetricsDataType.Histogram, is_monotonic: false, @@ -309,7 +309,7 @@ describe('clickhouse - getMultiSeriesChart', () => { ), clickhouse.bulkInsertTeamMetricStream( buildMetricSeries({ - name: 'test.four_timestamps', + name: 'test.four_timestamps_bucket', tags: { host: 'test2', runId }, data_type: clickhouse.MetricsDataType.Histogram, is_monotonic: false, @@ -336,7 +336,7 @@ describe('clickhouse - getMultiSeriesChart', () => { ), clickhouse.bulkInsertTeamMetricStream( buildMetricSeries({ - name: 'test.counter_reset_with_inconsistent_bucket_resolution', + name: 'test.counter_reset_with_inconsistent_bucket_resolution_bucket', tags: { host: 'test2', runId }, data_type: clickhouse.MetricsDataType.Histogram, is_monotonic: false, @@ -367,7 +367,7 @@ describe('clickhouse - getMultiSeriesChart', () => { ), clickhouse.bulkInsertTeamMetricStream([ ...buildMetricSeries({ - name: 'test.counter_might_reset', + name: 'test.counter_might_reset_bucket', tags: { host: 'host-a', region: 'region-a' }, data_type: clickhouse.MetricsDataType.Histogram, is_monotonic: false, @@ -397,7 +397,7 @@ describe('clickhouse - getMultiSeriesChart', () => { team_id: teamId, }), ...buildMetricSeries({ - name: 'test.counter_might_reset', + name: 'test.counter_might_reset_bucket', tags: { host: 'host-b', region: 'region-a' }, data_type: clickhouse.MetricsDataType.Histogram, is_monotonic: false, From 21eb158fbb390f319f2237d1796e7be090e2e54d Mon Sep 17 00:00:00 2001 From: Warren <5959690+wrn14897@users.noreply.github.com> Date: Wed, 7 Aug 2024 11:41:33 -0700 Subject: [PATCH 05/12] fix: select last value (sum and count are both monotonic) --- packages/api/src/clickhouse/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/api/src/clickhouse/index.ts b/packages/api/src/clickhouse/index.ts index 26f5dfce6..b751d1fa1 100644 --- a/packages/api/src/clickhouse/index.ts +++ b/packages/api/src/clickhouse/index.ts @@ -1146,12 +1146,12 @@ export const buildMetricSeriesQuery = async ({ switch (aggFn) { case AggFn.Sum: name = `${name}_sum`; - gaugeMetricSelectClause.push('SUM(value) as value'); + gaugeMetricSelectClause.push('LAST_VALUE(value) as value'); selectClause.push('SUM(value) as data'); break; case AggFn.Count: name = `${name}_count`; - gaugeMetricSelectClause.push('SUM(value) as value'); + gaugeMetricSelectClause.push('LAST_VALUE(value) as value'); selectClause.push('SUM(value) as data'); break; case AggFn.P50: From ad7b7c83b714b7beaf8bc9d46cda17ae603fef01 Mon Sep 17 00:00:00 2001 From: Warren <5959690+wrn14897@users.noreply.github.com> Date: Wed, 7 Aug 2024 11:56:16 -0700 Subject: [PATCH 06/12] test: add histogram sum + count tests --- .../clickhouseGetMultiSeriesChart.test.ts | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts b/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts index 3a2855178..7d67846a2 100644 --- a/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts +++ b/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts @@ -115,6 +115,40 @@ describe('clickhouse - getMultiSeriesChart', () => { team_id: teamId, }), ), + clickhouse.bulkInsertTeamMetricStream( + buildMetricSeries({ + name: 'test.three_timestamps_sum', + tags: { host: 'test2', runId }, + data_type: clickhouse.MetricsDataType.Histogram, + is_monotonic: false, + is_delta: false, + unit: '', + points: [ + { value: 0, timestamp: now }, + { value: 1, timestamp: now + ms('1m') }, + { value: 2, timestamp: now + ms('1m') }, + { value: 3, timestamp: now + ms('2m') }, + ], + team_id: teamId, + }), + ), + clickhouse.bulkInsertTeamMetricStream( + buildMetricSeries({ + name: 'test.three_timestamps_count', + tags: { host: 'test2', runId }, + data_type: clickhouse.MetricsDataType.Histogram, + is_monotonic: false, + is_delta: false, + unit: '', + points: [ + { value: 0, timestamp: now }, + { value: 1, timestamp: now + ms('1m') }, + { value: 2, timestamp: now + ms('1m') }, + { value: 3, timestamp: now + ms('2m') }, + ], + team_id: teamId, + }), + ), clickhouse.bulkInsertTeamMetricStream( buildMetricSeries({ name: 'test.two_timestamps_lower_bound_bucket', @@ -839,6 +873,88 @@ Array [ `); }); + it('three_timestamps histogram (sum + count)', async () => { + const sumData = ( + await clickhouse.getMultiSeriesChart({ + series: [ + { + type: 'time', + table: 'metrics', + aggFn: clickhouse.AggFn.Sum, + field: 'test.three_timestamps', + where: `runId:${runId}`, + groupBy: [], + metricDataType: clickhouse.MetricsDataType.Histogram, + }, + ], + tableVersion: undefined, + teamId, + startTime: now, + endTime: now + ms('2m'), + granularity: '1 minute', + maxNumGroups: 20, + seriesReturnType: clickhouse.SeriesReturnType.Column, + }) + ).data.map(d => { + return _.pick(d, ['group', 'series_0.data', 'ts_bucket']); + }); + + const countData = ( + await clickhouse.getMultiSeriesChart({ + series: [ + { + type: 'time', + table: 'metrics', + aggFn: clickhouse.AggFn.Count, + field: 'test.three_timestamps', + where: `runId:${runId}`, + groupBy: [], + metricDataType: clickhouse.MetricsDataType.Histogram, + }, + ], + tableVersion: undefined, + teamId, + startTime: now, + endTime: now + ms('2m'), + granularity: '1 minute', + maxNumGroups: 20, + seriesReturnType: clickhouse.SeriesReturnType.Column, + }) + ).data.map(d => { + return _.pick(d, ['group', 'series_0.data', 'ts_bucket']); + }); + + expect(sumData).toMatchInlineSnapshot(` +Array [ + Object { + "group": Array [], + "series_0.data": 0, + "ts_bucket": 1641340800, + }, + Object { + "group": Array [], + "series_0.data": 2, + "ts_bucket": 1641340860, + }, +] +`); + + expect(countData).toMatchInlineSnapshot(` +Array [ + Object { + "group": Array [], + "series_0.data": 0, + "ts_bucket": 1641340800, + }, + Object { + "group": Array [], + "series_0.data": 2, + "ts_bucket": 1641340860, + }, +] +`); + }); + it('two_timestamps_lower_bound histogram (p50)', async () => { const p50Data = ( await clickhouse.getMultiSeriesChart({ From d565eabe9406d656fe2488204ec8b7e5b3d3db3c Mon Sep 17 00:00:00 2001 From: Warren <5959690+wrn14897@users.noreply.github.com> Date: Wed, 7 Aug 2024 13:06:54 -0700 Subject: [PATCH 07/12] feat: histogram metric name backward compatibility --- packages/api/src/clickhouse/index.ts | 11 +++++++---- packages/app/src/ChartUtils.tsx | 6 ++++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/api/src/clickhouse/index.ts b/packages/api/src/clickhouse/index.ts index b751d1fa1..a2fbcaa31 100644 --- a/packages/api/src/clickhouse/index.ts +++ b/packages/api/src/clickhouse/index.ts @@ -1143,6 +1143,9 @@ export const buildMetricSeriesQuery = async ({ })(${isRate ? 'rate' : 'value'}) as data`, ); } else if (dataType === MetricsDataType.Histogram) { + // TODO: remove this (backward compatibility) + const isOlderVersion = name.endsWith('_bucket'); + switch (aggFn) { case AggFn.Sum: name = `${name}_sum`; @@ -1156,19 +1159,19 @@ export const buildMetricSeriesQuery = async ({ break; case AggFn.P50: isHistogram = true; - name = `${name}_bucket`; + name = isOlderVersion ? name : `${name}_bucket`; break; case AggFn.P90: isHistogram = true; - name = `${name}_bucket`; + name = isOlderVersion ? name : `${name}_bucket`; break; case AggFn.P95: isHistogram = true; - name = `${name}_bucket`; + name = isOlderVersion ? name : `${name}_bucket`; break; case AggFn.P99: isHistogram = true; - name = `${name}_bucket`; + name = isOlderVersion ? name : `${name}_bucket`; break; default: throw new Error(`Unsupported aggFn for Histogram: ${aggFn}`); diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index bda8d14b6..e1255d25a 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -398,6 +398,12 @@ export function MetricSelect({ // TODO: Dedup with metric rate checkbox const { data: metricNamesData, isLoading, isError } = api.useMetricsNames(); + // TODO: depcrcate this (backward compatibility) + if (metricName?.endsWith('- Histogram')) { + // patch for old histogram metric names + metricName = metricName.replace('_bucket', ''); + } + const aggFnWithMaybeRate = (aggFn: AggFn, isRate: boolean) => { if (isRate) { if (aggFn.includes('_rate')) { From 1ca085829a15b68e00d5508ceefa1674f0f3699f Mon Sep 17 00:00:00 2001 From: Warren <5959690+wrn14897@users.noreply.github.com> Date: Wed, 7 Aug 2024 13:08:39 -0700 Subject: [PATCH 08/12] docs: update changeset --- .changeset/chatty-maps-march.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/chatty-maps-march.md b/.changeset/chatty-maps-march.md index a7d40da09..30360c11e 100644 --- a/.changeset/chatty-maps-march.md +++ b/.changeset/chatty-maps-march.md @@ -1,6 +1,6 @@ --- -'@hyperdx/api': major -'@hyperdx/app': major +'@hyperdx/api': minor +'@hyperdx/app': minor --- feat: support sum + count aggregation for histogram metrics + merge histogram From 8155c2c2a3662f38f7668115c79371e91cf1392d Mon Sep 17 00:00:00 2001 From: Warren <5959690+wrn14897@users.noreply.github.com> Date: Wed, 7 Aug 2024 13:13:06 -0700 Subject: [PATCH 09/12] test: update endTime --- .../clickhouseGetMultiSeriesChart.test.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts b/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts index 7d67846a2..ad0c19b4b 100644 --- a/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts +++ b/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts @@ -890,7 +890,7 @@ Array [ tableVersion: undefined, teamId, startTime: now, - endTime: now + ms('2m'), + endTime: now + ms('3m'), granularity: '1 minute', maxNumGroups: 20, seriesReturnType: clickhouse.SeriesReturnType.Column, @@ -915,7 +915,7 @@ Array [ tableVersion: undefined, teamId, startTime: now, - endTime: now + ms('2m'), + endTime: now + ms('3m'), granularity: '1 minute', maxNumGroups: 20, seriesReturnType: clickhouse.SeriesReturnType.Column, @@ -936,6 +936,11 @@ Array [ "series_0.data": 2, "ts_bucket": 1641340860, }, + Object { + "group": Array [], + "series_0.data": 3, + "ts_bucket": 1641340920, + }, ] `); @@ -951,6 +956,11 @@ Array [ "series_0.data": 2, "ts_bucket": 1641340860, }, + Object { + "group": Array [], + "series_0.data": 3, + "ts_bucket": 1641340920, + }, ] `); }); From fa9355c93f853df2244eb3bcc368cf0909e0cd50 Mon Sep 17 00:00:00 2001 From: Warren <5959690+wrn14897@users.noreply.github.com> Date: Wed, 7 Aug 2024 14:32:06 -0700 Subject: [PATCH 10/12] feat: suppert histogram sum + count rate metrics --- packages/api/src/clickhouse/index.ts | 14 ++++++++++++-- packages/app/src/ChartUtils.tsx | 9 +++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/api/src/clickhouse/index.ts b/packages/api/src/clickhouse/index.ts index a2fbcaa31..8cba6e83f 100644 --- a/packages/api/src/clickhouse/index.ts +++ b/packages/api/src/clickhouse/index.ts @@ -62,6 +62,7 @@ export enum AggFn { Avg = 'avg', AvgRate = 'avg_rate', Count = 'count', + CountRate = 'count_rate', CountDistinct = 'count_distinct', CountPerHour = 'count_per_hour', CountPerMin = 'count_per_min', @@ -828,6 +829,7 @@ export const getMetricsTags = async ({ export const isRateAggFn = (aggFn: AggFn) => { return ( + aggFn === AggFn.CountRate || aggFn === AggFn.SumRate || aggFn === AggFn.AvgRate || aggFn === AggFn.MaxRate || @@ -1148,28 +1150,36 @@ export const buildMetricSeriesQuery = async ({ switch (aggFn) { case AggFn.Sum: + case AggFn.SumRate: name = `${name}_sum`; gaugeMetricSelectClause.push('LAST_VALUE(value) as value'); - selectClause.push('SUM(value) as data'); + selectClause.push(`SUM(${isRate ? 'rate' : 'value'}) as data`); break; case AggFn.Count: + case AggFn.CountRate: name = `${name}_count`; gaugeMetricSelectClause.push('LAST_VALUE(value) as value'); - selectClause.push('SUM(value) as data'); + selectClause.push(`SUM(${isRate ? 'rate' : 'value'}) as data`); break; + // FIXME: currently non-rate percentiles are not supported + // FIXME: the old p50, p90, p95, p99 are incorrect; they are meant to be rate percentiles case AggFn.P50: + case AggFn.P50Rate: isHistogram = true; name = isOlderVersion ? name : `${name}_bucket`; break; case AggFn.P90: + case AggFn.P90Rate: isHistogram = true; name = isOlderVersion ? name : `${name}_bucket`; break; case AggFn.P95: + case AggFn.P95Rate: isHistogram = true; name = isOlderVersion ? name : `${name}_bucket`; break; case AggFn.P99: + case AggFn.P99Rate: isHistogram = true; name = isOlderVersion ? name : `${name}_bucket`; break; diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index e1255d25a..067aa4886 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -432,7 +432,11 @@ export function MetricSelect({ metric => metric.name === name, )?.data_type; - const newAggFn = aggFnWithMaybeRate(aggFn, metricType === 'Sum'); + const newAggFn = aggFnWithMaybeRate( + aggFn, + metricType === MetricsDataType.Sum || + metricType === MetricsDataType.Histogram, + ); setFieldAndAggFn(name, newAggFn); }} @@ -469,7 +473,8 @@ export function MetricRateSelect({ return ( <> - {metricType === 'Sum' ? ( + {metricType === MetricsDataType.Sum || + metricType === MetricsDataType.Histogram ? ( Date: Wed, 7 Aug 2024 14:40:52 -0700 Subject: [PATCH 11/12] test: sum + count rate functions --- .../clickhouseGetMultiSeriesChart.test.ts | 185 +++++++++++++----- 1 file changed, 136 insertions(+), 49 deletions(-) diff --git a/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts b/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts index ad0c19b4b..eb1a38011 100644 --- a/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts +++ b/packages/api/src/clickhouse/__tests__/clickhouseGetMultiSeriesChart.test.ts @@ -874,55 +874,104 @@ Array [ }); it('three_timestamps histogram (sum + count)', async () => { - const sumData = ( - await clickhouse.getMultiSeriesChart({ - series: [ - { - type: 'time', - table: 'metrics', - aggFn: clickhouse.AggFn.Sum, - field: 'test.three_timestamps', - where: `runId:${runId}`, - groupBy: [], - metricDataType: clickhouse.MetricsDataType.Histogram, - }, - ], - tableVersion: undefined, - teamId, - startTime: now, - endTime: now + ms('3m'), - granularity: '1 minute', - maxNumGroups: 20, - seriesReturnType: clickhouse.SeriesReturnType.Column, - }) - ).data.map(d => { - return _.pick(d, ['group', 'series_0.data', 'ts_bucket']); - }); - - const countData = ( - await clickhouse.getMultiSeriesChart({ - series: [ - { - type: 'time', - table: 'metrics', - aggFn: clickhouse.AggFn.Count, - field: 'test.three_timestamps', - where: `runId:${runId}`, - groupBy: [], - metricDataType: clickhouse.MetricsDataType.Histogram, - }, - ], - tableVersion: undefined, - teamId, - startTime: now, - endTime: now + ms('3m'), - granularity: '1 minute', - maxNumGroups: 20, - seriesReturnType: clickhouse.SeriesReturnType.Column, - }) - ).data.map(d => { - return _.pick(d, ['group', 'series_0.data', 'ts_bucket']); - }); + const [sumData, sumRateData, countData, countRateData] = await Promise.all([ + ( + await clickhouse.getMultiSeriesChart({ + series: [ + { + type: 'time', + table: 'metrics', + aggFn: clickhouse.AggFn.Sum, + field: 'test.three_timestamps', + where: `runId:${runId}`, + groupBy: [], + metricDataType: clickhouse.MetricsDataType.Histogram, + }, + ], + tableVersion: undefined, + teamId, + startTime: now, + endTime: now + ms('3m'), + granularity: '1 minute', + maxNumGroups: 20, + seriesReturnType: clickhouse.SeriesReturnType.Column, + }) + ).data.map(d => { + return _.pick(d, ['group', 'series_0.data', 'ts_bucket']); + }), + ( + await clickhouse.getMultiSeriesChart({ + series: [ + { + type: 'time', + table: 'metrics', + aggFn: clickhouse.AggFn.SumRate, + field: 'test.three_timestamps', + where: `runId:${runId}`, + groupBy: [], + metricDataType: clickhouse.MetricsDataType.Histogram, + }, + ], + tableVersion: undefined, + teamId, + startTime: now, + endTime: now + ms('3m'), + granularity: '1 minute', + maxNumGroups: 20, + seriesReturnType: clickhouse.SeriesReturnType.Column, + }) + ).data.map(d => { + return _.pick(d, ['group', 'series_0.data', 'ts_bucket']); + }), + ( + await clickhouse.getMultiSeriesChart({ + series: [ + { + type: 'time', + table: 'metrics', + aggFn: clickhouse.AggFn.Count, + field: 'test.three_timestamps', + where: `runId:${runId}`, + groupBy: [], + metricDataType: clickhouse.MetricsDataType.Histogram, + }, + ], + tableVersion: undefined, + teamId, + startTime: now, + endTime: now + ms('3m'), + granularity: '1 minute', + maxNumGroups: 20, + seriesReturnType: clickhouse.SeriesReturnType.Column, + }) + ).data.map(d => { + return _.pick(d, ['group', 'series_0.data', 'ts_bucket']); + }), + ( + await clickhouse.getMultiSeriesChart({ + series: [ + { + type: 'time', + table: 'metrics', + aggFn: clickhouse.AggFn.CountRate, + field: 'test.three_timestamps', + where: `runId:${runId}`, + groupBy: [], + metricDataType: clickhouse.MetricsDataType.Histogram, + }, + ], + tableVersion: undefined, + teamId, + startTime: now, + endTime: now + ms('3m'), + granularity: '1 minute', + maxNumGroups: 20, + seriesReturnType: clickhouse.SeriesReturnType.Column, + }) + ).data.map(d => { + return _.pick(d, ['group', 'series_0.data', 'ts_bucket']); + }), + ]); expect(sumData).toMatchInlineSnapshot(` Array [ @@ -942,6 +991,25 @@ Array [ "ts_bucket": 1641340920, }, ] +`); + expect(sumRateData).toMatchInlineSnapshot(` +Array [ + Object { + "group": Array [], + "series_0.data": 0, + "ts_bucket": 1641340800, + }, + Object { + "group": Array [], + "series_0.data": 2, + "ts_bucket": 1641340860, + }, + Object { + "group": Array [], + "series_0.data": 1, + "ts_bucket": 1641340920, + }, +] `); expect(countData).toMatchInlineSnapshot(` @@ -962,6 +1030,25 @@ Array [ "ts_bucket": 1641340920, }, ] +`); + expect(countRateData).toMatchInlineSnapshot(` +Array [ + Object { + "group": Array [], + "series_0.data": 0, + "ts_bucket": 1641340800, + }, + Object { + "group": Array [], + "series_0.data": 2, + "ts_bucket": 1641340860, + }, + Object { + "group": Array [], + "series_0.data": 1, + "ts_bucket": 1641340920, + }, +] `); }); From dc09603346be83c8aebe546fe113c195fceadcc3 Mon Sep 17 00:00:00 2001 From: Warren <5959690+wrn14897@users.noreply.github.com> Date: Wed, 7 Aug 2024 15:49:21 -0700 Subject: [PATCH 12/12] feat: histogram metric should be rate only --- packages/app/src/ChartUtils.tsx | 3 +++ packages/app/src/types.ts | 1 + 2 files changed, 4 insertions(+) diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index 067aa4886..916cf49f8 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -471,6 +471,8 @@ export function MetricRateSelect({ ?.data_type; }, [metricNamesData, metricName]); + const isDisabled = metricType === MetricsDataType.Histogram; + return ( <> {metricType === MetricsDataType.Sum || @@ -482,6 +484,7 @@ export function MetricRateSelect({ labelClassName="fs-7" checked={isRate} onChange={() => setIsRate(!isRate)} + disabled={isDisabled} label="Use Increase" /> ) : null} diff --git a/packages/app/src/types.ts b/packages/app/src/types.ts index e425daa78..f57f3e398 100644 --- a/packages/app/src/types.ts +++ b/packages/app/src/types.ts @@ -181,6 +181,7 @@ export type AggFn = | 'avg' | 'count_distinct' | 'count' + | 'count_rate' | 'count_per_sec' | 'count_per_min' | 'count_per_hour'