Alerting: various rule form fixes (#34272)

2025-02-25 18:55:37 -06:00 · 2021-05-18 19:14:57 +03:00
parent 7375115a98
commit 5721019573
17 changed files with 211 additions and 123 deletions
--- a/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx
@@ -58,7 +58,7 @@ export const AlertRuleForm: FC<Props> = ({ existing }) => {
  const type = watch('type');
  const dataSourceName = watch('dataSourceName');

-  const showStep2 = Boolean(type && (type === RuleFormType.threshold || !!dataSourceName));
+  const showStep2 = Boolean(type && (type === RuleFormType.grafana || !!dataSourceName));

  const submitState = useUnifiedAlertingSelector((state) => state.ruleForm.saveRule) || initialAsyncRequestState;
  useCleanup((state) => state.unifiedAlerting.ruleForm.saveRule);
@@ -69,7 +69,7 @@ export const AlertRuleForm: FC<Props> = ({ existing }) => {
        values: {
          ...defaultValues,
          ...values,
-          annotations: values.annotations?.filter(({ key }) => !!key) ?? [],
+          annotations: values.annotations?.filter(({ key, value }) => !!key && !!value) ?? [],
          labels: values.labels?.filter(({ key }) => !!key) ?? [],
        },
        existing,
@@ -80,7 +80,7 @@ export const AlertRuleForm: FC<Props> = ({ existing }) => {

  return (
    <FormProvider {...formAPI}>
-      <form onSubmit={handleSubmit((values) => submit(values, false))} className={styles.form}>
+      <form onSubmit={(e) => e.preventDefault()} className={styles.form}>
        <PageToolbar title="Create alert rule" pageIcon="bell">
          <Link to={returnTo}>
            <Button variant="secondary" disabled={submitState.loading} type="button" fill="outline">
--- a/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx
@@ -1,6 +1,6 @@
 import React, { FC, useCallback, useEffect } from 'react';
-import { DataSourceInstanceSettings, GrafanaTheme, SelectableValue } from '@grafana/data';
-import { Field, Input, InputControl, Select, useStyles } from '@grafana/ui';
+import { DataSourceInstanceSettings, GrafanaTheme2, SelectableValue } from '@grafana/data';
+import { Field, Input, InputControl, Select, useStyles2 } from '@grafana/ui';
 import { css } from '@emotion/css';

 import { RuleEditorSection } from './RuleEditorSection';
@@ -14,16 +14,16 @@ import { contextSrv } from 'app/core/services/context_srv';

 const alertTypeOptions: SelectableValue[] = [
  {
-    label: 'Threshold',
-    value: RuleFormType.threshold,
-    description: 'Metric alert based on a defined threshold',
+    label: 'Grafana managed alert',
+    value: RuleFormType.grafana,
+    description: 'Classic Grafana alerts based on thresholds.',
  },
 ];

 if (contextSrv.isEditor) {
  alertTypeOptions.push({
-    label: 'System or application',
-    value: RuleFormType.system,
+    label: 'Cortex/Loki managed alert',
+    value: RuleFormType.cloud,
    description: 'Alert based on a system or application behavior. Based on Prometheus.',
  });
 }
@@ -33,7 +33,7 @@ interface Props {
 }

 export const AlertTypeStep: FC<Props> = ({ editingExistingRule }) => {
-  const styles = useStyles(getStyles);
+  const styles = useStyles2(getStyles);

  const {
    register,
@@ -52,7 +52,7 @@ export const AlertTypeStep: FC<Props> = ({ editingExistingRule }) => {

  const dataSourceFilter = useCallback(
    (ds: DataSourceInstanceSettings): boolean => {
-      if (ruleFormType === RuleFormType.threshold) {
+      if (ruleFormType === RuleFormType.grafana) {
        return !!ds.meta.alerting;
      } else {
        // filter out only rules sources that support ruler and thus can have alerts edited
@@ -92,7 +92,7 @@ export const AlertTypeStep: FC<Props> = ({ editingExistingRule }) => {
                  const value = v?.value;
                  // when switching to system alerts, null out data source selection if it's not a rules source with ruler
                  if (
-                    value === RuleFormType.system &&
+                    value === RuleFormType.cloud &&
                    dataSourceName &&
                    !rulesSourcesWithRuler.find(({ name }) => name === dataSourceName)
                  ) {
@@ -109,7 +109,7 @@ export const AlertTypeStep: FC<Props> = ({ editingExistingRule }) => {
            }}
          />
        </Field>
-        {ruleFormType === RuleFormType.system && (
+        {ruleFormType === RuleFormType.cloud && (
          <Field
            className={styles.formInput}
            label="Select data source"
@@ -140,10 +140,10 @@ export const AlertTypeStep: FC<Props> = ({ editingExistingRule }) => {
          </Field>
        )}
      </div>
-      {ruleFormType === RuleFormType.system && dataSourceName && (
+      {ruleFormType === RuleFormType.cloud && dataSourceName && (
        <GroupAndNamespaceFields dataSourceName={dataSourceName} />
      )}
-      {ruleFormType === RuleFormType.threshold && (
+      {ruleFormType === RuleFormType.grafana && (
        <Field
          label="Folder"
          className={styles.formInput}
@@ -165,11 +165,11 @@ export const AlertTypeStep: FC<Props> = ({ editingExistingRule }) => {
  );
 };

-const getStyles = (theme: GrafanaTheme) => ({
+const getStyles = (theme: GrafanaTheme2) => ({
  formInput: css`
    width: 330px;
    & + & {
-      margin-left: ${theme.spacing.sm};
+      margin-left: ${theme.spacing(3)};
    }
  `,
  flexRow: css`
--- a/public/app/features/alerting/unified/components/rule-editor/AnnotationsField.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/AnnotationsField.tsx
@@ -1,5 +1,5 @@
 import React, { FC, useCallback } from 'react';
-import { Button, Field, FieldArray, InputControl, Label, TextArea, useStyles } from '@grafana/ui';
+import { Button, Field, FieldArray, Input, InputControl, Label, TextArea, useStyles } from '@grafana/ui';
 import { GrafanaTheme } from '@grafana/data';
 import { css, cx } from '@emotion/css';
 import { useFormContext } from 'react-hook-form';
@@ -28,53 +28,55 @@ const AnnotationsField: FC = () => {
        {({ fields, append, remove }) => {
          return (
            <div className={styles.flexColumn}>
-              {fields.map((field, index) => (
-                <div key={field.id} className={styles.flexRow}>
-                  <Field
-                    className={styles.field}
-                    invalid={!!errors.annotations?.[index]?.key?.message}
-                    error={errors.annotations?.[index]?.key?.message}
-                  >
-                    <InputControl
-                      name={`annotations[${index}].key`}
-                      render={({ field: { ref, ...field } }) => (
-                        <AnnotationKeyInput {...field} existingKeys={existingKeys(index)} width={18} />
-                      )}
-                      control={control}
-                      rules={{ required: { value: !!annotations[index]?.value, message: 'Required.' } }}
+              {fields.map((field, index) => {
+                const isUrl = annotations[index]?.key?.toLocaleLowerCase().endsWith('url');
+                const ValueInputComponent = isUrl ? Input : TextArea;
+                return (
+                  <div key={field.id} className={styles.flexRow}>
+                    <Field
+                      className={styles.field}
+                      invalid={!!errors.annotations?.[index]?.key?.message}
+                      error={errors.annotations?.[index]?.key?.message}
+                    >
+                      <InputControl
+                        name={`annotations[${index}].key`}
+                        render={({ field: { ref, ...field } }) => (
+                          <AnnotationKeyInput {...field} existingKeys={existingKeys(index)} width={18} />
+                        )}
+                        control={control}
+                        rules={{ required: { value: !!annotations[index]?.value, message: 'Required.' } }}
+                      />
+                    </Field>
+                    <Field
+                      className={cx(styles.flexRowItemMargin, styles.field)}
+                      invalid={!!errors.annotations?.[index]?.value?.message}
+                      error={errors.annotations?.[index]?.value?.message}
+                    >
+                      <ValueInputComponent
+                        className={cx(styles.annotationValueInput, { [styles.textarea]: !isUrl })}
+                        {...register(`annotations[${index}].value`)}
+                        placeholder={isUrl ? 'https://' : `Text`}
+                        defaultValue={field.value}
+                      />
+                    </Field>
+                    <Button
+                      type="button"
+                      className={styles.flexRowItemMargin}
+                      aria-label="delete annotation"
+                      icon="trash-alt"
+                      variant="secondary"
+                      onClick={() => remove(index)}
                    />
-                  </Field>
-                  <Field
-                    className={cx(styles.flexRowItemMargin, styles.field)}
-                    invalid={!!errors.annotations?.[index]?.value?.message}
-                    error={errors.annotations?.[index]?.value?.message}
-                  >
-                    <TextArea
-                      className={styles.annotationTextArea}
-                      {...register(`annotations[${index}].value`, {
-                        required: { value: !!annotations[index]?.key, message: 'Required.' },
-                      })}
-                      placeholder={`value`}
-                      defaultValue={field.value}
-                    />
-                  </Field>
-                  <Button
-                    type="button"
-                    className={styles.flexRowItemMargin}
-                    aria-label="delete annotation"
-                    icon="trash-alt"
-                    variant="secondary"
-                    onClick={() => remove(index)}
-                  />
-                </div>
-              ))}
+                  </div>
+                );
+              })}
              <Button
                className={styles.addAnnotationsButton}
                icon="plus-circle"
                type="button"
                variant="secondary"
                onClick={() => {
-                  append({});
+                  append({ key: '', value: '' });
                }}
              >
                Add info
@@ -88,14 +90,16 @@ const AnnotationsField: FC = () => {
 };

 const getStyles = (theme: GrafanaTheme) => ({
-  annotationTextArea: css`
+  annotationValueInput: css`
    width: 426px;
+  `,
+  textarea: css`
    height: 76px;
  `,
  addAnnotationsButton: css`
    flex-grow: 0;
    align-self: flex-start;
-    margin-left: 124px;
+    margin-left: 148px;
  `,
  flexColumn: css`
    display: flex;
--- a/public/app/features/alerting/unified/components/rule-editor/ConditionsStep.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/ConditionsStep.tsx
@@ -1,6 +1,6 @@
 import React, { FC, useState } from 'react';
 import { css } from '@emotion/css';
-import { GrafanaTheme } from '@grafana/data';
+import { GrafanaTheme, parseDuration, addDurationToDate } from '@grafana/data';
 import { Field, InlineLabel, Input, InputControl, Select, Switch, useStyles } from '@grafana/ui';
 import { useFormContext, RegisterOptions } from 'react-hook-form';
 import { RuleFormType, RuleFormValues } from '../../types/rule-form';
@@ -9,12 +9,29 @@ import { ConditionField } from './ConditionField';
 import { GrafanaAlertStatePicker } from './GrafanaAlertStatePicker';
 import { RuleEditorSection } from './RuleEditorSection';

+const MIN_TIME_RANGE_STEP_S = 10; // 10 seconds
+
 const timeRangeValidationOptions: RegisterOptions = {
  required: {
    value: true,
    message: 'Required.',
  },
  pattern: timeValidationPattern,
+  validate: (value: string) => {
+    const duration = parseDuration(value);
+    if (Object.keys(duration).length) {
+      const from = new Date();
+      const to = addDurationToDate(from, duration);
+      const diff = to.getTime() - from.getTime();
+      if (diff < 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) {
+        return `Must be a multiple of ${MIN_TIME_RANGE_STEP_S} seconds.`;
+      }
+    }
+    return true;
+  },
 };

 export const ConditionsStep: FC = () => {
@@ -31,7 +48,7 @@ export const ConditionsStep: FC = () => {

  return (
    <RuleEditorSection stepNo={3} title="Define alert conditions">
-      {type === RuleFormType.threshold && (
+      {type === RuleFormType.grafana && (
        <>
          <ConditionField />
          <Field label="Evaluate">
@@ -43,6 +60,7 @@ export const ConditionsStep: FC = () => {
                className={styles.inlineField}
                error={errors.evaluateEvery?.message}
                invalid={!!errors.evaluateEvery?.message}
+                validationMessageHorizontalOverflow={true}
              >
                <Input width={8} {...register('evaluateEvery', timeRangeValidationOptions)} />
              </Field>
@@ -56,6 +74,7 @@ export const ConditionsStep: FC = () => {
                className={styles.inlineField}
                error={errors.evaluateFor?.message}
                invalid={!!errors.evaluateFor?.message}
+                validationMessageHorizontalOverflow={true}
              >
                <Input width={8} {...register('evaluateFor', timeRangeValidationOptions)} />
              </Field>
@@ -69,7 +88,12 @@ export const ConditionsStep: FC = () => {
              <Field label="Alert state if no data or all values are null">
                <InputControl
                  render={({ field: { onChange, ref, ...field } }) => (
-                    <GrafanaAlertStatePicker {...field} width={42} onChange={(value) => onChange(value?.value)} />
+                    <GrafanaAlertStatePicker
+                      {...field}
+                      width={42}
+                      includeNoData={true}
+                      onChange={(value) => onChange(value?.value)}
+                    />
                  )}
                  name="noDataState"
                />
@@ -77,7 +101,12 @@ export const ConditionsStep: FC = () => {
              <Field label="Alert state if execution error or timeout">
                <InputControl
                  render={({ field: { onChange, ref, ...field } }) => (
-                    <GrafanaAlertStatePicker {...field} width={42} onChange={(value) => onChange(value?.value)} />
+                    <GrafanaAlertStatePicker
+                      {...field}
+                      width={42}
+                      includeNoData={false}
+                      onChange={(value) => onChange(value?.value)}
+                    />
                  )}
                  name="execErrState"
                />
@@ -86,7 +115,7 @@ export const ConditionsStep: FC = () => {
          )}
        </>
      )}
-      {type === RuleFormType.system && (
+      {type === RuleFormType.cloud && (
        <>
          <Field label="For" description="Expression has to be true for this long for the alert to be fired.">
            <div className={styles.flexRow}>
--- a/public/app/features/alerting/unified/components/rule-editor/GrafanaAlertStatePicker.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/GrafanaAlertStatePicker.tsx
@@ -2,15 +2,24 @@ import { SelectableValue } from '@grafana/data';
 import { Select } from '@grafana/ui';
 import { SelectBaseProps } from '@grafana/ui/src/components/Select/types';
 import { GrafanaAlertStateDecision } from 'app/types/unified-alerting-dto';
-import React, { FC } from 'react';
+import React, { FC, useMemo } from 'react';

-type Props = Omit<SelectBaseProps<GrafanaAlertStateDecision>, 'options'>;
+type Props = Omit<SelectBaseProps<GrafanaAlertStateDecision>, 'options'> & {
+  includeNoData: boolean;
+};

 const options: SelectableValue[] = [
  { value: GrafanaAlertStateDecision.Alerting, label: 'Alerting' },
  { value: GrafanaAlertStateDecision.NoData, label: 'No Data' },
-  { value: GrafanaAlertStateDecision.KeepLastState, label: 'Keep Last State' },
  { value: GrafanaAlertStateDecision.OK, label: 'OK' },
 ];

-export const GrafanaAlertStatePicker: FC<Props> = (props) => <Select options={options} {...props} />;
+export const GrafanaAlertStatePicker: FC<Props> = ({ includeNoData, ...props }) => {
+  const opts = useMemo(() => {
+    if (includeNoData) {
+      return options;
+    }
+    return options.filter((opt) => opt.value !== GrafanaAlertStateDecision.NoData);
+  }, [includeNoData]);
+  return <Select options={opts} {...props} />;
+};
--- a/public/app/features/alerting/unified/components/rule-editor/GroupAndNamespaceFields.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/GroupAndNamespaceFields.tsx
@@ -4,9 +4,9 @@ import { useUnifiedAlertingSelector } from '../../hooks/useUnifiedAlertingSelect
 import { fetchRulerRulesAction } from '../../state/actions';
 import { RuleFormValues } from '../../types/rule-form';
 import { useFormContext } from 'react-hook-form';
-import { SelectableValue } from '@grafana/data';
+import { GrafanaTheme2, SelectableValue } from '@grafana/data';
 import { SelectWithAdd } from './SelectWIthAdd';
-import { Field, InputControl } from '@grafana/ui';
+import { Field, InputControl, useStyles2 } from '@grafana/ui';
 import { css } from '@emotion/css';

 interface Props {
@@ -21,6 +21,8 @@ export const GroupAndNamespaceFields: FC<Props> = ({ dataSourceName }) => {
    setValue,
  } = useFormContext<RuleFormValues>();

+  const style = useStyles2(getStyle);
+
  const [customGroup, setCustomGroup] = useState(false);

  const rulerRequests = useUnifiedAlertingSelector((state) => state.rulerRules);
@@ -46,13 +48,13 @@ export const GroupAndNamespaceFields: FC<Props> = ({ dataSourceName }) => {
  );

  return (
-    <>
+    <div className={style.flexRow}>
      <Field label="Namespace" error={errors.namespace?.message} invalid={!!errors.namespace?.message}>
        <InputControl
          render={({ field: { onChange, ref, ...field } }) => (
            <SelectWithAdd
              {...field}
-              className={inputStyle}
+              className={style.input}
              onChange={(value) => {
                setValue('group', ''); //reset if namespace changes
                onChange(value);
@@ -74,7 +76,7 @@ export const GroupAndNamespaceFields: FC<Props> = ({ dataSourceName }) => {
      <Field label="Group" error={errors.group?.message} invalid={!!errors.group?.message}>
        <InputControl
          render={({ field: { ref, ...field } }) => (
-            <SelectWithAdd {...field} options={groupOptions} width={42} custom={customGroup} className={inputStyle} />
+            <SelectWithAdd {...field} options={groupOptions} width={42} custom={customGroup} className={style.input} />
          )}
          name="group"
          control={control}
@@ -83,10 +85,21 @@ export const GroupAndNamespaceFields: FC<Props> = ({ dataSourceName }) => {
          }}
        />
      </Field>
-    </>
+    </div>
  );
 };

-const inputStyle = css`
-  width: 330px;
-`;
+const getStyle = (theme: GrafanaTheme2) => ({
+  flexRow: css`
+    display: flex;
+    flex-direction: row;
+    justify-content: flex-start;
+
+    & > * + * {
+      margin-left: ${theme.spacing(3)};
+    }
+  `,
+  input: css`
+    width: 330px !important;
+  `,
+});
--- a/public/app/features/alerting/unified/components/rule-editor/QueryStep.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/QueryStep.tsx
@@ -16,7 +16,7 @@ export const QueryStep: FC = () => {
  const dataSourceName = watch('dataSourceName');
  return (
    <RuleEditorSection stepNo={2} title="Create a query to be alerted on">
-      {type === RuleFormType.system && dataSourceName && (
+      {type === RuleFormType.cloud && dataSourceName && (
        <Field error={errors.expression?.message} invalid={!!errors.expression?.message}>
          <InputControl
            name="expression"
@@ -28,7 +28,7 @@ export const QueryStep: FC = () => {
          />
        </Field>
      )}
-      {type === RuleFormType.threshold && (
+      {type === RuleFormType.grafana && (
        <Field
          invalid={!!errors.queries}
          error={(!!errors.queries && 'Must provide at least one valid query.') || undefined}
--- a/public/app/features/alerting/unified/components/rule-editor/RuleEditorSection.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/RuleEditorSection.tsx
@@ -1,6 +1,6 @@
 import { css } from '@emotion/css';
-import { GrafanaTheme } from '@grafana/data';
-import { FieldSet, useStyles } from '@grafana/ui';
+import { GrafanaTheme2 } from '@grafana/data';
+import { FieldSet, useStyles2 } from '@grafana/ui';
 import React, { FC } from 'react';

 export interface RuleEditorSectionProps {
@@ -10,7 +10,7 @@ export interface RuleEditorSectionProps {
 }

 export const RuleEditorSection: FC<RuleEditorSectionProps> = ({ title, stepNo, children, description }) => {
-  const styles = useStyles(getStyles);
+  const styles = useStyles2(getStyles);

  return (
    <div className={styles.parent}>
@@ -18,7 +18,7 @@ export const RuleEditorSection: FC<RuleEditorSectionProps> = ({ title, stepNo, c
        <span className={styles.stepNo}>{stepNo}</span>
      </div>
      <div className={styles.content}>
-        <FieldSet label={title}>
+        <FieldSet label={title} className={styles.fieldset}>
          {description && <p className={styles.description}>{description}</p>}
          {children}
        </FieldSet>
@@ -27,26 +27,35 @@ export const RuleEditorSection: FC<RuleEditorSectionProps> = ({ title, stepNo, c
  );
 };

-const getStyles = (theme: GrafanaTheme) => ({
+const getStyles = (theme: GrafanaTheme2) => ({
+  fieldset: css`
+    legend {
+      font-size: 16px;
+      padding-top: ${theme.spacing(0.5)};
+    }
+  `,
  parent: css`
    display: flex;
    flex-direction: row;
-    max-width: ${theme.breakpoints.xl};
+    max-width: ${theme.breakpoints.values.xl};
+    & + & {
+      margin-top: ${theme.spacing(4)};
+    }
  `,
  description: css`
-    margin-top: -${theme.spacing.md};
+    margin-top: -${theme.spacing(2)};
  `,
  stepNo: css`
    display: inline-block;
-    width: ${theme.spacing.xl};
-    height: ${theme.spacing.xl};
-    line-height: ${theme.spacing.xl};
-    border-radius: ${theme.spacing.md};
+    width: ${theme.spacing(4)};
+    height: ${theme.spacing(4)};
+    line-height: ${theme.spacing(4)};
+    border-radius: ${theme.spacing(4)};
    text-align: center;
-    color: ${theme.colors.textStrong};
-    background-color: ${theme.colors.bg3};
+    color: ${theme.colors.text.maxContrast};
+    background-color: ${theme.colors.background.canvas};
    font-size: ${theme.typography.size.lg};
-    margin-right: ${theme.spacing.md};
+    margin-right: ${theme.spacing(2)};
  `,
  content: css`
    flex: 1;