public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[v10.1.x] Loki: Remove distinct operation (#74003)

Browse Source 
Loki: Remove `distinct` operation (#73938)

* remove distinct

* trigger ci

* update yarn.lock

* fix import

(cherry picked from commit 07eb4b1b90)
This commit is contained in:
Sven Grossmann
2023-08-29 16:29:12 +02:00
committed by GitHub
parent 82d44b41be
commit d1e9a733d9
14 changed files with 9 additions and 223 deletions
Download Patch File Download Diff File Expand all files Collapse all files
package.json
+1 -1
View File
@@ -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",
public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.test.ts
-32
View File
@@ -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', () => {
public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.ts
-21
View File
@@ -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);
}
public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.test.ts
-12
View File
@@ -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 ',
});
});
});
public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.ts
-37
View File
@@ -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<string, LabelOperator>([
@@ -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)
public/app/plugins/datasource/loki/querySplitting.test.ts
+2 -16
View File
@@ -340,19 +340,6 @@ describe('runSplitQuery()', () => {
expect(datasource.runQuery).toHaveBeenCalledTimes(1);
});
});
test('Groups queries using distinct', async () => {
const request = getQueryOptions<LokiQuery>({
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<LokiQuery>({
@@ -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<LokiQuery>({
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);
});
});
public/app/plugins/datasource/loki/querySplitting.ts
+1 -2
View File
@@ -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)
public/app/plugins/datasource/loki/queryUtils.test.ts
-13
View File
@@ -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);
public/app/plugins/datasource/loki/queryUtils.ts
-5
View File
@@ -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) {
public/app/plugins/datasource/loki/querybuilder/operations.ts
-22
View File
@@ -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,
public/app/plugins/datasource/loki/querybuilder/parsing.test.ts
-30
View File
@@ -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) {
public/app/plugins/datasource/loki/querybuilder/parsing.ts
-24
View File
@@ -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,
};
}
public/app/plugins/datasource/loki/querybuilder/types.ts
-1
View File
@@ -38,7 +38,6 @@ export enum LokiOperationId {
Regexp = 'regexp',
Pattern = 'pattern',
Unpack = 'unpack',
Distinct = 'distinct',
LineFormat = 'line_format',
LabelFormat = 'label_format',
Decolorize = 'decolorize',
yarn.lock
+5 -7
View File
@@ -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