From 1945f2b64e66c268465adc617158538ec03add8b Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Mon, 10 Jul 2023 13:03:35 +0200 Subject: [PATCH] Alerting: Use new "Label" components for alert instance labels (#70997) --- .../alerting/unified/AlertGroups.test.tsx | 20 +++---- .../alerting/unified/RuleList.test.tsx | 8 +-- .../unified/components/AlertLabels.tsx | 36 ++++++++++-- .../alerting/unified/components/Label.tsx | 58 ++++++++++++++----- .../components/alert-groups/AlertGroup.tsx | 6 +- .../alert-groups/AlertGroupAlertsTable.tsx | 2 +- .../components/rule-viewer/RuleViewer.v1.tsx | 3 +- .../components/rules/AlertInstancesTable.tsx | 2 +- .../silences/SilencedAlertsTableRow.tsx | 2 +- .../silences/SilencedInstancesPreview.tsx | 5 +- .../plugins/panel/alertGroups/AlertGroup.tsx | 4 +- .../alertGroups/AlertGroupsPanel.test.tsx | 2 +- .../panel/alertlist/UnifiedalertList.test.tsx | 4 +- 13 files changed, 102 insertions(+), 50 deletions(-) diff --git a/public/app/features/alerting/unified/AlertGroups.test.tsx b/public/app/features/alerting/unified/AlertGroups.test.tsx index e6c6c87163c..fd0dd82b153 100644 --- a/public/app/features/alerting/unified/AlertGroups.test.tsx +++ b/public/app/features/alerting/unified/AlertGroups.test.tsx @@ -78,7 +78,7 @@ describe('AlertGroups', () => { expect(groups).toHaveLength(2); expect(groups[0]).toHaveTextContent('No grouping'); - expect(groups[1]).toHaveTextContent('severity=warningregion=US-Central'); + expect(groups[1]).toHaveTextContent('severitywarning regionUS-Central'); await userEvent.click(ui.groupCollapseToggle.get(groups[0])); expect(ui.groupTable.get()).toBeDefined(); @@ -111,9 +111,9 @@ describe('AlertGroups', () => { const groupByWrapper = ui.groupByContainer.get(); expect(groups).toHaveLength(3); - expect(groups[0]).toHaveTextContent('region=NASA'); - expect(groups[1]).toHaveTextContent('region=EMEA'); - expect(groups[2]).toHaveTextContent('region=APAC'); + expect(groups[0]).toHaveTextContent('regionNASA'); + expect(groups[1]).toHaveTextContent('regionEMEA'); + expect(groups[2]).toHaveTextContent('regionAPAC'); await userEvent.type(groupByInput, 'appName{enter}'); @@ -123,9 +123,9 @@ describe('AlertGroups', () => { await waitFor(() => expect(ui.clearButton.get()).toBeInTheDocument()); expect(groups).toHaveLength(3); - expect(groups[0]).toHaveTextContent('appName=billing'); - expect(groups[1]).toHaveTextContent('appName=auth'); - expect(groups[2]).toHaveTextContent('appName=frontend'); + expect(groups[0]).toHaveTextContent('appNamebilling'); + expect(groups[1]).toHaveTextContent('appNameauth'); + expect(groups[2]).toHaveTextContent('appNamefrontend'); await userEvent.click(ui.clearButton.get()); await waitFor(() => expect(groupByWrapper).not.toHaveTextContent('appName')); @@ -136,8 +136,8 @@ describe('AlertGroups', () => { groups = await ui.group.findAll(); expect(groups).toHaveLength(2); - expect(groups[0]).toHaveTextContent('env=production'); - expect(groups[1]).toHaveTextContent('env=staging'); + expect(groups[0]).toHaveTextContent('envproduction'); + expect(groups[1]).toHaveTextContent('envstaging'); await userEvent.click(ui.clearButton.get()); await waitFor(() => expect(groupByWrapper).not.toHaveTextContent('env')); @@ -148,7 +148,7 @@ describe('AlertGroups', () => { groups = await ui.group.findAll(); expect(groups).toHaveLength(2); expect(groups[0]).toHaveTextContent('No grouping'); - expect(groups[1]).toHaveTextContent('uniqueLabel=true'); + expect(groups[1]).toHaveTextContent('uniqueLabeltrue'); }); it('should combine multiple ungrouped groups', async () => { diff --git a/public/app/features/alerting/unified/RuleList.test.tsx b/public/app/features/alerting/unified/RuleList.test.tsx index d191bdb743b..3ffda3e9d2e 100644 --- a/public/app/features/alerting/unified/RuleList.test.tsx +++ b/public/app/features/alerting/unified/RuleList.test.tsx @@ -362,7 +362,7 @@ describe('RuleList', () => { const ruleDetails = ui.expandedContent.get(ruleRows[1]); - expect(ruleDetails).toHaveTextContent('Labelsseverity=warningfoo=bar'); + expect(ruleDetails).toHaveTextContent('Labels severitywarning foobar'); expect(ruleDetails).toHaveTextContent('Expressiontopk ( 5 , foo ) [ 5m ]'); expect(ruleDetails).toHaveTextContent('messagegreat alert'); expect(ruleDetails).toHaveTextContent('Matching instances'); @@ -373,8 +373,8 @@ describe('RuleList', () => { const instanceRows = byTestId('row').getAll(instancesTable); expect(instanceRows).toHaveLength(2); - expect(instanceRows![0]).toHaveTextContent('Firing foo=barseverity=warning2021-03-18 08:47:05'); - expect(instanceRows![1]).toHaveTextContent('Firing foo=bazseverity=error2021-03-18 08:47:05'); + expect(instanceRows![0]).toHaveTextContent('Firing foobar severitywarning2021-03-18 08:47:05'); + expect(instanceRows![1]).toHaveTextContent('Firing foobaz severityerror2021-03-18 08:47:05'); // expand details of an instance await userEvent.click(ui.ruleCollapseToggle.get(instanceRows![0])); @@ -515,7 +515,7 @@ describe('RuleList', () => { await userEvent.click(ui.ruleCollapseToggle.get(ruleRows[0])); const ruleDetails = ui.expandedContent.get(ruleRows[0]); - expect(ruleDetails).toHaveTextContent('Labelsseverity=warningfoo=bar'); + expect(ruleDetails).toHaveTextContent('Labels severitywarning foobar'); // Check for different label matchers await userEvent.clear(filterInput); diff --git a/public/app/features/alerting/unified/components/AlertLabels.tsx b/public/app/features/alerting/unified/components/AlertLabels.tsx index 8c6c37cd82d..cadcaaeec59 100644 --- a/public/app/features/alerting/unified/components/AlertLabels.tsx +++ b/public/app/features/alerting/unified/components/AlertLabels.tsx @@ -1,17 +1,41 @@ +import { css } from '@emotion/css'; +import { chain } from 'lodash'; import React from 'react'; -import { TagList } from '@grafana/ui'; +import { GrafanaTheme2 } from '@grafana/data'; +import { getTagColorsFromName, useStyles2 } from '@grafana/ui'; + +import { Label, LabelSize } from './Label'; interface Props { labels: Record; - className?: string; + size?: LabelSize; } -export const AlertLabels = ({ labels, className }: Props) => { - const pairs = Object.entries(labels).filter(([key]) => !(key.startsWith('__') && key.endsWith('__'))); +export const AlertLabels = ({ labels, size }: Props) => { + const styles = useStyles2((theme) => getStyles(theme, size)); + const pairs = chain(labels).toPairs().reject(isPrivateKey).value(); + return ( -
- `${label}=${value}`)} className={className} /> +
+ {pairs.map(([label, value]) => ( +
); }; + +function getLabelColor(input: string): string { + return getTagColorsFromName(input).color; +} + +const isPrivateKey = ([key, _]: [string, string]) => key.startsWith('__') && key.endsWith('__'); + +const getStyles = (theme: GrafanaTheme2, size?: LabelSize) => ({ + wrapper: css` + display: flex; + flex-wrap: wrap; + + gap: ${size === 'md' ? theme.spacing() : theme.spacing(0.5)}; + `, +}); diff --git a/public/app/features/alerting/unified/components/Label.tsx b/public/app/features/alerting/unified/components/Label.tsx index 83dbf664569..20ac5505dde 100644 --- a/public/app/features/alerting/unified/components/Label.tsx +++ b/public/app/features/alerting/unified/components/Label.tsx @@ -1,61 +1,87 @@ import { css } from '@emotion/css'; import React, { ReactNode } from 'react'; +import tinycolor2 from 'tinycolor2'; import { GrafanaTheme2, IconName } from '@grafana/data'; import { Stack } from '@grafana/experimental'; import { Icon, useStyles2 } from '@grafana/ui'; +export type LabelSize = 'md' | 'sm'; + interface Props { icon?: IconName; label?: ReactNode; value: ReactNode; color?: string; + size?: LabelSize; } // TODO allow customization with color prop -const Label = ({ label, value, icon }: Props) => { - const styles = useStyles2(getStyles); +const Label = ({ label, value, icon, color, size = 'md' }: Props) => { + const styles = useStyles2((theme) => getStyles(theme, color, size)); return ( -
- -
+
+ +
{icon && } {label ?? ''}
-
{value}
+
{value}
); }; -const getStyles = (theme: GrafanaTheme2) => ({ - meta: (color?: string) => ({ +const getStyles = (theme: GrafanaTheme2, color?: string, size?: string) => { + const backgroundColor = color ?? theme.colors.secondary.main; + + const borderColor = theme.isDark + ? tinycolor2(backgroundColor).lighten(5).toString() + : tinycolor2(backgroundColor).darken(5).toString(); + + const valueBackgroundColor = theme.isDark + ? tinycolor2(backgroundColor).darken(5).toString() + : tinycolor2(backgroundColor).lighten(5).toString(); + + const fontColor = color + ? tinycolor2.mostReadable(backgroundColor, ['#000', '#fff']).toString() + : theme.colors.text.primary; + + const padding = + size === 'md' ? `${theme.spacing(0.33)} ${theme.spacing(1)}` : `${theme.spacing(0.2)} ${theme.spacing(0.6)}`; + + return { wrapper: css` + color: ${fontColor}; font-size: ${theme.typography.bodySmall.fontSize}; + + border-radius: ${theme.shape.borderRadius(2)}; `, label: css` display: flex; align-items: center; + color: inherit; - padding: ${theme.spacing(0.33)} ${theme.spacing(1)}; - background: ${theme.colors.secondary.transparent}; + padding: ${padding}; + background: ${backgroundColor}; - border: solid 1px ${theme.colors.border.medium}; + border: solid 1px ${borderColor}; border-top-left-radius: ${theme.shape.borderRadius(2)}; border-bottom-left-radius: ${theme.shape.borderRadius(2)}; `, value: css` - padding: ${theme.spacing(0.33)} ${theme.spacing(1)}; - font-weight: ${theme.typography.fontWeightBold}; + color: inherit; + padding: ${padding}; + background: ${valueBackgroundColor}; - border: solid 1px ${theme.colors.border.medium}; + border: solid 1px ${borderColor}; border-left: none; border-top-right-radius: ${theme.shape.borderRadius(2)}; border-bottom-right-radius: ${theme.shape.borderRadius(2)}; `, - }), -}); + }; +}; export { Label }; diff --git a/public/app/features/alerting/unified/components/alert-groups/AlertGroup.tsx b/public/app/features/alerting/unified/components/alert-groups/AlertGroup.tsx index c89bd2f3cf5..62b27ad8e4f 100644 --- a/public/app/features/alerting/unified/components/alert-groups/AlertGroup.tsx +++ b/public/app/features/alerting/unified/components/alert-groups/AlertGroup.tsx @@ -30,7 +30,11 @@ export const AlertGroup = ({ alertManagerSourceName, group }: Props) => { onToggle={() => setIsCollapsed(!isCollapsed)} data-testid="alert-group-collapse-toggle" /> - {Object.keys(group.labels).length ? : No grouping} + {Object.keys(group.labels).length ? ( + + ) : ( + No grouping + )}
diff --git a/public/app/features/alerting/unified/components/alert-groups/AlertGroupAlertsTable.tsx b/public/app/features/alerting/unified/components/alert-groups/AlertGroupAlertsTable.tsx index 6883839f34b..578490c78c4 100644 --- a/public/app/features/alerting/unified/components/alert-groups/AlertGroupAlertsTable.tsx +++ b/public/app/features/alerting/unified/components/alert-groups/AlertGroupAlertsTable.tsx @@ -47,7 +47,7 @@ export const AlertGroupAlertsTable = ({ alerts, alertManagerSourceName }: Props) id: 'labels', label: 'Labels', // eslint-disable-next-line react/display-name - renderCell: ({ data: { labels } }) => , + renderCell: ({ data: { labels } }) => , size: 1, }, ], diff --git a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.v1.tsx b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.v1.tsx index d190e5101d9..252422d0bd8 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.v1.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.v1.tsx @@ -171,7 +171,7 @@ export function RuleViewer({ match }: RuleViewerProps) { )} {!!rule.labels && !!Object.keys(rule.labels).length && ( - + )} @@ -297,6 +297,7 @@ const getStyles = (theme: GrafanaTheme2) => { `, leftSide: css` flex: 1; + overflow: hidden; `, rightSide: css` padding-right: ${theme.spacing(3)}; diff --git a/public/app/features/alerting/unified/components/rules/AlertInstancesTable.tsx b/public/app/features/alerting/unified/components/rules/AlertInstancesTable.tsx index 38cbaefef2b..dd2bd4302dc 100644 --- a/public/app/features/alerting/unified/components/rules/AlertInstancesTable.tsx +++ b/public/app/features/alerting/unified/components/rules/AlertInstancesTable.tsx @@ -53,7 +53,7 @@ const columns: AlertTableColumnProps[] = [ id: 'labels', label: 'Labels', // eslint-disable-next-line react/display-name - renderCell: ({ data: { labels } }) => , + renderCell: ({ data: { labels } }) => , }, { id: 'created', diff --git a/public/app/features/alerting/unified/components/silences/SilencedAlertsTableRow.tsx b/public/app/features/alerting/unified/components/silences/SilencedAlertsTableRow.tsx index e4b785659a3..d5692a73be5 100644 --- a/public/app/features/alerting/unified/components/silences/SilencedAlertsTableRow.tsx +++ b/public/app/features/alerting/unified/components/silences/SilencedAlertsTableRow.tsx @@ -42,7 +42,7 @@ export const SilencedAlertsTableRow = ({ alert, className }: Props) => { - + )} diff --git a/public/app/features/alerting/unified/components/silences/SilencedInstancesPreview.tsx b/public/app/features/alerting/unified/components/silences/SilencedInstancesPreview.tsx index 8c1b4490b29..a45d210b0a7 100644 --- a/public/app/features/alerting/unified/components/silences/SilencedInstancesPreview.tsx +++ b/public/app/features/alerting/unified/components/silences/SilencedInstancesPreview.tsx @@ -90,7 +90,7 @@ function useColumns(): Array> { id: 'labels', label: 'Labels', renderCell: function renderName({ data }) { - return ; + return ; }, size: 'auto', }, @@ -123,7 +123,4 @@ const getStyles = (theme: GrafanaTheme2) => ({ display: flex; align-items: center; `, - alertLabels: css` - justify-content: flex-start; - `, }); diff --git a/public/app/plugins/panel/alertGroups/AlertGroup.tsx b/public/app/plugins/panel/alertGroups/AlertGroup.tsx index c2ba8196295..2ffb2de9aa3 100644 --- a/public/app/plugins/panel/alertGroups/AlertGroup.tsx +++ b/public/app/plugins/panel/alertGroups/AlertGroup.tsx @@ -26,7 +26,7 @@ export const AlertGroup = ({ alertManagerSourceName, group, expandAll }: Props) return (
{Object.keys(group.labels).length > 0 ? ( - + ) : (
No grouping
)} @@ -49,7 +49,7 @@ export const AlertGroup = ({ alertManagerSourceName, group, expandAll }: Props) {state} for {interval}
- +
{alert.status.state === AlertState.Suppressed && ( diff --git a/public/app/plugins/panel/alertGroups/AlertGroupsPanel.test.tsx b/public/app/plugins/panel/alertGroups/AlertGroupsPanel.test.tsx index 5499e623250..6e8b34f1b9c 100644 --- a/public/app/plugins/panel/alertGroups/AlertGroupsPanel.test.tsx +++ b/public/app/plugins/panel/alertGroups/AlertGroupsPanel.test.tsx @@ -123,7 +123,7 @@ describe('AlertGroupsPanel', () => { expect(groups).toHaveLength(2); expect(groups[0]).toHaveTextContent('No grouping'); - expect(groups[1]).toHaveTextContent('severity=warningregion=US-Central'); + expect(groups[1]).toHaveTextContent('severitywarning regionUS-Central'); const alerts = ui.alert.queryAll(); expect(alerts).toHaveLength(0); diff --git a/public/app/plugins/panel/alertlist/UnifiedalertList.test.tsx b/public/app/plugins/panel/alertlist/UnifiedalertList.test.tsx index e84e34486d7..e98637b8e7d 100644 --- a/public/app/plugins/panel/alertlist/UnifiedalertList.test.tsx +++ b/public/app/plugins/panel/alertlist/UnifiedalertList.test.tsx @@ -188,8 +188,8 @@ describe('UnifiedAlertList', () => { await user.click(expandElement); - const tagsElement = await byRole('list', { name: 'Tags' }).find(); - expect(await byRole('listitem').find(tagsElement)).toHaveTextContent('severity=critical'); + const labelsElement = await byRole('list', { name: 'Labels' }).find(); + expect(await byRole('listitem').find(labelsElement)).toHaveTextContent('severitycritical'); expect(replaceVarsSpy).toHaveBeenLastCalledWith('$label'); expect(filterAlertsSpy).toHaveBeenLastCalledWith(