Loki: Autocomplete returning labels already in use when cursor is before or between other labels (#75925)

* Fix issue getting current labels that would only grab values to the left of the cursor

* Loki: Fix Autocomplete in stream selector overwriting existing label names, or inserting autocomplete result within label value (#76485)

* Better autocomplete functionality in loki, changing the word pattern to include label value separator (=), include valid sting chars (-), and value wrapper ("), adding some more logic in the range calculation to prevent autocomplete results from partially overwriting adjacent label names or portions of the current label value
This commit is contained in:
Galen Kistler
2023-10-13 10:26:32 -05:00
committed by GitHub
parent c4b02d7063
commit f2ad66620f
6 changed files with 320 additions and 19 deletions

View File

@@ -61,13 +61,20 @@ const LANG_ID = 'logql';
// we must only run the lang-setup code once // we must only run the lang-setup code once
let LANGUAGE_SETUP_STARTED = false; let LANGUAGE_SETUP_STARTED = false;
export const defaultWordPattern = /(-?\d*\.\d\w*)|([^`~!#%^&*()\-=+\[{\]}\\|;:'",.<>\/?\s]+)/g;
function ensureLogQL(monaco: Monaco) { function ensureLogQL(monaco: Monaco) {
if (LANGUAGE_SETUP_STARTED === false) { if (LANGUAGE_SETUP_STARTED === false) {
LANGUAGE_SETUP_STARTED = true; LANGUAGE_SETUP_STARTED = true;
monaco.languages.register({ id: LANG_ID }); monaco.languages.register({ id: LANG_ID });
monaco.languages.setMonarchTokensProvider(LANG_ID, monarchlanguage); monaco.languages.setMonarchTokensProvider(LANG_ID, monarchlanguage);
monaco.languages.setLanguageConfiguration(LANG_ID, languageConfiguration); monaco.languages.setLanguageConfiguration(LANG_ID, {
...languageConfiguration,
wordPattern: /(-?\d*\.\d\w*)|([^`~!#%^&*()+\[{\]}\\|;:',.<>\/?\s]+)/g,
// Default: /(-?\d*\.\d\w*)|([^`~!#%^&*()\-=+\[{\]}\\|;:'",.<>\/?\s]+)/g
// Removed `"`, `=`, and `-`, from the exclusion list, so now the completion provider can decide to overwrite any matching words, or just insert text at the cursor
});
} }
} }

View File

