From b0172427b17a6f8077b7cbbdb39275a5538d1f0e Mon Sep 17 00:00:00 2001 From: David Date: Thu, 4 Oct 2018 15:48:08 +0200 Subject: [PATCH 1/2] Extract query hints --- .../datasource/prometheus/datasource.ts | 101 +----------------- .../datasource/prometheus/query_hints.ts | 99 +++++++++++++++++ .../prometheus/specs/datasource.test.ts | 79 -------------- .../prometheus/specs/query_hints.test.ts | 79 ++++++++++++++ 4 files changed, 180 insertions(+), 178 deletions(-) create mode 100644 public/app/plugins/datasource/prometheus/query_hints.ts create mode 100644 public/app/plugins/datasource/prometheus/specs/query_hints.test.ts diff --git a/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts index 17a4b6b0f95..856ab035ea0 100644 --- a/public/app/plugins/datasource/prometheus/datasource.ts +++ b/public/app/plugins/datasource/prometheus/datasource.ts @@ -8,6 +8,7 @@ import { ResultTransformer } from './result_transformer'; import { BackendSrv } from 'app/core/services/backend_srv'; import addLabelToQuery from './add_label_to_query'; +import { getQueryHints } from './query_hints'; export function alignRange(start, end, step) { const alignedEnd = Math.ceil(end / step) * step; @@ -18,104 +19,6 @@ export function alignRange(start, end, step) { }; } -export function determineQueryHints(series: any[], datasource?: any): any[] { - const hints = series.map((s, i) => { - const query: string = s.query; - const index: number = s.responseIndex; - if (query === undefined || index === undefined) { - return null; - } - - // ..._bucket metric needs a histogram_quantile() - const histogramMetric = query.trim().match(/^\w+_bucket$/); - if (histogramMetric) { - const label = 'Time series has buckets, you probably wanted a histogram.'; - return { - index, - label, - fix: { - label: 'Fix by adding histogram_quantile().', - action: { - type: 'ADD_HISTOGRAM_QUANTILE', - query, - index, - }, - }, - }; - } - - // Check for monotony - const datapoints: number[][] = s.datapoints; - if (query.indexOf('rate(') === -1 && datapoints.length > 1) { - let increasing = false; - const monotonic = datapoints.filter(dp => dp[0] !== null).every((dp, index) => { - if (index === 0) { - return true; - } - increasing = increasing || dp[0] > datapoints[index - 1][0]; - // monotonic? - return dp[0] >= datapoints[index - 1][0]; - }); - if (increasing && monotonic) { - const simpleMetric = query.trim().match(/^\w+$/); - let label = 'Time series is monotonously increasing.'; - let fix; - if (simpleMetric) { - fix = { - label: 'Fix by adding rate().', - action: { - type: 'ADD_RATE', - query, - index, - }, - }; - } else { - label = `${label} Try applying a rate() function.`; - } - return { - label, - index, - fix, - }; - } - } - - // Check for recording rules expansion - if (datasource && datasource.ruleMappings) { - const mapping = datasource.ruleMappings; - const mappingForQuery = Object.keys(mapping).reduce((acc, ruleName) => { - if (query.search(ruleName) > -1) { - return { - ...acc, - [ruleName]: mapping[ruleName], - }; - } - return acc; - }, {}); - if (_.size(mappingForQuery) > 0) { - const label = 'Query contains recording rules.'; - return { - label, - index, - fix: { - label: 'Expand rules', - action: { - type: 'EXPAND_RULES', - query, - index, - mapping: mappingForQuery, - }, - }, - }; - } - } - - // No hint found - return null; - }); - return hints; -} - export function extractRuleMappingFromGroups(groups: any[]) { return groups.reduce( (mapping, group) => @@ -300,7 +203,7 @@ export class PrometheusDatasource { result = [...result, ...series]; if (queries[index].hinting) { - const queryHints = determineQueryHints(series, this); + const queryHints = getQueryHints(series, this); hints = [...hints, ...queryHints]; } }); diff --git a/public/app/plugins/datasource/prometheus/query_hints.ts b/public/app/plugins/datasource/prometheus/query_hints.ts new file mode 100644 index 00000000000..42bba54905c --- /dev/null +++ b/public/app/plugins/datasource/prometheus/query_hints.ts @@ -0,0 +1,99 @@ +import _ from 'lodash'; + +export function getQueryHints(series: any[], datasource?: any): any[] { + const hints = series.map((s, i) => { + const query: string = s.query; + const index: number = s.responseIndex; + if (query === undefined || index === undefined) { + return null; + } + + // ..._bucket metric needs a histogram_quantile() + const histogramMetric = query.trim().match(/^\w+_bucket$/); + if (histogramMetric) { + const label = 'Time series has buckets, you probably wanted a histogram.'; + return { + index, + label, + fix: { + label: 'Fix by adding histogram_quantile().', + action: { + type: 'ADD_HISTOGRAM_QUANTILE', + query, + index, + }, + }, + }; + } + + // Check for monotony + const datapoints: number[][] = s.datapoints; + if (query.indexOf('rate(') === -1 && datapoints.length > 1) { + let increasing = false; + const monotonic = datapoints.filter(dp => dp[0] !== null).every((dp, index) => { + if (index === 0) { + return true; + } + increasing = increasing || dp[0] > datapoints[index - 1][0]; + // monotonic? + return dp[0] >= datapoints[index - 1][0]; + }); + if (increasing && monotonic) { + const simpleMetric = query.trim().match(/^\w+$/); + let label = 'Time series is monotonously increasing.'; + let fix; + if (simpleMetric) { + fix = { + label: 'Fix by adding rate().', + action: { + type: 'ADD_RATE', + query, + index, + }, + }; + } else { + label = `${label} Try applying a rate() function.`; + } + return { + label, + index, + fix, + }; + } + } + + // Check for recording rules expansion + if (datasource && datasource.ruleMappings) { + const mapping = datasource.ruleMappings; + const mappingForQuery = Object.keys(mapping).reduce((acc, ruleName) => { + if (query.search(ruleName) > -1) { + return { + ...acc, + [ruleName]: mapping[ruleName], + }; + } + return acc; + }, {}); + if (_.size(mappingForQuery) > 0) { + const label = 'Query contains recording rules.'; + return { + label, + index, + fix: { + label: 'Expand rules', + action: { + type: 'EXPAND_RULES', + query, + index, + mapping: mappingForQuery, + }, + }, + }; + } + } + + // No hint found + return null; + }); + return hints; +} diff --git a/public/app/plugins/datasource/prometheus/specs/datasource.test.ts b/public/app/plugins/datasource/prometheus/specs/datasource.test.ts index 692a1827247..bff35c7ba88 100644 --- a/public/app/plugins/datasource/prometheus/specs/datasource.test.ts +++ b/public/app/plugins/datasource/prometheus/specs/datasource.test.ts @@ -3,7 +3,6 @@ import moment from 'moment'; import q from 'q'; import { alignRange, - determineQueryHints, extractRuleMappingFromGroups, PrometheusDatasource, prometheusSpecialRegexEscape, @@ -216,84 +215,6 @@ describe('PrometheusDatasource', () => { }); }); - describe('determineQueryHints()', () => { - it('returns no hints for no series', () => { - expect(determineQueryHints([])).toEqual([]); - }); - - it('returns no hints for empty series', () => { - expect(determineQueryHints([{ datapoints: [], query: '' }])).toEqual([null]); - }); - - it('returns no hint for a monotonously decreasing series', () => { - const series = [{ datapoints: [[23, 1000], [22, 1001]], query: 'metric', responseIndex: 0 }]; - const hints = determineQueryHints(series); - expect(hints).toEqual([null]); - }); - - it('returns a rate hint for a monotonously increasing series', () => { - const series = [{ datapoints: [[23, 1000], [24, 1001]], query: 'metric', responseIndex: 0 }]; - const hints = determineQueryHints(series); - expect(hints.length).toBe(1); - expect(hints[0]).toMatchObject({ - label: 'Time series is monotonously increasing.', - index: 0, - fix: { - action: { - type: 'ADD_RATE', - query: 'metric', - }, - }, - }); - }); - - it('returns no rate hint for a monotonously increasing series that already has a rate', () => { - const series = [{ datapoints: [[23, 1000], [24, 1001]], query: 'rate(metric[1m])', responseIndex: 0 }]; - const hints = determineQueryHints(series); - expect(hints).toEqual([null]); - }); - - it('returns a rate hint w/o action for a complex monotonously increasing series', () => { - const series = [{ datapoints: [[23, 1000], [24, 1001]], query: 'sum(metric)', responseIndex: 0 }]; - const hints = determineQueryHints(series); - expect(hints.length).toBe(1); - expect(hints[0].label).toContain('rate()'); - expect(hints[0].fix).toBeUndefined(); - }); - - it('returns a rate hint for a monotonously increasing series with missing data', () => { - const series = [{ datapoints: [[23, 1000], [null, 1001], [24, 1002]], query: 'metric', responseIndex: 0 }]; - const hints = determineQueryHints(series); - expect(hints.length).toBe(1); - expect(hints[0]).toMatchObject({ - label: 'Time series is monotonously increasing.', - index: 0, - fix: { - action: { - type: 'ADD_RATE', - query: 'metric', - }, - }, - }); - }); - - it('returns a histogram hint for a bucket series', () => { - const series = [{ datapoints: [[23, 1000]], query: 'metric_bucket', responseIndex: 0 }]; - const hints = determineQueryHints(series); - expect(hints.length).toBe(1); - expect(hints[0]).toMatchObject({ - label: 'Time series has buckets, you probably wanted a histogram.', - index: 0, - fix: { - action: { - type: 'ADD_HISTOGRAM_QUANTILE', - query: 'metric_bucket', - }, - }, - }); - }); - }); - describe('extractRuleMappingFromGroups()', () => { it('returns empty mapping for no rule groups', () => { expect(extractRuleMappingFromGroups([])).toEqual({}); diff --git a/public/app/plugins/datasource/prometheus/specs/query_hints.test.ts b/public/app/plugins/datasource/prometheus/specs/query_hints.test.ts new file mode 100644 index 00000000000..bdfa8f5f834 --- /dev/null +++ b/public/app/plugins/datasource/prometheus/specs/query_hints.test.ts @@ -0,0 +1,79 @@ +import { getQueryHints } from '../query_hints'; + +describe('getQueryHints()', () => { + it('returns no hints for no series', () => { + expect(getQueryHints([])).toEqual([]); + }); + + it('returns no hints for empty series', () => { + expect(getQueryHints([{ datapoints: [], query: '' }])).toEqual([null]); + }); + + it('returns no hint for a monotonously decreasing series', () => { + const series = [{ datapoints: [[23, 1000], [22, 1001]], query: 'metric', responseIndex: 0 }]; + const hints = getQueryHints(series); + expect(hints).toEqual([null]); + }); + + it('returns a rate hint for a monotonously increasing series', () => { + const series = [{ datapoints: [[23, 1000], [24, 1001]], query: 'metric', responseIndex: 0 }]; + const hints = getQueryHints(series); + expect(hints.length).toBe(1); + expect(hints[0]).toMatchObject({ + label: 'Time series is monotonously increasing.', + index: 0, + fix: { + action: { + type: 'ADD_RATE', + query: 'metric', + }, + }, + }); + }); + + it('returns no rate hint for a monotonously increasing series that already has a rate', () => { + const series = [{ datapoints: [[23, 1000], [24, 1001]], query: 'rate(metric[1m])', responseIndex: 0 }]; + const hints = getQueryHints(series); + expect(hints).toEqual([null]); + }); + + it('returns a rate hint w/o action for a complex monotonously increasing series', () => { + const series = [{ datapoints: [[23, 1000], [24, 1001]], query: 'sum(metric)', responseIndex: 0 }]; + const hints = getQueryHints(series); + expect(hints.length).toBe(1); + expect(hints[0].label).toContain('rate()'); + expect(hints[0].fix).toBeUndefined(); + }); + + it('returns a rate hint for a monotonously increasing series with missing data', () => { + const series = [{ datapoints: [[23, 1000], [null, 1001], [24, 1002]], query: 'metric', responseIndex: 0 }]; + const hints = getQueryHints(series); + expect(hints.length).toBe(1); + expect(hints[0]).toMatchObject({ + label: 'Time series is monotonously increasing.', + index: 0, + fix: { + action: { + type: 'ADD_RATE', + query: 'metric', + }, + }, + }); + }); + + it('returns a histogram hint for a bucket series', () => { + const series = [{ datapoints: [[23, 1000]], query: 'metric_bucket', responseIndex: 0 }]; + const hints = getQueryHints(series); + expect(hints.length).toBe(1); + expect(hints[0]).toMatchObject({ + label: 'Time series has buckets, you probably wanted a histogram.', + index: 0, + fix: { + action: { + type: 'ADD_HISTOGRAM_QUANTILE', + query: 'metric_bucket', + }, + }, + }); + }); +}); From 107bef2d6d1f10401f7763a862f1497c750d0b53 Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Thu, 4 Oct 2018 16:32:32 +0200 Subject: [PATCH 2/2] Fix rate function hint for series with nulls --- public/app/plugins/datasource/prometheus/query_hints.ts | 7 ++++--- .../datasource/prometheus/specs/query_hints.test.ts | 8 ++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/public/app/plugins/datasource/prometheus/query_hints.ts b/public/app/plugins/datasource/prometheus/query_hints.ts index 42bba54905c..5efaadb77be 100644 --- a/public/app/plugins/datasource/prometheus/query_hints.ts +++ b/public/app/plugins/datasource/prometheus/query_hints.ts @@ -30,13 +30,14 @@ export function getQueryHints(series: any[], datasource?: any): any[] { const datapoints: number[][] = s.datapoints; if (query.indexOf('rate(') === -1 && datapoints.length > 1) { let increasing = false; - const monotonic = datapoints.filter(dp => dp[0] !== null).every((dp, index) => { + const nonNullData = datapoints.filter(dp => dp[0] !== null); + const monotonic = nonNullData.every((dp, index) => { if (index === 0) { return true; } - increasing = increasing || dp[0] > datapoints[index - 1][0]; + increasing = increasing || dp[0] > nonNullData[index - 1][0]; // monotonic? - return dp[0] >= datapoints[index - 1][0]; + return dp[0] >= nonNullData[index - 1][0]; }); if (increasing && monotonic) { const simpleMetric = query.trim().match(/^\w+$/); diff --git a/public/app/plugins/datasource/prometheus/specs/query_hints.test.ts b/public/app/plugins/datasource/prometheus/specs/query_hints.test.ts index bdfa8f5f834..2c72203137d 100644 --- a/public/app/plugins/datasource/prometheus/specs/query_hints.test.ts +++ b/public/app/plugins/datasource/prometheus/specs/query_hints.test.ts @@ -15,6 +15,14 @@ describe('getQueryHints()', () => { expect(hints).toEqual([null]); }); + it('returns no hint for a flat series', () => { + const series = [ + { datapoints: [[null, 1000], [23, 1001], [null, 1002], [23, 1003]], query: 'metric', responseIndex: 0 }, + ]; + const hints = getQueryHints(series); + expect(hints).toEqual([null]); + }); + it('returns a rate hint for a monotonously increasing series', () => { const series = [{ datapoints: [[23, 1000], [24, 1001]], query: 'metric', responseIndex: 0 }]; const hints = getQueryHints(series);