From a7f4e4c56ac94c6b42c6cc13e3fa0e7a72637854 Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Mon, 9 Dec 2019 09:46:52 +0100 Subject: [PATCH] Prometheus: Refactor labels caching (#20898) --- package.json | 2 + .../src/slate-plugins/suggestions.tsx | 2 +- .../prometheus/language_provider.test.ts | 116 +++++++++------ .../prometheus/language_provider.ts | 140 ++++++++++-------- yarn.lock | 5 + 5 files changed, 153 insertions(+), 112 deletions(-) diff --git a/package.json b/package.json index 7ce9c9517e0..5f4cbf94f00 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "@types/jest": "24.0.13", "@types/jquery": "1.10.35", "@types/lodash": "4.14.123", + "@types/lru-cache": "^5.1.0", "@types/marked": "0.6.5", "@types/mousetrap": "1.6.3", "@types/node": "11.13.4", @@ -223,6 +224,7 @@ "is-hotkey": "0.1.4", "jquery": "3.4.1", "lodash": "4.17.15", + "lru-cache": "^5.1.1", "marked": "0.6.2", "memoize-one": "5.1.1", "moment": "2.24.0", diff --git a/packages/grafana-ui/src/slate-plugins/suggestions.tsx b/packages/grafana-ui/src/slate-plugins/suggestions.tsx index 818a05e8bdf..1a0fe11245f 100644 --- a/packages/grafana-ui/src/slate-plugins/suggestions.tsx +++ b/packages/grafana-ui/src/slate-plugins/suggestions.tsx @@ -10,7 +10,7 @@ import { Typeahead } from '../components/Typeahead/Typeahead'; import { CompletionItem, TypeaheadOutput, TypeaheadInput, SuggestionsState } from '../types/completion'; import { makeFragment } from '../utils/slate'; -export const TYPEAHEAD_DEBOUNCE = 100; +export const TYPEAHEAD_DEBOUNCE = 250; // Commands added to the editor by this plugin. interface SuggestionsPluginCommands { diff --git a/public/app/plugins/datasource/prometheus/language_provider.test.ts b/public/app/plugins/datasource/prometheus/language_provider.test.ts index 11ec8d3d7ed..8b91e0e9d77 100644 --- a/public/app/plugins/datasource/prometheus/language_provider.test.ts +++ b/public/app/plugins/datasource/prometheus/language_provider.test.ts @@ -26,7 +26,8 @@ describe('Language completion provider', () => { }); it('returns default suggestions with metrics on empty context when metrics were provided', async () => { - const instance = new LanguageProvider(datasource, { metrics: ['foo', 'bar'] }); + const instance = new LanguageProvider(datasource); + instance.metrics = ['foo', 'bar']; const value = Plain.deserialize(''); const result = await instance.provideCompletionItems({ text: '', prefix: '', value, wrapperClasses: [] }); expect(result.context).toBeUndefined(); @@ -101,7 +102,8 @@ describe('Language completion provider', () => { describe('metric suggestions', () => { it('returns metrics and function suggestions in an unknown context', async () => { - const instance = new LanguageProvider(datasource, { metrics: ['foo', 'bar'] }); + const instance = new LanguageProvider(datasource); + instance.metrics = ['foo', 'bar']; let value = Plain.deserialize('a'); value = value.setSelection({ anchor: { offset: 1 }, focus: { offset: 1 } }); const result = await instance.provideCompletionItems({ text: 'a', prefix: 'a', value, wrapperClasses: [] }); @@ -117,7 +119,8 @@ describe('Language completion provider', () => { }); it('returns metrics and function suggestions after a binary operator', async () => { - const instance = new LanguageProvider(datasource, { metrics: ['foo', 'bar'] }); + const instance = new LanguageProvider(datasource); + instance.metrics = ['foo', 'bar']; const value = Plain.deserialize('*'); const result = await instance.provideCompletionItems({ text: '*', prefix: '', value, wrapperClasses: [] }); expect(result.context).toBeUndefined(); @@ -132,7 +135,7 @@ describe('Language completion provider', () => { }); it('returns no suggestions at the beginning of a non-empty function', async () => { - const instance = new LanguageProvider(datasource, { metrics: ['foo', 'bar'] }); + const instance = new LanguageProvider(datasource); const value = Plain.deserialize('sum(up)'); const ed = new SlateEditor({ value }); @@ -169,7 +172,7 @@ describe('Language completion provider', () => { metadataRequest: () => ({ data: { data: [{ __name__: 'metric', bar: 'bazinga' }] as any[] } }), getTimeRange: () => ({ start: 0, end: 1 }), } as any) as PrometheusDatasource; - const instance = new LanguageProvider(datasources, { labelKeys: { '{__name__="metric"}': ['bar'] } }); + const instance = new LanguageProvider(datasources); const value = Plain.deserialize('metric{}'); const ed = new SlateEditor({ value }); const valueWithSelection = ed.moveForward(7).value; @@ -184,7 +187,7 @@ describe('Language completion provider', () => { }); it('returns label suggestions on label context but leaves out labels that already exist', async () => { - const datasources: PrometheusDatasource = ({ + const datasource: PrometheusDatasource = ({ metadataRequest: () => ({ data: { data: [ @@ -200,11 +203,7 @@ describe('Language completion provider', () => { }), getTimeRange: () => ({ start: 0, end: 1 }), } as any) as PrometheusDatasource; - const instance = new LanguageProvider(datasources, { - labelKeys: { - '{job1="foo",job2!="foo",job3=~"foo",__name__="metric"}': ['bar', 'job1', 'job2', 'job3', '__name__'], - }, - }); + const instance = new LanguageProvider(datasource); const value = Plain.deserialize('{job1="foo",job2!="foo",job3=~"foo",__name__="metric",}'); const ed = new SlateEditor({ value }); const valueWithSelection = ed.moveForward(54).value; @@ -219,31 +218,33 @@ describe('Language completion provider', () => { }); it('returns label value suggestions inside a label value context after a negated matching operator', async () => { - const instance = new LanguageProvider(datasource, { - labelKeys: { '{}': ['label'] }, - labelValues: { '{}': { label: ['a', 'b', 'c'] } }, - }); - const value = Plain.deserialize('{label!=}'); + const instance = new LanguageProvider(({ + ...datasource, + metadataRequest: () => { + return { data: { data: ['value1', 'value2'] } }; + }, + } as any) as PrometheusDatasource); + const value = Plain.deserialize('{job!=}'); const ed = new SlateEditor({ value }); const valueWithSelection = ed.moveForward(8).value; const result = await instance.provideCompletionItems({ text: '!=', prefix: '', wrapperClasses: ['context-labels'], - labelKey: 'label', + labelKey: 'job', value: valueWithSelection, }); expect(result.context).toBe('context-label-values'); expect(result.suggestions).toEqual([ { - items: [{ label: 'a' }, { label: 'b' }, { label: 'c' }], - label: 'Label values for "label"', + items: [{ label: 'value1' }, { label: 'value2' }], + label: 'Label values for "job"', }, ]); }); it('returns a refresher on label context and unavailable metric', async () => { - const instance = new LanguageProvider(datasource, { labelKeys: { '{__name__="foo"}': ['bar'] } }); + const instance = new LanguageProvider(datasource); const value = Plain.deserialize('metric{}'); const ed = new SlateEditor({ value }); const valueWithSelection = ed.moveForward(7).value; @@ -258,10 +259,10 @@ describe('Language completion provider', () => { }); it('returns label values on label context when given a metric and a label key', async () => { - const instance = new LanguageProvider(datasource, { - labelKeys: { '{__name__="metric"}': ['bar'] }, - labelValues: { '{__name__="metric"}': { bar: ['baz'] } }, - }); + const instance = new LanguageProvider(({ + ...datasource, + metadataRequest: () => simpleMetricLabelsResponse, + } as any) as PrometheusDatasource); const value = Plain.deserialize('metric{bar=ba}'); const ed = new SlateEditor({ value }); const valueWithSelection = ed.moveForward(13).value; @@ -277,7 +278,10 @@ describe('Language completion provider', () => { }); it('returns label suggestions on aggregation context and metric w/ selector', async () => { - const instance = new LanguageProvider(datasource, { labelKeys: { '{__name__="metric",foo="xx"}': ['bar'] } }); + const instance = new LanguageProvider(({ + ...datasource, + metadataRequest: () => simpleMetricLabelsResponse, + } as any) as PrometheusDatasource); const value = Plain.deserialize('sum(metric{foo="xx"}) by ()'); const ed = new SlateEditor({ value }); const valueWithSelection = ed.moveForward(26).value; @@ -292,7 +296,10 @@ describe('Language completion provider', () => { }); it('returns label suggestions on aggregation context and metric w/o selector', async () => { - const instance = new LanguageProvider(datasource, { labelKeys: { '{__name__="metric"}': ['bar'] } }); + const instance = new LanguageProvider(({ + ...datasource, + metadataRequest: () => simpleMetricLabelsResponse, + } as any) as PrometheusDatasource); const value = Plain.deserialize('sum(metric) by ()'); const ed = new SlateEditor({ value }); const valueWithSelection = ed.moveForward(16).value; @@ -307,9 +314,10 @@ describe('Language completion provider', () => { }); it('returns label suggestions inside a multi-line aggregation context', async () => { - const instance = new LanguageProvider(datasource, { - labelKeys: { '{__name__="metric"}': ['label1', 'label2', 'label3'] }, - }); + const instance = new LanguageProvider(({ + ...datasource, + metadataRequest: () => simpleMetricLabelsResponse, + } as any) as PrometheusDatasource); const value = Plain.deserialize('sum(\nmetric\n)\nby ()'); const aggregationTextBlock = value.document.getBlocks().get(3); const ed = new SlateEditor({ value }); @@ -324,16 +332,17 @@ describe('Language completion provider', () => { expect(result.context).toBe('context-aggregation'); expect(result.suggestions).toEqual([ { - items: [{ label: 'label1' }, { label: 'label2' }, { label: 'label3' }], + items: [{ label: 'bar' }], label: 'Labels', }, ]); }); it('returns label suggestions inside an aggregation context with a range vector', async () => { - const instance = new LanguageProvider(datasource, { - labelKeys: { '{__name__="metric"}': ['label1', 'label2', 'label3'] }, - }); + const instance = new LanguageProvider(({ + ...datasource, + metadataRequest: () => simpleMetricLabelsResponse, + } as any) as PrometheusDatasource); const value = Plain.deserialize('sum(rate(metric[1h])) by ()'); const ed = new SlateEditor({ value }); const valueWithSelection = ed.moveForward(26).value; @@ -346,16 +355,17 @@ describe('Language completion provider', () => { expect(result.context).toBe('context-aggregation'); expect(result.suggestions).toEqual([ { - items: [{ label: 'label1' }, { label: 'label2' }, { label: 'label3' }], + items: [{ label: 'bar' }], label: 'Labels', }, ]); }); it('returns label suggestions inside an aggregation context with a range vector and label', async () => { - const instance = new LanguageProvider(datasource, { - labelKeys: { '{__name__="metric",label1="value"}': ['label1', 'label2', 'label3'] }, - }); + const instance = new LanguageProvider(({ + ...datasource, + metadataRequest: () => simpleMetricLabelsResponse, + } as any) as PrometheusDatasource); const value = Plain.deserialize('sum(rate(metric{label1="value"}[1h])) by ()'); const ed = new SlateEditor({ value }); const valueWithSelection = ed.moveForward(42).value; @@ -368,16 +378,14 @@ describe('Language completion provider', () => { expect(result.context).toBe('context-aggregation'); expect(result.suggestions).toEqual([ { - items: [{ label: 'label1' }, { label: 'label2' }, { label: 'label3' }], + items: [{ label: 'bar' }], label: 'Labels', }, ]); }); it('returns no suggestions inside an unclear aggregation context using alternate syntax', async () => { - const instance = new LanguageProvider(datasource, { - labelKeys: { '{__name__="metric"}': ['label1', 'label2', 'label3'] }, - }); + const instance = new LanguageProvider(datasource); const value = Plain.deserialize('sum by ()'); const ed = new SlateEditor({ value }); const valueWithSelection = ed.moveForward(8).value; @@ -392,9 +400,10 @@ describe('Language completion provider', () => { }); it('returns label suggestions inside an aggregation context using alternate syntax', async () => { - const instance = new LanguageProvider(datasource, { - labelKeys: { '{__name__="metric"}': ['label1', 'label2', 'label3'] }, - }); + const instance = new LanguageProvider(({ + ...datasource, + metadataRequest: () => simpleMetricLabelsResponse, + } as any) as PrometheusDatasource); const value = Plain.deserialize('sum by () (metric)'); const ed = new SlateEditor({ value }); const valueWithSelection = ed.moveForward(8).value; @@ -407,7 +416,7 @@ describe('Language completion provider', () => { expect(result.context).toBe('context-aggregation'); expect(result.suggestions).toEqual([ { - items: [{ label: 'label1' }, { label: 'label2' }, { label: 'label3' }], + items: [{ label: 'bar' }], label: 'Labels', }, ]); @@ -429,11 +438,24 @@ describe('Language completion provider', () => { wrapperClasses: ['context-labels'], value: valueWithSelection, }; - await instance.provideCompletionItems(args); + const promise1 = instance.provideCompletionItems(args); // one call for 2 default labels job, instance expect((datasource.metadataRequest as Mock).mock.calls.length).toBe(2); - await instance.provideCompletionItems(args); + const promise2 = instance.provideCompletionItems(args); + expect((datasource.metadataRequest as Mock).mock.calls.length).toBe(2); + await Promise.all([promise1, promise2]); expect((datasource.metadataRequest as Mock).mock.calls.length).toBe(2); }); }); }); + +const simpleMetricLabelsResponse = { + data: { + data: [ + { + __name__: 'metric', + bar: 'baz', + }, + ], + }, +}; diff --git a/public/app/plugins/datasource/prometheus/language_provider.ts b/public/app/plugins/datasource/prometheus/language_provider.ts index 2be3bcfac80..813ed57feb4 100644 --- a/public/app/plugins/datasource/prometheus/language_provider.ts +++ b/public/app/plugins/datasource/prometheus/language_provider.ts @@ -1,4 +1,5 @@ import _ from 'lodash'; +import LRU from 'lru-cache'; import { dateTime, LanguageProvider, HistoryItem } from '@grafana/data'; import { CompletionItem, TypeaheadInput, TypeaheadOutput, CompletionItemGroup } from '@grafana/ui'; @@ -42,23 +43,24 @@ export function addHistoryMetadata(item: CompletionItem, history: any[]): Comple export default class PromQlLanguageProvider extends LanguageProvider { histogramMetrics?: string[]; timeRange?: { start: number; end: number }; - labelKeys?: { [index: string]: string[] }; // metric -> [labelKey,...] - labelValues?: { [index: string]: { [index: string]: string[] } }; // metric -> labelKey -> [labelValue,...] metrics?: string[]; startTask: Promise; datasource: PrometheusDatasource; - constructor(datasource: PrometheusDatasource, initialValues?: any) { + /** + * Cache for labels of series. This is bit simplistic in the sense that it just counts responses each as a 1 and does + * not account for different size of a response. If that is needed a `length` function can be added in the options. + * 10 as a max size is totally arbitrary right now. + */ + private labelsCache = new LRU>(10); + + constructor(datasource: PrometheusDatasource) { super(); this.datasource = datasource; this.histogramMetrics = []; this.timeRange = { start: 0, end: 0 }; - this.labelKeys = {}; - this.labelValues = {}; this.metrics = []; - - Object.assign(this, initialValues); } // Strip syntax chars @@ -216,20 +218,7 @@ export default class PromQlLanguageProvider extends LanguageProvider { }; } - roundToMinutes(seconds: number): number { - return Math.floor(seconds / 60); - } - - timeRangeChanged(): boolean { - const dsRange = this.datasource.getTimeRange(); - return ( - this.roundToMinutes(dsRange.end) !== this.roundToMinutes(this.timeRange.end) || - this.roundToMinutes(dsRange.start) !== this.roundToMinutes(this.timeRange.start) - ); - } - - getAggregationCompletionItems = ({ value }: TypeaheadInput): TypeaheadOutput => { - const refresher: Promise = null; + getAggregationCompletionItems = async ({ value }: TypeaheadInput): Promise => { const suggestions: CompletionItemGroup[] = []; // Stitch all query lines together to support multi-line queries @@ -258,7 +247,6 @@ export default class PromQlLanguageProvider extends LanguageProvider { } const result = { - refresher, suggestions, context: 'context-aggregation', }; @@ -275,13 +263,10 @@ export default class PromQlLanguageProvider extends LanguageProvider { const selector = parseSelector(selectorString, selectorString.length - 2).selector; - const labelKeys = this.labelKeys[selector]; - if (labelKeys && !this.timeRangeChanged()) { - suggestions.push({ label: 'Labels', items: labelKeys.map(wrapLabel) }); - } else { - result.refresher = this.fetchSeriesLabels(selector); + const labelValues = await this.getLabelValues(selector); + if (labelValues) { + suggestions.push({ label: 'Labels', items: Object.keys(labelValues).map(wrapLabel) }); } - return result; }; @@ -307,36 +292,31 @@ export default class PromQlLanguageProvider extends LanguageProvider { const containsMetric = selector.includes('__name__='); const existingKeys = parsedSelector ? parsedSelector.labelKeys : []; + const suggestions: CompletionItemGroup[] = []; + let labelValues; // Query labels for selector if (selector) { - if (selector === EMPTY_SELECTOR) { - // For empty selector we do not need to check range - if (!this.labelValues[selector]) { - // Query label values for default labels - await Promise.all(DEFAULT_KEYS.map(key => this.fetchLabelValues(key))); - } - } else { - if (!this.labelValues[selector] || this.timeRangeChanged()) { - await this.fetchSeriesLabels(selector, !containsMetric); - } - } + labelValues = await this.getLabelValues(selector, !containsMetric); + } + + if (!labelValues) { + console.warn(`Server did not return any values for selector = ${selector}`); + return { suggestions }; } - const suggestions = []; let context: string; if ((text && text.match(/^!?=~?/)) || wrapperClasses.includes('attr-value')) { // Label values - if (labelKey && this.labelValues[selector] && this.labelValues[selector][labelKey]) { - const labelValues = this.labelValues[selector][labelKey]; + if (labelKey && labelValues[labelKey]) { context = 'context-label-values'; suggestions.push({ label: `Label values for "${labelKey}"`, - items: labelValues.map(wrapLabel), + items: labelValues[labelKey].map(wrapLabel), }); } } else { // Label keys - const labelKeys = this.labelKeys[selector] || (containsMetric ? null : DEFAULT_KEYS); + const labelKeys = labelValues ? Object.keys(labelValues) : containsMetric ? null : DEFAULT_KEYS; if (labelKeys) { const possibleKeys = _.difference(labelKeys, existingKeys); @@ -352,30 +332,62 @@ export default class PromQlLanguageProvider extends LanguageProvider { return { context, suggestions }; }; - fetchLabelValues = async (key: string) => { + async getLabelValues(selector: string, withName?: boolean) { try { - const data = await this.request(`/api/v1/label/${key}/values`); - const existingValues = this.labelValues[EMPTY_SELECTOR]; - const values = { - ...existingValues, - [key]: data, - }; - this.labelValues[EMPTY_SELECTOR] = values; - } catch (e) { - console.error(e); + if (selector === EMPTY_SELECTOR) { + return await this.fetchDefaultLabels(); + } else { + return await this.fetchSeriesLabels(selector, withName); + } + } catch (error) { + // TODO: better error handling + console.error(error); + return undefined; } + } + + fetchLabelValues = async (key: string): Promise> => { + const data = await this.request(`/api/v1/label/${key}/values`); + return { [key]: data }; }; - fetchSeriesLabels = async (name: string, withName?: boolean) => { - try { - const tRange = this.datasource.getTimeRange(); - const data = await this.request(`/api/v1/series?match[]=${name}&start=${tRange['start']}&end=${tRange['end']}`); - const { keys, values } = processLabels(data, withName); - this.labelKeys[name] = keys; - this.labelValues[name] = values; - this.timeRange = tRange; - } catch (e) { - console.error(e); + roundToMinutes(seconds: number): number { + return Math.floor(seconds / 60); + } + + /** + * Fetch labels for a series. This is cached by it's args but also by the global timeRange currently selected as + * they can change over requested time. + * @param name + * @param withName + */ + fetchSeriesLabels = async (name: string, withName?: boolean): Promise> => { + const tRange = this.datasource.getTimeRange(); + const url = `/api/v1/series?match[]=${name}&start=${tRange['start']}&end=${tRange['end']}`; + // Cache key is a bit different here. We add the `withName` param and also round up to a minute the intervals. + // The rounding may seem strange but makes relative intervals like now-1h less prone to need separate request every + // millisecond while still actually getting all the keys for the correct interval. This still can create problems + // when user does not the newest values for a minute if already cached. + const cacheKey = `/api/v1/series?match[]=${name}&start=${this.roundToMinutes( + tRange['start'] + )}&end=${this.roundToMinutes(tRange['end'])}&withName=${!!withName}`; + let value = this.labelsCache.get(cacheKey); + if (!value) { + const data = await this.request(url); + const { values } = processLabels(data, withName); + value = values; + this.labelsCache.set(cacheKey, value); } + return value; }; + + /** + * Fetch this only one as we assume this won't change over time. This is cached differently from fetchSeriesLabels + * because we can cache more aggressively here and also we do not want to invalidate this cache the same way as in + * fetchSeriesLabels. + */ + fetchDefaultLabels = _.once(async () => { + const values = await Promise.all(DEFAULT_KEYS.map(key => this.fetchLabelValues(key))); + return values.reduce((acc, value) => ({ ...acc, ...value }), {}); + }); } diff --git a/yarn.lock b/yarn.lock index 60ce2b07115..4fb3480a7d4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4071,6 +4071,11 @@ resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.123.tgz#39be5d211478c8dd3bdae98ee75bb7efe4abfe4d" integrity sha512-pQvPkc4Nltyx7G1Ww45OjVqUsJP4UsZm+GWJpigXgkikZqJgRm4c48g027o6tdgubWHwFRF15iFd+Y4Pmqv6+Q== +"@types/lru-cache@^5.1.0": + version "5.1.0" + resolved "https://registry.yarnpkg.com/@types/lru-cache/-/lru-cache-5.1.0.tgz#57f228f2b80c046b4a1bd5cac031f81f207f4f03" + integrity sha512-RaE0B+14ToE4l6UqdarKPnXwVDuigfFv+5j9Dze/Nqr23yyuqdNvzcZi3xB+3Agvi5R4EOgAksfv3lXX4vBt9w== + "@types/marked@0.6.5": version "0.6.5" resolved "https://registry.yarnpkg.com/@types/marked/-/marked-0.6.5.tgz#3cf2a56ef615dad24aaf99784ef90a9eba4e29d8"