@@ -1,10 +1,14 @@
import { Monaco, monacoTypes } from '@grafana/ui/src';
import LokiLanguageProvider from '../../../LanguageProvider'; import LokiLanguageProvider from '../../../LanguageProvider';
import { LokiDatasource } from '../../../datasource'; import { LokiDatasource } from '../../../datasource';
import { createLokiDatasource } from '../../../mocks'; import { createLokiDatasource } from '../../../mocks';
import { CompletionDataProvider } from './CompletionDataProvider'; import { CompletionDataProvider } from './CompletionDataProvider';
import { getAfterSelectorCompletions, getCompletions } from './completions'; import { getAfterSelectorCompletions, getCompletions } from './completions';
import { Label, Situation } from './situation'; import { getSituation, Label, Situation } from './situation';
import { calculateRange } from './index';
jest.mock('../../../querybuilder/operations', () => ({ jest.mock('../../../querybuilder/operations', () => ({
explainOperator: () => 'Operator docs', explainOperator: () => 'Operator docs',
@@ -797,4 +801,101 @@ describe('IN_LOGFMT completions', () => {
] ]
`); `);
}); });
describe('calculateRange', () => {
let monaco: Monaco;
beforeEach(() => {
monaco = {
Range: {
lift(range: monacoTypes.Range): monacoTypes.Range {
return range;
},
},
} as Monaco;
});
it('getSituation fails to return autocomplete when inserting before any other labels', () => {
// Ideally we'd be able to autocomplete in this situation as well, but currently not supported to insert labels at the start.
//{^label1="value1",label2="value2"}
const situation: Situation | null = getSituation('{label1="value1",label2="value2"}', 1);
expect(situation).toBe(null);
});
it('tests inserting new label before existing label name', () => {
const situation: Situation | null = getSituation('{label1="value1",label2="value2"}', 17);
expect(situation?.type).toBe('IN_LABEL_SELECTOR_NO_LABEL_NAME');
const word: monacoTypes.editor.IWordAtPosition = {
word: 'label2="value2"',
startColumn: 17,
endColumn: 32,
};
const wordUntil: monacoTypes.editor.IWordAtPosition = {
word: '',
startColumn: 17,
endColumn: 17,
};
const position: monacoTypes.Position = {
lineNumber: 1,
column: 17,
} as monacoTypes.Position;
expect(calculateRange(situation, word, wordUntil, monaco, position)).toMatchObject({
startLineNumber: 1,
endLineNumber: 1,
startColumn: 17,
endColumn: 32,
});
});
it('tests inserting new label within existing label value', () => {
//{label1="value1",label2="^value2"}
const situation: Situation | null = getSituation('{label1="value1",label2="value"}', 25);
expect(situation?.type).toBe('IN_LABEL_SELECTOR_WITH_LABEL_NAME');
const word: monacoTypes.editor.IWordAtPosition = {
word: 'label2="value2"',
startColumn: 18,
endColumn: 33,
};
const wordUntil: monacoTypes.editor.IWordAtPosition = {
word: 'label2="',
startColumn: 18,
endColumn: 26,
};
const position: monacoTypes.Position = {
lineNumber: 1,
column: 25,
} as monacoTypes.Position;
expect(calculateRange(situation, word, wordUntil, monaco, position)).toMatchObject({
startLineNumber: 1,
endLineNumber: 1,
startColumn: 26,
endColumn: 32,
});
});
it('tests inserting new label within existing label value containing dashes', () => {
// {label1="value1",label2="value2^-value"}
const situation: Situation | null = getSituation('{label1="value1",label2="value2-value"}', 30);
expect(situation?.type).toBe('IN_LABEL_SELECTOR_WITH_LABEL_NAME');
const word: monacoTypes.editor.IWordAtPosition = {
word: 'label2="value2-value"',
startColumn: 18,
endColumn: 39,
};
const wordUntil: monacoTypes.editor.IWordAtPosition = {
word: 'label2="value2',
startColumn: 18,
endColumn: 32,
};
const position: monacoTypes.Position = {
lineNumber: 1,
column: 25,
} as monacoTypes.Position;
expect(calculateRange(situation, word, wordUntil, monaco, position)).toMatchObject({
startLineNumber: 1,
endLineNumber: 1,
startColumn: 26,
endColumn: 38,
});
});
});
}); });

View File

