New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Alerting: Fix evaluation interval validation #56115
Changes from 4 commits
9ed36cb
6f8f3b3
4501acc
dc8a1de
a422217
80ba5c1
9492c4e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import { css } from '@emotion/css'; | ||
import React from 'react'; | ||
import React, { useMemo } from 'react'; | ||
|
||
import { GrafanaTheme2 } from '@grafana/data/src'; | ||
import { config } from '@grafana/runtime/src'; | ||
|
@@ -15,7 +15,10 @@ interface RuleConfigStatusProps { | |
export function RuleConfigStatus({ rule }: RuleConfigStatusProps) { | ||
const styles = useStyles2(getStyles); | ||
|
||
const { exceedsLimit } = checkEvaluationIntervalGlobalLimit(rule.group.interval); | ||
const { exceedsLimit } = useMemo( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 |
||
() => checkEvaluationIntervalGlobalLimit(rule.group.interval), | ||
[rule.group.interval] | ||
); | ||
|
||
if (!exceedsLimit) { | ||
return null; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
import { config } from '@grafana/runtime'; | ||
|
||
import { checkEvaluationIntervalGlobalLimit } from './config'; | ||
|
||
describe('checkEvaluationIntervalGlobalLimit', () => { | ||
it('should NOT exceed limit if evaluate every is not valid duration', () => { | ||
config.unifiedAlerting.minInterval = '2m30s'; | ||
|
||
const { globalLimit, exceedsLimit } = checkEvaluationIntervalGlobalLimit('123notvalidduration'); | ||
|
||
expect(globalLimit).toBe(150 * 1000); | ||
expect(exceedsLimit).toBe(false); | ||
}); | ||
|
||
it('should NOT exceed limit if config minInterval is not valid duration', () => { | ||
config.unifiedAlerting.minInterval = '1A8IU3A'; | ||
|
||
const { globalLimit, exceedsLimit } = checkEvaluationIntervalGlobalLimit('1m30s'); | ||
|
||
expect(globalLimit).toBe(0); | ||
expect(exceedsLimit).toBe(false); | ||
}); | ||
|
||
it.each([ | ||
['2m30s', '1m30s'], | ||
['30s', '10s'], | ||
['1d2h', '2h'], | ||
['1y', '90d'], | ||
])( | ||
'should exceed limit if config minInterval (%s) is greater than evaluate every (%s)', | ||
(minInterval, evaluateEvery) => { | ||
config.unifiedAlerting.minInterval = minInterval; | ||
|
||
const { globalLimit, exceedsLimit } = checkEvaluationIntervalGlobalLimit(evaluateEvery); | ||
|
||
expect(globalLimit).toBeGreaterThan(0); | ||
expect(exceedsLimit).toBe(true); | ||
} | ||
); | ||
|
||
it.each([ | ||
['1m30s', '2m30s'], | ||
['30s', '1d'], | ||
['1m10s', '1h30m15s'], | ||
])('should NOT exceed limit if config minInterval is lesser than evaluate every', (minInterval, evaluateEvery) => { | ||
config.unifiedAlerting.minInterval = minInterval; | ||
|
||
const { globalLimit, exceedsLimit } = checkEvaluationIntervalGlobalLimit(evaluateEvery); | ||
|
||
expect(globalLimit).toBeGreaterThan(0); | ||
expect(exceedsLimit).toBe(false); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,21 +1,26 @@ | ||
import { DataSourceInstanceSettings, DataSourceJsonData, isValidGoDuration, rangeUtil } from '@grafana/data'; | ||
import { DataSourceInstanceSettings, DataSourceJsonData } from '@grafana/data'; | ||
import { config } from '@grafana/runtime'; | ||
|
||
import { isValidPrometheusDuration, parsePrometheusDuration } from './time'; | ||
|
||
export function getAllDataSources(): Array<DataSourceInstanceSettings<DataSourceJsonData>> { | ||
return Object.values(config.datasources); | ||
} | ||
|
||
export function checkEvaluationIntervalGlobalLimit(alertGroupEvaluateEvery?: string) { | ||
if (!alertGroupEvaluateEvery || !isValidGoDuration(alertGroupEvaluateEvery)) { | ||
// config.unifiedAlerting.minInterval should be Prometheus-compatible duration | ||
// However, Go's gtime library has issues with parsing y,w,d | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically it's our own There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, so why can't we fix it? 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that might introduce some unwanted side-effects :D |
||
if (!isValidPrometheusDuration(config.unifiedAlerting.minInterval)) { | ||
return { globalLimit: 0, exceedsLimit: false }; | ||
} | ||
|
||
if (!isValidGoDuration(config.unifiedAlerting.minInterval)) { | ||
return { globalLimit: 0, exceedsLimit: false }; | ||
const evaluateEveryGlobalLimitMs = parsePrometheusDuration(config.unifiedAlerting.minInterval); | ||
|
||
if (!alertGroupEvaluateEvery || !isValidPrometheusDuration(alertGroupEvaluateEvery)) { | ||
return { globalLimit: evaluateEveryGlobalLimitMs, exceedsLimit: false }; | ||
} | ||
|
||
const evaluateEveryMs = rangeUtil.intervalToMs(alertGroupEvaluateEvery); | ||
const evaluateEveryGlobalLimitMs = rangeUtil.intervalToMs(config.unifiedAlerting.minInterval); | ||
const evaluateEveryMs = parsePrometheusDuration(alertGroupEvaluateEvery); | ||
|
||
const exceedsLimit = evaluateEveryGlobalLimitMs > evaluateEveryMs && evaluateEveryMs > 0; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import { isValidGoDuration, isValidPrometheusDuration } from './time'; | ||
|
||
describe('isValidPrometheusDuration', () => { | ||
const validDurations = ['20h30m10s45ms', '1m30s', '20s4h', '90s', '10s', '20h20h', '2d4h20m']; | ||
|
||
it.each(validDurations)('%s should be valid', (duration) => { | ||
expect(isValidPrometheusDuration(duration)).toBe(true); | ||
}); | ||
|
||
const invalidDurations = ['20h 30m 10s 45ms', '10Y', 'sample text', 'm']; | ||
|
||
it.each(invalidDurations)('%s should NOT be valid', (duration) => { | ||
expect(isValidPrometheusDuration(duration)).toBe(false); | ||
}); | ||
}); | ||
|
||
describe('isValidGoDuration', () => { | ||
const validDurations = ['20h30m10s45ms', '1m30s', '20s4h', '90s', '10s', '20h20h']; | ||
|
||
it.each(validDurations)('%s should be valid', (duration) => { | ||
expect(isValidGoDuration(duration)).toBe(true); | ||
}); | ||
|
||
const invalidDurations = ['20h 30m 10s 45ms', '2d4h20m', 'sample text', 'h']; | ||
|
||
it.each(invalidDurations)('%s should NOT be valid', (duration) => { | ||
expect(isValidGoDuration(duration)).toBe(false); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,13 @@ import { describeInterval } from '@grafana/data/src/datetime/rangeutil'; | |
|
||
import { TimeOptions } from '../types/time'; | ||
|
||
/** | ||
* ⚠️ | ||
* Some of these functions might be confusing, but there is a significant difference between "Golang duration", | ||
* supported by the time.ParseDuration() function and "prometheus duration" which is similar but does not support anything | ||
* smaller than seconds and adds the following supported units: "d, w, y" | ||
*/ | ||
|
||
export function parseInterval(value: string): [number, string] { | ||
const match = value.match(/(\d+)(\w+)/); | ||
if (match) { | ||
|
@@ -21,22 +28,78 @@ export const timeOptions = Object.entries(TimeOptions).map(([key, value]) => ({ | |
value: value, | ||
})); | ||
|
||
// 1h, 10m and such | ||
export const positiveDurationValidationPattern = { | ||
value: new RegExp(`^\\d+(${Object.values(TimeOptions).join('|')})$`), | ||
message: `Must be of format "(number)(unit)" , for example "1m". Available units: ${Object.values(TimeOptions).join( | ||
', ' | ||
)}`, | ||
export function parseDurationToMilliseconds(duration: string) { | ||
return durationToMilliseconds(parseDuration(duration)); | ||
} | ||
|
||
export function isValidPrometheusDuration(duration: string): boolean { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👏 |
||
try { | ||
parsePrometheusDuration(duration); | ||
return true; | ||
} catch (err) { | ||
return false; | ||
} | ||
} | ||
|
||
const PROMETHEUS_SUFFIX_MULTIPLIER: Record<string, number> = { | ||
ms: 1, | ||
s: 1000, | ||
m: 60 * 1000, | ||
h: 60 * 60 * 1000, | ||
d: 24 * 60 * 60 * 1000, | ||
w: 7 * 24 * 60 * 60 * 1000, | ||
y: 365 * 24 * 60 * 60 * 1000, | ||
}; | ||
|
||
// 1h, 10m or 0 (without units) | ||
export const durationValidationPattern = { | ||
value: new RegExp(`^\\d+(${Object.values(TimeOptions).join('|')})|0$`), | ||
message: `Must be of format "(number)(unit)", for example "1m", or just "0". Available units: ${Object.values( | ||
const DURATION_REGEXP = new RegExp(/^(?:(?<value>\d+)(?<type>ms|s|m|h|d|w|y))|0$/); | ||
const INVALID_FORMAT = new Error( | ||
`Must be of format "(number)(unit)", for example "1m", or just "0". Available units: ${Object.values( | ||
TimeOptions | ||
).join(', ')}`, | ||
}; | ||
).join(', ')}` | ||
); | ||
|
||
export function parseDurationToMilliseconds(duration: string) { | ||
return durationToMilliseconds(parseDuration(duration)); | ||
/** | ||
* According to https://prometheus.io/docs/alerting/latest/configuration/#configuration-file | ||
* see <duration> | ||
* | ||
* @returns Duration in milliseconds | ||
*/ | ||
export function parsePrometheusDuration(duration: string): number { | ||
let input = duration; | ||
let parts: Array<[number, string]> = []; | ||
|
||
function matchDuration(part: string) { | ||
const match = DURATION_REGEXP.exec(part); | ||
const hasValueAndType = match?.groups?.value && match?.groups?.type; | ||
|
||
if (!match || !hasValueAndType) { | ||
throw INVALID_FORMAT; | ||
} | ||
|
||
if (match && match.groups?.value && match.groups?.type) { | ||
input = input.replace(match[0], ''); | ||
parts.push([Number(match.groups.value), match.groups.type]); | ||
} | ||
|
||
if (input) { | ||
matchDuration(input); | ||
} | ||
} | ||
|
||
matchDuration(duration); | ||
|
||
if (!parts.length) { | ||
throw INVALID_FORMAT; | ||
} | ||
|
||
const totalDuration = parts.reduce((acc, [value, type]) => { | ||
const duration = value * PROMETHEUS_SUFFIX_MULTIPLIER[type]; | ||
return acc + duration; | ||
}, 0); | ||
|
||
return totalDuration; | ||
} | ||
|
||
export function isValidGoDuration(duration: string): boolean { | ||
return /^(?:\d+(h|m|s|ms|us|µs|ns))+$/.test(duration); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would name this const with
millisEvery
ormillisEvaluateEvery
to be consistent with theFor
, and I think it would be more clear