mirror of
https://github.com/SigNoz/signoz.git
synced 2026-06-19 23:10:25 +01:00
Compare commits
2 Commits
issue_5267
...
fix/clickh
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
8212043c1f | ||
|
|
654f328984 |
@@ -274,4 +274,110 @@ describe('convertV5ResponseToLegacy', () => {
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('clickhouse_sql scalar keeps each value column distinct (regression: all-"A" collapse)', () => {
|
||||
const scalar: ScalarData = {
|
||||
columns: [
|
||||
{
|
||||
name: 'service.name',
|
||||
queryName: 'A',
|
||||
aggregationIndex: 0,
|
||||
columnType: 'group',
|
||||
} as unknown as ScalarData['columns'][number],
|
||||
{
|
||||
name: 'current_availability',
|
||||
queryName: 'A',
|
||||
aggregationIndex: 0,
|
||||
columnType: 'aggregation',
|
||||
} as unknown as ScalarData['columns'][number],
|
||||
{
|
||||
name: 'error_budget_remaining',
|
||||
queryName: 'A',
|
||||
aggregationIndex: 1,
|
||||
columnType: 'aggregation',
|
||||
} as unknown as ScalarData['columns'][number],
|
||||
{
|
||||
name: 'budget_status',
|
||||
queryName: 'A',
|
||||
aggregationIndex: 2,
|
||||
columnType: 'group',
|
||||
} as unknown as ScalarData['columns'][number],
|
||||
{
|
||||
name: 'total_requests',
|
||||
queryName: 'A',
|
||||
aggregationIndex: 4,
|
||||
columnType: 'aggregation',
|
||||
} as unknown as ScalarData['columns'][number],
|
||||
],
|
||||
data: [['kuja-api_gateway-service', 99.985, 0.985, 'Healthy ✅', 2181216]],
|
||||
};
|
||||
|
||||
const v5Data: QueryRangeResponseV5 = {
|
||||
type: 'scalar',
|
||||
data: { results: [scalar] },
|
||||
meta: { rowsScanned: 0, bytesScanned: 0, durationMs: 0, stepIntervals: {} },
|
||||
};
|
||||
|
||||
// A clickhouse_sql envelope contributes no aggregation metadata.
|
||||
const params = makeBaseParams('scalar', [
|
||||
{
|
||||
type: 'clickhouse_sql',
|
||||
spec: {
|
||||
name: 'A',
|
||||
query: 'SELECT ...',
|
||||
disabled: false,
|
||||
},
|
||||
} as unknown as QueryRangeRequestV5['compositeQuery']['queries'][number],
|
||||
]);
|
||||
|
||||
const input: SuccessResponse<MetricRangePayloadV5, QueryRangeRequestV5> =
|
||||
makeBaseSuccess({ data: v5Data }, params);
|
||||
// formatForWeb=true is the table-panel path.
|
||||
const result = convertV5ResponseToLegacy(input, { A: '' }, true);
|
||||
|
||||
const [tableEntry] = result.payload.data.result;
|
||||
// Headers keep their real names instead of collapsing to "A".
|
||||
expect(tableEntry.table?.columns).toStrictEqual([
|
||||
{
|
||||
name: 'service.name',
|
||||
queryName: 'A',
|
||||
isValueColumn: false,
|
||||
id: 'service.name',
|
||||
},
|
||||
{
|
||||
name: 'current_availability',
|
||||
queryName: 'A',
|
||||
isValueColumn: true,
|
||||
id: 'current_availability',
|
||||
},
|
||||
{
|
||||
name: 'error_budget_remaining',
|
||||
queryName: 'A',
|
||||
isValueColumn: true,
|
||||
id: 'error_budget_remaining',
|
||||
},
|
||||
{
|
||||
name: 'budget_status',
|
||||
queryName: 'A',
|
||||
isValueColumn: false,
|
||||
id: 'budget_status',
|
||||
},
|
||||
{
|
||||
name: 'total_requests',
|
||||
queryName: 'A',
|
||||
isValueColumn: true,
|
||||
id: 'total_requests',
|
||||
},
|
||||
]);
|
||||
// Ids are unique, so value columns don't overwrite each other in the row.
|
||||
expect(tableEntry.table?.rows?.[0]).toStrictEqual({
|
||||
data: {
|
||||
'service.name': 'kuja-api_gateway-service',
|
||||
current_availability: 99.985,
|
||||
error_budget_remaining: 0.985,
|
||||
budget_status: 'Healthy ✅',
|
||||
total_requests: 2181216,
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -15,6 +15,7 @@ function getColName(
|
||||
col: ScalarData['columns'][number],
|
||||
legendMap: Record<string, string>,
|
||||
aggregationPerQuery: Record<string, any>,
|
||||
clickhouseQueryNames: Set<string>,
|
||||
): string {
|
||||
if (col.columnType === 'group') {
|
||||
return col.name;
|
||||
@@ -39,16 +40,32 @@ function getColName(
|
||||
return alias || expression || col.queryName;
|
||||
}
|
||||
|
||||
// clickhouse_sql value columns carry their real SQL alias in col.name — use
|
||||
// it so each value column keeps its own header instead of collapsing onto
|
||||
// the query name. Formulas/promql use placeholder names, so they fall back
|
||||
// to legend || queryName.
|
||||
if (clickhouseQueryNames.has(col.queryName)) {
|
||||
return col.name;
|
||||
}
|
||||
return legend || col.queryName;
|
||||
}
|
||||
|
||||
function getColId(
|
||||
col: ScalarData['columns'][number],
|
||||
aggregationPerQuery: Record<string, any>,
|
||||
clickhouseQueryNames: Set<string>,
|
||||
): string {
|
||||
if (col.columnType === 'group') {
|
||||
return col.name;
|
||||
}
|
||||
|
||||
// clickhouse_sql value columns are keyed by their real SQL alias so multiple
|
||||
// value columns stay unique instead of all collapsing onto the query name
|
||||
// (which would overwrite every cell in the row with the last column's value).
|
||||
if (clickhouseQueryNames.has(col.queryName)) {
|
||||
return col.name;
|
||||
}
|
||||
|
||||
const aggregation =
|
||||
aggregationPerQuery?.[col.queryName]?.[col.aggregationIndex];
|
||||
const expression = aggregation?.expression || '';
|
||||
@@ -141,6 +158,7 @@ function convertScalarDataArrayToTable(
|
||||
scalarDataArray: ScalarData[],
|
||||
legendMap: Record<string, string>,
|
||||
aggregationPerQuery: Record<string, any>,
|
||||
clickhouseQueryNames: Set<string>,
|
||||
): QueryDataV3[] {
|
||||
// If no scalar data, return empty structure
|
||||
|
||||
@@ -166,10 +184,10 @@ function convertScalarDataArrayToTable(
|
||||
|
||||
// Collect columns for this specific query
|
||||
const columns = scalarData?.columns?.map((col) => ({
|
||||
name: getColName(col, legendMap, aggregationPerQuery),
|
||||
name: getColName(col, legendMap, aggregationPerQuery, clickhouseQueryNames),
|
||||
queryName: col.queryName,
|
||||
isValueColumn: col.columnType === 'aggregation',
|
||||
id: getColId(col, aggregationPerQuery),
|
||||
id: getColId(col, aggregationPerQuery, clickhouseQueryNames),
|
||||
}));
|
||||
|
||||
// Process rows for this specific query
|
||||
@@ -177,8 +195,13 @@ function convertScalarDataArrayToTable(
|
||||
const rowData: Record<string, any> = {};
|
||||
|
||||
scalarData?.columns?.forEach((col, colIndex) => {
|
||||
const columnName = getColName(col, legendMap, aggregationPerQuery);
|
||||
const columnId = getColId(col, aggregationPerQuery);
|
||||
const columnName = getColName(
|
||||
col,
|
||||
legendMap,
|
||||
aggregationPerQuery,
|
||||
clickhouseQueryNames,
|
||||
);
|
||||
const columnId = getColId(col, aggregationPerQuery, clickhouseQueryNames);
|
||||
rowData[columnId || columnName] = dataRow[colIndex];
|
||||
});
|
||||
|
||||
@@ -202,6 +225,7 @@ function convertScalarWithFormatForWeb(
|
||||
scalarDataArray: ScalarData[],
|
||||
legendMap: Record<string, string>,
|
||||
aggregationPerQuery: Record<string, any>,
|
||||
clickhouseQueryNames: Set<string>,
|
||||
): QueryDataV3[] {
|
||||
if (!scalarDataArray || scalarDataArray.length === 0) {
|
||||
return [];
|
||||
@@ -210,13 +234,18 @@ function convertScalarWithFormatForWeb(
|
||||
return scalarDataArray.map((scalarData) => {
|
||||
const columns =
|
||||
scalarData.columns?.map((col) => {
|
||||
const colName = getColName(col, legendMap, aggregationPerQuery);
|
||||
const colName = getColName(
|
||||
col,
|
||||
legendMap,
|
||||
aggregationPerQuery,
|
||||
clickhouseQueryNames,
|
||||
);
|
||||
|
||||
return {
|
||||
name: colName,
|
||||
queryName: col.queryName,
|
||||
isValueColumn: col.columnType === 'aggregation',
|
||||
id: getColId(col, aggregationPerQuery),
|
||||
id: getColId(col, aggregationPerQuery, clickhouseQueryNames),
|
||||
};
|
||||
}) || [];
|
||||
|
||||
@@ -289,6 +318,7 @@ function convertV5DataByType(
|
||||
v5Data: any,
|
||||
legendMap: Record<string, string>,
|
||||
aggregationPerQuery: Record<string, any>,
|
||||
clickhouseQueryNames: Set<string>,
|
||||
): MetricRangePayloadV3['data'] {
|
||||
switch (v5Data?.type) {
|
||||
case 'time_series': {
|
||||
@@ -307,6 +337,7 @@ function convertV5DataByType(
|
||||
scalarData,
|
||||
legendMap,
|
||||
aggregationPerQuery,
|
||||
clickhouseQueryNames,
|
||||
);
|
||||
return {
|
||||
resultType: 'scalar',
|
||||
@@ -373,6 +404,15 @@ export function convertV5ResponseToLegacy(
|
||||
{} as Record<string, any>,
|
||||
) || {};
|
||||
|
||||
// clickhouse_sql queries have no aggregation metadata; their value columns
|
||||
// are named/keyed by the real SQL alias the response carries (see getColId).
|
||||
const clickhouseQueryNames = new Set<string>(
|
||||
(params?.compositeQuery?.queries ?? [])
|
||||
.filter((query) => query.type === 'clickhouse_sql')
|
||||
.map((query) => (query.spec as { name?: string })?.name)
|
||||
.filter((name): name is string => !!name),
|
||||
);
|
||||
|
||||
// If formatForWeb is true, return as-is (like existing logic)
|
||||
if (formatForWeb && v5Data?.type === 'scalar') {
|
||||
const scalarData = v5Data.data.results as ScalarData[];
|
||||
@@ -380,6 +420,7 @@ export function convertV5ResponseToLegacy(
|
||||
scalarData,
|
||||
legendMap,
|
||||
aggregationPerQuery,
|
||||
clickhouseQueryNames,
|
||||
);
|
||||
|
||||
return {
|
||||
@@ -402,6 +443,7 @@ export function convertV5ResponseToLegacy(
|
||||
v5Data,
|
||||
legendMap,
|
||||
aggregationPerQuery,
|
||||
clickhouseQueryNames,
|
||||
);
|
||||
|
||||
// Create legacy-compatible response structure
|
||||
|
||||
@@ -5,6 +5,7 @@ import type {
|
||||
|
||||
import {
|
||||
extractAggregationsPerQuery,
|
||||
extractClickhouseQueryNames,
|
||||
prepareScalarTables,
|
||||
} from '../prepareScalarTables';
|
||||
|
||||
@@ -56,6 +57,24 @@ describe('extractAggregationsPerQuery', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('extractClickhouseQueryNames', () => {
|
||||
it('collects names of clickhouse_sql queries, ignoring other envelope types', () => {
|
||||
const request = requestWith([
|
||||
{ type: 'clickhouse_sql', spec: { name: 'A', query: 'SELECT 1' } },
|
||||
{
|
||||
type: 'builder_query',
|
||||
spec: { name: 'B', aggregations: [{ expression: 'count()' }] },
|
||||
},
|
||||
{ type: 'promql', spec: { name: 'P', query: 'up' } },
|
||||
]);
|
||||
expect(extractClickhouseQueryNames(request)).toStrictEqual(new Set(['A']));
|
||||
});
|
||||
|
||||
it('returns an empty set for an undefined payload', () => {
|
||||
expect(extractClickhouseQueryNames(undefined)).toStrictEqual(new Set());
|
||||
});
|
||||
});
|
||||
|
||||
describe('prepareScalarTables', () => {
|
||||
it('builds keyed rows with group + aggregation columns (V1 getColName/getColId parity)', () => {
|
||||
const [table] = prepareScalarTables({
|
||||
@@ -194,18 +213,115 @@ describe('prepareScalarTables', () => {
|
||||
expect(tables.map((t) => t.queryName)).toStrictEqual(['A', 'B']);
|
||||
});
|
||||
|
||||
it('queries without aggregation metadata fall back to legend || queryName', () => {
|
||||
it('clickhouse_sql single value column uses the SQL alias over the legend', () => {
|
||||
const [table] = prepareScalarTables({
|
||||
results: [
|
||||
scalarResult(
|
||||
[
|
||||
{
|
||||
name: 'current_availability',
|
||||
queryName: 'A',
|
||||
columnType: 'aggregation',
|
||||
},
|
||||
],
|
||||
[],
|
||||
),
|
||||
],
|
||||
legendMap: { A: 'Legend' },
|
||||
requestPayload: requestWith([
|
||||
{ type: 'clickhouse_sql', spec: { name: 'A', query: 'SELECT ...' } },
|
||||
]),
|
||||
});
|
||||
// The query is clickhouse_sql, so the response column's real SQL alias is
|
||||
// used for both header and key (a single legend can't be the column name).
|
||||
expect(table.columns[0].name).toBe('current_availability');
|
||||
expect(table.columns[0].id).toBe('current_availability');
|
||||
});
|
||||
|
||||
it('non-clickhouse query without aggregation metadata falls back to legend || queryName', () => {
|
||||
const [table] = prepareScalarTables({
|
||||
results: [
|
||||
// Formulas/promql carry placeholder names and are not clickhouse_sql,
|
||||
// so they must not adopt the response column name.
|
||||
scalarResult(
|
||||
[{ name: '__result_0', queryName: 'A', columnType: 'aggregation' }],
|
||||
[],
|
||||
),
|
||||
],
|
||||
legendMap: { A: 'Legend' },
|
||||
requestPayload: requestWith([]),
|
||||
requestPayload: requestWith([
|
||||
{ type: 'promql', spec: { name: 'A', query: 'up' } },
|
||||
]),
|
||||
});
|
||||
expect(table.columns[0].name).toBe('Legend');
|
||||
expect(table.columns[0].id).toBe('A');
|
||||
});
|
||||
|
||||
it('clickhouse_sql query keeps each value column distinct (regression: all-"A" collapse)', () => {
|
||||
const [table] = prepareScalarTables({
|
||||
results: [
|
||||
scalarResult(
|
||||
[
|
||||
{ name: 'service.name', queryName: 'A', columnType: 'group' },
|
||||
{
|
||||
name: 'current_availability',
|
||||
queryName: 'A',
|
||||
columnType: 'aggregation',
|
||||
aggregationIndex: 0,
|
||||
},
|
||||
{
|
||||
name: 'error_budget_remaining',
|
||||
queryName: 'A',
|
||||
columnType: 'aggregation',
|
||||
aggregationIndex: 1,
|
||||
},
|
||||
{ name: 'budget_status', queryName: 'A', columnType: 'group' },
|
||||
{
|
||||
name: 'total_requests',
|
||||
queryName: 'A',
|
||||
columnType: 'aggregation',
|
||||
aggregationIndex: 4,
|
||||
},
|
||||
],
|
||||
[['kuja-api_gateway-service', 99.985, 0.985, 'Healthy ✅', 2181216]],
|
||||
),
|
||||
],
|
||||
legendMap: { A: '' },
|
||||
// A clickhouse_sql envelope contributes no aggregation metadata.
|
||||
requestPayload: requestWith([
|
||||
{
|
||||
type: 'clickhouse_sql',
|
||||
spec: { name: 'A', query: 'SELECT ...' },
|
||||
},
|
||||
]),
|
||||
});
|
||||
|
||||
// Headers keep their real names instead of collapsing to "A".
|
||||
expect(table.columns.map((col) => col.name)).toStrictEqual([
|
||||
'service.name',
|
||||
'current_availability',
|
||||
'error_budget_remaining',
|
||||
'budget_status',
|
||||
'total_requests',
|
||||
]);
|
||||
// Ids are unique, so value columns don't overwrite each other in the row.
|
||||
expect(table.columns.map((col) => col.id)).toStrictEqual([
|
||||
'service.name',
|
||||
'current_availability',
|
||||
'error_budget_remaining',
|
||||
'budget_status',
|
||||
'total_requests',
|
||||
]);
|
||||
expect(table.rows).toStrictEqual([
|
||||
{
|
||||
data: {
|
||||
'service.name': 'kuja-api_gateway-service',
|
||||
current_availability: 99.985,
|
||||
error_budget_remaining: 0.985,
|
||||
budget_status: 'Healthy ✅',
|
||||
total_requests: 2181216,
|
||||
},
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type {
|
||||
Querybuildertypesv5ColumnDescriptorDTO,
|
||||
Querybuildertypesv5QueryEnvelopeClickHouseSQLDTO,
|
||||
Querybuildertypesv5QueryRangeRequestDTO,
|
||||
Querybuildertypesv5ScalarDataDTO,
|
||||
} from 'api/generated/services/sigNoz.schemas';
|
||||
@@ -44,16 +45,43 @@ export function extractAggregationsPerQuery(
|
||||
return perQuery;
|
||||
}
|
||||
|
||||
/**
|
||||
* Names of the request's clickhouse_sql queries. These have no aggregation
|
||||
* metadata, but their value columns carry the user's real SQL alias in the
|
||||
* response `col.name` — so columns of these queries are named/keyed by that
|
||||
* alias rather than collapsing onto the query name. Builder/formula/promql use
|
||||
* placeholder names (`__result`/`__result_N`) and are excluded here.
|
||||
*/
|
||||
export function extractClickhouseQueryNames(
|
||||
requestPayload: Querybuildertypesv5QueryRangeRequestDTO | undefined,
|
||||
): Set<string> {
|
||||
const names = new Set<string>();
|
||||
(requestPayload?.compositeQuery?.queries ?? []).forEach((envelope) => {
|
||||
if (envelope.type !== Querybuildertypesv5QueryTypeDTO.clickhouse_sql) {
|
||||
return;
|
||||
}
|
||||
const spec = (envelope as Querybuildertypesv5QueryEnvelopeClickHouseSQLDTO)
|
||||
.spec;
|
||||
if (spec?.name) {
|
||||
names.add(spec.name);
|
||||
}
|
||||
});
|
||||
return names;
|
||||
}
|
||||
|
||||
/**
|
||||
* Column display name. Group columns keep their field name; aggregation
|
||||
* columns resolve alias > legend > expression > queryName — with the legend
|
||||
* skipped when the query has multiple aggregations, because one legend can't
|
||||
* label several value columns. (Port of V1 `getColName`.)
|
||||
* label several value columns. clickhouse_sql columns have no aggregation
|
||||
* metadata, so their value columns are named by the real SQL alias the
|
||||
* response carries in `col.name`. (Port of V1 `getColName`.)
|
||||
*/
|
||||
function getColName(
|
||||
col: Querybuildertypesv5ColumnDescriptorDTO,
|
||||
legendMap: Record<string, string>,
|
||||
aggregationsPerQuery: AggregationsPerQuery,
|
||||
clickhouseQueryNames: Set<string>,
|
||||
): string {
|
||||
if (col.columnType === 'group') {
|
||||
return col.name;
|
||||
@@ -74,6 +102,13 @@ function getColName(
|
||||
return alias || expression || queryName;
|
||||
}
|
||||
|
||||
// clickhouse_sql value columns carry their real SQL alias in col.name — use
|
||||
// it so each value column keeps its own header instead of collapsing onto
|
||||
// the query name. Formulas/promql use placeholder names, so they fall back
|
||||
// to legend || queryName.
|
||||
if (clickhouseQueryNames.has(queryName)) {
|
||||
return col.name;
|
||||
}
|
||||
return legend || queryName;
|
||||
}
|
||||
|
||||
@@ -85,15 +120,23 @@ function getColName(
|
||||
function getColId(
|
||||
col: Querybuildertypesv5ColumnDescriptorDTO,
|
||||
aggregationsPerQuery: AggregationsPerQuery,
|
||||
clickhouseQueryNames: Set<string>,
|
||||
): string {
|
||||
if (col.columnType === 'group') {
|
||||
return col.name;
|
||||
}
|
||||
|
||||
const queryName = col.queryName ?? '';
|
||||
|
||||
// clickhouse_sql value columns are keyed by their real SQL alias so multiple
|
||||
// value columns stay unique instead of all collapsing onto the query name
|
||||
// (which would overwrite every cell in the row with the last column's value).
|
||||
if (clickhouseQueryNames.has(queryName)) {
|
||||
return col.name;
|
||||
}
|
||||
|
||||
const aggregations = aggregationsPerQuery[queryName];
|
||||
const expression = aggregations?.[col.aggregationIndex ?? 0]?.expression || '';
|
||||
|
||||
if ((aggregations?.length || 0) > 1 && expression) {
|
||||
return `${queryName}.${expression}`;
|
||||
}
|
||||
@@ -119,6 +162,7 @@ export function prepareScalarTables({
|
||||
requestPayload,
|
||||
}: PrepareScalarTablesArgs): PanelTable[] {
|
||||
const aggregationsPerQuery = extractAggregationsPerQuery(requestPayload);
|
||||
const clickhouseQueryNames = extractClickhouseQueryNames(requestPayload);
|
||||
|
||||
return results.map((scalarData) => {
|
||||
if (!scalarData) {
|
||||
@@ -132,10 +176,10 @@ export function prepareScalarTables({
|
||||
const queryName = scalarData.columns?.[0]?.queryName ?? '';
|
||||
|
||||
const columns: PanelTableColumn[] = (scalarData.columns ?? []).map((col) => ({
|
||||
name: getColName(col, legendMap, aggregationsPerQuery),
|
||||
name: getColName(col, legendMap, aggregationsPerQuery, clickhouseQueryNames),
|
||||
queryName: col.queryName ?? '',
|
||||
isValueColumn: col.columnType === 'aggregation',
|
||||
id: getColId(col, aggregationsPerQuery),
|
||||
id: getColId(col, aggregationsPerQuery, clickhouseQueryNames),
|
||||
}));
|
||||
|
||||
const rows = (scalarData.data ?? []).map((dataRow) => {
|
||||
|
||||
Reference in New Issue
Block a user