@@ -2,8 +2,8 @@ import type { Monaco, monacoTypes } from '@grafana/ui';
import { CompletionDataProvider } from './CompletionDataProvider'; import { CompletionDataProvider } from './CompletionDataProvider';
import { NeverCaseError } from './NeverCaseError'; import { NeverCaseError } from './NeverCaseError';
import { getCompletions, CompletionType } from './completions'; import { CompletionType, getCompletions } from './completions';
import { getSituation } from './situation'; import { getSituation, Situation } from './situation';
// from: monacoTypes.languages.CompletionItemInsertTextRule.InsertAsSnippet // from: monacoTypes.languages.CompletionItemInsertTextRule.InsertAsSnippet
const INSERT_AS_SNIPPET_ENUM_VALUE = 4; const INSERT_AS_SNIPPET_ENUM_VALUE = 4;
@@ -53,6 +53,7 @@ function getMonacoCompletionItemKind(type: CompletionType, monaco: Monaco): mona
throw new NeverCaseError(type); throw new NeverCaseError(type);
} }
} }
export function getCompletionProvider( export function getCompletionProvider(
monaco: Monaco, monaco: Monaco,
dataProvider: CompletionDataProvider dataProvider: CompletionDataProvider
@@ -62,15 +63,8 @@ export function getCompletionProvider(
position: monacoTypes.Position position: monacoTypes.Position
): monacoTypes.languages.ProviderResult<monacoTypes.languages.CompletionList> => { ): monacoTypes.languages.ProviderResult<monacoTypes.languages.CompletionList> => {
const word = model.getWordAtPosition(position); const word = model.getWordAtPosition(position);
const range = const wordUntil = model.getWordUntilPosition(position);
word != null
? monaco.Range.lift({
startLineNumber: position.lineNumber,
endLineNumber: position.lineNumber,
startColumn: word.startColumn,
endColumn: word.endColumn,
})
: monaco.Range.fromPositions(position);
// documentation says `position` will be "adjusted" in `getOffsetAt` // documentation says `position` will be "adjusted" in `getOffsetAt`
// i don't know what that means, to be sure i clone it // i don't know what that means, to be sure i clone it
const positionClone = { const positionClone = {
@@ -79,6 +73,7 @@ export function getCompletionProvider(
}; };
const offset = model.getOffsetAt(positionClone); const offset = model.getOffsetAt(positionClone);
const situation = getSituation(model.getValue(), offset); const situation = getSituation(model.getValue(), offset);
const range = calculateRange(situation, word, wordUntil, monaco, position);
const completionsPromise = situation != null ? getCompletions(situation, dataProvider) : Promise.resolve([]); const completionsPromise = situation != null ? getCompletions(situation, dataProvider) : Promise.resolve([]);
return completionsPromise.then((items) => { return completionsPromise.then((items) => {
// monaco by default alphabetically orders the items. // monaco by default alphabetically orders the items.
@@ -93,7 +88,7 @@ export function getCompletionProvider(
detail: item.detail, detail: item.detail,
documentation: item.documentation, documentation: item.documentation,
sortText: index.toString().padStart(maxIndexDigits, '0'), // to force the order we have sortText: index.toString().padStart(maxIndexDigits, '0'), // to force the order we have
range, range: range,
command: item.triggerOnInsert command: item.triggerOnInsert
? { ? {
id: 'editor.action.triggerSuggest', id: 'editor.action.triggerSuggest',
@@ -110,3 +105,68 @@ export function getCompletionProvider(
provideCompletionItems, provideCompletionItems,
}; };
} }
export const calculateRange = (
situation: Situation | null,
word: monacoTypes.editor.IWordAtPosition | null,
wordUntil: monacoTypes.editor.IWordAtPosition,
monaco: Monaco,
position: monacoTypes.Position
): monacoTypes.Range => {
if (
situation &&
situation?.type === 'IN_LABEL_SELECTOR_WITH_LABEL_NAME' &&
'betweenQuotes' in situation &&
situation.betweenQuotes
) {
// Word until won't have second quote if they are between quotes
const indexOfFirstQuote = wordUntil?.word?.indexOf('"') ?? 0;
const indexOfLastQuote = word?.word?.lastIndexOf('"') ?? 0;
const indexOfEquals = word?.word.indexOf('=');
const indexOfLastEquals = word?.word.lastIndexOf('=');
// Just one equals "=" the cursor is somewhere within a label value
// e.g. value="labe^l-value" or value="^label-value" etc
// We want the word to include everything within the quotes, so the result from autocomplete overwrites the existing label value
if (
indexOfLastEquals === indexOfEquals &&
indexOfFirstQuote !== -1 &&
indexOfLastQuote !== -1 &&
indexOfLastEquals !== -1
) {
return word != null
? monaco.Range.lift({
startLineNumber: position.lineNumber,
endLineNumber: position.lineNumber,
startColumn: wordUntil.startColumn + indexOfFirstQuote + 1,
endColumn: wordUntil.startColumn + indexOfLastQuote,
})
: monaco.Range.fromPositions(position);
}
}
if (situation && situation.type === 'IN_LABEL_SELECTOR_WITH_LABEL_NAME') {
// Otherwise we want the range to be calculated as the cursor position, as we want to insert the autocomplete, instead of overwriting existing text
// The cursor position is the length of the wordUntil
return word != null
? monaco.Range.lift({
startLineNumber: position.lineNumber,
endLineNumber: position.lineNumber,
startColumn: wordUntil.endColumn,
endColumn: wordUntil.endColumn,
})
: monaco.Range.fromPositions(position);
}
// And for all other non-label cases, we want to use the word start and end column
return word != null
? monaco.Range.lift({
startLineNumber: position.lineNumber,
endLineNumber: position.lineNumber,
startColumn: word.startColumn,
endColumn: word.endColumn,
})
: monaco.Range.fromPositions(position);
};

View File

@@ -288,6 +288,98 @@ describe('situation', () => {
}); });
}); });
it('identifies all labels from queries when cursor is at start', () => {
assertSituation('{^,one="val1",two!="val2",three=~"val3",four!~"val4"}', {
type: 'IN_LABEL_SELECTOR_NO_LABEL_NAME',
otherLabels: [
{ name: 'one', value: 'val1', op: '=' },
{ name: 'two', value: 'val2', op: '!=' },
{ name: 'three', value: 'val3', op: '=~' },
{ name: 'four', value: 'val4', op: '!~' },
],
});
});
describe('tests that should return IN_LABEL_SELECTOR_NO_LABEL_NAME, but currently are null', () => {
it('fails to identify when cursor is before any labels with no comma before first label', () => {
assertSituation('{^ one="val1",two!="val2",three=~"val3",four!~"val4"}', null);
});
it('fails to identify situation when missing space within label list', () => {
assertSituation('{one="val1",two!="val2"^ three=~"val3",four!~"val4"}', null);
});
it('fails to identify situation when missing comma within label list', () => {
assertSituation('{one="val1",two!="val2" ^ three=~"val3",four!~"val4"}', null);
});
it('fails to identify situation when missing comma at end of label list', () => {
assertSituation('{one="val1",two!="val2",three=~"val3",four!~"val4" ^ }', null);
});
it('fails to identify situation when attempting to insert before first label', () => {
assertSituation('{^one="val1",two!="val2",three=~"val3",four!~"val4"}', null);
});
});
it('identifies all labels correctly when error node is from missing comma within label list', () => {
assertSituation('{one="val1",two!="val2",^ three=~"val3",four!~"val4"}', {
type: 'IN_LABEL_SELECTOR_NO_LABEL_NAME',
otherLabels: [
{ name: 'one', value: 'val1', op: '=' },
{ name: 'two', value: 'val2', op: '!=' },
{ name: 'three', value: 'val3', op: '=~' },
{ name: 'four', value: 'val4', op: '!~' },
],
});
});
it('identifies labels when inserting before last, no space, size 2', () => {
assertSituation('{one="val1",^two!="val2"}', {
type: 'IN_LABEL_SELECTOR_NO_LABEL_NAME',
otherLabels: [
{ name: 'one', value: 'val1', op: '=' },
{ name: 'two', value: 'val2', op: '!=' },
],
});
});
it('identifies labels when inserting before last, no space, size 3', () => {
assertSituation('{one="val1",two!="val2",^three="val3"}', {
type: 'IN_LABEL_SELECTOR_NO_LABEL_NAME',
otherLabels: [
{ name: 'one', value: 'val1', op: '=' },
{ name: 'two', value: 'val2', op: '!=' },
{ name: 'three', value: 'val3', op: '=' },
],
});
});
it('identifies all labels from queries when cursor is in middle', () => {
// Note the extra whitespace, if the cursor is after whitespace, the situation will fail to resolve
assertSituation('{one="val1", ^,two!="val2",three=~"val3",four!~"val4"}', {
type: 'IN_LABEL_SELECTOR_NO_LABEL_NAME',
otherLabels: [
{ name: 'one', value: 'val1', op: '=' },
{ name: 'two', value: 'val2', op: '!=' },
{ name: 'three', value: 'val3', op: '=~' },
{ name: 'four', value: 'val4', op: '!~' },
],
});
});
it('identifies all labels from queries when cursor is at end', () => {
// Note the extra whitespace, if the cursor is after whitespace, the situation will fail to resolve
assertSituation('{one="val1",two!="val2",three=~"val3",four!~"val4",^}', {
type: 'IN_LABEL_SELECTOR_NO_LABEL_NAME',
otherLabels: [
{ name: 'one', value: 'val1', op: '=' },
{ name: 'two', value: 'val2', op: '!=' },
{ name: 'three', value: 'val3', op: '=~' },
{ name: 'four', value: 'val4', op: '!~' },
],
});
});
it('identifies AFTER_UNWRAP autocomplete situations', () => { it('identifies AFTER_UNWRAP autocomplete situations', () => {
assertSituation('sum(sum_over_time({one="val1"} | unwrap^', { assertSituation('sum(sum_over_time({one="val1"} | unwrap^', {
type: 'AFTER_UNWRAP', type: 'AFTER_UNWRAP',

View File

@@ -42,6 +42,30 @@ function move(node: SyntaxNode, direction: Direction): SyntaxNode | null {
return node[direction]; return node[direction];
} }
/**
* Iteratively calls walk with given path until it returns null, then we return the last non-null node.
* @param node
* @param path
*/
function traverse(node: SyntaxNode, path: Path): SyntaxNode | null {
let current: SyntaxNode | null = node;
let next = walk(current, path);
while (next) {
let nextTmp = walk(next, path);
if (nextTmp) {
next = nextTmp;
} else {
return next;
}
}
return null;
}
/**
* Walks a single step from the provided node, following the path.
* @param node
* @param path
*/
function walk(node: SyntaxNode, path: Path): SyntaxNode | null { function walk(node: SyntaxNode, path: Path): SyntaxNode | null {
let current: SyntaxNode | null = node; let current: SyntaxNode | null = node;
for (const [direction, expectedNode] of path) { for (const [direction, expectedNode] of path) {
@@ -161,7 +185,7 @@ const ERROR_NODE_ID = 0;
const RESOLVERS: Resolver[] = [ const RESOLVERS: Resolver[] = [
{ {
paths: [[Selector], [ERROR_NODE_ID, Matchers, Selector]], paths: [[Selector], [Selector, Matchers], [Matchers], [ERROR_NODE_ID, Matchers, Selector]],
fun: resolveSelector, fun: resolveSelector,
}, },
{ {
@@ -194,7 +218,10 @@ const RESOLVERS: Resolver[] = [
fun: resolveLogRange, fun: resolveLogRange,
}, },
{ {
paths: [[ERROR_NODE_ID, Matcher]], paths: [
[ERROR_NODE_ID, Matcher],
[ERROR_NODE_ID, Matchers, Selector],
],
fun: resolveMatcher, fun: resolveMatcher,
}, },
{ {
@@ -276,11 +303,25 @@ function getLabel(matcherNode: SyntaxNode, text: string): Label | null {
} }
function getLabels(selectorNode: SyntaxNode, text: string): Label[] { function getLabels(selectorNode: SyntaxNode, text: string): Label[] {
if (selectorNode.type.id !== Selector) { if (selectorNode.type.id !== Selector && selectorNode.type.id !== Matchers) {
return []; return [];
} }
let listNode: SyntaxNode | null = walk(selectorNode, [['firstChild', Matchers]]); let listNode: SyntaxNode | null = null;
// If parent node is selector, we want to start with the current Matcher node
if (selectorNode?.parent?.type.id === Selector) {
listNode = selectorNode;
} else {
// Parent node needs to be returned first because otherwise both of the other walks will return a non-null node and this function will return the labels on the left side of the current node, the other two walks should be mutually exclusive when the parent is null
listNode =
// Node in-between labels
traverse(selectorNode, [['parent', Matchers]]) ??
// Node after all other labels
walk(selectorNode, [['firstChild', Matchers]]) ??
// Node before all other labels
walk(selectorNode, [['lastChild', Matchers]]);
}
const labels: Label[] = []; const labels: Label[] = [];

View File

@@ -259,7 +259,7 @@ export function handleExpression(expr: string, node: SyntaxNode, context: Contex
function getLabel(expr: string, node: SyntaxNode): QueryBuilderLabelFilter { function getLabel(expr: string, node: SyntaxNode): QueryBuilderLabelFilter {
const labelNode = node.getChild(Identifier); const labelNode = node.getChild(Identifier);
const label = getString(expr, labelNode); const label = getString(expr, labelNode);
const op = getString(expr, labelNode!.nextSibling); const op = getString(expr, labelNode?.nextSibling);
let value = getString(expr, node.getChild(String)); let value = getString(expr, node.getChild(String));
// `value` is wrapped in double quotes, so we need to remove them. As a value can contain double quotes, we can't use RegEx here. // `value` is wrapped in double quotes, so we need to remove them. As a value can contain double quotes, we can't use RegEx here.
value = value.substring(1, value.length - 1); value = value.substring(1, value.length - 1);