From cb3d91b53c08622b818f67497c4b337c479504e3 Mon Sep 17 00:00:00 2001 From: David Date: Fri, 3 Jan 2020 12:55:50 +0100 Subject: [PATCH] Prometheus: user metrics metadata to inform query hints (#21304) * Prometheus: user metrics metadata to inform query hints - no longer analyse datapoints - either use metrics metadata to determine metrics type or use metric name suffix - removed testcases based on datapoints * Added hint certainty and tests --- .../datasource/prometheus/query_hints.test.ts | 140 +++++------------- .../datasource/prometheus/query_hints.ts | 76 +++++----- 2 files changed, 80 insertions(+), 136 deletions(-) diff --git a/public/app/plugins/datasource/prometheus/query_hints.test.ts b/public/app/plugins/datasource/prometheus/query_hints.test.ts index a59e252522f..9345a5f4083 100644 --- a/public/app/plugins/datasource/prometheus/query_hints.test.ts +++ b/public/app/plugins/datasource/prometheus/query_hints.test.ts @@ -1,5 +1,6 @@ import { getQueryHints, SUM_HINT_THRESHOLD_COUNT } from './query_hints'; -////// +import { PrometheusDatasource } from './datasource'; + describe('getQueryHints()', () => { it('returns no hints for no series', () => { expect(getQueryHints('', [])).toEqual(null); @@ -9,35 +10,7 @@ describe('getQueryHints()', () => { expect(getQueryHints('', [{ datapoints: [] }])).toEqual(null); }); - it('returns no hint for a monotonically decreasing series', () => { - const series = [ - { - datapoints: [ - [23, 1000], - [22, 1001], - ], - }, - ]; - const hints = getQueryHints('metric', series); - expect(hints).toEqual(null); - }); - - it('returns no hint for a flat series', () => { - const series = [ - { - datapoints: [ - [null, 1000], - [23, 1001], - [null, 1002], - [23, 1003], - ], - }, - ]; - const hints = getQueryHints('metric', series); - expect(hints).toEqual(null); - }); - - it('returns a rate hint for a monotonically increasing series', () => { + it('returns a rate hint for a counter metric', () => { const series = [ { datapoints: [ @@ -46,21 +19,21 @@ describe('getQueryHints()', () => { ], }, ]; - const hints = getQueryHints('metric', series); + const hints = getQueryHints('metric_total', series); expect(hints!.length).toBe(1); expect(hints![0]).toMatchObject({ - label: 'Time series is monotonically increasing.', + label: 'Metric metric_total looks like a counter.', fix: { action: { type: 'ADD_RATE', - query: 'metric', + query: 'metric_total', }, }, }); }); - it('returns no rate hint for a monotonically increasing series that already has a rate', () => { + it('returns a certain rate hint for a counter metric', () => { const series = [ { datapoints: [ @@ -69,11 +42,40 @@ describe('getQueryHints()', () => { ], }, ]; - const hints = getQueryHints('rate(metric[1m])', series); + const mock: unknown = { languageProvider: { metricsMetadata: { foo: [{ type: 'counter' }] } } }; + const datasource = mock as PrometheusDatasource; + + let hints = getQueryHints('foo', series, datasource); + expect(hints!.length).toBe(1); + expect(hints![0]).toMatchObject({ + label: 'Metric foo is a counter.', + fix: { + action: { + type: 'ADD_RATE', + query: 'foo', + }, + }, + }); + + // Test substring match not triggering hint + hints = getQueryHints('foo_foo', series, datasource); + expect(hints).toBe(null); + }); + + it('returns no rate hint for a counter metric that already has a rate', () => { + const series = [ + { + datapoints: [ + [23, 1000], + [24, 1001], + ], + }, + ]; + const hints = getQueryHints('rate(metric_total[1m])', series); expect(hints).toEqual(null); }); - it('returns a rate hint w/o action for a complex monotonically increasing series', () => { + it('returns a rate hint w/o action for a complex counter metric', () => { const series = [ { datapoints: [ @@ -82,35 +84,12 @@ describe('getQueryHints()', () => { ], }, ]; - const hints = getQueryHints('sum(metric)', series); + const hints = getQueryHints('sum(metric_total)', series); expect(hints!.length).toBe(1); expect(hints![0].label).toContain('rate()'); expect(hints![0].fix).toBeUndefined(); }); - it('returns a rate hint for a monotonically increasing series with missing data', () => { - const series = [ - { - datapoints: [ - [23, 1000], - [null, 1001], - [24, 1002], - ], - }, - ]; - const hints = getQueryHints('metric', series); - expect(hints!.length).toBe(1); - expect(hints![0]).toMatchObject({ - label: 'Time series is monotonically increasing.', - fix: { - action: { - type: 'ADD_RATE', - query: 'metric', - }, - }, - }); - }); - it('returns a histogram hint for a bucket series', () => { const series = [{ datapoints: [[23, 1000]] }]; const hints = getQueryHints('metric_bucket', series); @@ -149,45 +128,4 @@ describe('getQueryHints()', () => { }, }); }); - - describe('when called without datapoints in series', () => { - it('then it should use rows instead and return correct hint', () => { - const series = [ - { - fields: [ - { - name: 'Some Name', - }, - ], - rows: [[1], [2]], - }, - ]; - - const result = getQueryHints('up', series); - expect(result).toEqual([ - { - fix: { action: { query: 'up', type: 'ADD_RATE' }, label: 'Fix by adding rate().' }, - label: 'Time series is monotonically increasing.', - type: 'APPLY_RATE', - }, - ]); - }); - }); - - describe('when called without datapoints and rows in series', () => { - it('then it should use an empty array and return null', () => { - const series = [ - { - fields: [ - { - name: 'Some Name', - }, - ], - }, - ]; - - const result = getQueryHints('up', series); - expect(result).toEqual(null); - }); - }); }); diff --git a/public/app/plugins/datasource/prometheus/query_hints.ts b/public/app/plugins/datasource/prometheus/query_hints.ts index 6f2fea082c2..946ae5e0f1a 100644 --- a/public/app/plugins/datasource/prometheus/query_hints.ts +++ b/public/app/plugins/datasource/prometheus/query_hints.ts @@ -1,12 +1,13 @@ import _ from 'lodash'; import { QueryHint, QueryFix } from '@grafana/data'; +import { PrometheusDatasource } from './datasource'; /** * Number of time series results needed before starting to suggest sum aggregation hints */ export const SUM_HINT_THRESHOLD_COUNT = 20; -export function getQueryHints(query: string, series?: any[], datasource?: any): QueryHint[] | null { +export function getQueryHints(query: string, series?: any[], datasource?: PrometheusDatasource): QueryHint[] | null { const hints = []; // ..._bucket metric needs a histogram_quantile() @@ -26,44 +27,49 @@ export function getQueryHints(query: string, series?: any[], datasource?: any): }); } - // Check for monotonicity on series (table results are being ignored here) - if (series && series.length > 0) { - series.forEach(s => { - const datapoints: number[][] = s.datapoints || s.rows || []; - if (query.indexOf('rate(') === -1 && datapoints.length > 1) { - let increasing = false; - const nonNullData = datapoints.filter(dp => dp[0] !== null); - const monotonic = nonNullData.every((dp, index) => { - if (index === 0) { + // Check for need of rate() + if (query.indexOf('rate(') === -1) { + // Use metric metadata for exact types + const nameMatch = query.match(/\b(\w+_(total|sum|count))\b/); + let counterNameMetric = nameMatch ? nameMatch[1] : ''; + const metricsMetadata = datasource?.languageProvider?.metricsMetadata; + let certain = false; + if (_.size(metricsMetadata) > 0) { + counterNameMetric = Object.keys(metricsMetadata).find(metricName => { + // Only considering first type information, could be non-deterministic + const metadata = metricsMetadata[metricName][0]; + if (metadata.type.toLowerCase() === 'counter') { + const metricRegex = new RegExp(`\\b${metricName}\\b`); + if (query.match(metricRegex)) { + certain = true; return true; } - increasing = increasing || dp[0] > nonNullData[index - 1][0]; - // monotonic? - return dp[0] >= nonNullData[index - 1][0]; - }); - if (increasing && monotonic) { - const simpleMetric = query.trim().match(/^\w+$/); - let label = 'Time series is monotonically increasing.'; - let fix: QueryFix; - if (simpleMetric) { - fix = { - label: 'Fix by adding rate().', - action: { - type: 'ADD_RATE', - query, - }, - } as QueryFix; - } else { - label = `${label} Try applying a rate() function.`; - } - hints.push({ - type: 'APPLY_RATE', - label, - fix, - }); } + return false; + }); + } + if (counterNameMetric) { + const simpleMetric = query.trim().match(/^\w+$/); + const verb = certain ? 'is' : 'looks like'; + let label = `Metric ${counterNameMetric} ${verb} a counter.`; + let fix: QueryFix; + if (simpleMetric) { + fix = { + label: 'Fix by adding rate().', + action: { + type: 'ADD_RATE', + query, + }, + } as QueryFix; + } else { + label = `${label} Try applying a rate() function.`; } - }); + hints.push({ + type: 'APPLY_RATE', + label, + fix, + }); + } } // Check for recording rules expansion