Commit cb3d91b5 by David Committed by GitHub

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
parent 512a42d2
import { getQueryHints, SUM_HINT_THRESHOLD_COUNT } from './query_hints'; import { getQueryHints, SUM_HINT_THRESHOLD_COUNT } from './query_hints';
////// import { PrometheusDatasource } from './datasource';
describe('getQueryHints()', () => { describe('getQueryHints()', () => {
it('returns no hints for no series', () => { it('returns no hints for no series', () => {
expect(getQueryHints('', [])).toEqual(null); expect(getQueryHints('', [])).toEqual(null);
...@@ -9,35 +10,30 @@ describe('getQueryHints()', () => { ...@@ -9,35 +10,30 @@ describe('getQueryHints()', () => {
expect(getQueryHints('', [{ datapoints: [] }])).toEqual(null); expect(getQueryHints('', [{ datapoints: [] }])).toEqual(null);
}); });
it('returns no hint for a monotonically decreasing series', () => { it('returns a rate hint for a counter metric', () => {
const series = [ const series = [
{ {
datapoints: [ datapoints: [
[23, 1000], [23, 1000],
[22, 1001], [24, 1001],
], ],
}, },
]; ];
const hints = getQueryHints('metric', series); const hints = getQueryHints('metric_total', series);
expect(hints).toEqual(null);
});
it('returns no hint for a flat series', () => { expect(hints!.length).toBe(1);
const series = [ expect(hints![0]).toMatchObject({
{ label: 'Metric metric_total looks like a counter.',
datapoints: [ fix: {
[null, 1000], action: {
[23, 1001], type: 'ADD_RATE',
[null, 1002], query: 'metric_total',
[23, 1003],
],
}, },
]; },
const hints = getQueryHints('metric', series); });
expect(hints).toEqual(null);
}); });
it('returns a rate hint for a monotonically increasing series', () => { it('returns a certain rate hint for a counter metric', () => {
const series = [ const series = [
{ {
datapoints: [ datapoints: [
...@@ -46,21 +42,27 @@ describe('getQueryHints()', () => { ...@@ -46,21 +42,27 @@ describe('getQueryHints()', () => {
], ],
}, },
]; ];
const hints = getQueryHints('metric', 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!.length).toBe(1);
expect(hints![0]).toMatchObject({ expect(hints![0]).toMatchObject({
label: 'Time series is monotonically increasing.', label: 'Metric foo is a counter.',
fix: { fix: {
action: { action: {
type: 'ADD_RATE', type: 'ADD_RATE',
query: 'metric', 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 monotonically increasing series that already has a rate', () => { it('returns no rate hint for a counter metric that already has a rate', () => {
const series = [ const series = [
{ {
datapoints: [ datapoints: [
...@@ -69,11 +71,11 @@ describe('getQueryHints()', () => { ...@@ -69,11 +71,11 @@ describe('getQueryHints()', () => {
], ],
}, },
]; ];
const hints = getQueryHints('rate(metric[1m])', series); const hints = getQueryHints('rate(metric_total[1m])', series);
expect(hints).toEqual(null); 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 = [ const series = [
{ {
datapoints: [ datapoints: [
...@@ -82,35 +84,12 @@ describe('getQueryHints()', () => { ...@@ -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!.length).toBe(1);
expect(hints![0].label).toContain('rate()'); expect(hints![0].label).toContain('rate()');
expect(hints![0].fix).toBeUndefined(); 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', () => { it('returns a histogram hint for a bucket series', () => {
const series = [{ datapoints: [[23, 1000]] }]; const series = [{ datapoints: [[23, 1000]] }];
const hints = getQueryHints('metric_bucket', series); const hints = getQueryHints('metric_bucket', series);
...@@ -149,45 +128,4 @@ describe('getQueryHints()', () => { ...@@ -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);
});
});
}); });
import _ from 'lodash'; import _ from 'lodash';
import { QueryHint, QueryFix } from '@grafana/data'; import { QueryHint, QueryFix } from '@grafana/data';
import { PrometheusDatasource } from './datasource';
/** /**
* Number of time series results needed before starting to suggest sum aggregation hints * Number of time series results needed before starting to suggest sum aggregation hints
*/ */
export const SUM_HINT_THRESHOLD_COUNT = 20; 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 = []; const hints = [];
// ..._bucket metric needs a histogram_quantile() // ..._bucket metric needs a histogram_quantile()
...@@ -26,24 +27,31 @@ export function getQueryHints(query: string, series?: any[], datasource?: any): ...@@ -26,24 +27,31 @@ export function getQueryHints(query: string, series?: any[], datasource?: any):
}); });
} }
// Check for monotonicity on series (table results are being ignored here) // Check for need of rate()
if (series && series.length > 0) { if (query.indexOf('rate(') === -1) {
series.forEach(s => { // Use metric metadata for exact types
const datapoints: number[][] = s.datapoints || s.rows || []; const nameMatch = query.match(/\b(\w+_(total|sum|count))\b/);
if (query.indexOf('rate(') === -1 && datapoints.length > 1) { let counterNameMetric = nameMatch ? nameMatch[1] : '';
let increasing = false; const metricsMetadata = datasource?.languageProvider?.metricsMetadata;
const nonNullData = datapoints.filter(dp => dp[0] !== null); let certain = false;
const monotonic = nonNullData.every((dp, index) => { if (_.size(metricsMetadata) > 0) {
if (index === 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; return true;
} }
increasing = increasing || dp[0] > nonNullData[index - 1][0]; }
// monotonic? return false;
return dp[0] >= nonNullData[index - 1][0];
}); });
if (increasing && monotonic) { }
if (counterNameMetric) {
const simpleMetric = query.trim().match(/^\w+$/); const simpleMetric = query.trim().match(/^\w+$/);
let label = 'Time series is monotonically increasing.'; const verb = certain ? 'is' : 'looks like';
let label = `Metric ${counterNameMetric} ${verb} a counter.`;
let fix: QueryFix; let fix: QueryFix;
if (simpleMetric) { if (simpleMetric) {
fix = { fix = {
...@@ -63,8 +71,6 @@ export function getQueryHints(query: string, series?: any[], datasource?: any): ...@@ -63,8 +71,6 @@ export function getQueryHints(query: string, series?: any[], datasource?: any):
}); });
} }
} }
});
}
// Check for recording rules expansion // Check for recording rules expansion
if (datasource && datasource.ruleMappings) { if (datasource && datasource.ruleMappings) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment