From d1e9a733d90418c74ba6ceb4523da65a5fff5826 Mon Sep 17 00:00:00 2001 From: Sven Grossmann Date: Tue, 29 Aug 2023 16:29:12 +0200 Subject: [PATCH] [v10.1.x] Loki: Remove `distinct` operation (#74003) Loki: Remove `distinct` operation (#73938) * remove distinct * trigger ci * update yarn.lock * fix import (cherry picked from commit 07eb4b1b901eb372e729712a199136975026009b) --- package.json | 2 +- .../completions.test.ts | 32 ---------------- .../monaco-completion-provider/completions.ts | 21 ----------- .../situation.test.ts | 12 ------ .../monaco-completion-provider/situation.ts | 37 ------------------- .../datasource/loki/querySplitting.test.ts | 18 +-------- .../plugins/datasource/loki/querySplitting.ts | 3 +- .../datasource/loki/queryUtils.test.ts | 13 ------- .../app/plugins/datasource/loki/queryUtils.ts | 5 --- .../loki/querybuilder/operations.ts | 22 ----------- .../loki/querybuilder/parsing.test.ts | 30 --------------- .../datasource/loki/querybuilder/parsing.ts | 24 ------------ .../datasource/loki/querybuilder/types.ts | 1 - yarn.lock | 12 +++--- 14 files changed, 9 insertions(+), 223 deletions(-) diff --git a/package.json b/package.json index b0a7e1f29fb..57c8f4a812f 100644 --- a/package.json +++ b/package.json @@ -265,7 +265,7 @@ "@grafana/faro-core": "1.1.0", "@grafana/faro-web-sdk": "1.1.0", "@grafana/google-sdk": "0.1.1", - "@grafana/lezer-logql": "0.1.8", + "@grafana/lezer-logql": "0.1.11", "@grafana/monaco-logql": "^0.0.7", "@grafana/runtime": "workspace:*", "@grafana/scenes": "0.22.0", diff --git a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.test.ts b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.test.ts index 911a23fb4f2..0c294c454ee 100644 --- a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.test.ts +++ b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.test.ts @@ -124,12 +124,6 @@ const afterSelectorCompletions = [ type: 'PIPE_OPERATION', documentation: 'Operator docs', }, - { - documentation: 'Operator docs', - insertText: '| distinct', - label: 'distinct', - type: 'PIPE_OPERATION', - }, ]; function buildAfterSelectorCompletions( @@ -388,32 +382,6 @@ describe('getCompletions', () => { ]); expect(functionCompletions).toHaveLength(3); }); - - test('Returns completion options when the situation is AFTER_DISTINCT', async () => { - const situation: Situation = { type: 'AFTER_DISTINCT', logQuery: '{label="value"}' }; - const completions = await getCompletions(situation, completionProvider); - - expect(completions).toEqual([ - { - insertText: 'extracted', - label: 'extracted', - triggerOnInsert: false, - type: 'LABEL_NAME', - }, - { - insertText: 'place', - label: 'place', - triggerOnInsert: false, - type: 'LABEL_NAME', - }, - { - insertText: 'source', - label: 'source', - triggerOnInsert: false, - type: 'LABEL_NAME', - }, - ]); - }); }); describe('getAfterSelectorCompletions', () => { diff --git a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.ts b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.ts index d40703e3640..8e984d0c37b 100644 --- a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.ts +++ b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.ts @@ -288,13 +288,6 @@ export async function getAfterSelectorCompletions( documentation: explainOperator(LokiOperationId.Decolorize), }); - completions.push({ - type: 'PIPE_OPERATION', - label: 'distinct', - insertText: `${prefix}distinct`, - documentation: explainOperator(LokiOperationId.Distinct), - }); - // Let's show label options only if query has parser if (hasQueryParser) { extractedLabelKeys.forEach((key) => { @@ -347,18 +340,6 @@ async function getAfterUnwrapCompletions( return [...labelCompletions, ...UNWRAP_FUNCTION_COMPLETIONS]; } -async function getAfterDistinctCompletions(logQuery: string, dataProvider: CompletionDataProvider) { - const { extractedLabelKeys } = await dataProvider.getParserAndLabelKeys(logQuery); - const labelCompletions: Completion[] = extractedLabelKeys.map((label) => ({ - type: 'LABEL_NAME', - label, - insertText: label, - triggerOnInsert: false, - })); - - return [...labelCompletions]; -} - export async function getCompletions( situation: Situation, dataProvider: CompletionDataProvider @@ -393,8 +374,6 @@ export async function getCompletions( return getAfterUnwrapCompletions(situation.logQuery, dataProvider); case 'IN_AGGREGATION': return [...FUNCTION_COMPLETIONS, ...AGGREGATION_COMPLETIONS]; - case 'AFTER_DISTINCT': - return getAfterDistinctCompletions(situation.logQuery, dataProvider); default: throw new NeverCaseError(situation); } diff --git a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.test.ts b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.test.ts index 6077b769a9e..9a8be4fa4a1 100644 --- a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.test.ts +++ b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.test.ts @@ -264,16 +264,4 @@ describe('situation', () => { ], }); }); - - it('identifies AFTER_DISTINCT autocomplete situations', () => { - assertSituation('{label="value"} | logfmt | distinct^', { - type: 'AFTER_DISTINCT', - logQuery: '{label="value"} | logfmt ', - }); - - assertSituation('{label="value"} | logfmt | distinct id,^', { - type: 'AFTER_DISTINCT', - logQuery: '{label="value"} | logfmt ', - }); - }); }); diff --git a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.ts b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.ts index 2d1efbc386e..7bfa9b7fb70 100644 --- a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.ts +++ b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.ts @@ -20,8 +20,6 @@ import { LiteralExpr, MetricExpr, UnwrapExpr, - DistinctFilter, - DistinctLabel, } from '@grafana/lezer-logql'; import { getLogQueryFromMetricsQuery } from '../../../queryUtils'; @@ -127,10 +125,6 @@ export type Situation = | { type: 'AFTER_UNWRAP'; logQuery: string; - } - | { - type: 'AFTER_DISTINCT'; - logQuery: string; }; type Resolver = { @@ -197,14 +191,6 @@ const RESOLVERS: Resolver[] = [ path: [UnwrapExpr], fun: resolveAfterUnwrap, }, - { - path: [ERROR_NODE_ID, DistinctFilter], - fun: resolveAfterDistinct, - }, - { - path: [ERROR_NODE_ID, DistinctLabel], - fun: resolveAfterDistinct, - }, ]; const LABEL_OP_MAP = new Map([ @@ -509,29 +495,6 @@ function resolveSelector(node: SyntaxNode, text: string, pos: number): Situation }; } -function resolveAfterDistinct(node: SyntaxNode, text: string, pos: number): Situation | null { - let logQuery = getLogQueryFromMetricsQuery(text).trim(); - - let distinctFilterParent: SyntaxNode | null = null; - let parent = node.parent; - while (parent !== null) { - if (parent.type.id === PipelineStage) { - distinctFilterParent = parent; - break; - } - parent = parent.parent; - } - - if (distinctFilterParent?.type.id === PipelineStage) { - logQuery = logQuery.slice(0, distinctFilterParent.from); - } - - return { - type: 'AFTER_DISTINCT', - logQuery, - }; -} - // we find the first error-node in the tree that is at the cursor-position. // NOTE: this might be too slow, might need to optimize it // (ideas: we do not need to go into every subtree, based on from/to) diff --git a/public/app/plugins/datasource/loki/querySplitting.test.ts b/public/app/plugins/datasource/loki/querySplitting.test.ts index 28149bf8860..61701843c75 100644 --- a/public/app/plugins/datasource/loki/querySplitting.test.ts +++ b/public/app/plugins/datasource/loki/querySplitting.test.ts @@ -340,19 +340,6 @@ describe('runSplitQuery()', () => { expect(datasource.runQuery).toHaveBeenCalledTimes(1); }); }); - test('Groups queries using distinct', async () => { - const request = getQueryOptions({ - targets: [ - { expr: '{a="b"} | distinct field', refId: 'A' }, - { expr: 'count_over_time({c="d"} | distinct something [1m])', refId: 'B' }, - ], - range, - }); - await expect(runSplitQuery(datasource, request)).toEmitValuesWith(() => { - // Queries using distinct are omitted from splitting - expect(datasource.runQuery).toHaveBeenCalledTimes(1); - }); - }); test('Respects maxLines of logs queries', async () => { const { logFrameA } = getMockFrames(); const request = getQueryOptions({ @@ -370,18 +357,17 @@ describe('runSplitQuery()', () => { expect(datasource.runQuery).toHaveBeenCalledTimes(4); }); }); - test('Groups multiple queries into logs, queries, instant, and distinct', async () => { + test('Groups multiple queries into logs, queries, instant', async () => { const request = getQueryOptions({ targets: [ { expr: 'count_over_time({a="b"}[1m])', refId: 'A', queryType: LokiQueryType.Instant }, { expr: '{c="d"}', refId: 'B' }, { expr: 'count_over_time({c="d"}[1m])', refId: 'C' }, - { expr: 'count_over_time({c="d"} | distinct id [1m])', refId: 'D' }, ], range, }); await expect(runSplitQuery(datasource, request)).toEmitValuesWith(() => { - // 3 days, 3 chunks, 3x Logs + 3x Metric + (1x Instant | Distinct), 7 requests. + // 3 days, 3 chunks, 3x Logs + 3x Metric + (1x Instant), 7 requests. expect(datasource.runQuery).toHaveBeenCalledTimes(7); }); }); diff --git a/public/app/plugins/datasource/loki/querySplitting.ts b/public/app/plugins/datasource/loki/querySplitting.ts index 03118c494ea..f2a5e9d6a0b 100644 --- a/public/app/plugins/datasource/loki/querySplitting.ts +++ b/public/app/plugins/datasource/loki/querySplitting.ts @@ -18,7 +18,7 @@ import { LoadingState } from '@grafana/schema'; import { LokiDatasource } from './datasource'; import { splitTimeRange as splitLogsTimeRange } from './logsTimeSplitting'; import { splitTimeRange as splitMetricTimeRange } from './metricTimeSplitting'; -import { isLogsQuery, isQueryWithDistinct, isQueryWithRangeVariable } from './queryUtils'; +import { isLogsQuery, isQueryWithRangeVariable } from './queryUtils'; import { combineResponses } from './responseUtils'; import { trackGroupedQueries } from './tracking'; import { LokiGroupedRequest, LokiQuery, LokiQueryType } from './types'; @@ -208,7 +208,6 @@ function getNextRequestPointers(requests: LokiGroupedRequest[], requestGroup: nu function querySupportsSplitting(query: LokiQuery) { return ( query.queryType !== LokiQueryType.Instant && - !isQueryWithDistinct(query.expr) && // Queries with $__range variable should not be split because then the interpolated $__range variable is incorrect // because it is interpolated on the backend with the split timeRange !isQueryWithRangeVariable(query.expr) diff --git a/public/app/plugins/datasource/loki/queryUtils.test.ts b/public/app/plugins/datasource/loki/queryUtils.test.ts index f6034cf7f5a..739b2a7e74a 100644 --- a/public/app/plugins/datasource/loki/queryUtils.test.ts +++ b/public/app/plugins/datasource/loki/queryUtils.test.ts @@ -12,7 +12,6 @@ import { getParserFromQuery, obfuscate, requestSupportsSplitting, - isQueryWithDistinct, isQueryWithRangeVariable, isQueryPipelineErrorFiltering, getLogQueryFromMetricsQuery, @@ -310,18 +309,6 @@ describe('isQueryWithLabelFormat', () => { }); }); -describe('isQueryWithDistinct', () => { - it('identifies queries using distinct', () => { - expect(isQueryWithDistinct('{job="grafana"} | distinct id')).toBe(true); - expect(isQueryWithDistinct('count_over_time({job="grafana"} | distinct id [1m])')).toBe(true); - }); - - it('does not return false positives', () => { - expect(isQueryWithDistinct('{label="distinct"} | logfmt')).toBe(false); - expect(isQueryWithDistinct('count_over_time({job="distinct"} | json [1m])')).toBe(false); - }); -}); - describe('isQueryWithRangeVariableDuration', () => { it('identifies queries using $__range variable', () => { expect(isQueryWithRangeVariable('rate({job="grafana"}[$__range])')).toBe(true); diff --git a/public/app/plugins/datasource/loki/queryUtils.ts b/public/app/plugins/datasource/loki/queryUtils.ts index 717b90e97b6..53ac6b4b703 100644 --- a/public/app/plugins/datasource/loki/queryUtils.ts +++ b/public/app/plugins/datasource/loki/queryUtils.ts @@ -17,7 +17,6 @@ import { MetricExpr, Matcher, Identifier, - Distinct, Range, formatLokiQuery, } from '@grafana/lezer-logql'; @@ -248,10 +247,6 @@ export function isQueryWithLineFilter(query: string): boolean { return isQueryWithNode(query, LineFilter); } -export function isQueryWithDistinct(query: string): boolean { - return isQueryWithNode(query, Distinct); -} - export function isQueryWithRangeVariable(query: string): boolean { const rangeNodes = getNodesFromQuery(query, [Range]); for (const node of rangeNodes) { diff --git a/public/app/plugins/datasource/loki/querybuilder/operations.ts b/public/app/plugins/datasource/loki/querybuilder/operations.ts index 4cff3263c96..8c542c50447 100644 --- a/public/app/plugins/datasource/loki/querybuilder/operations.ts +++ b/public/app/plugins/datasource/loki/querybuilder/operations.ts @@ -1,4 +1,3 @@ -import { LabelParamEditor } from '../../prometheus/querybuilder/components/LabelParamEditor'; import { createAggregationOperation, createAggregationOperationWithParam, @@ -487,27 +486,6 @@ Example: \`\`error_level=\`level\` \`\` addOperationHandler: addLokiOperation, explainHandler: () => `This will remove ANSI color codes from log lines.`, }, - { - id: LokiOperationId.Distinct, - name: 'Distinct', - params: [ - { - name: 'Label', - type: 'string', - restParam: true, - optional: true, - editor: LabelParamEditor, - }, - ], - defaultParams: [''], - alternativesKey: 'format', - category: LokiVisualQueryOperationCategory.Formats, - orderRank: LokiOperationOrder.Unwrap, - renderer: (op, def, innerExpr) => `${innerExpr} | distinct ${op.params.join(',')}`, - addOperationHandler: addLokiOperation, - explainHandler: () => - 'Allows filtering log lines using their original and extracted labels to filter out duplicate label values. The first line occurrence of a distinct value is returned, and the others are dropped.', - }, ...binaryScalarOperations, { id: LokiOperationId.NestedQuery, diff --git a/public/app/plugins/datasource/loki/querybuilder/parsing.test.ts b/public/app/plugins/datasource/loki/querybuilder/parsing.test.ts index f9508866d47..beae01478e7 100644 --- a/public/app/plugins/datasource/loki/querybuilder/parsing.test.ts +++ b/public/app/plugins/datasource/loki/querybuilder/parsing.test.ts @@ -746,36 +746,6 @@ describe('buildVisualQueryFromString', () => { }, }); }); - - it('parses a log query with distinct and no labels', () => { - expect(buildVisualQueryFromString('{app="frontend"} | distinct')).toEqual( - noErrors({ - labels: [ - { - op: '=', - value: 'frontend', - label: 'app', - }, - ], - operations: [{ id: LokiOperationId.Distinct, params: [] }], - }) - ); - }); - - it('parses a log query with distinct and labels', () => { - expect(buildVisualQueryFromString('{app="frontend"} | distinct id, email')).toEqual( - noErrors({ - labels: [ - { - op: '=', - value: 'frontend', - label: 'app', - }, - ], - operations: [{ id: LokiOperationId.Distinct, params: ['id', 'email'] }], - }) - ); - }); }); function noErrors(query: LokiVisualQuery) { diff --git a/public/app/plugins/datasource/loki/querybuilder/parsing.ts b/public/app/plugins/datasource/loki/querybuilder/parsing.ts index 2bedcf3c227..d234c831073 100644 --- a/public/app/plugins/datasource/loki/querybuilder/parsing.ts +++ b/public/app/plugins/datasource/loki/querybuilder/parsing.ts @@ -8,8 +8,6 @@ import { By, ConvOp, Decolorize, - DistinctFilter, - DistinctLabel, Filter, FilterOp, Grouping, @@ -207,11 +205,6 @@ export function handleExpression(expr: string, node: SyntaxNode, context: Contex break; } - case DistinctFilter: { - visQuery.operations.push(handleDistinctFilter(expr, node, context)); - break; - } - default: { // Any other nodes we just ignore and go to its children. This should be fine as there are lots of wrapper // nodes that can be skipped. @@ -643,20 +636,3 @@ function isEmptyQuery(query: LokiVisualQuery) { } return false; } - -function handleDistinctFilter(expr: string, node: SyntaxNode, context: Context): QueryBuilderOperation { - const labels: string[] = []; - let exploringNode = node.getChild(DistinctLabel); - while (exploringNode) { - const label = getString(expr, exploringNode.getChild(Identifier)); - if (label) { - labels.push(label); - } - exploringNode = exploringNode?.getChild(DistinctLabel); - } - labels.reverse(); - return { - id: LokiOperationId.Distinct, - params: labels, - }; -} diff --git a/public/app/plugins/datasource/loki/querybuilder/types.ts b/public/app/plugins/datasource/loki/querybuilder/types.ts index 6e534b62418..fdd9ec57d20 100644 --- a/public/app/plugins/datasource/loki/querybuilder/types.ts +++ b/public/app/plugins/datasource/loki/querybuilder/types.ts @@ -38,7 +38,6 @@ export enum LokiOperationId { Regexp = 'regexp', Pattern = 'pattern', Unpack = 'unpack', - Distinct = 'distinct', LineFormat = 'line_format', LabelFormat = 'label_format', Decolorize = 'decolorize', diff --git a/yarn.lock b/yarn.lock index 0541dbf1ca3..9c8f7fd59c3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3959,14 +3959,12 @@ __metadata: languageName: node linkType: hard -"@grafana/lezer-logql@npm:0.1.8": - version: 0.1.8 - resolution: "@grafana/lezer-logql@npm:0.1.8" - dependencies: - lodash: ^4.17.21 +"@grafana/lezer-logql@npm:0.1.11": + version: 0.1.11 + resolution: "@grafana/lezer-logql@npm:0.1.11" peerDependencies: "@lezer/lr": ^1.0.0 - checksum: f0f301b6d4fbd2d79563b5b4e34303257be0ea995b2b9fa1f012648654b4afaa9cea91642bc59eddb70e9fa24ec8804489c161f7065b41eef49db68d3a2ca561 + checksum: 6a624b9a8d31ff854fcf9708c35e6a7498e78c4bda884639681d0b6d0fffe5527fbaeab1198e5a7694f913181657334345f31156a4a15ff64e3019b30ba6ca2a languageName: node linkType: hard @@ -19272,7 +19270,7 @@ __metadata: "@grafana/faro-core": 1.1.0 "@grafana/faro-web-sdk": 1.1.0 "@grafana/google-sdk": 0.1.1 - "@grafana/lezer-logql": 0.1.8 + "@grafana/lezer-logql": 0.1.11 "@grafana/monaco-logql": ^0.0.7 "@grafana/runtime": "workspace:*" "@grafana/scenes": 0.22.0