diff --git a/.changeset/brave-points-hunt.md b/.changeset/brave-points-hunt.md new file mode 100644 index 00000000..b72deb0d --- /dev/null +++ b/.changeset/brave-points-hunt.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/api': patch +'@hyperdx/app': patch +--- + +feat: support multi group-bys in event series query diff --git a/packages/api/src/clickhouse/__tests__/clickhouse.test.ts b/packages/api/src/clickhouse/__tests__/clickhouse.test.ts index 1f9b970b..8e9bc54b 100644 --- a/packages/api/src/clickhouse/__tests__/clickhouse.test.ts +++ b/packages/api/src/clickhouse/__tests__/clickhouse.test.ts @@ -115,18 +115,21 @@ Array [ timestamp: now, runId, testGroup: 'group1', + testOtherGroup: 'otherGroup1', awesomeNumber: 1, }), buildEvent({ timestamp: now + ms('1m'), runId, testGroup: 'group1', + testOtherGroup: 'otherGroup1', awesomeNumber: 15, }), buildEvent({ timestamp: now + ms('2m'), runId, testGroup: 'group1', + testOtherGroup: 'otherGroup2', awesomeNumber: 61, }), // Group 1, sum: 7, avg: 2.3333333 @@ -134,18 +137,21 @@ Array [ timestamp: now + ms('6m'), runId, testGroup: 'group1', + testOtherGroup: 'otherGroup2', awesomeNumber: 4, }), buildEvent({ timestamp: now + ms('7m'), runId, testGroup: 'group1', + testOtherGroup: 'otherGroup2', awesomeNumber: 2, }), buildEvent({ timestamp: now + ms('8m'), runId, testGroup: 'group1', + testOtherGroup: 'otherGroup3', awesomeNumber: 1, }), // Group 2, sum: 777, avg: 259 @@ -153,24 +159,28 @@ Array [ timestamp: now, runId, testGroup: 'group2', + testOtherGroup: 'otherGroup1', awesomeNumber: 70, }), buildEvent({ timestamp: now + ms('4m'), runId, testGroup: 'group2', + testOtherGroup: 'otherGroup1', awesomeNumber: 700, }), buildEvent({ timestamp: now + ms('1m'), runId, testGroup: 'group2', + testOtherGroup: 'otherGroup1', awesomeNumber: 7, }), ]); mockLogsPropertyTypeMappingsModel({ testGroup: 'string', + testOtherGroup: 'string', awesomeNumber: 'number', runId: 'string', }); @@ -216,24 +226,103 @@ Array [ expect(data).toMatchInlineSnapshot(` Array [ Object { - "group": "group2", + "group": Array [ + "group2", + ], "series_0.data": 777, "series_1.data": 259, "ts_bucket": 1641340800, }, Object { - "group": "group1", + "group": Array [ + "group1", + ], "series_0.data": 77, "series_1.data": 25.666666666666668, "ts_bucket": 1641340800, }, Object { - "group": "group1", + "group": Array [ + "group1", + ], "series_0.data": 7, "series_1.data": 2.3333333333333335, "ts_bucket": 1641341100, }, ] +`); + const multiGroupBysData = ( + await clickhouse.getMultiSeriesChart({ + series: [ + { + type: 'time', + table: 'logs', + aggFn: clickhouse.AggFn.Sum, + field: 'awesomeNumber', + where: `runId:${runId}`, + groupBy: ['testGroup', 'testOtherGroup'], + }, + ], + tableVersion: undefined, + teamId, + startTime: now, + endTime: now + ms('10m'), + granularity: '5 minute', + maxNumGroups: 20, + seriesReturnType: clickhouse.SeriesReturnType.Column, + }) + ).data.map(d => { + return _.pick(d, [ + 'group', + 'series_0.data', + 'series_1.data', + 'ts_bucket', + ]); + }); + expect(multiGroupBysData.length).toEqual(5); + expect(multiGroupBysData).toMatchInlineSnapshot(` +Array [ + Object { + "group": Array [ + "group2", + "otherGroup1", + ], + "series_0.data": 777, + "ts_bucket": 1641340800, + }, + Object { + "group": Array [ + "group1", + "otherGroup2", + ], + "series_0.data": 61, + "ts_bucket": 1641340800, + }, + Object { + "group": Array [ + "group1", + "otherGroup1", + ], + "series_0.data": 16, + "ts_bucket": 1641340800, + }, + Object { + "group": Array [ + "group1", + "otherGroup2", + ], + "series_0.data": 6, + "ts_bucket": 1641341100, + }, + Object { + "group": Array [ + "group1", + "otherGroup3", + ], + "series_0.data": 1, + "ts_bucket": 1641341100, + }, +] `); const ratioData = ( @@ -272,24 +361,28 @@ Array [ expect(ratioData).toMatchInlineSnapshot(` Array [ Object { - "group": "group1", + "group": Array [ + "group1", + ], "series_0.data": 3, "ts_bucket": 1641340800, }, Object { - "group": "group2", + "group": Array [ + "group2", + ], "series_0.data": 3, "ts_bucket": 1641340800, }, Object { - "group": "group1", + "group": Array [ + "group1", + ], "series_0.data": 3, "ts_bucket": 1641341100, }, ] `); - - jest.clearAllMocks(); }); it('fetches multi-series metric time chart correctly', async () => { @@ -421,22 +514,22 @@ Array [ expect(singleSumSeriesData).toMatchInlineSnapshot(` Array [ Object { - "group": "", + "group": Array [], "series_0.data": 19, "ts_bucket": 1641340800, }, Object { - "group": "", + "group": Array [], "series_0.data": 79, "ts_bucket": 1641341100, }, Object { - "group": "", + "group": Array [], "series_0.data": 5813, "ts_bucket": 1641341400, }, Object { - "group": "", + "group": Array [], "series_0.data": 78754, "ts_bucket": 1641341700, }, @@ -471,22 +564,30 @@ Array [ expect(singleGaugeGroupedSeriesData).toMatchInlineSnapshot(` Array [ Object { - "group": "test1", + "group": Array [ + "test1", + ], "series_0.data": 6.25, "ts_bucket": 1641340800, }, Object { - "group": "test2", + "group": Array [ + "test2", + ], "series_0.data": 4, "ts_bucket": 1641340800, }, Object { - "group": "test1", + "group": Array [ + "test1", + ], "series_0.data": 80, "ts_bucket": 1641341100, }, Object { - "group": "test2", + "group": Array [ + "test2", + ], "series_0.data": 4, "ts_bucket": 1641341100, }, @@ -521,12 +622,12 @@ Array [ expect(singleGaugeSeriesData).toMatchInlineSnapshot(` Array [ Object { - "group": "", + "group": Array [], "series_0.data": 5.125, "ts_bucket": 1641340800, }, Object { - "group": "", + "group": Array [], "series_0.data": 42, "ts_bucket": 1641341100, }, @@ -561,12 +662,12 @@ Array [ expect(singleGaugeSeriesSummedData).toMatchInlineSnapshot(` Array [ Object { - "group": "", + "group": Array [], "series_0.data": 10.25, "ts_bucket": 1641340800, }, Object { - "group": "", + "group": Array [], "series_0.data": 84, "ts_bucket": 1641341100, }, @@ -610,39 +711,45 @@ Array [ expect(ratioData).toMatchInlineSnapshot(` Array [ Object { - "group": "test1", + "group": Array [ + "test1", + ], "series_0.data": 0.78125, "ts_bucket": 1641340800, }, Object { - "group": "test2", + "group": Array [ + "test2", + ], "series_0.data": 0.36363636363636365, "ts_bucket": 1641340800, }, Object { - "group": "test1", + "group": Array [ + "test1", + ], "series_0.data": 80, "ts_bucket": 1641341100, }, Object { - "group": "test2", + "group": Array [ + "test2", + ], "series_0.data": 0.05128205128205128, "ts_bucket": 1641341100, }, Object { - "group": "", + "group": Array [], "series_0.data": null, "ts_bucket": 1641341400, }, Object { - "group": "", + "group": Array [], "series_0.data": null, "ts_bucket": 1641341700, }, ] `); - - jest.clearAllMocks(); }); it('fetches multi series metric table chart correctly', async () => { @@ -788,15 +895,13 @@ Array [ expect(singleSumSeriesData).toMatchInlineSnapshot(` Array [ Object { - "group": "", + "group": Array [], "series_0.data": 84665, "series_1.data": 42, "ts_bucket": "0", }, ] `); - - jest.clearAllMocks(); }); it('limits groups and sorts multi series charts properly', async () => { @@ -869,37 +974,49 @@ Array [ expect(ascData).toMatchInlineSnapshot(` Array [ Object { - "group": "group0", + "group": Array [ + "group0", + ], "series_0.data": 1, "series_1.data": 0, "ts_bucket": 1641340800, }, Object { - "group": "group1", + "group": Array [ + "group1", + ], "series_0.data": 1, "series_1.data": 1, "ts_bucket": 1641340800, }, Object { - "group": "group2", + "group": Array [ + "group2", + ], "series_0.data": 1, "series_1.data": 2, "ts_bucket": 1641340800, }, Object { - "group": "group0", + "group": Array [ + "group0", + ], "series_0.data": 1, "series_1.data": 0, "ts_bucket": 1641341100, }, Object { - "group": "group2", + "group": Array [ + "group2", + ], "series_0.data": 1, "series_1.data": 2, "ts_bucket": 1641341100, }, Object { - "group": "group1", + "group": Array [ + "group1", + ], "series_0.data": 1, "series_1.data": 19, "ts_bucket": 1641341100, @@ -944,37 +1061,49 @@ Array [ expect(descData).toMatchInlineSnapshot(` Array [ Object { - "group": "group5", + "group": Array [ + "group5", + ], "series_0.data": 1, "series_1.data": 5, "ts_bucket": 1641340800, }, Object { - "group": "group3", + "group": Array [ + "group3", + ], "series_0.data": 1, "series_1.data": 3, "ts_bucket": 1641340800, }, Object { - "group": "group1", + "group": Array [ + "group1", + ], "series_0.data": 1, "series_1.data": 1, "ts_bucket": 1641340800, }, Object { - "group": "group1", + "group": Array [ + "group1", + ], "series_0.data": 1, "series_1.data": 19, "ts_bucket": 1641341100, }, Object { - "group": "group3", + "group": Array [ + "group3", + ], "series_0.data": 1, "series_1.data": 17, "ts_bucket": 1641341100, }, Object { - "group": "group5", + "group": Array [ + "group5", + ], "series_0.data": 1, "series_1.data": 15, "ts_bucket": 1641341100, @@ -1019,26 +1148,31 @@ Array [ expect(descDataSimple).toMatchInlineSnapshot(` Array [ Object { - "group": "group9", + "group": Array [ + "group9", + ], "series_0.data": 1, "series_1.data": 9, "ts_bucket": 1641340800, }, Object { - "group": "group8", + "group": Array [ + "group8", + ], "series_0.data": 1, "series_1.data": 8, "ts_bucket": 1641340800, }, Object { - "group": "group7", + "group": Array [ + "group7", + ], "series_0.data": 1, "series_1.data": 7, "ts_bucket": 1641340800, }, ] `); - jest.clearAllMocks(); }); it('fetches legacy format correctly for alerts', async () => { @@ -1153,25 +1287,29 @@ Array [ Array [ Object { "data": 777, - "group": "group2", + "group": Array [ + "group2", + ], "ts_bucket": 1641340800, }, Object { "data": 77, - "group": "group1", + "group": Array [ + "group1", + ], "ts_bucket": 1641340800, }, Object { "data": 7, - "group": "group1", + "group": Array [ + "group1", + ], "ts_bucket": 1641341100, }, ] `); expect(data).toMatchObject(oldData); - - jest.clearAllMocks(); }); it('clientInsertWithRetries (success)', async () => { diff --git a/packages/api/src/clickhouse/index.ts b/packages/api/src/clickhouse/index.ts index ccd3af29..c1dfb446 100644 --- a/packages/api/src/clickhouse/index.ts +++ b/packages/api/src/clickhouse/index.ts @@ -858,6 +858,7 @@ export const getMetricsChart = async ({ return result; }; +// TODO: support multiple groupBy export const buildMetricSeriesQuery = async ({ aggFn, dataType, @@ -911,8 +912,8 @@ export const buildMetricSeriesQuery = async ({ ) : "'0' as ts_bucket", groupBy - ? SqlString.format(`_string_attributes[?] AS group`, [groupBy]) - : "'' AS group", + ? SqlString.format(`[_string_attributes[?]] AS group`, [groupBy]) + : '[] AS group', ]; const hasGroupBy = groupBy != '' && groupBy != null; @@ -1065,7 +1066,7 @@ const buildEventSeriesQuery = async ({ endTime: number; // unix in ms, field?: string; granularity: string | undefined; // can be undefined in the number chart - groupBy: string; + groupBy: string[]; propertyTypeMappingsModel: LogsPropertyTypeMappingsModel; q: string; sortOrder?: 'asc' | 'desc'; @@ -1096,14 +1097,36 @@ const buildEventSeriesQuery = async ({ ? buildSearchColumnName(propertyTypeMappingsModel.get(field), field) : ''; - const hasGroupBy = groupBy != '' && groupBy != null; const isCountFn = aggFn === AggFn.Count; - const groupByField = - hasGroupBy && - buildSearchColumnName(propertyTypeMappingsModel.get(groupBy), groupBy); + const groupByColumnNames = groupBy.map(g => { + const columnName = buildSearchColumnName( + propertyTypeMappingsModel.get(g), + g, + ); + if (columnName != null) { + return columnName; + } + throw new Error(`Group by field ${g} does not exist`); + }); + + const hasGroupBy = groupByColumnNames.length > 0; const serializer = new SQLSerializer(propertyTypeMappingsModel); + // compute additional where clause for group-by fields + select field + let additionalSelectFieldCheck = ''; + let additionalGroupByFieldCheck = ''; + if (!isCountFn && field != null) { + const _condition = await serializer.isNotNull(field, false); + additionalSelectFieldCheck = ` AND (${_condition})`; + } + if (hasGroupBy) { + const _conditions = await Promise.all( + groupBy.map(g => serializer.isNotNull(g, false)), + ); + additionalGroupByFieldCheck = ` AND (${_conditions.join(' AND ')})`; + } + const label = SqlString.escape(`${aggFn}(${field})`); const selectClause = [ @@ -1131,11 +1154,11 @@ const buildEventSeriesQuery = async ({ granularity != null ? `toUnixTimestamp(toStartOfInterval(timestamp, INTERVAL ${granularity})) as ts_bucket` : "'0' as ts_bucket", - groupByField ? `${groupByField} as group` : `'' as group`, // FIXME: should we fallback to use aggFn as group + hasGroupBy ? `[${groupByColumnNames.join(',')}] as group` : `[] as group`, // FIXME: should we fallback to use aggFn as group `${label} as label`, ].join(','); - const groupByClause = `ts_bucket ${groupByField ? `, ${groupByField}` : ''}`; + const groupByClause = ['ts_bucket', ...groupByColumnNames].join(','); const query = SqlString.format( ` @@ -1160,16 +1183,8 @@ const buildEventSeriesQuery = async ({ tableName, buildTeamLogStreamWhereCondition(tableVersion, teamId), SqlString.raw(whereClause), - SqlString.raw( - !isCountFn && field != null - ? ` AND (${await serializer.isNotNull(field, false)})` - : '', - ), - SqlString.raw( - hasGroupBy - ? ` AND (${await serializer.isNotNull(groupBy, false)})` - : '', - ), + SqlString.raw(additionalSelectFieldCheck), + SqlString.raw(additionalGroupByFieldCheck), SqlString.raw(groupByClause), ...(granularity != null ? [ @@ -1302,7 +1317,7 @@ export const queryMultiSeriesChart = async ({ const result = await rows.json< ResponseJSON<{ ts_bucket: number; - group: string; + group: string[]; [series_data: `series_${number}.data`]: number; }> >(); @@ -1386,7 +1401,7 @@ export const getMultiSeriesChart = async ({ endTime, field: s.field, granularity, - groupBy: s.groupBy[0], + groupBy: s.groupBy, propertyTypeMappingsModel, q: s.where, sortOrder: s.type === 'table' ? s.sortOrder : undefined, @@ -2022,6 +2037,8 @@ export const getLogById = async ( return result; }; +// TODO: support multiple group bys +// FIXME: return 'group' field should be array type export const checkAlert = async ({ endTime, groupBy, diff --git a/packages/api/src/tasks/checkAlerts.ts b/packages/api/src/tasks/checkAlerts.ts index 889e917e..b9dd432d 100644 --- a/packages/api/src/tasks/checkAlerts.ts +++ b/packages/api/src/tasks/checkAlerts.ts @@ -482,7 +482,9 @@ export const processAlert = async (now: Date, alert: AlertDocument) => { alert, dashboard: targetDashboard, endTime: fns.addMinutes(bucketStart, windowSizeInMins), - group: checkData.group, + group: Array.isArray(checkData.group) + ? checkData.group.join(', ') + : checkData.group, logView, startTime: bucketStart, totalCount, diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index b32d9061..a8fe9e08 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -267,14 +267,16 @@ const HDXMultiSeriesLineChart = memo( // per series/group const dataKey = `series_${i}.data:::${row.group}`; + const hasGroup = Array.isArray(row.group) && row.group.length > 0; + const displayName = series.length === 1 ? // If there's only one series, just show the group, unless there is no group - row.group + hasGroup ? `${row.group}` : meta.displayName : // Otherwise, show the series and a group if there is any - `${row.group ? `${row.group} • ` : ''}${meta.displayName}`; + `${hasGroup ? `${row.group} • ` : ''}${meta.displayName}`; acc[dataKey] = row[meta.dataKey]; lineDataMap[dataKey] = {