Skip to content

Commit

Permalink
Merge branch 'main' into ei/HDX-923
Browse files Browse the repository at this point in the history
  • Loading branch information
kodiakhq[bot] authored Jul 16, 2024
2 parents bdde29e + 90a5ca7 commit 5d5aad3
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 47 deletions.
5 changes: 5 additions & 0 deletions .changeset/fair-lizards-yawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@hyperdx/api': patch
---

fix: validate the min length of series field at charts series endpoint
12 changes: 7 additions & 5 deletions packages/api/src/clickhouse/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1412,18 +1412,18 @@ const buildEventSeriesQuery = async ({
? buildSearchColumnName(propertyTypeMappingsModel.get(field), field)
: '';

const groupByColumnNames = groupBy.map(g => {
const groupByColumnExpressions = groupBy.map(g => {
const columnName = buildSearchColumnName(
propertyTypeMappingsModel.get(g),
g,
);
if (columnName != null) {
return columnName;
return `toString(${columnName})`;
}
throw new Error(`Group by field ${g} does not exist`);
});

const hasGroupBy = groupByColumnNames.length > 0;
const hasGroupBy = groupByColumnExpressions.length > 0;

const serializer = new SQLSerializer(propertyTypeMappingsModel);

Expand Down Expand Up @@ -1497,11 +1497,13 @@ const buildEventSeriesQuery = async ({
granularity != null
? `toUnixTimestamp(toStartOfInterval(timestamp, INTERVAL ${granularity})) as ts_bucket`
: "'0' as ts_bucket",
hasGroupBy ? `[${groupByColumnNames.join(',')}] as group` : `[] as group`,
hasGroupBy
? `[${groupByColumnExpressions.join(',')}] as group`
: `[] as group`,
`${label} as label`,
].join(',');

const groupByClause = ['ts_bucket', ...groupByColumnNames].join(',');
const groupByClause = ['ts_bucket', ...groupByColumnExpressions].join(',');

const query = SqlString.format(
`
Expand Down
2 changes: 1 addition & 1 deletion packages/api/src/routers/api/chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ router.post(
'/series',
validateRequest({
body: z.object({
series: z.array(chartSeriesSchema),
series: z.array(chartSeriesSchema).min(1),
endTime: z.number(),
granularity: z.nativeEnum(clickhouse.Granularity).optional(),
startTime: z.number(),
Expand Down
1 change: 1 addition & 0 deletions packages/api/src/routers/external-api/v1/charts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ router.post(
body: z.object({
series: z
.array(externalQueryChartSeriesSchema)
.min(1)
.max(5)
.refine(
series => {
Expand Down
19 changes: 11 additions & 8 deletions packages/app/src/ChartUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
import { NumberFormatInput } from './components/NumberFormat';
import api from './api';
import Checkbox from './Checkbox';
import FieldMultiSelect from './FieldMultiSelect';
import MetricTagFilterInput from './MetricTagFilterInput';
import SearchInput from './SearchInput';
import { AggFn, ChartSeries, MetricsDataType, SourceTable } from './types';
Expand Down Expand Up @@ -930,12 +931,12 @@ export function ChartSeriesFormCompact({
}: {
aggFn: AggFn;
field: string | undefined;
groupBy?: string | undefined;
groupBy?: string[] | undefined;
setAggFn: (fn: AggFn) => void;
setField: (field: string | undefined) => void;
setFieldAndAggFn: (field: string | undefined, fn: AggFn) => void;
setTableAndAggFn?: (table: SourceTable, fn: AggFn) => void;
setGroupBy?: (groupBy: string | undefined) => void;
setGroupBy?: (groupBy: string[] | undefined) => void;
setSortOrder?: (sortOrder: SortOrder) => void;
setWhere: (where: string) => void;
sortOrder?: string;
Expand Down Expand Up @@ -1049,10 +1050,12 @@ export function ChartSeriesFormCompact({
<div className="d-flex align-items-center">
<div className="text-muted">Group By</div>
<div className="ms-3 flex-grow-1" style={{ minWidth: 300 }}>
<GroupBySelect
groupBy={groupBy}
table={table}
setGroupBy={setGroupBy}
<FieldMultiSelect
types={['number', 'bool', 'string']}
values={groupBy ?? []}
setValues={(values: string[]) => {
setGroupBy(values);
}}
/>
</div>
</div>
Expand Down Expand Up @@ -1093,10 +1096,10 @@ export function ChartSeriesFormCompact({
<div className="text-muted fw-500">Group By</div>
<div className="ms-3 flex-grow-1" style={{ minWidth: 300 }}>
<GroupBySelect
groupBy={groupBy}
groupBy={groupBy?.[0]}
fields={field != null ? [field] : []}
table={table}
setGroupBy={setGroupBy}
setGroupBy={g => setGroupBy(g != null ? [g] : undefined)}
/>
{/* <MetricTagSelect
value={groupBy}
Expand Down
90 changes: 58 additions & 32 deletions packages/app/src/EditChartForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
import Checkbox from './Checkbox';
import * as config from './config';
import EditChartFormAlerts from './EditChartFormAlerts';
import FieldMultiSelect from './FieldMultiSelect';
import GranularityPicker from './GranularityPicker';
import HDXHistogramChart from './HDXHistogramChart';
import HDXMarkdownChart from './HDXMarkdownChart';
Expand Down Expand Up @@ -1003,7 +1004,7 @@ export const EditMultiSeriesChartForm = ({
table={series.table ?? 'logs'}
aggFn={series.aggFn}
where={series.where}
groupBy={series.type !== 'number' ? series.groupBy[0] : undefined}
groupBy={series.type !== 'number' ? series.groupBy : undefined}
field={series.field ?? ''}
numberFormat={series.numberFormat}
setAggFn={aggFn =>
Expand Down Expand Up @@ -1037,7 +1038,7 @@ export const EditMultiSeriesChartForm = ({
draftSeries.type !== 'number'
) {
if (groupBy != undefined) {
draftSeries.groupBy[0] = groupBy;
draftSeries.groupBy = groupBy;
} else {
draftSeries.groupBy = [];
}
Expand Down Expand Up @@ -1092,33 +1093,58 @@ export const EditMultiSeriesChartForm = ({
<Flex align="center" gap="md" mb="sm">
<div className="text-muted">Group By</div>
<div className="flex-grow-1">
<GroupBySelect
table={editedChart.series[0].table ?? 'logs'}
groupBy={editedChart.series[0].groupBy[0]}
fields={
editedChart.series
.map(s => (s as TimeChartSeries).field)
.filter(f => f != null) as string[]
}
setGroupBy={groupBy => {
setEditedChart(
produce(editedChart, draft => {
draft.series.forEach((series, i) => {
if (
series.type === chartType &&
series.type !== 'number'
) {
if (groupBy != undefined) {
series.groupBy[0] = groupBy;
} else {
series.groupBy = [];
{editedChart.series[0].table === 'logs' ? (
<FieldMultiSelect
types={['number', 'bool', 'string']}
values={editedChart.series[0].groupBy}
setValues={(values: string[]) => {
setEditedChart(
produce(editedChart, draft => {
draft.series.forEach((series, i) => {
if (
series.type === chartType &&
series.type !== 'number'
) {
if (values != undefined) {
series.groupBy = values;
} else {
series.groupBy = [];
}
}
}
});
}),
);
}}
/>
});
}),
);
}}
/>
) : (
<GroupBySelect
table={editedChart.series[0].table ?? 'logs'}
groupBy={editedChart.series[0].groupBy[0]}
fields={
editedChart.series
.map(s => (s as TimeChartSeries).field)
.filter(f => f != null) as string[]
}
setGroupBy={groupBy => {
setEditedChart(
produce(editedChart, draft => {
draft.series.forEach((series, i) => {
if (
series.type === chartType &&
series.type !== 'number'
) {
if (groupBy != undefined) {
series.groupBy[0] = groupBy;
} else {
series.groupBy = [];
}
}
});
}),
);
}}
/>
)}
</div>
</Flex>
)}
Expand Down Expand Up @@ -1268,10 +1294,10 @@ export const EditLineChartForm = ({
: null,
[_editedChart, alertEnabled, editedAlert?.interval, dateRange, granularity],
);
const previewConfig = useDebounce(
withDashboardFilter(chartConfig, dashboardQuery),
500,
);
const chartConfigWithDashboardFilter = useMemo(() => {
return withDashboardFilter(chartConfig, dashboardQuery);
}, [chartConfig, dashboardQuery]);
const previewConfig = useDebounce(chartConfigWithDashboardFilter, 500);

if (
chartConfig == null ||
Expand Down
5 changes: 4 additions & 1 deletion packages/app/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,10 @@ export const useMultiSeriesChartV2 = (
},
})
.json()
.then((data: any) => setData(data as MultiSeriesChartResponse))
.then((data: any) => {
setData(data as MultiSeriesChartResponse);
setIsError(false);
})
.catch(() => setIsError(true))
.finally(() => {
setIsLoading(false);
Expand Down

0 comments on commit 5d5aad3

Please sign in to comment.