Alerting: Fix the silence url's matcher parameters (#43898)

* Split silence matchers parameter into a separate entry for each label

* Unify the silence link creation

* Remove duplicated matchers when parsing to/from query params

* Add tests for matchers

* Add a comment with a duplication removal explanation

* Improve label duplication comment

* Remove redundant code

* Use uniqBy to simplify the code. Rename matchers parameter

* Fix Silence test data

public/app/features/alerting/unified/Silences.test.tsx

@@ -174,9 +174,10 @@ describe('Silence edit', () => {
   it(
     'prefills the matchers field with matchers params',
     async () => {
-      renderSilences(
+      const matchersParams = ['foo=bar', 'bar=~ba.+', 'hello!=world', 'cluster!~us-central.*'];
-        `${baseUrlPath}?matchers=${encodeURIComponent('foo=bar,bar=~ba.+,hello!=world,cluster!~us-central.*')}`
+      const matchersQueryString = matchersParams.map((matcher) => `matcher=${encodeURIComponent(matcher)}`).join('&');
-      );
+
+      renderSilences(`${baseUrlPath}?${matchersQueryString}`);
       await waitFor(() => expect(ui.editor.durationField.query()).not.toBeNull());
 
       const matchers = ui.editor.matchersField.queryAll();

public/app/features/alerting/unified/components/alert-groups/AlertDetails.tsx

@@ -2,11 +2,9 @@ import { css } from '@emotion/css';
 import { GrafanaTheme2 } from '@grafana/data';
 import { LinkButton, useStyles2 } from '@grafana/ui';
 import { AlertmanagerAlert, AlertState } from 'app/plugins/datasource/alertmanager/types';
-
 import React, { FC } from 'react';
-import { makeAMLink } from '../../utils/misc';
+import { makeAMLink, makeLabelBasedSilenceLink } from '../../utils/misc';
 import { AnnotationDetailsField } from '../AnnotationDetailsField';
-import { getMatcherQueryParams } from '../../utils/matchers';
 
 interface AmNotificationsAlertDetailsProps {
   alertManagerSourceName: string;
@@ -33,9 +31,7 @@ export const AlertDetails: FC<AmNotificationsAlertDetailsProps> = ({ alert, aler
         )}
         {alert.status.state === AlertState.Active && (
           <LinkButton
-            href={`${makeAMLink('/alerting/silence/new', alertManagerSourceName)}&${getMatcherQueryParams(
+            href={makeLabelBasedSilenceLink(alertManagerSourceName, alert.labels)}
-              alert.labels
-            )}`}
             className={styles.button}
             icon={'bell-slash'}
             size={'sm'}

public/app/features/alerting/unified/components/rules/RuleDetailsActionButtons.tsx

@@ -10,7 +10,7 @@ import { appEvents } from 'app/core/core';
 import { useIsRuleEditable } from '../../hooks/useIsRuleEditable';
 import { Annotation } from '../../utils/constants';
 import { getRulesSourceName, isCloudRulesSource, isGrafanaRulesSource } from '../../utils/datasource';
-import { createExploreLink, createViewLink, makeSilenceLink } from '../../utils/misc';
+import { createExploreLink, createViewLink, makeRuleBasedSilenceLink } from '../../utils/misc';
 import * as ruleId from '../../utils/rule-id';
 import { deleteRuleAction } from '../../state/actions';
 import { CombinedRule, RulesSource } from 'app/types/unified-alerting';
@@ -140,7 +140,7 @@ export const RuleDetailsActionButtons: FC<Props> = ({ rule, rulesSource }) => {
         key="silence"
         icon="bell-slash"
         target="__blank"
-        href={makeSilenceLink(alertmanagerSourceName, rule)}
+        href={makeRuleBasedSilenceLink(alertmanagerSourceName, rule)}
       >
         Silence
       </LinkButton>

public/app/features/alerting/unified/components/silences/SilencesEditor.tsx

@@ -8,7 +8,6 @@ import {
   addDurationToDate,
   dateTime,
   isValidDate,
-  UrlQueryMap,
   GrafanaTheme2,
 } from '@grafana/data';
 import { useDebounce } from 'react-use';
@@ -24,35 +23,34 @@ import { css, cx } from '@emotion/css';
 import { useUnifiedAlertingSelector } from '../../hooks/useUnifiedAlertingSelector';
 import { makeAMLink } from '../../utils/misc';
 import { useCleanup } from 'app/core/hooks/useCleanup';
-import { useQueryParams } from 'app/core/hooks/useQueryParams';
 import { parseQueryParamMatchers } from '../../utils/matchers';
 import { matcherToMatcherField, matcherFieldToMatcher } from '../../utils/alertmanager';
+import { useURLSearchParams } from '../../hooks/useURLSearchParams';
 
 interface Props {
   silence?: Silence;
   alertManagerSourceName: string;
 }
 
-const defaultsFromQuery = (queryParams: UrlQueryMap): Partial<SilenceFormFields> => {
+const defaultsFromQuery = (searchParams: URLSearchParams): Partial<SilenceFormFields> => {
   const defaults: Partial<SilenceFormFields> = {};
 
-  const { matchers, comment } = queryParams;
+  const comment = searchParams.get('comment');
+  const matchers = searchParams.getAll('matcher');
 
-  if (typeof matchers === 'string') {
+  const formMatchers = parseQueryParamMatchers(matchers);
-    const formMatchers = parseQueryParamMatchers(matchers);
+  if (formMatchers.length) {
-    if (formMatchers.length) {
+    defaults.matchers = formMatchers.map(matcherToMatcherField);
-      defaults.matchers = formMatchers.map(matcherToMatcherField);
-    }
   }
 
-  if (typeof comment === 'string') {
+  if (comment) {
     defaults.comment = comment;
   }
 
   return defaults;
 };
 
-const getDefaultFormValues = (queryParams: UrlQueryMap, silence?: Silence): SilenceFormFields => {
+const getDefaultFormValues = (searchParams: URLSearchParams, silence?: Silence): SilenceFormFields => {
   const now = new Date();
   if (silence) {
     const isExpired = Date.parse(silence.endsAt) < Date.now();
@@ -89,14 +87,15 @@ const getDefaultFormValues = (queryParams: UrlQueryMap, silence?: Silence): Sile
       matcherName: '',
       matcherValue: '',
       timeZone: DefaultTimeZone,
-      ...defaultsFromQuery(queryParams),
+      ...defaultsFromQuery(searchParams),
     };
   }
 };
 
 export const SilencesEditor: FC<Props> = ({ silence, alertManagerSourceName }) => {
-  const [queryParams] = useQueryParams();
+  const [urlSearchParams] = useURLSearchParams();
-  const defaultValues = useMemo(() => getDefaultFormValues(queryParams, silence), [silence, queryParams]);
+
+  const defaultValues = useMemo(() => getDefaultFormValues(urlSearchParams, silence), [silence, urlSearchParams]);
   const formAPI = useForm({ defaultValues });
   const dispatch = useDispatch();
   const styles = useStyles2(getStyles);

public/app/features/alerting/unified/hooks/useURLSearchParams.ts

@@ -0,0 +1,8 @@
+import { useMemo } from 'react';
+import { useLocation } from 'react-router-dom';
+
+export function useURLSearchParams(): [URLSearchParams] {
+  const { search } = useLocation();
+  const queryParams = useMemo(() => new URLSearchParams(search), [search]);
+  return [queryParams];
+}

public/app/features/alerting/unified/utils/matchers.test.ts

@@ -0,0 +1,36 @@
+import { getMatcherQueryParams, parseQueryParamMatchers } from './matchers';
+
+describe('Unified Alerting matchers', () => {
+  describe('getMatcherQueryParams tests', () => {
+    it('Should create an entry for each label', () => {
+      const params = getMatcherQueryParams({ foo: 'bar', alertname: 'TestData - No data', rule_uid: 'YNZBpGJnk' });
+
+      const matcherParams = params.getAll('matcher');
+
+      expect(matcherParams).toHaveLength(3);
+      expect(matcherParams).toContain('foo=bar');
+      expect(matcherParams).toContain('alertname=TestData - No data');
+      expect(matcherParams).toContain('rule_uid=YNZBpGJnk');
+    });
+  });
+
+  describe('parseQueryParamMatchers tests', () => {
+    it('Should create a matcher for each unique label-expression pair', () => {
+      const matchers = parseQueryParamMatchers(['alertname=TestData 1', 'rule_uid=YNZBpGJnk']);
+
+      expect(matchers).toHaveLength(2);
+      expect(matchers[0].name).toBe('alertname');
+      expect(matchers[0].value).toBe('TestData 1');
+      expect(matchers[1].name).toBe('rule_uid');
+      expect(matchers[1].value).toBe('YNZBpGJnk');
+    });
+
+    it('Should create one matcher, using the first occurence when duplicated labels exists', () => {
+      const matchers = parseQueryParamMatchers(['alertname=TestData 1', 'alertname=TestData 2']);
+
+      expect(matchers).toHaveLength(1);
+      expect(matchers[0].name).toBe('alertname');
+      expect(matchers[0].value).toBe('TestData 1');
+    });
+  });
+});

public/app/features/alerting/unified/utils/matchers.ts

@@ -1,22 +1,26 @@
 import { Matcher } from 'app/plugins/datasource/alertmanager/types';
 import { Labels } from '@grafana/data';
 import { parseMatcher } from './alertmanager';
+import { uniqBy } from 'lodash';
 
-// parses comma separated matchers like "foo=bar,baz=~bad*" into SilenceMatcher[]
+// Parses a list of entries like like "['foo=bar', 'baz=~bad*']" into SilenceMatcher[]
-export function parseQueryParamMatchers(paramValue: string): Matcher[] {
+export function parseQueryParamMatchers(matcherPairs: string[]): Matcher[] {
-  return paramValue
+  const parsedMatchers = matcherPairs.filter((x) => !!x.trim()).map((x) => parseMatcher(x.trim()));
-    .split(',')
+
-    .filter((x) => !!x.trim())
+  // Due to migration, old alert rules might have a duplicated alertname label
-    .map((x) => parseMatcher(x.trim()));
+  // To handle that case want to filter out duplicates and make sure there are only unique labels
+  return uniqBy(parsedMatchers, (matcher) => matcher.name);
 }
 
 export const getMatcherQueryParams = (labels: Labels) => {
-  return `matchers=${encodeURIComponent(
+  const validMatcherLabels = Object.entries(labels).filter(
-    Object.entries(labels)
+    ([labelKey]) => !(labelKey.startsWith('__') && labelKey.endsWith('__'))
-      .filter(([labelKey]) => !(labelKey.startsWith('__') && labelKey.endsWith('__')))
+  );
-      .map(([labelKey, labelValue]) => {
+
-        return `${labelKey}=${labelValue}`;
+  const matcherUrlParams = new URLSearchParams();
-      })
+  validMatcherLabels.forEach(([labelKey, labelValue]) =>
-      .join(',')
+    matcherUrlParams.append('matcher', `${labelKey}=${labelValue}`)
-  )}`;
+  );
+
+  return matcherUrlParams;
 };

public/app/features/alerting/unified/utils/misc.ts

@@ -1,4 +1,4 @@
-import { urlUtil, UrlQueryMap } from '@grafana/data';
+import { urlUtil, UrlQueryMap, Labels } from '@grafana/data';
 import { config } from '@grafana/runtime';
 import { Alert, CombinedRule, FilterState, RulesSource, SilenceFilterState } from 'app/types/unified-alerting';
 import { ALERTMANAGER_NAME_QUERY_KEY } from './constants';
@@ -8,6 +8,7 @@ import { SortOrder } from 'app/plugins/panel/alertlist/types';
 import { alertInstanceKey } from 'app/features/alerting/unified/utils/rules';
 import { sortBy } from 'lodash';
 import { GrafanaAlertState, PromAlertingRuleState } from 'app/types/unified-alerting-dto';
+import { getMatcherQueryParams } from './matchers';
 
 export function createViewLink(ruleSource: RulesSource, rule: CombinedRule, returnTo: string): string {
   const sourceName = getRulesSourceName(ruleSource);
@@ -65,13 +66,23 @@ export function makeAMLink(path: string, alertManagerName?: string, options?: Re
   return `${path}?${search.toString()}`;
 }
 
-export function makeSilenceLink(alertmanagerSourceName: string, rule: CombinedRule) {
+export function makeRuleBasedSilenceLink(alertManagerSourceName: string, rule: CombinedRule) {
-  return (
+  const labels: Labels = {
-    `${config.appSubUrl}/alerting/silence/new?alertmanager=${alertmanagerSourceName}` +
+    alertname: rule.name,
-    `&matchers=alertname=${rule.name},${Object.entries(rule.labels)
+    ...rule.labels,
-      .map(([key, value]) => encodeURIComponent(`${key}=${value}`))
+  };
-      .join(',')}`
+
-  );
+  return makeLabelBasedSilenceLink(alertManagerSourceName, labels);
+}
+
+export function makeLabelBasedSilenceLink(alertManagerSourceName: string, labels: Labels) {
+  const silenceUrlParams = new URLSearchParams();
+  silenceUrlParams.append('alertmanager', alertManagerSourceName);
+
+  const matcherParams = getMatcherQueryParams(labels);
+  matcherParams.forEach((value, key) => silenceUrlParams.append(key, value));
+
+  return `${config.appSubUrl}/alerting/silence/new?${silenceUrlParams.toString()}`;
 }
 
 // keep retrying fn if it's error passes shouldRetry(error) and timeout has not elapsed yet

public/app/plugins/panel/alertGroups/AlertGroup.tsx

@@ -8,8 +8,7 @@ import { AlertLabels } from 'app/features/alerting/unified/components/AlertLabel
 import { AlertGroupHeader } from 'app/features/alerting/unified/components/alert-groups/AlertGroupHeader';
 import { CollapseToggle } from 'app/features/alerting/unified/components/CollapseToggle';
 import { getNotificationsTextColors } from 'app/features/alerting/unified/styles/notifications';
-import { makeAMLink } from 'app/features/alerting/unified/utils/misc';
+import { makeAMLink, makeLabelBasedSilenceLink } from 'app/features/alerting/unified/utils/misc';
-import { getMatcherQueryParams } from 'app/features/alerting/unified/utils/matchers';
 
 type Props = {
   alertManagerSourceName: string;
@@ -68,9 +67,7 @@ export const AlertGroup = ({ alertManagerSourceName, group, expandAll }: Props)
                   )}
                   {alert.status.state === AlertState.Active && (
                     <LinkButton
-                      href={`${makeAMLink('/alerting/silence/new', alertManagerSourceName)}&${getMatcherQueryParams(
+                      href={makeLabelBasedSilenceLink(alertManagerSourceName, alert.labels)}
-                        alert.labels
-                      )}`}
                       className={styles.button}
                       icon={'bell-slash'}
                       size={'sm'}