From d8a5d8d8825650949d4ff05a84e8c6b76f82795d Mon Sep 17 00:00:00 2001 From: Lauren <61048546+laurenashleigh@users.noreply.github.com> Date: Fri, 14 Nov 2025 09:45:57 +0000 Subject: [PATCH] Alerting: Update UI of instance counts on triage page (#113660) * Alerting: Update UI of instance counts on triage page * make boxes full height * resolve PR comments- use grafana-ui components * refactoring --- .../alerting/unified/triage/Workbench.tsx | 2 +- .../unified/triage/scene/SummaryStats.test.ts | 79 +++---- .../unified/triage/scene/SummaryStats.tsx | 201 ++++++++---------- public/locales/en-US/grafana.json | 6 +- 4 files changed, 138 insertions(+), 150 deletions(-) diff --git a/public/app/features/alerting/unified/triage/Workbench.tsx b/public/app/features/alerting/unified/triage/Workbench.tsx index 66c6ccfec5b..3ee34133494 100644 --- a/public/app/features/alerting/unified/triage/Workbench.tsx +++ b/public/app/features/alerting/unified/triage/Workbench.tsx @@ -247,7 +247,7 @@ export const getStyles = (theme: GrafanaTheme2) => { overflow: 'hidden', // Let AutoSizer handle the overflow }), summaryContainer: css({ - gridTemplateRows: summaryHeight, + minHeight: summaryHeight, marginBottom: theme.spacing(2), }), headerContainer: css({ diff --git a/public/app/features/alerting/unified/triage/scene/SummaryStats.test.ts b/public/app/features/alerting/unified/triage/scene/SummaryStats.test.ts index b232733d430..ec968ea1214 100644 --- a/public/app/features/alerting/unified/triage/scene/SummaryStats.test.ts +++ b/public/app/features/alerting/unified/triage/scene/SummaryStats.test.ts @@ -4,31 +4,37 @@ import { PromAlertingRuleState } from 'app/types/unified-alerting-dto'; import { RuleFrame, countRules, parseAlertstateFilter } from './SummaryStats'; describe('parseAlertstateFilter', () => { - it('should return "firing" when filter contains alertstate="firing"', () => { - expect(parseAlertstateFilter('alertstate="firing"')).toBe(PromAlertingRuleState.Firing); + it('should return array with "firing" when filter contains alertstate="firing"', () => { + expect(parseAlertstateFilter('alertstate="firing"')).toEqual([PromAlertingRuleState.Firing]); }); - it('should return "pending" when filter contains alertstate="pending"', () => { - expect(parseAlertstateFilter('alertstate="pending"')).toBe(PromAlertingRuleState.Pending); + it('should return array with "pending" when filter contains alertstate="pending"', () => { + expect(parseAlertstateFilter('alertstate="pending"')).toEqual([PromAlertingRuleState.Pending]); }); - it('should return null when filter contains both firing and pending', () => { - expect(parseAlertstateFilter('alertstate=~"firing|pending"')).toBe(null); + it('should return both states when filter contains both firing and pending', () => { + expect(parseAlertstateFilter('alertstate=~"firing|pending"')).toEqual([ + PromAlertingRuleState.Firing, + PromAlertingRuleState.Pending, + ]); }); - it('should return null when no alertstate filter', () => { - expect(parseAlertstateFilter('')).toBe(null); - expect(parseAlertstateFilter('namespace="default"')).toBe(null); + it('should return both states when no alertstate filter', () => { + expect(parseAlertstateFilter('')).toEqual([PromAlertingRuleState.Firing, PromAlertingRuleState.Pending]); + expect(parseAlertstateFilter('namespace="default"')).toEqual([ + PromAlertingRuleState.Firing, + PromAlertingRuleState.Pending, + ]); }); it('should handle regex operator =~', () => { - expect(parseAlertstateFilter('alertstate=~"firing"')).toBe(PromAlertingRuleState.Firing); - expect(parseAlertstateFilter('alertstate=~"pending"')).toBe(PromAlertingRuleState.Pending); + expect(parseAlertstateFilter('alertstate=~"firing"')).toEqual([PromAlertingRuleState.Firing]); + expect(parseAlertstateFilter('alertstate=~"pending"')).toEqual([PromAlertingRuleState.Pending]); }); it('should handle whitespace', () => { - expect(parseAlertstateFilter('alertstate = "firing"')).toBe(PromAlertingRuleState.Firing); - expect(parseAlertstateFilter('alertstate =~ "pending"')).toBe(PromAlertingRuleState.Pending); + expect(parseAlertstateFilter('alertstate = "firing"')).toEqual([PromAlertingRuleState.Firing]); + expect(parseAlertstateFilter('alertstate =~ "pending"')).toEqual([PromAlertingRuleState.Pending]); }); }); @@ -58,7 +64,7 @@ describe('countRules', () => { { ruleUID: 'rule3', alertstate: PromAlertingRuleState.Pending }, ]); - const result = countRules(ruleDfv, null); + const result = countRules(ruleDfv, [PromAlertingRuleState.Firing, PromAlertingRuleState.Pending]); expect(result.firing).toBe(2); expect(result.pending).toBe(1); @@ -71,13 +77,13 @@ describe('countRules', () => { { ruleUID: 'rule3', alertstate: PromAlertingRuleState.Firing }, ]); - const result = countRules(ruleDfv, null); + const result = countRules(ruleDfv, [PromAlertingRuleState.Firing, PromAlertingRuleState.Pending]); expect(result.firing).toBe(1); expect(result.pending).toBe(2); }); - it('should count rules with BOTH firing and pending as firing (firing takes precedence)', () => { + it('should count rules with BOTH firing and pending in both counts', () => { const ruleDfv = createMockRuleDfv([ { ruleUID: 'rule1', alertstate: PromAlertingRuleState.Firing }, { ruleUID: 'rule1', alertstate: PromAlertingRuleState.Pending }, // Same rule, both states @@ -85,13 +91,13 @@ describe('countRules', () => { { ruleUID: 'rule3', alertstate: PromAlertingRuleState.Pending }, // Only pending ]); - const result = countRules(ruleDfv, null); + const result = countRules(ruleDfv, [PromAlertingRuleState.Firing, PromAlertingRuleState.Pending]); - // rule1 should be counted as firing (firing takes precedence) - // rule2 should be counted as firing - // rule3 should be counted as pending + // rule1 has both states, so counted in both + // rule2 counted only in firing + // rule3 counted only in pending expect(result.firing).toBe(2); // rule1 and rule2 - expect(result.pending).toBe(1); // only rule3 + expect(result.pending).toBe(2); // rule1 and rule3 }); it('should handle multiple instances of the same rule with same state', () => { @@ -102,7 +108,7 @@ describe('countRules', () => { { ruleUID: 'rule2', alertstate: PromAlertingRuleState.Pending }, // Same rule, duplicate entry ]); - const result = countRules(ruleDfv, null); + const result = countRules(ruleDfv, [PromAlertingRuleState.Firing, PromAlertingRuleState.Pending]); // Each rule should only be counted once despite multiple entries expect(result.firing).toBe(1); @@ -112,7 +118,7 @@ describe('countRules', () => { it('should return 0 for both counts when no rules', () => { const ruleDfv = createMockRuleDfv([]); - const result = countRules(ruleDfv, null); + const result = countRules(ruleDfv, [PromAlertingRuleState.Firing, PromAlertingRuleState.Pending]); expect(result.firing).toBe(0); expect(result.pending).toBe(0); @@ -128,7 +134,7 @@ describe('countRules', () => { { ruleUID: 'rule3', alertstate: PromAlertingRuleState.Firing }, // Only firing ]); - const result = countRules(ruleDfv, PromAlertingRuleState.Pending); + const result = countRules(ruleDfv, [PromAlertingRuleState.Pending]); // Should count rule1 and rule2 (both have pending instances) expect(result.pending).toBe(2); @@ -144,7 +150,7 @@ describe('countRules', () => { { ruleUID: 'rule3', alertstate: PromAlertingRuleState.Pending }, ]); - const result = countRules(ruleDfv, PromAlertingRuleState.Pending); + const result = countRules(ruleDfv, [PromAlertingRuleState.Pending]); // Should count all three rules (all have pending instances) expect(result.pending).toBe(3); @@ -158,14 +164,13 @@ describe('countRules', () => { { ruleUID: 'rule2', alertstate: PromAlertingRuleState.Pending }, ]); - const noFilterResult = countRules(ruleDfv, null); - const pendingFilterResult = countRules(ruleDfv, PromAlertingRuleState.Pending); + const noFilterResult = countRules(ruleDfv, [PromAlertingRuleState.Firing, PromAlertingRuleState.Pending]); + const pendingFilterResult = countRules(ruleDfv, [PromAlertingRuleState.Pending]); // With pending filter: should count rule1 and rule2 = 2 - // Without filter: should count only rule2 = 1 (rule1 is counted as firing, not pending) - expect(pendingFilterResult.pending).toBeGreaterThanOrEqual(noFilterResult.pending); + // Without filter: should also count both rules = 2 (both have pending instances) expect(pendingFilterResult.pending).toBe(2); - expect(noFilterResult.pending).toBe(1); // Only rule2 (rule1 is firing) + expect(noFilterResult.pending).toBe(2); // Both rule1 and rule2 have pending instances expect(noFilterResult.firing).toBe(1); // rule1 }); }); @@ -179,7 +184,7 @@ describe('countRules', () => { { ruleUID: 'rule3', alertstate: PromAlertingRuleState.Pending }, // Only pending ]); - const result = countRules(ruleDfv, PromAlertingRuleState.Firing); + const result = countRules(ruleDfv, [PromAlertingRuleState.Firing]); // Should count rule1 and rule2 (both have firing instances) expect(result.firing).toBe(2); @@ -208,18 +213,18 @@ describe('countRules', () => { { ruleUID: 'ruleE', alertstate: PromAlertingRuleState.Pending }, ]); - // No filter: Firing takes precedence - const noFilter = countRules(ruleDfv, null); - expect(noFilter.firing).toBe(3); // Rule A, C, and D (C has firing so it's counted as firing) - expect(noFilter.pending).toBe(2); // Rule B and E (only those with NO firing) + // No filter: Count all rules with at least one instance in each state + const noFilter = countRules(ruleDfv, [PromAlertingRuleState.Firing, PromAlertingRuleState.Pending]); + expect(noFilter.firing).toBe(3); // Rule A, C, and D (all have at least one firing instance) + expect(noFilter.pending).toBe(3); // Rule B, C, and E (all have at least one pending instance) // With pending filter: Count all rules with ANY pending instances - const pendingFilter = countRules(ruleDfv, PromAlertingRuleState.Pending); + const pendingFilter = countRules(ruleDfv, [PromAlertingRuleState.Pending]); expect(pendingFilter.pending).toBe(3); // Rule B, C, and E expect(pendingFilter.firing).toBe(0); // With firing filter: Count all rules with ANY firing instances - const firingFilter = countRules(ruleDfv, PromAlertingRuleState.Firing); + const firingFilter = countRules(ruleDfv, [PromAlertingRuleState.Firing]); expect(firingFilter.firing).toBe(3); // Rule A, C, and D expect(firingFilter.pending).toBe(0); }); diff --git a/public/app/features/alerting/unified/triage/scene/SummaryStats.tsx b/public/app/features/alerting/unified/triage/scene/SummaryStats.tsx index b26e9b9aa8f..3088d618c41 100644 --- a/public/app/features/alerting/unified/triage/scene/SummaryStats.tsx +++ b/public/app/features/alerting/unified/triage/scene/SummaryStats.tsx @@ -1,11 +1,12 @@ -import { DataFrameView } from '@grafana/data'; +import { css } from '@emotion/css'; + +import { DataFrameView, GrafanaTheme2 } from '@grafana/data'; import { Trans } from '@grafana/i18n'; import { SceneObjectBase, SceneObjectState } from '@grafana/scenes'; import { useQueryRunner } from '@grafana/scenes-react'; -import { ErrorBoundaryAlert, Stack, Text } from '@grafana/ui'; +import { Box, ErrorBoundaryAlert, Grid, useStyles2 } from '@grafana/ui'; import { PromAlertingRuleState } from 'app/types/unified-alerting-dto'; -import { Spacer } from '../../components/Spacer'; import { METRIC_NAME } from '../constants'; import { getDataQuery, useQueryFilter } from './utils'; @@ -25,63 +26,44 @@ export interface RuleFrame { Value: number; } -export function parseAlertstateFilter(filter: string): AlertState | null { - const firingMatch = filter.match(/alertstate\s*=~?\s*"firing"/); - const pendingMatch = filter.match(/alertstate\s*=~?\s*"pending"/); +export function parseAlertstateFilter(filter: string): AlertState[] { + const hasFiring = filter.match(/alertstate\s*=~?\s*"firing"/); + const hasPending = filter.match(/alertstate\s*=~?\s*"pending"/); - if (firingMatch && pendingMatch) { - return null; + const states: AlertState[] = []; + + // If both or neither match, include both states + if ((hasFiring && hasPending) || (!hasFiring && !hasPending)) { + return [PromAlertingRuleState.Firing, PromAlertingRuleState.Pending]; } - if (firingMatch) { - return PromAlertingRuleState.Firing; + if (hasFiring) { + states.push(PromAlertingRuleState.Firing); + } + if (hasPending) { + states.push(PromAlertingRuleState.Pending); } - if (pendingMatch) { - return PromAlertingRuleState.Pending; - } - - return null; + return states; } -export function countRules(ruleDfv: DataFrameView, alertstateFilter: AlertState | null) { - const rulesWithFiring = new Set(); - const rulesWithPending = new Set(); +export function countRules(ruleDfv: DataFrameView, alertstateFilter: AlertState[]) { + const counts = { + [PromAlertingRuleState.Firing]: new Set(), + [PromAlertingRuleState.Pending]: new Set(), + }; + // Only count rules for states we're interested in ruleDfv.fields.grafana_rule_uid.values.forEach((ruleUID, i) => { const alertstate = ruleDfv.fields.alertstate.values[i]; - if (alertstate === PromAlertingRuleState.Firing) { - rulesWithFiring.add(ruleUID); - } - if (alertstate === PromAlertingRuleState.Pending) { - rulesWithPending.add(ruleUID); + if (alertstateFilter.includes(alertstate)) { + counts[alertstate].add(ruleUID); } }); - // When filtering by pending, count all rules with pending instances (may also have firing) - if (alertstateFilter === PromAlertingRuleState.Pending) { - return { - firing: 0, - pending: rulesWithPending.size, - }; - } - - // When filtering by firing, count all rules with firing instances (may also have pending) - if (alertstateFilter === PromAlertingRuleState.Firing) { - return { - firing: rulesWithFiring.size, - pending: 0, - }; - } - - // When no filter: firing takes precedence - // A rule is "firing" if it has ANY firing instances (even if it also has pending) - // A rule is "pending" ONLY if it has pending instances but NO firing instances - const onlyPending = new Set([...rulesWithPending].filter((uid) => !rulesWithFiring.has(uid))); - return { - firing: rulesWithFiring.size, // ALL rules with firing instances - pending: onlyPending.size, // ONLY rules with no firing instances + firing: counts[PromAlertingRuleState.Firing].size, + pending: counts[PromAlertingRuleState.Pending].size, }; } @@ -93,23 +75,57 @@ function countInstances(instanceDfv: DataFrameView) { return { firing: getValue(PromAlertingRuleState.Firing), pending: getValue(PromAlertingRuleState.Pending) }; } -interface StatRowProps { +interface StatBoxProps { i18nKey: string; + value: number; color: 'error' | 'warning'; - values: Record; children: React.ReactNode; } -function StatRow({ i18nKey, color, values, children }: StatRowProps) { +function StatBox({ i18nKey, value, color, children }: StatBoxProps) { + const styles = useStyles2(getStatBoxStyles); + const colorClass = color === 'error' ? styles.errorColor : styles.warningColor; + return ( - - - {children} - - + +
{children}
+
{value}
+ ); } +const getStatBoxStyles = (theme: GrafanaTheme2) => ({ + label: css({ + fontSize: theme.typography.bodySmall.fontSize, + color: theme.colors.text.primary, + wordWrap: 'break-word', + whiteSpace: 'normal', + textAlign: 'center', + }), + value: css({ + fontSize: theme.typography.h1.fontSize, + fontWeight: theme.typography.fontWeightMedium, + lineHeight: 1.2, + textAlign: 'center', + }), + errorColor: css({ + color: theme.colors.error.text, + }), + warningColor: css({ + color: theme.colors.warning.text, + }), +}); + function SummaryStatsContent() { const filter = useQueryFilter(); const alertstateFilter = parseAlertstateFilter(filter); @@ -161,65 +177,28 @@ function SummaryStatsContent() { const rules = countRules(ruleDfv, alertstateFilter); return ( - - - {alertstateFilter === PromAlertingRuleState.Firing && ( - <> - - {'{{count}} firing alert rules'} - - - {'{{firingCount}} firing instances'} - - + + {alertstateFilter.includes(PromAlertingRuleState.Firing) && ( + + + Firing alert instances + + + Alert rules with firing instances + + )} - {alertstateFilter === PromAlertingRuleState.Pending && ( - <> - - {'{{count}} rules with pending instances'} - - - {'{{pendingCount}} pending instances'} - - + {alertstateFilter.includes(PromAlertingRuleState.Pending) && ( + + + Pending alert instances + + + Alert rules with pending instances + + )} - {!alertstateFilter && ( - <> - - {'{{count}} firing alert rules'} - - - {'{{firingCount}} firing instances'} - - - {'{{count}} pending alert rules'} - - - {'{{pendingCount}} pending instances'} - - - )} - + ); } diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 4ca17a2d075..6e89681bd14 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -2945,6 +2945,7 @@ "triage": { "alert-instances": "Alert instances", "error-loading-rule": "Error loading rule", + "firing-instances-count": "Firing alert instances", "instance-details-drawer": { "instance-details": "Instance details" }, @@ -2954,6 +2955,7 @@ "no-matching-instances-with-filters": "No alert instances match your current set of filters for the selected time range.", "open-in-sidebar": "Open in sidebar", "open-rule-details": "Open rule details", + "pending-instances-count": "Pending alert instances", "rule-details": { "subtitle": "Rule details and conditions", "title": "Rule Details" @@ -2961,7 +2963,9 @@ "rule-not-found": { "description": "The requested rule could not be found.", "title": "Rule not found" - } + }, + "rules-with-firing-instances": "Alert rules with firing instances", + "rules-with-pending-instances": "Alert rules with pending instances" }, "type-selector-button": { "add-expression": "Add expression"