From 5e748243af0d1a3e472fe95e02d8c6f22010debb Mon Sep 17 00:00:00 2001 From: Michael Huynh Date: Sat, 3 Nov 2018 07:56:13 +0800 Subject: [PATCH 1/2] Handle suggestions for alternate syntax aggregation contexts In aggregation contexts using the alternate syntax form, labels will precede metrics. A cursor at the label position cannot provide meaningful suggestions unless a metric is specified. In the latter case, no suggestions are presented at all. Related: #13690 --- .../prometheus/language_provider.ts | 38 +++++++++++++------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/public/app/plugins/datasource/prometheus/language_provider.ts b/public/app/plugins/datasource/prometheus/language_provider.ts index 5f97ebfa7b5..fa9c2f1310a 100644 --- a/public/app/plugins/datasource/prometheus/language_provider.ts +++ b/public/app/plugins/datasource/prometheus/language_provider.ts @@ -156,7 +156,7 @@ export default class PromQlLanguageProvider extends LanguageProvider { } getAggregationCompletionItems({ value }: TypeaheadInput): TypeaheadOutput { - let refresher: Promise = null; + const refresher: Promise = null; const suggestions: CompletionItemGroup[] = []; // Stitch all query lines together to support multi-line queries @@ -172,12 +172,30 @@ export default class PromQlLanguageProvider extends LanguageProvider { return text; }, ''); - const leftSide = queryText.slice(0, queryOffset); - const openParensAggregationIndex = leftSide.lastIndexOf('('); - const openParensSelectorIndex = leftSide.slice(0, openParensAggregationIndex).lastIndexOf('('); - const closeParensSelectorIndex = leftSide.slice(openParensSelectorIndex).indexOf(')') + openParensSelectorIndex; + // Try search for selector part on the left-hand side, such as `sum (m) by (l)` + const openParensAggregationIndex = queryText.lastIndexOf('(', queryOffset); + let openParensSelectorIndex = queryText.lastIndexOf('(', openParensAggregationIndex - 1); + let closeParensSelectorIndex = queryText.indexOf(')', openParensSelectorIndex); - let selectorString = leftSide.slice(openParensSelectorIndex + 1, closeParensSelectorIndex); + // Try search for selector part of an alternate aggregation clause, such as `sum by (l) (m)` + if (openParensSelectorIndex === -1) { + const closeParensAggregationIndex = queryText.indexOf(')', queryOffset); + closeParensSelectorIndex = queryText.indexOf(')', closeParensAggregationIndex + 1); + openParensSelectorIndex = queryText.lastIndexOf('(', closeParensSelectorIndex); + } + + const result = { + refresher, + suggestions, + context: 'context-aggregation', + }; + + // Suggestions are useless for alternative aggregation clauses without a selector in context + if (openParensSelectorIndex === -1) { + return result; + } + + let selectorString = queryText.slice(openParensSelectorIndex + 1, closeParensSelectorIndex); // Range vector syntax not accounted for by subsequent parse so discard it if present selectorString = selectorString.replace(/\[[^\]]+\]$/, ''); @@ -188,14 +206,10 @@ export default class PromQlLanguageProvider extends LanguageProvider { if (labelKeys) { suggestions.push({ label: 'Labels', items: labelKeys.map(wrapLabel) }); } else { - refresher = this.fetchSeriesLabels(selector); + result.refresher = this.fetchSeriesLabels(selector); } - return { - refresher, - suggestions, - context: 'context-aggregation', - }; + return result; } getLabelCompletionItems({ text, wrapperClasses, labelKey, value }: TypeaheadInput): TypeaheadOutput { From f79b790ef68cb05f5ac6373c8c75aa99fec0b366 Mon Sep 17 00:00:00 2001 From: Michael Huynh Date: Sat, 3 Nov 2018 08:07:08 +0800 Subject: [PATCH 2/2] Add tests covering alternate syntax for aggregation contexts Related: #13690 --- .../specs/language_provider.test.ts | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/public/app/plugins/datasource/prometheus/specs/language_provider.test.ts b/public/app/plugins/datasource/prometheus/specs/language_provider.test.ts index 20e148efd57..784a8b59739 100644 --- a/public/app/plugins/datasource/prometheus/specs/language_provider.test.ts +++ b/public/app/plugins/datasource/prometheus/specs/language_provider.test.ts @@ -269,5 +269,48 @@ describe('Language completion provider', () => { }, ]); }); + + it('returns no suggestions inside an unclear aggregation context using alternate syntax', () => { + const instance = new LanguageProvider(datasource, { + labelKeys: { '{__name__="metric"}': ['label1', 'label2', 'label3'] }, + }); + const value = Plain.deserialize('sum by ()'); + const range = value.selection.merge({ + anchorOffset: 8, + }); + const valueWithSelection = value.change().select(range).value; + const result = instance.provideCompletionItems({ + text: '', + prefix: '', + wrapperClasses: ['context-aggregation'], + value: valueWithSelection, + }); + expect(result.context).toBe('context-aggregation'); + expect(result.suggestions).toEqual([]); + }); + + it('returns label suggestions inside an aggregation context using alternate syntax', () => { + const instance = new LanguageProvider(datasource, { + labelKeys: { '{__name__="metric"}': ['label1', 'label2', 'label3'] }, + }); + const value = Plain.deserialize('sum by () (metric)'); + const range = value.selection.merge({ + anchorOffset: 8, + }); + const valueWithSelection = value.change().select(range).value; + const result = instance.provideCompletionItems({ + text: '', + prefix: '', + wrapperClasses: ['context-aggregation'], + value: valueWithSelection, + }); + expect(result.context).toBe('context-aggregation'); + expect(result.suggestions).toEqual([ + { + items: [{ label: 'label1' }, { label: 'label2' }, { label: 'label3' }], + label: 'Labels', + }, + ]); + }); }); });