Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add postGroupWhere filter option to /chart/series endpoint #249

Merged
merged 2 commits into from
Jan 18, 2024
Merged
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
5 changes: 5 additions & 0 deletions .changeset/hip-pans-hear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@hyperdx/api': patch
---

Add postGroupWhere filter option to /chart/series endpoint
71 changes: 71 additions & 0 deletions packages/api/src/clickhouse/__tests__/clickhouse.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,77 @@ Array [
]
`);
});

it('filters using postGroupWhere properly', async () => {
const queryConfig: Parameters<typeof clickhouse.getMultiSeriesChart>[0] =
{
series: [
{
type: 'time',
table: 'metrics',
aggFn: clickhouse.AggFn.LastValue,
field: 'test.cpu',
where: `runId:${runId}`,
groupBy: ['host'],
metricDataType: clickhouse.MetricsDataType.Gauge,
},
],
tableVersion: undefined,
teamId,
startTime: now,
endTime: now + ms('20m'),
granularity: undefined,
maxNumGroups: 20,
seriesReturnType: clickhouse.SeriesReturnType.Column,
postGroupWhere: 'series_0:4',
};

// Exclude postGroupWhere to assert we get the test data we expect at first
const data = (
await clickhouse.getMultiSeriesChart(
_.omit(queryConfig, ['postGroupWhere']),
)
).data.map(d => {
return _.pick(d, ['group', 'series_0.data', 'ts_bucket']);
});

expect(data).toMatchInlineSnapshot(`
Array [
Object {
"group": Array [
"test1",
],
"series_0.data": 80,
"ts_bucket": "0",
},
Object {
"group": Array [
"test2",
],
"series_0.data": 4,
"ts_bucket": "0",
},
]
`);

const filteredData = (
await clickhouse.getMultiSeriesChart(queryConfig)
).data.map(d => {
return _.pick(d, ['group', 'series_0.data', 'ts_bucket']);
});

expect(filteredData).toMatchInlineSnapshot(`
Array [
Object {
"group": Array [
"test2",
],
"series_0.data": 4,
"ts_bucket": "0",
},
]
`);
});
});

it('limits groups and sorts multi series charts properly', async () => {
Expand Down
12 changes: 12 additions & 0 deletions packages/api/src/clickhouse/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
MetricsPropertyTypeMappingsModel,
} from './propertyTypeMappingsModel';
import {
buildPostGroupWhereCondition,
buildSearchColumnName,
buildSearchColumnName_OLD,
buildSearchQueryWhereCondition,
Expand Down Expand Up @@ -1251,12 +1252,14 @@ export const queryMultiSeriesChart = async ({
teamId,
seriesReturnType = SeriesReturnType.Column,
queries,
postGroupWhere,
}: {
maxNumGroups: number;
tableVersion: number | undefined;
teamId: string;
seriesReturnType?: SeriesReturnType;
queries: { query: string; hasGroupBy: boolean; sortOrder?: 'desc' | 'asc' }[];
postGroupWhere?: string;
}) => {
// For now only supports same-table series with the same groupBy

Expand Down Expand Up @@ -1296,6 +1299,10 @@ export const queryMultiSeriesChart = async ({
.join(',\n')
: 'series_0.data / series_1.data as "series_0.data"';

const postGroupWhereClause = postGroupWhere
? buildPostGroupWhereCondition({ query: postGroupWhere })
: '1=1';

// Return each series data as a separate column
const query = SqlString.format(
`WITH ?
Expand All @@ -1306,6 +1313,7 @@ export const queryMultiSeriesChart = async ({
series_0.group as group
FROM series_0 AS series_0
?
WHERE ?
), groups AS (
SELECT *, ?(?) OVER (PARTITION BY group) as rank_order_by_value
FROM raw_groups
Expand All @@ -1323,6 +1331,7 @@ export const queryMultiSeriesChart = async ({
SqlString.raw(seriesCTEs),
SqlString.raw(select),
SqlString.raw(leftJoin),
SqlString.raw(postGroupWhereClause),
// Setting rank_order_by_value
SqlString.raw(sortOrder === 'asc' ? 'MIN' : 'MAX'),
SqlString.raw(
Expand Down Expand Up @@ -1373,6 +1382,7 @@ export const getMultiSeriesChart = async ({
tableVersion,
teamId,
seriesReturnType = SeriesReturnType.Column,
postGroupWhere,
}: {
series: z.infer<typeof chartSeriesSchema>[];
endTime: number; // unix in ms,
Expand All @@ -1382,6 +1392,7 @@ export const getMultiSeriesChart = async ({
tableVersion: number | undefined;
teamId: string;
seriesReturnType?: SeriesReturnType;
postGroupWhere?: string;
}) => {
let queries: {
query: string;
Expand Down Expand Up @@ -1496,6 +1507,7 @@ export const getMultiSeriesChart = async ({
teamId,
seriesReturnType,
queries,
postGroupWhere,
});
};

Expand Down
18 changes: 18 additions & 0 deletions packages/api/src/clickhouse/searchQueryParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -652,3 +652,21 @@ export const buildSearchQueryWhereCondition = async ({
builder.teamId = teamId;
return await builder.timestampInBetween(startTime, endTime).build();
};

export const buildPostGroupWhereCondition = ({ query }: { query: string }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I wonder why we don't pass query string directly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh like why use a kwargs-like setup? I suspect this function will grow over time and copied the buildSearchQueryWhereCondition format.

// This needs to be replaced with the proper query builder
// after generalizing it for arbitrary field resolutions
// the query can only specify one field from the series and an exact match
const [field, value] = query.split(':', 2);
const seriesNumber = parseInt(field.replace('series_', ''), 10);
const floatValue = parseFloat(value);

if (Number.isSafeInteger(seriesNumber) === false) {
throw new Error('Invalid series number');
}
if (Number.isNaN(floatValue)) {
throw new Error('Invalid value');
}

return SqlString.format(`series_${seriesNumber}.data = ?`, [floatValue]);
};
12 changes: 10 additions & 2 deletions packages/api/src/routers/api/chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,20 @@ router.post(
granularity: z.nativeEnum(clickhouse.Granularity).optional(),
startTime: z.number(),
seriesReturnType: z.optional(z.nativeEnum(clickhouse.SeriesReturnType)),
postGroupWhere: z.optional(z.string().max(1024)),
}),
}),
async (req, res, next) => {
try {
const teamId = req.user?.team;
const { endTime, granularity, startTime, seriesReturnType, series } =
req.body;
const {
endTime,
granularity,
startTime,
seriesReturnType,
series,
postGroupWhere,
} = req.body;

if (teamId == null) {
return res.sendStatus(403);
Expand All @@ -161,6 +168,7 @@ router.post(
tableVersion: team.logStreamTableVersion,
teamId: teamId.toString(),
seriesReturnType,
postGroupWhere,
}),
);
} catch (e) {
Expand Down
Loading