Skip to content
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

Merged
merged 7 commits into from
Oct 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,12 @@ import { css } from '@emotion/css';
import React, { FC, useState } from 'react';
import { RegisterOptions, useFormContext } from 'react-hook-form';

import { durationToMilliseconds, GrafanaTheme2, parseDuration } from '@grafana/data';
import { GrafanaTheme2 } from '@grafana/data';
import { Field, InlineLabel, Input, InputControl, useStyles2 } from '@grafana/ui';

import { RuleFormValues } from '../../types/rule-form';
import { checkEvaluationIntervalGlobalLimit } from '../../utils/config';
import {
durationValidationPattern,
parseDurationToMilliseconds,
positiveDurationValidationPattern,
} from '../../utils/time';
import { parsePrometheusDuration } from '../../utils/time';
import { CollapseToggle } from '../CollapseToggle';
import { EvaluationIntervalLimitExceeded } from '../InvalidIntervalWarning';

Expand All @@ -20,43 +16,62 @@ import { RuleEditorSection } from './RuleEditorSection';

const MIN_TIME_RANGE_STEP_S = 10; // 10 seconds

const forValidationOptions = (evaluateEvery: string): RegisterOptions => ({
export const forValidationOptions = (evaluateEvery: string): RegisterOptions => ({
required: {
value: true,
message: 'Required.',
},
pattern: durationValidationPattern,
validate: (value) => {
const millisFor = parseDurationToMilliseconds(value);
const millisEvery = parseDurationToMilliseconds(evaluateEvery);

// 0 is a special value meaning for equals evaluation interval
if (millisFor === 0) {
validate: (value: string) => {
// parsePrometheusDuration does not allow 0 but does allow 0s
if (value === '0') {
return true;
}

return millisFor >= millisEvery ? true : 'For must be greater than or equal to evaluate every.';
try {
const millisFor = parsePrometheusDuration(value);

// 0 is a special value meaning for equals evaluation interval
if (millisFor === 0) {
return true;
}

try {
const millisEvery = parsePrometheusDuration(evaluateEvery);
return millisFor >= millisEvery
? true
: 'For duration must be greater than or equal to the evaluation interval.';
} catch (err) {
// if we fail to parse "every", assume validation is successful, or the error messages
// will overlap in the UI
return true;
}
} catch (error) {
return error instanceof Error ? error.message : 'Failed to parse duration';
}
},
});

const evaluateEveryValidationOptions: RegisterOptions = {
export const evaluateEveryValidationOptions: RegisterOptions = {
required: {
value: true,
message: 'Required.',
},
pattern: positiveDurationValidationPattern,
validate: (value: string) => {
const duration = parseDuration(value);
if (Object.keys(duration).length) {
const diff = durationToMilliseconds(duration);
if (diff < MIN_TIME_RANGE_STEP_S * 1000) {
try {
const duration = parsePrometheusDuration(value);
Copy link
Member

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 withmillisEvery or millisEvaluateEvery to be consistent with the For, and I think it would be more clear


if (duration < MIN_TIME_RANGE_STEP_S * 1000) {
return `Cannot be less than ${MIN_TIME_RANGE_STEP_S} seconds.`;
}
if (diff % (MIN_TIME_RANGE_STEP_S * 1000) !== 0) {

if (duration % (MIN_TIME_RANGE_STEP_S * 1000) !== 0) {
return `Must be a multiple of ${MIN_TIME_RANGE_STEP_S} seconds.`;
}

return true;
} catch (error) {
return error instanceof Error ? error.message : 'Failed to parse duration';
}
return true;
},
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { css } from '@emotion/css';
import React, { useEffect, useMemo } from 'react';

import { isValidGoDuration } from '@grafana/data';
import { Modal, Button, Form, Field, Input, useStyles2 } from '@grafana/ui';
import { useCleanup } from 'app/core/hooks/useCleanup';
import { useDispatch } from 'app/types';
Expand All @@ -12,8 +11,8 @@ import { updateLotexNamespaceAndGroupAction } from '../../state/actions';
import { checkEvaluationIntervalGlobalLimit } from '../../utils/config';
import { getRulesSourceName } from '../../utils/datasource';
import { initialAsyncRequestState } from '../../utils/redux';
import { durationValidationPattern } from '../../utils/time';
import { EvaluationIntervalLimitExceeded } from '../InvalidIntervalWarning';
import { evaluateEveryValidationOptions } from '../rule-editor/GrafanaEvaluationBehavior';

interface ModalProps {
namespace: CombinedRuleNamespace;
Expand Down Expand Up @@ -100,22 +99,7 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement {
<Input
id="groupInterval"
placeholder="1m"
{...register('groupInterval', {
pattern: durationValidationPattern,
validate: (input) => {
const validDuration = isValidGoDuration(input);
if (!validDuration) {
return 'Invalid duration. Valid example: 1m (Available units: h, m, s)';
}

const limitExceeded = !checkEvaluationIntervalGlobalLimit(input).exceedsLimit;
if (limitExceeded) {
return true;
}

return false;
},
})}
{...register('groupInterval', evaluateEveryValidationOptions)}
/>
</Field>
{checkEvaluationIntervalGlobalLimit(watch('groupInterval')).exceedsLimit && (
Expand Down
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';
Expand All @@ -15,7 +15,10 @@ interface RuleConfigStatusProps {
export function RuleConfigStatus({ rule }: RuleConfigStatusProps) {
const styles = useStyles2(getStyles);

const { exceedsLimit } = checkEvaluationIntervalGlobalLimit(rule.group.interval);
const { exceedsLimit } = useMemo(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡

() => checkEvaluationIntervalGlobalLimit(rule.group.interval),
[rule.group.interval]
);

if (!exceedsLimit) {
return null;
Expand Down
53 changes: 53 additions & 0 deletions public/app/features/alerting/unified/utils/config.test.ts
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);
});
});
17 changes: 11 additions & 6 deletions public/app/features/alerting/unified/utils/config.ts
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically it's our own grafana-sdk-go's gtime package :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so why can't we fix it? 😄

Copy link
Member

Choose a reason for hiding this comment

The 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;

Expand Down
15 changes: 15 additions & 0 deletions public/app/features/alerting/unified/utils/time.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { 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);
});
});
87 changes: 73 additions & 14 deletions public/app/features/alerting/unified/utils/time.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -21,22 +28,74 @@ 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 {
Copy link
Member

Choose a reason for hiding this comment

The 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;
}