Bug fix: Restore dashboard filters, fetch log property type mappings correctly for charts (#182)

This commit is contained in:
Mike Shi 2024-01-04 15:02:58 -08:00 committed by GitHub
parent 3c29bcf6fd
commit 2910461669
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 120 additions and 113 deletions

View file

@ -0,0 +1,7 @@
---
'@hyperdx/api': patch
'@hyperdx/app': patch
---
Bug fix: Restore dashboard filters, use correct field lookup for metrics, and
remove extra log property type mapping fetches.

View file

@ -169,7 +169,7 @@ Array [
}),
]);
const propertyTypeMappingsModel = mockLogsPropertyTypeMappingsModel({
mockLogsPropertyTypeMappingsModel({
testGroup: 'string',
awesomeNumber: 'number',
runId: 'string',
@ -202,7 +202,6 @@ Array [
granularity: '5 minute',
maxNumGroups: 20,
seriesReturnType: clickhouse.SeriesReturnType.Column,
propertyTypeMappingsModel,
})
).data.map(d => {
return _.pick(d, [
@ -264,7 +263,6 @@ Array [
granularity: '5 minute',
maxNumGroups: 20,
seriesReturnType: clickhouse.SeriesReturnType.Ratio,
propertyTypeMappingsModel,
})
).data.map(d => {
return _.pick(d, ['group', 'series_0.data', 'ts_bucket']);
@ -388,7 +386,7 @@ Array [
}),
);
const propertyTypeMappingsModel = mockLogsPropertyTypeMappingsModel({});
mockLogsPropertyTypeMappingsModel({});
mockSpyMetricPropertyTypeMappingsModel({
runId: 'string',
@ -415,7 +413,6 @@ Array [
granularity: '5 minute',
maxNumGroups: 20,
seriesReturnType: clickhouse.SeriesReturnType.Column,
propertyTypeMappingsModel,
})
).data.map(d => {
return _.pick(d, ['group', 'series_0.data', 'ts_bucket']);
@ -466,7 +463,6 @@ Array [
granularity: '5 minute',
maxNumGroups: 20,
seriesReturnType: clickhouse.SeriesReturnType.Column,
propertyTypeMappingsModel,
})
).data.map(d => {
return _.pick(d, ['group', 'series_0.data', 'ts_bucket']);
@ -517,7 +513,6 @@ Array [
granularity: '5 minute',
maxNumGroups: 20,
seriesReturnType: clickhouse.SeriesReturnType.Column,
propertyTypeMappingsModel,
})
).data.map(d => {
return _.pick(d, ['group', 'series_0.data', 'ts_bucket']);
@ -558,7 +553,6 @@ Array [
granularity: '5 minute',
maxNumGroups: 20,
seriesReturnType: clickhouse.SeriesReturnType.Column,
propertyTypeMappingsModel,
})
).data.map(d => {
return _.pick(d, ['group', 'series_0.data', 'ts_bucket']);
@ -608,7 +602,6 @@ Array [
granularity: '5 minute',
maxNumGroups: 20,
seriesReturnType: clickhouse.SeriesReturnType.Ratio,
propertyTypeMappingsModel,
})
).data.map(d => {
return _.pick(d, ['group', 'series_0.data', 'ts_bucket']);
@ -746,7 +739,7 @@ Array [
}),
);
const propertyTypeMappingsModel = mockLogsPropertyTypeMappingsModel({});
mockLogsPropertyTypeMappingsModel({});
mockSpyMetricPropertyTypeMappingsModel({
runId: 'string',
@ -782,7 +775,6 @@ Array [
granularity: undefined,
maxNumGroups: 20,
seriesReturnType: clickhouse.SeriesReturnType.Column,
propertyTypeMappingsModel,
})
).data.map(d => {
return _.pick(d, [
@ -834,7 +826,7 @@ Array [
]),
);
const propertyTypeMappingsModel = mockLogsPropertyTypeMappingsModel({
mockLogsPropertyTypeMappingsModel({
testGroup: 'string',
awesomeNumber: 'number',
runId: 'string',
@ -869,7 +861,6 @@ Array [
granularity: '5 minute',
maxNumGroups: 3,
seriesReturnType: clickhouse.SeriesReturnType.Column,
propertyTypeMappingsModel,
})
).data.map(d =>
_.pick(d, ['series_0.data', 'series_1.data', 'group', 'ts_bucket']),
@ -945,7 +936,6 @@ Array [
granularity: '5 minute',
maxNumGroups: 3,
seriesReturnType: clickhouse.SeriesReturnType.Column,
propertyTypeMappingsModel,
})
).data.map(d =>
_.pick(d, ['series_0.data', 'series_1.data', 'group', 'ts_bucket']),
@ -1021,7 +1011,6 @@ Array [
granularity: '5 minute',
maxNumGroups: 3,
seriesReturnType: clickhouse.SeriesReturnType.Column,
propertyTypeMappingsModel,
})
).data.map(d =>
_.pick(d, ['series_0.data', 'series_1.data', 'group', 'ts_bucket']),
@ -1142,7 +1131,6 @@ Array [
granularity: '5 minute',
maxNumGroups: 20,
seriesReturnType: clickhouse.SeriesReturnType.Column,
propertyTypeMappingsModel,
})
).data;

View file

@ -869,6 +869,7 @@ export const buildMetricSeriesQuery = async ({
startTime,
teamId,
sortOrder,
propertyTypeMappingsModel,
}: {
aggFn: AggFn;
dataType: MetricsDataType;
@ -880,12 +881,9 @@ export const buildMetricSeriesQuery = async ({
startTime: number; // unix in ms
teamId: string;
sortOrder?: 'asc' | 'desc';
propertyTypeMappingsModel: MetricsPropertyTypeMappingsModel;
}) => {
const tableName = `default.${TableName.Metric}`;
const propertyTypeMappingsModel = await buildMetricsPropertyTypeMappingsModel(
undefined, // default version
teamId,
);
const isRate = isRateAggFn(aggFn);
@ -1299,12 +1297,6 @@ export const queryMultiSeriesChart = async ({
const rows = await client.query({
query,
format: 'JSON',
clickhouse_settings: {
additional_table_filters: buildLogStreamAdditionalFilters(
tableVersion,
teamId,
),
},
});
const result = await rows.json<
@ -1322,7 +1314,6 @@ export const getMultiSeriesChart = async ({
endTime,
granularity,
maxNumGroups,
propertyTypeMappingsModel,
startTime,
tableVersion,
teamId,
@ -1333,7 +1324,6 @@ export const getMultiSeriesChart = async ({
startTime: number; // unix in ms
granularity: string | undefined; // can be undefined in the number chart
maxNumGroups: number;
propertyTypeMappingsModel?: LogsPropertyTypeMappingsModel;
tableVersion: number | undefined;
teamId: string;
seriesReturnType?: SeriesReturnType;
@ -1349,8 +1339,37 @@ export const getMultiSeriesChart = async ({
(series[0].table === 'logs' || series[0].table == null)) ||
!('table' in series[0])
) {
if (propertyTypeMappingsModel == null) {
throw new Error('propertyTypeMappingsModel is required for logs chart');
const propertyTypeMappingsModel = await buildLogsPropertyTypeMappingsModel(
tableVersion,
teamId.toString(),
startTime,
endTime,
);
const propertySet = new Set<string>();
series.map(s => {
if ('field' in s && s.field != null) {
propertySet.add(s.field);
}
if ('groupBy' in s && s.groupBy.length > 0) {
s.groupBy.map(g => propertySet.add(g));
}
});
// Hack to refresh property cache if needed
const properties = Array.from(propertySet);
if (
properties.some(p => {
return !doesLogsPropertyExist(p, propertyTypeMappingsModel);
})
) {
logger.warn({
message: `getChart: Property type mappings cache is out of date (${properties.join(
', ',
)})`,
});
await propertyTypeMappingsModel.refresh();
}
queries = await Promise.all(
@ -1378,6 +1397,12 @@ export const getMultiSeriesChart = async ({
}),
);
} else if ('table' in series[0] && series[0].table === 'metrics') {
const propertyTypeMappingsModel =
await buildMetricsPropertyTypeMappingsModel(
undefined, // default version
teamId,
);
queries = await Promise.all(
series.map(s => {
if (s.type != 'time' && s.type != 'table') {
@ -1404,6 +1429,7 @@ export const getMultiSeriesChart = async ({
startTime,
teamId,
dataType: s.metricDataType,
propertyTypeMappingsModel,
});
}),
);
@ -1423,7 +1449,6 @@ export const getMultiSeriesChartLegacyFormat = async ({
endTime,
granularity,
maxNumGroups,
propertyTypeMappingsModel,
startTime,
tableVersion,
teamId,
@ -1434,7 +1459,6 @@ export const getMultiSeriesChartLegacyFormat = async ({
startTime: number; // unix in ms
granularity: string | undefined; // can be undefined in the number chart
maxNumGroups: number;
propertyTypeMappingsModel?: LogsPropertyTypeMappingsModel;
tableVersion: number | undefined;
teamId: string;
seriesReturnType?: SeriesReturnType;
@ -1444,7 +1468,6 @@ export const getMultiSeriesChartLegacyFormat = async ({
endTime,
granularity,
maxNumGroups,
propertyTypeMappingsModel,
startTime,
tableVersion,
teamId,

View file

@ -40,43 +40,6 @@ router.post(
return res.sendStatus(403);
}
const propertyTypeMappingsModel =
await clickhouse.buildLogsPropertyTypeMappingsModel(
team.logStreamTableVersion,
teamId.toString(),
startTime,
endTime,
);
const propertySet = new Set<string>();
series.map(s => {
if ('field' in s && s.field != null) {
propertySet.add(s.field);
}
if ('groupBy' in s && s.groupBy.length > 0) {
s.groupBy.map(g => propertySet.add(g));
}
});
// Hack to refresh property cache if needed
const properties = Array.from(propertySet);
if (
properties.some(p => {
return !clickhouse.doesLogsPropertyExist(
p,
propertyTypeMappingsModel,
);
})
) {
logger.warn({
message: `getChart: Property type mappings cache is out of date (${properties.join(
', ',
)})`,
});
await propertyTypeMappingsModel.refresh();
}
// TODO: expose this to the frontend
const MAX_NUM_GROUPS = 20;
@ -86,7 +49,6 @@ router.post(
endTime: endTime,
granularity,
maxNumGroups: MAX_NUM_GROUPS,
propertyTypeMappingsModel,
startTime: startTime,
tableVersion: team.logStreamTableVersion,
teamId: teamId.toString(),

View file

@ -394,20 +394,12 @@ export const processAlert = async (now: Date, alert: AlertDocument) => {
targetDashboard = dashboard;
const startTimeMs = fns.getTime(checkStartTime);
const endTimeMs = fns.getTime(checkEndTime);
const propertyTypeMappingsModel =
await clickhouse.buildLogsPropertyTypeMappingsModel(
dashboard.team.logStreamTableVersion,
dashboard.team._id.toString(),
startTimeMs,
endTimeMs,
);
checksData = await clickhouse.getMultiSeriesChartLegacyFormat({
series: chart.series,
endTime: endTimeMs,
granularity: `${windowSizeInMins} minute`,
maxNumGroups: MAX_NUM_GROUPS,
propertyTypeMappingsModel,
startTime: startTimeMs,
tableVersion: dashboard.team.logStreamTableVersion,
teamId: dashboard.team._id.toString(),

View file

@ -64,18 +64,32 @@ export type Granularity =
| '7 day'
| '30 day';
const seriesDisplayName = (s: ChartSeries) =>
s.type === 'time' || s.type === 'table'
? `${s.aggFn}${
s.aggFn !== 'count'
? `(${
s.table === 'metrics'
? s.field?.split(' - ')?.[0] ?? s.field
: s.field
})`
: '()'
}${s.where ? `{${s.where}}` : ''}`
: '';
const seriesDisplayName = (
s: ChartSeries,
{
showAggFn,
showField,
showWhere,
}: {
showAggFn?: boolean;
showField?: boolean;
showWhere?: boolean;
} = {},
) => {
if (s.type === 'time' || s.type === 'table') {
const displayField =
s.aggFn !== 'count'
? s.table === 'metrics'
? s.field?.split(' - ')?.[0] ?? s.field
: s.field
: '';
return `${showAggFn === false ? '' : s.aggFn}${
showField === false ? '' : `(${displayField})`
}${s.where && showWhere !== false ? `{${s.where}}` : ''}`;
}
return '';
};
export function seriesColumns({
series,
@ -84,14 +98,28 @@ export function seriesColumns({
seriesReturnType: 'ratio' | 'column';
series: ChartSeries[];
}) {
const uniqueWhere = new Set<string | undefined>(
series.map(s => ('where' in s ? s.where : undefined)),
);
const uniqueFields = new Set<string | undefined>(
series.map(s => ('field' in s ? s.field : undefined)),
);
const showField = uniqueFields.size > 1;
const showWhere = uniqueWhere.size > 1;
const seriesMeta =
seriesReturnType === 'ratio'
? [
{
dataKey: `series_0.data`,
displayName: `${seriesDisplayName(series[0])}/${seriesDisplayName(
series[1],
)}`,
displayName: `${seriesDisplayName(series[0], {
showField,
showWhere,
})}/${seriesDisplayName(series[1], {
showField,
showWhere,
})}`,
sortOrder:
'sortOrder' in series[0] ? series[0].sortOrder : undefined,
},
@ -99,7 +127,10 @@ export function seriesColumns({
: series.map((s, i) => {
return {
dataKey: `series_${i}.data`,
displayName: seriesDisplayName(s),
displayName: seriesDisplayName(s, {
showField,
showWhere,
}),
sortOrder: 'sortOrder' in s ? s.sortOrder : undefined,
};
});

View file

@ -138,6 +138,14 @@ const Tile = forwardRef(
granularity ?? convertDateRangeToGranularityString(dateRange, 60),
dateRange,
numberFormat: chart.series[0].numberFormat,
seriesReturnType: chart.seriesReturnType,
series: chart.series.map(s => ({
...s,
where: buildAndWhereClause(
query,
s.type === 'time' ? s.where : '',
),
})),
}
: type === 'table'
? {
@ -152,6 +160,14 @@ const Tile = forwardRef(
granularity ?? convertDateRangeToGranularityString(dateRange, 60),
dateRange,
numberFormat: chart.series[0].numberFormat,
series: chart.series.map(s => ({
...s,
where: buildAndWhereClause(
query,
s.type === 'table' ? s.where : '',
),
})),
seriesReturnType: chart.seriesReturnType,
}
: type === 'histogram'
? {
@ -272,22 +288,10 @@ const Tile = forwardRef(
onMouseDown={e => e.stopPropagation()}
>
{chart.series[0].type === 'time' && config.type === 'time' && (
<HDXMultiSeriesLineChart
config={{
...config,
seriesReturnType: chart.seriesReturnType,
series: chart.series,
}}
/>
<HDXMultiSeriesLineChart config={config} />
)}
{chart.series[0].type === 'table' && config.type === 'table' && (
<HDXMultiSeriesTableChart
config={{
...config,
seriesReturnType: chart.seriesReturnType,
series: chart.series,
}}
/>
<HDXMultiSeriesTableChart config={config} />
)}
{config.type === 'histogram' && (
<HDXHistogramChart config={config} onSettled={onSettled} />

View file

@ -304,7 +304,7 @@ export const HDXLineChartTooltip = memo((props: any) => {
.sort((a: any, b: any) => b.value - a.value)
.map((p: any) => (
<div key={p.dataKey} style={{ color: p.color }}>
{truncateMiddle(p.name ?? p.dataKey, 50)}:{' '}
{truncateMiddle(p.name ?? p.dataKey, 70)}:{' '}
{numberFormat ? formatNumber(p.value, numberFormat) : p.value}
</div>
))}

View file

@ -37,15 +37,15 @@
}
.chartTooltip {
background-color: rgba(0, 0, 0, 0.5);
backdrop-filter: blur(1px);
background-color: rgba(0, 0, 0, 0.8);
backdrop-filter: blur(2px);
border-radius: 4px;
font-size: 11px;
}
.chartTooltipHeader {
padding: 2px 12px;
color: $slate-400;
color: $slate-200;
border-bottom: 1px solid $slate-700;
}