From abeadc76727b456dc9cae37d524f602c1aefd008 Mon Sep 17 00:00:00 2001 From: Abhi kumar Date: Thu, 4 Sep 2025 18:51:16 +0530 Subject: [PATCH] fix: backward compatibility for explorer in case of aggregateAttribute is not present (#9000) --- .../QueryBuilderV2/__tests__/utils.test.ts | 203 ++++++++++++++++++ .../src/components/QueryBuilderV2/utils.ts | 37 +++- .../queryBuilder/useGetCompositeQueryParam.ts | 16 +- 3 files changed, 238 insertions(+), 18 deletions(-) diff --git a/frontend/src/components/QueryBuilderV2/__tests__/utils.test.ts b/frontend/src/components/QueryBuilderV2/__tests__/utils.test.ts index 08e256f015bd..c7b19ea8c97b 100644 --- a/frontend/src/components/QueryBuilderV2/__tests__/utils.test.ts +++ b/frontend/src/components/QueryBuilderV2/__tests__/utils.test.ts @@ -1,10 +1,16 @@ /* eslint-disable sonarjs/no-duplicate-string */ /* eslint-disable import/no-unresolved */ import { negateOperator, OPERATORS } from 'constants/antlrQueryConstants'; +import { + BaseAutocompleteData, + DataTypes, +} from 'types/api/queryBuilder/queryAutocompleteResponse'; import { TagFilter } from 'types/api/queryBuilder/queryBuilderData'; +import { DataSource } from 'types/common/queryBuilder'; import { extractQueryPairs } from 'utils/queryContextUtils'; import { + convertAggregationToExpression, convertFiltersToExpression, convertFiltersToExpressionWithExistingQuery, } from '../utils'; @@ -769,3 +775,200 @@ describe('convertFiltersToExpression', () => { expect(result.filter.expression).toBe("service.name = 'old-service'"); }); }); + +describe('convertAggregationToExpression', () => { + const mockAttribute: BaseAutocompleteData = { + id: 'test-id', + key: 'test_metric', + type: 'string', + dataType: DataTypes.String, + }; + + it('should return undefined when no aggregateOperator is provided', () => { + const result = convertAggregationToExpression({ + aggregateOperator: '', + aggregateAttribute: mockAttribute, + dataSource: DataSource.METRICS, + }); + expect(result).toBeUndefined(); + }); + + it('should convert metrics aggregation with required temporality field', () => { + const result = convertAggregationToExpression({ + aggregateOperator: 'sum', + aggregateAttribute: mockAttribute, + dataSource: DataSource.METRICS, + timeAggregation: 'avg', + spaceAggregation: 'max', + alias: 'test_alias', + reduceTo: 'sum', + temporality: 'delta', + }); + + expect(result).toEqual([ + { + metricName: 'test_metric', + timeAggregation: 'avg', + spaceAggregation: 'max', + reduceTo: 'sum', + temporality: 'delta', + }, + ]); + }); + + it('should handle noop operators by converting to count', () => { + const result = convertAggregationToExpression({ + aggregateOperator: 'noop', + aggregateAttribute: mockAttribute, + dataSource: DataSource.METRICS, + timeAggregation: 'noop', + spaceAggregation: 'noop', + }); + + expect(result).toEqual([ + { + metricName: 'test_metric', + timeAggregation: 'count', + spaceAggregation: 'count', + }, + ]); + }); + + it('should handle missing attribute key gracefully', () => { + const result = convertAggregationToExpression({ + aggregateOperator: 'sum', + aggregateAttribute: { ...mockAttribute, key: '' }, + dataSource: DataSource.METRICS, + }); + + expect(result).toEqual([ + { + metricName: '', + timeAggregation: 'sum', + spaceAggregation: 'sum', + }, + ]); + }); + + it('should convert traces aggregation to expression format', () => { + const result = convertAggregationToExpression({ + aggregateOperator: 'count', + aggregateAttribute: mockAttribute, + dataSource: DataSource.TRACES, + alias: 'trace_alias', + }); + + expect(result).toEqual([ + { + expression: 'count(test_metric)', + alias: 'trace_alias', + }, + ]); + }); + + it('should convert logs aggregation to expression format', () => { + const result = convertAggregationToExpression({ + aggregateOperator: 'avg', + aggregateAttribute: mockAttribute, + dataSource: DataSource.LOGS, + alias: 'log_alias', + }); + + expect(result).toEqual([ + { + expression: 'avg(test_metric)', + alias: 'log_alias', + }, + ]); + }); + + it('should handle aggregation without attribute key for traces/logs', () => { + const result = convertAggregationToExpression({ + aggregateOperator: 'count', + aggregateAttribute: { ...mockAttribute, key: '' }, + dataSource: DataSource.TRACES, + }); + + expect(result).toEqual([ + { + expression: 'count()', + }, + ]); + }); + + it('should handle missing alias for traces/logs', () => { + const result = convertAggregationToExpression({ + aggregateOperator: 'sum', + aggregateAttribute: mockAttribute, + dataSource: DataSource.LOGS, + }); + + expect(result).toEqual([ + { + expression: 'sum(test_metric)', + }, + ]); + }); + + it('should use aggregateOperator as fallback for time and space aggregation', () => { + const result = convertAggregationToExpression({ + aggregateOperator: 'max', + aggregateAttribute: mockAttribute, + dataSource: DataSource.METRICS, + }); + + expect(result).toEqual([ + { + metricName: 'test_metric', + timeAggregation: 'max', + spaceAggregation: 'max', + }, + ]); + }); + + it('should handle undefined aggregateAttribute parameter with metrics', () => { + const result = convertAggregationToExpression({ + aggregateOperator: 'sum', + aggregateAttribute: mockAttribute, + dataSource: DataSource.METRICS, + }); + + expect(result).toEqual([ + { + metricName: 'test_metric', + timeAggregation: 'sum', + spaceAggregation: 'sum', + reduceTo: undefined, + temporality: undefined, + }, + ]); + }); + + it('should handle undefined aggregateAttribute parameter with traces', () => { + const result = convertAggregationToExpression({ + aggregateOperator: 'noop', + aggregateAttribute: (undefined as unknown) as BaseAutocompleteData, + dataSource: DataSource.TRACES, + }); + + expect(result).toEqual([ + { + expression: 'count()', + }, + ]); + }); + + it('should handle undefined aggregateAttribute parameter with logs', () => { + const result = convertAggregationToExpression({ + aggregateOperator: 'noop', + aggregateAttribute: (undefined as unknown) as BaseAutocompleteData, + dataSource: DataSource.LOGS, + }); + + expect(result).toEqual([ + { + expression: 'count()', + }, + ]); + }); +}); diff --git a/frontend/src/components/QueryBuilderV2/utils.ts b/frontend/src/components/QueryBuilderV2/utils.ts index 2abca6d239b6..9b1f3683f737 100644 --- a/frontend/src/components/QueryBuilderV2/utils.ts +++ b/frontend/src/components/QueryBuilderV2/utils.ts @@ -22,7 +22,7 @@ import { TraceAggregation, } from 'types/api/v5/queryRange'; import { EQueryType } from 'types/common/dashboard'; -import { DataSource } from 'types/common/queryBuilder'; +import { DataSource, ReduceOperators } from 'types/common/queryBuilder'; import { extractQueryPairs } from 'utils/queryContextUtils'; import { unquote } from 'utils/stringUtils'; import { isFunctionOperator, isNonValueOperator } from 'utils/tokenUtils'; @@ -580,14 +580,25 @@ export const convertHavingToExpression = ( * @returns New aggregation format based on data source * */ -export const convertAggregationToExpression = ( - aggregateOperator: string, - aggregateAttribute: BaseAutocompleteData, - dataSource: DataSource, - timeAggregation?: string, - spaceAggregation?: string, - alias?: string, -): (TraceAggregation | LogAggregation | MetricAggregation)[] | undefined => { +export const convertAggregationToExpression = ({ + aggregateOperator, + aggregateAttribute, + dataSource, + timeAggregation, + spaceAggregation, + alias, + reduceTo, + temporality, +}: { + aggregateOperator: string; + aggregateAttribute: BaseAutocompleteData; + dataSource: DataSource; + timeAggregation?: string; + spaceAggregation?: string; + alias?: string; + reduceTo?: ReduceOperators; + temporality?: string; +}): (TraceAggregation | LogAggregation | MetricAggregation)[] | undefined => { // Skip if no operator or attribute key if (!aggregateOperator) { return undefined; @@ -605,7 +616,9 @@ export const convertAggregationToExpression = ( if (dataSource === DataSource.METRICS) { return [ { - metricName: aggregateAttribute.key, + metricName: aggregateAttribute?.key || '', + reduceTo, + temporality, timeAggregation: (normalizedTimeAggregation || normalizedOperator) as any, spaceAggregation: (normalizedSpaceAggregation || normalizedOperator) as any, } as MetricAggregation, @@ -613,7 +626,9 @@ export const convertAggregationToExpression = ( } // For traces and logs, use expression format - const expression = `${normalizedOperator}(${aggregateAttribute.key})`; + const expression = aggregateAttribute?.key + ? `${normalizedOperator}(${aggregateAttribute?.key})` + : `${normalizedOperator}()`; if (dataSource === DataSource.TRACES) { return [ diff --git a/frontend/src/hooks/queryBuilder/useGetCompositeQueryParam.ts b/frontend/src/hooks/queryBuilder/useGetCompositeQueryParam.ts index 7b24cb78829a..439292ab613f 100644 --- a/frontend/src/hooks/queryBuilder/useGetCompositeQueryParam.ts +++ b/frontend/src/hooks/queryBuilder/useGetCompositeQueryParam.ts @@ -47,13 +47,15 @@ export const useGetCompositeQueryParam = (): Query | null => { // Convert aggregation if needed if (!query.aggregations && query.aggregateOperator) { - const convertedAggregation = convertAggregationToExpression( - query.aggregateOperator, - query.aggregateAttribute as BaseAutocompleteData, - query.dataSource, - query.timeAggregation, - query.spaceAggregation, - ) as any; // Type assertion to handle union type + const convertedAggregation = convertAggregationToExpression({ + aggregateOperator: query.aggregateOperator, + aggregateAttribute: query.aggregateAttribute as BaseAutocompleteData, + dataSource: query.dataSource, + timeAggregation: query.timeAggregation, + spaceAggregation: query.spaceAggregation, + reduceTo: query.reduceTo, + temporality: query.temporality, + }) as any; // Type assertion to handle union type convertedQuery.aggregations = convertedAggregation; } return convertedQuery;