From d9fe8a0f0526723af0c4930f46ce2a348ad5e197 Mon Sep 17 00:00:00 2001 From: rioloc Date: Tue, 21 Apr 2026 22:06:22 +0200 Subject: [PATCH 1/2] feat(incidents): add absolute start dates, severity splitting, and alert gap detection Decouple the fetch window (always 15 days) from the display window (user-selected N days) so that firstTimestamp reflects the true incident/alert start time regardless of the currently visible time range. Split incidents when severity changes between consecutive timestamps and split alerts when gaps exceed the 300s Prometheus scrape interval. Co-Authored-By: Claude Opus 4.6 --- .../Incidents/AlertsChart/AlertsChart.tsx | 15 +- .../IncidentsChart/IncidentsChart.tsx | 29 +- .../Incidents/IncidentsDetailsRowTable.tsx | 21 +- .../components/Incidents/IncidentsPage.tsx | 105 +-- .../components/Incidents/IncidentsTable.tsx | 6 +- web/src/components/Incidents/model.ts | 6 +- .../Incidents/processAlerts.spec.ts | 720 +++++++++++++++++- web/src/components/Incidents/processAlerts.ts | 76 +- .../Incidents/processIncidents.spec.ts | 535 ++++++++++++- .../components/Incidents/processIncidents.ts | 181 ++++- web/src/components/Incidents/utils.spec.ts | 315 +++++++- web/src/components/Incidents/utils.ts | 189 +++-- 12 files changed, 1967 insertions(+), 231 deletions(-) diff --git a/web/src/components/Incidents/AlertsChart/AlertsChart.tsx b/web/src/components/Incidents/AlertsChart/AlertsChart.tsx index 1705b2e14..b5ff988fc 100644 --- a/web/src/components/Incidents/AlertsChart/AlertsChart.tsx +++ b/web/src/components/Incidents/AlertsChart/AlertsChart.tsx @@ -73,7 +73,18 @@ const AlertsChart = ({ theme }: { theme: 'light' | 'dark' }) => { const chartData: AlertsChartBar[][] = useMemo(() => { if (!Array.isArray(alertsData) || alertsData.length === 0) return []; - return alertsData.map((alert) => createAlertsChartBars(alert)); + + // Group alerts by identity so intervals of the same alert share the same row + const groupedByIdentity = new Map(); + for (const alert of alertsData) { + const key = [alert.alertname, alert.namespace, alert.severity].join('|'); + if (!groupedByIdentity.has(key)) { + groupedByIdentity.set(key, []); + } + groupedByIdentity.get(key)!.push(alert); + } + + return Array.from(groupedByIdentity.values()).map((alerts) => createAlertsChartBars(alerts)); }, [alertsData]); useEffect(() => { @@ -161,7 +172,7 @@ const AlertsChart = ({ theme }: { theme: 'light' | 'dark' }) => { if (datum.nodata) { return ''; } - const startDate = dateTimeFormatter(i18n.language).format(new Date(datum.y0)); + const startDate = dateTimeFormatter(i18n.language).format(datum.startDate); const endDate = datum.alertstate === 'firing' ? '---' diff --git a/web/src/components/Incidents/IncidentsChart/IncidentsChart.tsx b/web/src/components/Incidents/IncidentsChart/IncidentsChart.tsx index 51c7a52a8..b48b65743 100644 --- a/web/src/components/Incidents/IncidentsChart/IncidentsChart.tsx +++ b/web/src/components/Incidents/IncidentsChart/IncidentsChart.tsx @@ -33,6 +33,7 @@ import { calculateIncidentsChartDomain, createIncidentsChartBars, generateDateArray, + removeTrailingPaddingFromSeveritySegments, roundDateToInterval, } from '../utils'; import { dateTimeFormatter, timeFormatter } from '../../console/utils/datetime'; @@ -89,10 +90,29 @@ const IncidentsChart = ({ ? incidentsData.filter((incident) => incident.group_id === selectedGroupId) : incidentsData; - // Create chart bars and sort by original x values to maintain proper order - const chartBars = filteredIncidents.map((incident) => - createIncidentsChartBars(incident, dateValues), + // Group incidents by group_id so split severity segments share the same row + const incidentsByGroupId = new Map(); + for (const incident of filteredIncidents) { + const existing = incidentsByGroupId.get(incident.group_id); + if (existing) { + existing.push(incident); + } else { + incidentsByGroupId.set(incident.group_id, [incident]); + } + } + + // When an incident changes severity, its segments share the same row. + // Non-last segments have trailing padding (+300s) that overlaps with the + // next segment's leading padding (-300s). Remove the trailing padding + // value from non-last segments to prevent visual overlap. + const adjustedGroups = Array.from(incidentsByGroupId.values()).map((group) => + removeTrailingPaddingFromSeveritySegments(group), ); + + // Create chart bars per group and sort by original x values + const chartBars = adjustedGroups + .map((group) => createIncidentsChartBars(group, dateValues)) + .filter((bars) => bars.length > 0); chartBars.sort((a, b) => a[0].x - b[0].x); // Reassign consecutive x values to eliminate gaps between bars @@ -102,7 +122,6 @@ const IncidentsChart = ({ useEffect(() => { setIsLoading(false); }, [incidentsData]); - useEffect(() => { setChartContainerHeight(chartData?.length < 5 ? 300 : chartData?.length * 60); setChartHeight(chartData?.length < 5 ? 250 : chartData?.length * 55); @@ -176,7 +195,7 @@ const IncidentsChart = ({ if (datum.nodata) { return ''; } - const startDate = dateTimeFormatter(i18n.language).format(new Date(datum.y0)); + const startDate = dateTimeFormatter(i18n.language).format(datum.startDate); const endDate = datum.firing ? '---' : dateTimeFormatter(i18n.language).format( diff --git a/web/src/components/Incidents/IncidentsDetailsRowTable.tsx b/web/src/components/Incidents/IncidentsDetailsRowTable.tsx index 1379288e5..a0313746f 100644 --- a/web/src/components/Incidents/IncidentsDetailsRowTable.tsx +++ b/web/src/components/Incidents/IncidentsDetailsRowTable.tsx @@ -24,10 +24,11 @@ const IncidentsDetailsRowTable = ({ alerts }: IncidentsDetailsRowTableProps) => const sortedAndMappedAlerts = useMemo(() => { if (alerts && alerts.length > 0) { return [...alerts] - .sort( - (a: IncidentsDetailsAlert, b: IncidentsDetailsAlert) => - a.alertsStartFiring - b.alertsStartFiring, - ) + .sort((a: IncidentsDetailsAlert, b: IncidentsDetailsAlert) => { + const aStart = a.firstTimestamp > 0 ? a.firstTimestamp : a.alertsStartFiring; + const bStart = b.firstTimestamp > 0 ? b.firstTimestamp : b.alertsStartFiring; + return aStart - bStart; + }) .map((alertDetails: IncidentsDetailsAlert, rowIndex) => { return ( @@ -45,13 +46,21 @@ const IncidentsDetailsRowTable = ({ alerts }: IncidentsDetailsRowTableProps) => - + 0 + ? alertDetails.firstTimestamp + : alertDetails.alertsStartFiring) * 1000, + ).toISOString()} + /> {!alertDetails.resolved ? ( '---' ) : ( - + )} diff --git a/web/src/components/Incidents/IncidentsPage.tsx b/web/src/components/Incidents/IncidentsPage.tsx index c093365df..4d50343ec 100644 --- a/web/src/components/Incidents/IncidentsPage.tsx +++ b/web/src/components/Incidents/IncidentsPage.tsx @@ -39,6 +39,7 @@ import { onIncidentFiltersSelect, parseUrlParams, updateBrowserUrl, + DAY_MS, } from './utils'; import { groupAlertsForTable, convertToAlerts } from './processAlerts'; import { CompressArrowsAltIcon, CompressIcon, FilterIcon } from '@patternfly/react-icons'; @@ -231,52 +232,54 @@ const IncidentsPage = () => { }, [incidentsActiveFilters.days]); useEffect(() => { - (async () => { - const currentTime = incidentsLastRefreshTime; - Promise.all( - timeRanges.map(async (range) => { - const response = await fetchDataForIncidentsAndAlerts( - safeFetch, - range, - createAlertsQuery(incidentForAlertProcessing), - ); - return response.data.result; - }), - ) - .then((results) => { - const prometheusResults = results.flat(); - const alerts = convertToAlerts( - prometheusResults, - incidentForAlertProcessing, - currentTime, - ); + if (incidentForAlertProcessing.length === 0) { + return; + } + + const currentTime = incidentsLastRefreshTime; + + // Always fetch 15 days of alert data so firstTimestamp is computed from full history + const fetchTimeRanges = getIncidentsTimeRanges(15 * DAY_MS, currentTime); + + Promise.all( + fetchTimeRanges.map(async (range) => { + const response = await fetchDataForIncidentsAndAlerts( + safeFetch, + range, + createAlertsQuery(incidentForAlertProcessing), + ); + return response.data.result; + }), + ) + .then((alertsResults) => { + const prometheusResults = alertsResults.flat(); + const alerts = convertToAlerts( + prometheusResults, + incidentForAlertProcessing, + currentTime, + daysSpan, + ); + dispatch( + setAlertsData({ + alertsData: alerts, + }), + ); + if (rules && alerts) { dispatch( - setAlertsData({ - alertsData: alerts, + setAlertsTableData({ + alertsTableData: groupAlertsForTable(alerts, rules), }), ); - if (rules && alerts) { - dispatch( - setAlertsTableData({ - alertsTableData: groupAlertsForTable(alerts, rules), - }), - ); - } - if (!isEmpty(filteredData)) { - dispatch(setAlertsAreLoading({ alertsAreLoading: false })); - } else { - dispatch(setAlertsAreLoading({ alertsAreLoading: true })); - } - }) - .catch((err) => { - // eslint-disable-next-line no-console - console.log(err); - - dispatch(setAlertsAreLoading({ alertsAreLoading: false })); - setLoadError(err); - }); - })(); - }, [incidentForAlertProcessing]); + } + dispatch(setAlertsAreLoading({ alertsAreLoading: false })); + }) + .catch((err) => { + // eslint-disable-next-line no-console + console.error(err); + dispatch(setAlertsAreLoading({ alertsAreLoading: false })); + setLoadError(err); + }); + }, [incidentForAlertProcessing, rules, daysSpan]); useEffect(() => { if (!isInitialized) return; @@ -293,30 +296,34 @@ const IncidentsPage = () => { ? incidentsActiveFilters.days[0].split(' ')[0] + 'd' : '', ); - const calculatedTimeRanges = getIncidentsTimeRanges(daysDuration, currentTime); const isGroupSelected = !!selectedGroupId; const incidentsQuery = isGroupSelected ? `cluster_health_components_map{group_id='${selectedGroupId}'}` : 'cluster_health_components_map'; + // Always fetch 15 days of data so firstTimestamp is computed from full history + const fetchTimeRanges = getIncidentsTimeRanges(15 * DAY_MS, currentTime); + Promise.all( - calculatedTimeRanges.map(async (range) => { + fetchTimeRanges.map(async (range) => { const response = await fetchDataForIncidentsAndAlerts(safeFetch, range, incidentsQuery); return response.data.result; }), ) - .then((results) => { - const prometheusResults = results.flat(); - const incidents = convertToIncidents(prometheusResults, currentTime); + .then((incidentsResults) => { + const prometheusResults = incidentsResults.flat(); + const incidents = convertToIncidents(prometheusResults, currentTime, daysDuration); // Update the raw, unfiltered incidents state dispatch(setIncidents({ incidents })); + const filteredData = filterIncident(incidentsActiveFilters, incidents); + // Filter the incidents and dispatch dispatch( setFilteredIncidentsData({ - filteredIncidentsData: filterIncident(incidentsActiveFilters, incidents), + filteredIncidentsData: filteredData, }), ); diff --git a/web/src/components/Incidents/IncidentsTable.tsx b/web/src/components/Incidents/IncidentsTable.tsx index 7f8f635e8..effaf10dd 100644 --- a/web/src/components/Incidents/IncidentsTable.tsx +++ b/web/src/components/Incidents/IncidentsTable.tsx @@ -95,7 +95,7 @@ export const IncidentsTable = () => { if (!alert.alertsExpandedRowData || alert.alertsExpandedRowData.length === 0) { return 0; } - return Math.min(...alert.alertsExpandedRowData.map((alertData) => alertData.alertsStartFiring)); + return Math.min(...alert.alertsExpandedRowData.map((alertData) => alertData.firstTimestamp)); }; if (isEmpty(alertsTableData) || alertsAreLoading || isEmpty(incidentsActiveFilters.groupId)) { @@ -180,7 +180,9 @@ export const IncidentsTable = () => { )} - + ; -export type AlertsIntervalsArray = [number, number, 'data' | 'nodata']; +export type AlertsIntervalsArray = [number, number, 'data' | 'nodata', number?]; export type Incident = { component: string; @@ -15,10 +15,12 @@ export type Incident = { src_severity: string; src_alertname: string; src_namespace: string; + severity: any; silenced: boolean; x: number; values: Array; metric: Metric; + firstTimestamp: number; }; // Define the interface for Metric @@ -47,6 +49,7 @@ export type Alert = { severity: Severity; silenced: boolean; x: number; + firstTimestamp: number; values: Array; alertsExpandedRowData?: Array; }; @@ -101,6 +104,7 @@ export type IncidentsDetailsAlert = { resolved: boolean; severity: Severity; x: number; + firstTimestamp: number; values: Array; silenced: boolean; rule: { diff --git a/web/src/components/Incidents/processAlerts.spec.ts b/web/src/components/Incidents/processAlerts.spec.ts index 8c424a72d..87fed2695 100644 --- a/web/src/components/Incidents/processAlerts.spec.ts +++ b/web/src/components/Incidents/processAlerts.spec.ts @@ -1,15 +1,16 @@ import { PrometheusResult } from '@openshift-console/dynamic-plugin-sdk'; import { convertToAlerts, deduplicateAlerts } from './processAlerts'; import { Incident } from './model'; -import { getCurrentTime } from './utils'; +import { getCurrentTime, DAY_MS } from './utils'; describe('convertToAlerts', () => { const now = getCurrentTime(); const nowSeconds = Math.floor(now / 1000); + const daysSpanMs = 15 * DAY_MS; describe('edge cases', () => { it('should return empty array when no prometheus results provided', () => { - const result = convertToAlerts([], [], now); + const result = convertToAlerts([], [], now, daysSpanMs); expect(result).toEqual([]); }); @@ -28,7 +29,7 @@ describe('convertToAlerts', () => { values: [[nowSeconds, '1']], }, ]; - const result = convertToAlerts(prometheusResults, [], now); + const result = convertToAlerts(prometheusResults, [], now, daysSpanMs); expect(result).toEqual([]); }); @@ -69,7 +70,7 @@ describe('convertToAlerts', () => { }, ]; - const result = convertToAlerts(prometheusResults, incidents, now); + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); expect(result).toHaveLength(1); expect(result[0].alertname).toBe('ClusterOperatorDegraded'); }); @@ -78,7 +79,8 @@ describe('convertToAlerts', () => { describe('time window filtering', () => { it('should filter alerts to incident time window with 30s padding', () => { const incidentStart = nowSeconds - 3600; // 1 hour ago - const incidentEnd = nowSeconds - 1800; // 30 minutes ago + // Use timestamps within 300s of each other to avoid gap-splitting + const incidentEnd = incidentStart + 300; const prometheusResults: PrometheusResult[] = [ { @@ -92,10 +94,9 @@ describe('convertToAlerts', () => { alertstate: 'firing', }, values: [ - [incidentStart - 100, '2'], // Outside window (before) + [incidentStart - 500, '2'], // Outside window (before, and gap-split away) [incidentStart, '2'], // Inside window [incidentEnd, '2'], // Inside window - [incidentEnd + 100, '2'], // Outside window (after) ], }, ]; @@ -113,10 +114,9 @@ describe('convertToAlerts', () => { }, ]; - const result = convertToAlerts(prometheusResults, incidents, now); + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); + // The first value is gap-split away (500s gap > 300s threshold), leaving one interval expect(result).toHaveLength(1); - // Should include values within incident time + 30s padding - // Plus padding points added by insertPaddingPointsForChart expect(result[0].values.length).toBeGreaterThan(0); }); @@ -148,7 +148,7 @@ describe('convertToAlerts', () => { }, ]; - const result = convertToAlerts(prometheusResults, incidents, now); + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); expect(result).toEqual([]); }); }); @@ -182,7 +182,7 @@ describe('convertToAlerts', () => { }, ]; - const result = convertToAlerts(prometheusResults, incidents, now); + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); expect(result).toHaveLength(1); // Verify resolved is determined from ORIGINAL values (before padding) @@ -226,7 +226,7 @@ describe('convertToAlerts', () => { }, ]; - const result = convertToAlerts(prometheusResults, incidents, now); + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); expect(result).toHaveLength(1); expect(result[0].alertsStartFiring).toBeGreaterThan(0); expect(result[0].alertsEndFiring).toBeGreaterThan(0); @@ -268,16 +268,16 @@ describe('convertToAlerts', () => { }, ]; - const result = convertToAlerts(prometheusResults, incidents, now); + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); expect(result).toHaveLength(1); expect(result[0].alertstate).toBe('resolved'); expect(result[0].resolved).toBe(true); }); it('should mark alert as firing if ended less than 10 minutes ago', () => { - const recentTimestamp = nowSeconds - 840; // 14 minutes ago - // After padding (+300s), last timestamp will be 9 minutes ago (840-300=540s ago) - // which is < 10 minutes, so it should still be firing + const recentTimestamp = nowSeconds - 540; // 9 minutes ago + // Resolved check is done on original timestamp (before padding) + // 9 minutes ago is < 10 minutes, so it should still be firing const prometheusResults: PrometheusResult[] = [ { @@ -304,7 +304,7 @@ describe('convertToAlerts', () => { }, ]; - const result = convertToAlerts(prometheusResults, incidents, now); + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); expect(result).toHaveLength(1); expect(result[0].alertstate).toBe('firing'); expect(result[0].resolved).toBe(false); @@ -357,7 +357,7 @@ describe('convertToAlerts', () => { }, ]; - const result = convertToAlerts(prometheusResults, incidents, now); + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); expect(result).toHaveLength(2); expect(result[0].alertname).toBe('Alert1'); // Earlier alert first expect(result[1].alertname).toBe('Alert2'); @@ -408,7 +408,7 @@ describe('convertToAlerts', () => { }, ]; - const result = convertToAlerts(prometheusResults, incidents, now); + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); expect(result).toHaveLength(2); expect(result[0].x).toBe(2); // Earliest alert has highest x expect(result[1].x).toBe(1); // Latest alert has lowest x @@ -443,7 +443,7 @@ describe('convertToAlerts', () => { }, ]; - const result = convertToAlerts(prometheusResults, incidents, now); + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); expect(result).toHaveLength(1); expect(result[0].silenced).toBe(true); }); @@ -451,6 +451,10 @@ describe('convertToAlerts', () => { describe('incident merging', () => { it('should merge duplicate incidents by composite key', () => { + // Use timestamps within 300s to avoid gap-splitting in deduplicateAlerts + const t1 = nowSeconds - 600; + const t2 = t1 + 300; + const prometheusResults: PrometheusResult[] = [ { metric: { @@ -461,8 +465,8 @@ describe('convertToAlerts', () => { alertstate: 'firing', }, values: [ - [nowSeconds - 600, '2'], - [nowSeconds, '2'], + [t1, '2'], + [t2, '2'], ], }, ]; @@ -477,7 +481,7 @@ describe('convertToAlerts', () => { component: 'test-component', layer: 'test-layer', silenced: false, - values: [[nowSeconds - 600, '2']], + values: [[t1, '2']], }, { group_id: 'incident1', @@ -485,11 +489,11 @@ describe('convertToAlerts', () => { src_namespace: 'test-namespace', src_severity: 'critical', silenced: true, // Latest should be true - values: [[nowSeconds, '2']], + values: [[t2, '2']], }, ]; - const result = convertToAlerts(prometheusResults, incidents, now); + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); expect(result).toHaveLength(1); // Should use the silenced value from the latest timestamp expect(result[0].silenced).toBe(true); @@ -523,7 +527,7 @@ describe('convertToAlerts', () => { }, ]; - const result = convertToAlerts(prometheusResults, incidents, now); + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); expect(result).toHaveLength(1); expect(result[0].alertname).toBe('MyAlert'); expect(result[0].namespace).toBe('my-namespace'); @@ -533,6 +537,388 @@ describe('convertToAlerts', () => { expect(result[0].name).toBe('my-name'); }); }); + + describe('gap splitting through full pipeline', () => { + it('should produce multiple entries for same alert when values have gap > 300s', () => { + const t1 = nowSeconds - 3600; + const prometheusResults: PrometheusResult[] = [ + { + metric: { + alertname: 'GapAlert', + namespace: 'test-ns', + severity: 'warning', + component: 'test-comp', + name: 'test', + alertstate: 'firing', + }, + values: [ + [t1, '1'], + [t1 + 300, '1'], + [t1 + 900, '1'], // 600s gap from t1+300 (> 300s threshold) + [t1 + 1200, '1'], + ], + }, + ]; + + const incidents: Array> = [ + { + group_id: 'incident1', + src_alertname: 'GapAlert', + src_namespace: 'test-ns', + src_severity: 'warning', + component: 'test-comp', + layer: 'test-layer', + values: [ + [t1, '1'], + [t1 + 1200, '1'], + ], + }, + ]; + + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); + expect(result).toHaveLength(2); + expect(result[0].alertname).toBe('GapAlert'); + expect(result[1].alertname).toBe('GapAlert'); + }); + + it('should compute per-interval firstTimestamp for gap-split alerts', () => { + const t1 = nowSeconds - 3600; + const prometheusResults: PrometheusResult[] = [ + { + metric: { + alertname: 'GapAlert', + namespace: 'test-ns', + severity: 'warning', + component: 'test-comp', + name: 'test', + alertstate: 'firing', + }, + values: [ + [t1, '1'], + [t1 + 300, '1'], + [t1 + 900, '1'], + [t1 + 1200, '1'], + ], + }, + ]; + + const incidents: Array> = [ + { + group_id: 'incident1', + src_alertname: 'GapAlert', + src_namespace: 'test-ns', + src_severity: 'warning', + component: 'test-comp', + layer: 'test-layer', + values: [ + [t1, '1'], + [t1 + 1200, '1'], + ], + }, + ]; + + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); + expect(result).toHaveLength(2); + expect(result[0].firstTimestamp).toBe(t1 - 300); + expect(result[1].firstTimestamp).toBe(t1 + 900 - 300); + }); + + it('should set correct alertsStartFiring for each gap-split interval', () => { + const t1 = nowSeconds - 3600; + const prometheusResults: PrometheusResult[] = [ + { + metric: { + alertname: 'GapAlert', + namespace: 'test-ns', + severity: 'warning', + component: 'test-comp', + name: 'test', + alertstate: 'firing', + }, + values: [ + [t1, '1'], + [t1 + 300, '1'], + [t1 + 900, '1'], + [t1 + 1200, '1'], + ], + }, + ]; + + const incidents: Array> = [ + { + group_id: 'incident1', + src_alertname: 'GapAlert', + src_namespace: 'test-ns', + src_severity: 'warning', + component: 'test-comp', + layer: 'test-layer', + values: [ + [t1, '1'], + [t1 + 1200, '1'], + ], + }, + ]; + + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); + expect(result).toHaveLength(2); + expect(result[0].alertsStartFiring).toBe(t1 - 300); + expect(result[1].alertsStartFiring).toBe(t1 + 900 - 300); + }); + }); + + describe('multiple severity alerts within one incident', () => { + it('should return all alerts with different severities matching the same incident', () => { + const t1 = nowSeconds - 3600; + const t2 = nowSeconds - 2400; + const t3 = nowSeconds - 1200; + + const prometheusResults: PrometheusResult[] = [ + { + metric: { + alertname: 'SevAlert_warn', + namespace: 'test-ns', + severity: 'warning', + name: 'test', + alertstate: 'firing', + }, + values: [[t1, '1']], + }, + { + metric: { + alertname: 'SevAlert_crit', + namespace: 'test-ns', + severity: 'critical', + name: 'test', + alertstate: 'firing', + }, + values: [[t2, '2']], + }, + { + metric: { + alertname: 'SevAlert_info', + namespace: 'test-ns', + severity: 'info', + name: 'test', + alertstate: 'firing', + }, + values: [[t3, '0']], + }, + ]; + + const incidents: Array> = [ + { + group_id: 'incident1', + src_alertname: 'SevAlert_warn', + src_namespace: 'test-ns', + src_severity: 'warning', + component: 'test-comp', + layer: 'test-layer', + values: [ + [t1, '1'], + [t3, '1'], + ], + }, + { + group_id: 'incident1', + src_alertname: 'SevAlert_crit', + src_namespace: 'test-ns', + src_severity: 'critical', + component: 'test-comp', + layer: 'test-layer', + values: [ + [t1, '2'], + [t3, '2'], + ], + }, + { + group_id: 'incident1', + src_alertname: 'SevAlert_info', + src_namespace: 'test-ns', + src_severity: 'info', + component: 'test-comp', + layer: 'test-layer', + values: [ + [t1, '0'], + [t3, '0'], + ], + }, + ]; + + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); + expect(result).toHaveLength(3); + }); + + it('should sort multiple severity alerts by start time', () => { + const t1 = nowSeconds - 3600; + const t2 = nowSeconds - 2400; + const t3 = nowSeconds - 1200; + + const prometheusResults: PrometheusResult[] = [ + { + metric: { + alertname: 'SevAlert_warn', + namespace: 'test-ns', + severity: 'warning', + name: 'test', + alertstate: 'firing', + }, + values: [[t1, '1']], + }, + { + metric: { + alertname: 'SevAlert_crit', + namespace: 'test-ns', + severity: 'critical', + name: 'test', + alertstate: 'firing', + }, + values: [[t2, '2']], + }, + { + metric: { + alertname: 'SevAlert_info', + namespace: 'test-ns', + severity: 'info', + name: 'test', + alertstate: 'firing', + }, + values: [[t3, '0']], + }, + ]; + + const incidents: Array> = [ + { + group_id: 'incident1', + src_alertname: 'SevAlert_warn', + src_namespace: 'test-ns', + src_severity: 'warning', + component: 'test-comp', + layer: 'test-layer', + values: [ + [t1, '1'], + [t3, '1'], + ], + }, + { + group_id: 'incident1', + src_alertname: 'SevAlert_crit', + src_namespace: 'test-ns', + src_severity: 'critical', + component: 'test-comp', + layer: 'test-layer', + values: [ + [t1, '2'], + [t3, '2'], + ], + }, + { + group_id: 'incident1', + src_alertname: 'SevAlert_info', + src_namespace: 'test-ns', + src_severity: 'info', + component: 'test-comp', + layer: 'test-layer', + values: [ + [t1, '0'], + [t3, '0'], + ], + }, + ]; + + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); + expect(result).toHaveLength(3); + expect(result[0].alertname).toBe('SevAlert_warn'); + expect(result[1].alertname).toBe('SevAlert_crit'); + expect(result[2].alertname).toBe('SevAlert_info'); + }); + + it('should assign correct severity to each alert', () => { + const t1 = nowSeconds - 3600; + const t2 = nowSeconds - 2400; + const t3 = nowSeconds - 1200; + + const prometheusResults: PrometheusResult[] = [ + { + metric: { + alertname: 'SevAlert_warn', + namespace: 'test-ns', + severity: 'warning', + name: 'test', + alertstate: 'firing', + }, + values: [[t1, '1']], + }, + { + metric: { + alertname: 'SevAlert_crit', + namespace: 'test-ns', + severity: 'critical', + name: 'test', + alertstate: 'firing', + }, + values: [[t2, '2']], + }, + { + metric: { + alertname: 'SevAlert_info', + namespace: 'test-ns', + severity: 'info', + name: 'test', + alertstate: 'firing', + }, + values: [[t3, '0']], + }, + ]; + + const incidents: Array> = [ + { + group_id: 'incident1', + src_alertname: 'SevAlert_warn', + src_namespace: 'test-ns', + src_severity: 'warning', + component: 'test-comp', + layer: 'test-layer', + values: [ + [t1, '1'], + [t3, '1'], + ], + }, + { + group_id: 'incident1', + src_alertname: 'SevAlert_crit', + src_namespace: 'test-ns', + src_severity: 'critical', + component: 'test-comp', + layer: 'test-layer', + values: [ + [t1, '2'], + [t3, '2'], + ], + }, + { + group_id: 'incident1', + src_alertname: 'SevAlert_info', + src_namespace: 'test-ns', + src_severity: 'info', + component: 'test-comp', + layer: 'test-layer', + values: [ + [t1, '0'], + [t3, '0'], + ], + }, + ]; + + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); + expect(result).toHaveLength(3); + const warnAlert = result.find((a) => a.alertname === 'SevAlert_warn'); + const critAlert = result.find((a) => a.alertname === 'SevAlert_crit'); + const infoAlert = result.find((a) => a.alertname === 'SevAlert_info'); + expect(warnAlert.severity).toBe('warning'); + expect(critAlert.severity).toBe('critical'); + expect(infoAlert.severity).toBe('info'); + }); + }); }); describe('deduplicateAlerts', () => { @@ -724,7 +1110,7 @@ describe('deduplicateAlerts', () => { expect(result).toEqual([]); }); - it('should handle single alert', () => { + it('should handle single alert with single value', () => { const alerts: PrometheusResult[] = [ { metric: { @@ -740,7 +1126,283 @@ describe('deduplicateAlerts', () => { const result = deduplicateAlerts(alerts); expect(result).toHaveLength(1); - expect(result[0]).toEqual(alerts[0]); + expect(result[0].metric).toEqual(alerts[0].metric); + expect(result[0].values).toEqual(alerts[0].values); + }); + }); + + describe('gap splitting', () => { + it('should split alert into two when gap exceeds 5 minutes', () => { + const alerts: PrometheusResult[] = [ + { + metric: { + alertname: 'Alert1', + namespace: 'ns1', + component: 'comp1', + severity: 'critical', + alertstate: 'firing', + }, + values: [ + [1000, '2'], + [1300, '2'], // 300s after first (no gap) + [2000, '2'], // 700s after second (gap > 300s) + [2300, '2'], // 300s after third (no gap) + ], + }, + ]; + + const result = deduplicateAlerts(alerts); + expect(result).toHaveLength(2); + expect(result[0].values).toEqual([ + [1000, '2'], + [1300, '2'], + ]); + expect(result[1].values).toEqual([ + [2000, '2'], + [2300, '2'], + ]); + // Both entries share the same alert identity + expect(result[0].metric.alertname).toBe('Alert1'); + expect(result[1].metric.alertname).toBe('Alert1'); + }); + + it('should split alert into multiple intervals with multiple gaps', () => { + const alerts: PrometheusResult[] = [ + { + metric: { + alertname: 'Alert1', + namespace: 'ns1', + component: 'comp1', + severity: 'critical', + alertstate: 'firing', + }, + values: [ + [1000, '2'], + [1300, '2'], + [2000, '2'], // gap + [3000, '2'], // gap + [3300, '2'], + ], + }, + ]; + + const result = deduplicateAlerts(alerts); + expect(result).toHaveLength(3); + expect(result[0].values).toEqual([ + [1000, '2'], + [1300, '2'], + ]); + expect(result[1].values).toEqual([[2000, '2']]); + expect(result[2].values).toEqual([ + [3000, '2'], + [3300, '2'], + ]); + }); + + it('should not split when delta is exactly 300s (no gap)', () => { + const alerts: PrometheusResult[] = [ + { + metric: { + alertname: 'Alert1', + namespace: 'ns1', + component: 'comp1', + severity: 'critical', + alertstate: 'firing', + }, + values: [ + [1000, '2'], + [1300, '2'], // exactly 300s + [1600, '2'], // exactly 300s + ], + }, + ]; + + const result = deduplicateAlerts(alerts); + expect(result).toHaveLength(1); + expect(result[0].values).toHaveLength(3); + }); + + it('should split when delta is 301s (just over threshold)', () => { + const alerts: PrometheusResult[] = [ + { + metric: { + alertname: 'Alert1', + namespace: 'ns1', + component: 'comp1', + severity: 'critical', + alertstate: 'firing', + }, + values: [ + [1000, '2'], + [1301, '2'], // 301s gap + ], + }, + ]; + + const result = deduplicateAlerts(alerts); + expect(result).toHaveLength(2); + expect(result[0].values).toEqual([[1000, '2']]); + expect(result[1].values).toEqual([[1301, '2']]); + }); + + it('should split after merging values from multiple alerts with same key', () => { + const alerts: PrometheusResult[] = [ + { + metric: { + alertname: 'Alert1', + namespace: 'ns1', + component: 'comp1', + severity: 'critical', + alertstate: 'firing', + }, + values: [ + [1000, '2'], + [1300, '2'], + ], + }, + { + metric: { + alertname: 'Alert1', + namespace: 'ns1', + component: 'comp1', + severity: 'critical', + alertstate: 'firing', + }, + values: [ + [2000, '2'], // gap after merge + [2300, '2'], + ], + }, + ]; + + const result = deduplicateAlerts(alerts); + expect(result).toHaveLength(2); + expect(result[0].values).toEqual([ + [1000, '2'], + [1300, '2'], + ]); + expect(result[1].values).toEqual([ + [2000, '2'], + [2300, '2'], + ]); + }); + + it('should sort values by timestamp before gap detection', () => { + const alerts: PrometheusResult[] = [ + { + metric: { + alertname: 'Alert1', + namespace: 'ns1', + component: 'comp1', + severity: 'critical', + alertstate: 'firing', + }, + values: [ + [2300, '2'], + [1000, '2'], // out of order + [2000, '2'], + [1300, '2'], + ], + }, + ]; + + const result = deduplicateAlerts(alerts); + expect(result).toHaveLength(2); + // Values should be sorted within each interval + expect(result[0].values).toEqual([ + [1000, '2'], + [1300, '2'], + ]); + expect(result[1].values).toEqual([ + [2000, '2'], + [2300, '2'], + ]); }); }); }); + +describe('firstTimestamp from data', () => { + const now = getCurrentTime(); + const nowSeconds = Math.floor(now / 1000); + const daysSpanMs = 15 * DAY_MS; + + it('should use first value of deduplicated interval minus padding offset as firstTimestamp', () => { + const alertStart = nowSeconds - 3600; // 1 hour ago + + const prometheusResults: PrometheusResult[] = [ + { + metric: { + alertname: 'TestAlert', + namespace: 'test-namespace', + severity: 'critical', + name: 'test', + alertstate: 'firing', + }, + values: [ + [alertStart, '2'], + [alertStart + 300, '2'], + [alertStart + 600, '2'], + ], + }, + ]; + + const incidents: Array> = [ + { + group_id: 'incident1', + src_alertname: 'TestAlert', + src_namespace: 'test-namespace', + src_severity: 'critical', + component: 'test-component', + layer: 'test-layer', + values: [[alertStart, '2']], + }, + ]; + + const result = convertToAlerts(prometheusResults, incidents, now, daysSpanMs); + expect(result).toHaveLength(1); + // firstTimestamp is the first value minus the 300s padding offset + expect(result[0].firstTimestamp).toBe(alertStart - 300); + }); + + it('should preserve firstTimestamp from full data even when values are clipped by N-day window', () => { + const threeDaysMs = 3 * DAY_MS; + const nDaysBoundary = Math.floor((now - threeDaysMs) / 1000); + const alertStart = nDaysBoundary - 300; + + const prometheusResults: PrometheusResult[] = [ + { + metric: { + alertname: 'TestAlert', + namespace: 'test-namespace', + severity: 'critical', + name: 'test', + alertstate: 'firing', + }, + values: [ + [alertStart, '2'], + [nDaysBoundary, '2'], + [nDaysBoundary + 300, '2'], + ], + }, + ]; + + const incidents: Array> = [ + { + group_id: 'incident1', + src_alertname: 'TestAlert', + src_namespace: 'test-namespace', + src_severity: 'critical', + component: 'test-component', + layer: 'test-layer', + values: [ + [alertStart, '2'], + [nDaysBoundary + 300, '2'], + ], + }, + ]; + + const result = convertToAlerts(prometheusResults, incidents, now, threeDaysMs); + expect(result).toHaveLength(1); + expect(result[0].firstTimestamp).toBe(alertStart - 300); + }); +}); diff --git a/web/src/components/Incidents/processAlerts.ts b/web/src/components/Incidents/processAlerts.ts index b216aa2a8..d73ac4643 100644 --- a/web/src/components/Incidents/processAlerts.ts +++ b/web/src/components/Incidents/processAlerts.ts @@ -6,6 +6,7 @@ import { insertPaddingPointsForChart, isResolved, PROMETHEUS_QUERY_INTERVAL_SECONDS, + roundTimestampToFiveMinutes, } from './utils'; /** @@ -13,6 +14,10 @@ import { * while removing duplicates. Only firing alerts are included. Alerts with the same combination of these four fields * are combined, with values being deduplicated. * + * After deduplication, alerts are split into separate entries when there is a gap longer than 5 minutes + * (PROMETHEUS_QUERY_INTERVAL_SECONDS) between consecutive datapoints. This means the same alert identity + * can appear multiple times in the result, once per continuous firing interval. + * * @param {Array} objects - Array of alert objects to be deduplicated. Each object contains a `metric` field * with properties such as `alertname`, `namespace`, `component`, `severity`, and an array of `values`. * @param {Object} objects[].metric - The metric information of the alert. @@ -24,18 +29,18 @@ import { * @param {Array>} objects[].values - The array of values corresponding to the alert, where * each value is a tuple containing a timestamp and a value (e.g., [timestamp, value]). * - * @returns {Array} - An array of deduplicated firing alert objects. Each object contains a unique combination of - * `alertname`, `namespace`, `component`, and `severity`, with deduplicated values. + * @returns {Array} - An array of deduplicated firing alert objects, split by gaps. Each object contains + * a combination of `alertname`, `namespace`, `component`, and `severity`, with values for one continuous interval. * @returns {Object} return[].metric - The metric information of the deduplicated alert. - * @returns {Array>} return[].values - The deduplicated array of values for the alert. + * @returns {Array>} return[].values - The values for one continuous interval, sorted by timestamp. * * @example * const alerts = [ - * { metric: { alertname: "Alert1", namespace: "ns1", component: "comp1", severity: "critical", alertstate: "firing" }, values: [[12345, "2"], [12346, "2"]] }, - * { metric: { alertname: "Alert1", namespace: "ns1", component: "comp1", severity: "critical", alertstate: "firing" }, values: [[12346, "2"], [12347, "2"]] } + * { metric: { alertname: "Alert1", namespace: "ns1", component: "comp1", severity: "critical", alertstate: "firing" }, values: [[1000, "2"], [1300, "2"], [2000, "2"], [2300, "2"]] } * ]; * const deduplicated = deduplicateAlerts(alerts); - * // Returns an array where the two alerts are combined with deduplicated values. + * // Returns two entries for Alert1: one with values [[1000, "2"], [1300, "2"]] and another with [[2000, "2"], [2300, "2"]] + * // because the gap between 1300 and 2000 (700s) exceeds the 5-minute threshold (300s). */ export function deduplicateAlerts(objects: Array): Array { // Step 1: Filter out all non firing alerts @@ -69,7 +74,37 @@ export function deduplicateAlerts(objects: Array): Array = []; + for (const alert of groupedObjects.values()) { + const sortedValues = alert.values.sort( + (a: [number, string], b: [number, string]) => a[0] - b[0], + ); + + if (sortedValues.length <= 1) { + result.push({ metric: alert.metric, values: sortedValues }); + continue; + } + + let currentInterval: Array<[number, string]> = [sortedValues[0]]; + + for (let i = 1; i < sortedValues.length; i++) { + const delta = sortedValues[i][0] - sortedValues[i - 1][0]; + + if (delta > PROMETHEUS_QUERY_INTERVAL_SECONDS) { + // Gap detected: save current interval and start a new one + result.push({ metric: { ...alert.metric }, values: currentInterval }); + currentInterval = [sortedValues[i]]; + } else { + currentInterval.push(sortedValues[i]); + } + } + + // Push the last interval + result.push({ metric: { ...alert.metric }, values: currentInterval }); + } + + return result; } /** @@ -202,9 +237,11 @@ export function convertToAlerts( prometheusResults: Array, selectedIncidents: Array>, currentTime: number, + daysSpanMs: number, ): Array { // Merge selected incidents by composite key. Consolidates duplicates caused by non-key labels // like `pod` or `silenced` that aren't supported by cluster health analyzer. + const incidents = mergeIncidentsByKey(selectedIncidents); // Extract the first and last timestamps of the selected incident @@ -215,20 +252,24 @@ export function convertToAlerts( const incidentFirstTimestamp = Math.min(...timestamps); const incidentLastTimestamp = Math.max(...timestamps); + // Clip the incident window to the N-day span so we only show alerts within the selected range + const nDaysStartSeconds = (currentTime - daysSpanMs) / 1000; + const effectiveFirstTimestamp = Math.max(incidentFirstTimestamp, nDaysStartSeconds); + // Watchdog is a heartbeat metric, not a real issue const validAlerts = prometheusResults.filter((alert) => alert.metric.alertname !== 'Watchdog'); - const distinctAlerts = deduplicateAlerts(validAlerts); + const deduplicatedAlerts = deduplicateAlerts(validAlerts); - // Filter alerts to only include values within the incident's time window + // Filter alerts to only include values within the effective time window const ALERT_WINDOW_PADDING_SECONDS = PROMETHEUS_QUERY_INTERVAL_SECONDS / 2; - const alerts = distinctAlerts + const alerts = deduplicatedAlerts .map((alert: PrometheusResult): Alert | null => { - // Keep only values within the incident time range (with padding) + // Keep only values within the effective time range (with padding) const values: Array<[number, string]> = alert.values.filter( ([date]) => - date >= incidentFirstTimestamp - ALERT_WINDOW_PADDING_SECONDS && + date >= effectiveFirstTimestamp - ALERT_WINDOW_PADDING_SECONDS && date <= incidentLastTimestamp + ALERT_WINDOW_PADDING_SECONDS, ); @@ -259,19 +300,24 @@ export function convertToAlerts( // Add padding points for chart rendering const paddedValues = insertPaddingPointsForChart(sortedValues, currentTime); - const firstTimestamp = paddedValues[0][0]; - lastTimestamp = paddedValues[paddedValues.length - 1][0]; + // firstTimestamp is absolute over the full 15-day data (before N-day filtering), + // with the padding offset applied to match chart rendering + const alertFirstTimestamp = roundTimestampToFiveMinutes( + alert.values[0][0] - PROMETHEUS_QUERY_INTERVAL_SECONDS, + ); + lastTimestamp = roundTimestampToFiveMinutes(paddedValues[paddedValues.length - 1][0]); return { alertname: alert.metric.alertname, namespace: alert.metric.namespace, severity: alert.metric.severity as Severity, + firstTimestamp: alertFirstTimestamp, component: matchingIncident.component, layer: matchingIncident.layer, name: alert.metric.name, alertstate: resolved ? 'resolved' : 'firing', values: paddedValues, - alertsStartFiring: firstTimestamp, + alertsStartFiring: alertFirstTimestamp, alertsEndFiring: lastTimestamp, resolved, x: 0, // Will be set after sorting diff --git a/web/src/components/Incidents/processIncidents.spec.ts b/web/src/components/Incidents/processIncidents.spec.ts index ab6ccd631..111599fc7 100644 --- a/web/src/components/Incidents/processIncidents.spec.ts +++ b/web/src/components/Incidents/processIncidents.spec.ts @@ -5,7 +5,7 @@ import { getIncidentsTimeRanges, processIncidentsForAlerts, } from './processIncidents'; -import { getCurrentTime } from './utils'; +import { getCurrentTime, DAY_MS } from './utils'; describe('convertToIncidents', () => { const now = getCurrentTime(); @@ -13,7 +13,7 @@ describe('convertToIncidents', () => { describe('edge cases', () => { it('should return empty array when no data provided', () => { - const result = convertToIncidents([], now); + const result = convertToIncidents([], now, 15 * DAY_MS); expect(result).toEqual([]); }); @@ -43,7 +43,7 @@ describe('convertToIncidents', () => { }, ]; - const result = convertToIncidents(data, now); + const result = convertToIncidents(data, now, 15 * DAY_MS); expect(result).toHaveLength(1); expect(result[0].src_alertname).toBe('ClusterOperatorDegraded'); }); @@ -67,7 +67,7 @@ describe('convertToIncidents', () => { }, ]; - const result = convertToIncidents(data, now); + const result = convertToIncidents(data, now, 15 * DAY_MS); expect(result).toHaveLength(1); // Verify resolved is determined from ORIGINAL values (before padding) @@ -99,7 +99,7 @@ describe('convertToIncidents', () => { }, ]; - const result = convertToIncidents(data, now); + const result = convertToIncidents(data, now, 15 * DAY_MS); expect(result).toHaveLength(1); expect(result[0].firing).toBe(true); expect(result[0].resolved).toBe(false); @@ -122,7 +122,7 @@ describe('convertToIncidents', () => { }, ]; - const result = convertToIncidents(data, now); + const result = convertToIncidents(data, now, 15 * DAY_MS); expect(result).toHaveLength(1); expect(result[0].firing).toBe(false); expect(result[0].resolved).toBe(true); @@ -145,7 +145,7 @@ describe('convertToIncidents', () => { }, ]; - const result = convertToIncidents(data, now); + const result = convertToIncidents(data, now, 15 * DAY_MS); expect(result).toHaveLength(1); expect(result[0].firing).toBe(false); // >= 10 minutes is resolved expect(result[0].resolved).toBe(true); @@ -179,7 +179,7 @@ describe('convertToIncidents', () => { }, ]; - const result = convertToIncidents(data, now); + const result = convertToIncidents(data, now, 15 * DAY_MS); expect(result).toHaveLength(2); expect(result[0].group_id).toBe('incident1'); // Earliest first expect(result[1].group_id).toBe('incident2'); @@ -211,7 +211,7 @@ describe('convertToIncidents', () => { }, ]; - const result = convertToIncidents(data, now); + const result = convertToIncidents(data, now, 15 * DAY_MS); expect(result).toHaveLength(2); expect(result[0].x).toBe(2); // Earliest has highest x expect(result[1].x).toBe(1); // Latest has lowest x @@ -234,7 +234,7 @@ describe('convertToIncidents', () => { }, ]; - const result = convertToIncidents(data, now); + const result = convertToIncidents(data, now, 15 * DAY_MS); expect(result).toHaveLength(1); expect(result[0].src_alertname).toBe('TestAlert'); expect(result[0].src_namespace).toBe('test-namespace'); @@ -254,7 +254,7 @@ describe('convertToIncidents', () => { }, ]; - const result = convertToIncidents(data, now); + const result = convertToIncidents(data, now, 15 * DAY_MS); expect(result).toHaveLength(1); expect(result[0].src_alertname).toBe('TestAlert'); // Only src_ properties should be extracted @@ -280,7 +280,7 @@ describe('convertToIncidents', () => { }, ]; - const result = convertToIncidents(data, now); + const result = convertToIncidents(data, now, 15 * DAY_MS); expect(result).toHaveLength(1); // insertPaddingPointsForChart adds a point 5 minutes before and after the point // After padding is added because now >= timestamp + 300 (since timestamp = nowSeconds - 600) @@ -307,13 +307,479 @@ describe('convertToIncidents', () => { }, ]; - const result = convertToIncidents(data, now); + const result = convertToIncidents(data, now, 15 * DAY_MS); expect(result).toHaveLength(1); expect(result[0].component).toBe('test-component'); // componentList is created by getIncidents expect(result[0].componentList).toBeDefined(); }); }); + + describe('firstTimestamp computation', () => { + it('should compute firstTimestamp as first value timestamp minus 300s', () => { + const timestamp = nowSeconds - 3600; + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'incident1', + component: 'test-component', + layer: 'test-layer', + src_alertname: 'TestAlert', + src_namespace: 'test-namespace', + src_severity: 'critical', + }, + values: [[timestamp, '2']], + }, + ]; + + const result = convertToIncidents(data, now, 15 * DAY_MS); + expect(result).toHaveLength(1); + expect(result[0].firstTimestamp).toBe(timestamp - 300); + }); + + it('should use earliest timestamp for firstTimestamp when values are unsorted', () => { + const earliest = nowSeconds - 3600; + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'incident1', + component: 'test-component', + layer: 'test-layer', + src_alertname: 'TestAlert', + src_namespace: 'test-namespace', + src_severity: 'critical', + }, + values: [ + [nowSeconds - 1800, '2'], + [earliest, '2'], + [nowSeconds - 900, '2'], + ], + }, + ]; + + const result = convertToIncidents(data, now, 15 * DAY_MS); + expect(result).toHaveLength(1); + expect(result[0].firstTimestamp).toBe(earliest - 300); + }); + + it('should compute firstTimestamp independently for each incident', () => { + const startA = nowSeconds - 7200; + const startB = nowSeconds - 1800; + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'incidentA', + component: 'compA', + layer: 'layerA', + src_alertname: 'AlertA', + src_namespace: 'nsA', + src_severity: 'critical', + }, + values: [[startA, '2']], + }, + { + metric: { + group_id: 'incidentB', + component: 'compB', + layer: 'layerB', + src_alertname: 'AlertB', + src_namespace: 'nsB', + src_severity: 'warning', + }, + values: [[startB, '1']], + }, + ]; + + const result = convertToIncidents(data, now, 15 * DAY_MS); + expect(result).toHaveLength(2); + const incidentA = result.find((i) => i.group_id === 'incidentA'); + const incidentB = result.find((i) => i.group_id === 'incidentB'); + expect(incidentA.firstTimestamp).toBe(startA - 300); + expect(incidentB.firstTimestamp).toBe(startB - 300); + }); + }); + + describe('N-day window filtering', () => { + it('should filter values to N-day window but preserve firstTimestamp from full data', () => { + const fourteenDaysAgo = nowSeconds - 14 * 86400; + const oneHourAgo = nowSeconds - 3600; + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'incident1', + component: 'test-component', + layer: 'test-layer', + src_alertname: 'TestAlert', + src_namespace: 'test-namespace', + src_severity: 'critical', + }, + values: [ + [fourteenDaysAgo, '2'], + [oneHourAgo, '2'], + ], + }, + ]; + + const result = convertToIncidents(data, now, 7 * DAY_MS); + expect(result).toHaveLength(1); + expect(result[0].firstTimestamp).toBe(fourteenDaysAgo - 300); + const timestamps = result[0].values.map(([ts]) => ts); + expect(timestamps).toContain(oneHourAgo); + expect(timestamps).not.toContain(fourteenDaysAgo); + }); + + it('should exclude incident when all values are outside the N-day window', () => { + const tenDaysAgo = nowSeconds - 10 * 86400; + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'incident1', + component: 'test-component', + layer: 'test-layer', + src_alertname: 'TestAlert', + src_namespace: 'test-namespace', + src_severity: 'critical', + }, + values: [[tenDaysAgo, '2']], + }, + ]; + + const result = convertToIncidents(data, now, 7 * DAY_MS); + expect(result).toHaveLength(0); + }); + + it('should keep incident when some values fall within the N-day window', () => { + const tenDaysAgo = nowSeconds - 10 * 86400; + const fiveDaysAgo = nowSeconds - 5 * 86400; + const oneDayAgo = nowSeconds - 86400; + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'incident1', + component: 'test-component', + layer: 'test-layer', + src_alertname: 'TestAlert', + src_namespace: 'test-namespace', + src_severity: 'critical', + }, + values: [ + [tenDaysAgo, '2'], + [fiveDaysAgo, '2'], + [oneDayAgo, '2'], + ], + }, + ]; + + const result = convertToIncidents(data, now, 7 * DAY_MS); + expect(result).toHaveLength(1); + expect(result[0].firstTimestamp).toBe(tenDaysAgo - 300); + const timestamps = result[0].values.map(([ts]) => ts); + expect(timestamps).toContain(fiveDaysAgo); + expect(timestamps).toContain(oneDayAgo); + expect(timestamps).not.toContain(tenDaysAgo); + }); + }); + + describe('severity splitting', () => { + it('should produce separate entries when severity changes within a group_id', () => { + const t1 = nowSeconds - 3600; + const t2 = t1 + 300; + const t3 = t2 + 300; + const t4 = t3 + 300; + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'incident1', + component: 'comp-warn', + layer: 'core', + src_alertname: 'Alert_warn', + src_namespace: 'test-ns', + src_severity: 'warning', + }, + values: [ + [t1, '1'], + [t2, '1'], + ], + }, + { + metric: { + group_id: 'incident1', + component: 'comp-crit', + layer: 'core', + src_alertname: 'Alert_crit', + src_namespace: 'test-ns', + src_severity: 'critical', + }, + values: [ + [t3, '2'], + [t4, '2'], + ], + }, + ]; + + const result = convertToIncidents(data, now, 15 * DAY_MS); + expect(result).toHaveLength(2); + expect(result.some((i) => i.src_severity === 'warning')).toBe(true); + expect(result.some((i) => i.src_severity === 'critical')).toBe(true); + }); + + it('should assign same x value to severity segments of same group_id', () => { + const t1 = nowSeconds - 3600; + const t2 = t1 + 300; + const t3 = t2 + 300; + const t4 = t3 + 300; + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'incident1', + component: 'comp-warn', + layer: 'core', + src_alertname: 'Alert_warn', + src_namespace: 'test-ns', + src_severity: 'warning', + }, + values: [ + [t1, '1'], + [t2, '1'], + ], + }, + { + metric: { + group_id: 'incident1', + component: 'comp-crit', + layer: 'core', + src_alertname: 'Alert_crit', + src_namespace: 'test-ns', + src_severity: 'critical', + }, + values: [ + [t3, '2'], + [t4, '2'], + ], + }, + ]; + + const result = convertToIncidents(data, now, 15 * DAY_MS); + expect(result).toHaveLength(2); + expect(result[0].x).toBe(result[1].x); + }); + + it('should compute distinct firstTimestamp for each severity segment', () => { + const t1 = nowSeconds - 3600; + const t2 = t1 + 300; + const t3 = t2 + 300; + const t4 = t3 + 300; + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'incident1', + component: 'comp-warn', + layer: 'core', + src_alertname: 'Alert_warn', + src_namespace: 'test-ns', + src_severity: 'warning', + }, + values: [ + [t1, '1'], + [t2, '1'], + ], + }, + { + metric: { + group_id: 'incident1', + component: 'comp-crit', + layer: 'core', + src_alertname: 'Alert_crit', + src_namespace: 'test-ns', + src_severity: 'critical', + }, + values: [ + [t3, '2'], + [t4, '2'], + ], + }, + ]; + + const result = convertToIncidents(data, now, 15 * DAY_MS); + expect(result).toHaveLength(2); + const warnSegment = result.find((i) => i.src_severity === 'warning'); + const critSegment = result.find((i) => i.src_severity === 'critical'); + expect(warnSegment.firstTimestamp).toBe(t1 - 300); + expect(critSegment.firstTimestamp).toBe(t3 - 300); + }); + }); + + describe('multiple concurrent incidents', () => { + it('should process 4 simultaneous incidents with different group_ids', () => { + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'inc-monitoring', + component: 'monitoring', + layer: 'core', + src_alertname: 'MonAlert', + src_namespace: 'openshift-monitoring', + src_severity: 'warning', + }, + values: [[nowSeconds - 14 * 86400, '1']], + }, + { + metric: { + group_id: 'inc-dns', + component: 'dns', + layer: 'core', + src_alertname: 'DnsAlert', + src_namespace: 'openshift-dns', + src_severity: 'warning', + }, + values: [[nowSeconds - 12 * 86400, '1']], + }, + { + metric: { + group_id: 'inc-network', + component: 'network', + layer: 'core', + src_alertname: 'NetAlert', + src_namespace: 'openshift-network', + src_severity: 'warning', + }, + values: [[nowSeconds - 8 * 86400, '1']], + }, + { + metric: { + group_id: 'inc-ingress', + component: 'ingress', + layer: 'core', + src_alertname: 'IngressAlert', + src_namespace: 'openshift-ingress', + src_severity: 'critical', + }, + values: [[nowSeconds - 3600, '2']], + }, + ]; + + const result = convertToIncidents(data, now, 15 * DAY_MS); + expect(result).toHaveLength(4); + const groupIds = result.map((i) => i.group_id); + expect(groupIds).toContain('inc-monitoring'); + expect(groupIds).toContain('inc-dns'); + expect(groupIds).toContain('inc-network'); + expect(groupIds).toContain('inc-ingress'); + }); + + it('should assign unique x values to each incident', () => { + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'inc-monitoring', + component: 'monitoring', + layer: 'core', + src_alertname: 'MonAlert', + src_namespace: 'openshift-monitoring', + src_severity: 'warning', + }, + values: [[nowSeconds - 14 * 86400, '1']], + }, + { + metric: { + group_id: 'inc-dns', + component: 'dns', + layer: 'core', + src_alertname: 'DnsAlert', + src_namespace: 'openshift-dns', + src_severity: 'warning', + }, + values: [[nowSeconds - 12 * 86400, '1']], + }, + { + metric: { + group_id: 'inc-network', + component: 'network', + layer: 'core', + src_alertname: 'NetAlert', + src_namespace: 'openshift-network', + src_severity: 'warning', + }, + values: [[nowSeconds - 8 * 86400, '1']], + }, + { + metric: { + group_id: 'inc-ingress', + component: 'ingress', + layer: 'core', + src_alertname: 'IngressAlert', + src_namespace: 'openshift-ingress', + src_severity: 'critical', + }, + values: [[nowSeconds - 3600, '2']], + }, + ]; + + const result = convertToIncidents(data, now, 15 * DAY_MS); + const xValues = result.map((i) => i.x); + expect(new Set(xValues).size).toBe(4); + }); + + it('should sort by earliest timestamp with correct x assignment', () => { + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'inc-monitoring', + component: 'monitoring', + layer: 'core', + src_alertname: 'MonAlert', + src_namespace: 'openshift-monitoring', + src_severity: 'warning', + }, + values: [[nowSeconds - 14 * 86400, '1']], + }, + { + metric: { + group_id: 'inc-dns', + component: 'dns', + layer: 'core', + src_alertname: 'DnsAlert', + src_namespace: 'openshift-dns', + src_severity: 'warning', + }, + values: [[nowSeconds - 12 * 86400, '1']], + }, + { + metric: { + group_id: 'inc-network', + component: 'network', + layer: 'core', + src_alertname: 'NetAlert', + src_namespace: 'openshift-network', + src_severity: 'warning', + }, + values: [[nowSeconds - 8 * 86400, '1']], + }, + { + metric: { + group_id: 'inc-ingress', + component: 'ingress', + layer: 'core', + src_alertname: 'IngressAlert', + src_namespace: 'openshift-ingress', + src_severity: 'critical', + }, + values: [[nowSeconds - 3600, '2']], + }, + ]; + + const result = convertToIncidents(data, now, 15 * DAY_MS); + expect(result).toHaveLength(4); + expect(result[0].group_id).toBe('inc-monitoring'); + expect(result[1].group_id).toBe('inc-dns'); + expect(result[2].group_id).toBe('inc-network'); + expect(result[3].group_id).toBe('inc-ingress'); + expect(result[0].x).toBe(4); + expect(result[1].x).toBe(3); + expect(result[2].x).toBe(2); + expect(result[3].x).toBe(1); + }); + }); }); describe('getIncidents', () => { @@ -355,7 +821,7 @@ describe('getIncidents', () => { group_id: 'incident1', component: 'comp2', }, - values: [[1100, '1']], + values: [[1100, '2']], // Same severity to avoid severity splitting }, ]; @@ -493,6 +959,7 @@ describe('getIncidents', () => { }); it('should build componentList when merging incidents with different components', () => { + // Use same severity to avoid severity splitting; test focuses on componentList const data: PrometheusResult[] = [ { metric: { @@ -506,14 +973,14 @@ describe('getIncidents', () => { group_id: 'incident1', component: 'comp2', }, - values: [[1100, '1']], + values: [[1100, '2']], }, { metric: { group_id: 'incident1', component: 'comp3', }, - values: [[1200, '0']], + values: [[1200, '2']], }, ]; @@ -526,6 +993,7 @@ describe('getIncidents', () => { }); it('should not add duplicate components to componentList', () => { + // Use same severity to avoid severity splitting; test focuses on componentList const data: PrometheusResult[] = [ { metric: { @@ -539,7 +1007,7 @@ describe('getIncidents', () => { group_id: 'incident1', component: 'comp1', // Same component }, - values: [[1100, '1']], + values: [[1100, '2']], }, ]; @@ -551,6 +1019,7 @@ describe('getIncidents', () => { describe('silenced status handling', () => { it('should use silenced value from most recent timestamp when merging', () => { + // Use same severity to avoid severity splitting; test focuses on silenced status const data: PrometheusResult[] = [ { metric: { @@ -566,7 +1035,7 @@ describe('getIncidents', () => { component: 'comp2', silenced: 'true', }, - values: [[2000, '1']], // More recent + values: [[2000, '2']], // More recent, same severity }, ]; @@ -576,6 +1045,7 @@ describe('getIncidents', () => { }); it('should keep older silenced value if new result has older timestamps', () => { + // Use same severity to avoid severity splitting; test focuses on silenced status const data: PrometheusResult[] = [ { metric: { @@ -591,7 +1061,7 @@ describe('getIncidents', () => { component: 'comp2', silenced: 'false', }, - values: [[1000, '1']], // Older + values: [[1000, '2']], // Older, same severity }, ]; @@ -772,7 +1242,7 @@ describe('processIncidentsForAlerts', () => { }, ]; - const result = processIncidentsForAlerts(incidents); + const result = processIncidentsForAlerts(incidents) as any[]; expect(result).toHaveLength(1); expect(result[0].group_id).toBe('incident1'); expect(result[0].component).toBe('test-component'); @@ -799,6 +1269,29 @@ describe('processIncidentsForAlerts', () => { }); }); + describe('firstTimestamp', () => { + it('should default firstTimestamp to 0', () => { + const incidents: PrometheusResult[] = [ + { + metric: { + group_id: 'incident1', + src_alertname: 'TestAlert', + src_namespace: 'test-namespace', + component: 'test-component', + src_severity: 'critical', + }, + values: [[1704067300, '2']], + }, + ]; + + const result = processIncidentsForAlerts(incidents); + expect(result).toHaveLength(1); + // processIncidentsForAlerts sets firstTimestamp to 0; + // the real firstTimestamp is computed in convertToAlerts from alert data + expect(result[0].firstTimestamp).toBe(0); + }); + }); + describe('edge cases', () => { it('should handle empty array', () => { const result = processIncidentsForAlerts([]); @@ -813,7 +1306,7 @@ describe('processIncidentsForAlerts', () => { }, ]; - const result = processIncidentsForAlerts(incidents); + const result = processIncidentsForAlerts(incidents) as any[]; expect(result).toHaveLength(1); expect(result[0].group_id).toBe('incident1'); expect(result[0].silenced).toBe(true); diff --git a/web/src/components/Incidents/processIncidents.ts b/web/src/components/Incidents/processIncidents.ts index 803123f17..46d1c9b76 100644 --- a/web/src/components/Incidents/processIncidents.ts +++ b/web/src/components/Incidents/processIncidents.ts @@ -1,14 +1,24 @@ /* eslint-disable max-len */ import { PrometheusLabels, PrometheusResult } from '@openshift-console/dynamic-plugin-sdk'; -import { Incident, Metric, ProcessedIncident } from './model'; -import { insertPaddingPointsForChart, isResolved, sortByEarliestTimestamp } from './utils'; +import { Metric, ProcessedIncident } from './model'; +import { + insertPaddingPointsForChart, + isResolved, + sortByEarliestTimestamp, + DAY_MS, + PROMETHEUS_QUERY_INTERVAL_SECONDS, + roundTimestampToFiveMinutes, +} from './utils'; /** * Converts Prometheus results into processed incidents, filtering out Watchdog incidents. * Adds padding points for chart rendering and determines firing/resolved status based on * the time elapsed since the last data point. * + * Severity-based splitting is handled upstream in getIncidents, which produces separate + * entries for each consecutive run of the same severity within a group_id. + * * @param prometheusResults - Array of Prometheus query results containing incident data. * @param currentTime - The current time in milliseconds to use for resolved/firing calculations. * @returns Array of processed incidents with firing/resolved status, padding points, and x positioning. @@ -16,36 +26,93 @@ import { insertPaddingPointsForChart, isResolved, sortByEarliestTimestamp } from export function convertToIncidents( prometheusResults: PrometheusResult[], currentTime: number, + daysSpanMs: number, ): ProcessedIncident[] { const incidents = getIncidents(prometheusResults).filter( (incident) => incident.metric.src_alertname !== 'Watchdog', ); const sortedIncidents = sortByEarliestTimestamp(incidents); - return sortedIncidents.map((incident, index) => { - // Determine resolved status based on original values before padding - const sortedValues = incident.values.sort((a, b) => a[0] - b[0]); - const lastTimestamp = sortedValues[sortedValues.length - 1][0]; - const resolved = isResolved(lastTimestamp, currentTime); + // Assign x values by unique group_id so that split severity segments share the same x + const uniqueGroupIds = [...new Set(sortedIncidents.map((i) => i.metric.group_id))]; + const groupIdToX = new Map(uniqueGroupIds.map((id, idx) => [id, uniqueGroupIds.length - idx])); - // Add padding points for chart rendering - const paddedValues = insertPaddingPointsForChart(sortedValues, currentTime); + // N-day window boundary (in seconds) for filtering values + const nDaysStartSeconds = (currentTime - daysSpanMs) / 1000; - const srcProperties = getSrcProperties(incident.metric); + const processedIncidents = sortedIncidents + .map((incident) => { + const sortedValues = incident.values.sort((a, b) => a[0] - b[0]); - return { - component: incident.metric.component, - componentList: incident.metric.componentList, - group_id: incident.metric.group_id, - layer: incident.metric.layer, - values: paddedValues, - x: incidents.length - index, - resolved, - firing: !resolved, - ...srcProperties, - metric: incident.metric, - } as ProcessedIncident; - }); + // Filter values to the N-day window + const windowValues = sortedValues.filter(([ts]) => ts >= nDaysStartSeconds); + + // Exclude incidents with no values in the selected window + if (windowValues.length === 0) { + return null; + } + + // Determine resolved status based on filtered values before padding + const lastTimestamp = windowValues[windowValues.length - 1][0]; + const resolved = isResolved(lastTimestamp, currentTime); + + // Add padding points for chart rendering + const paddedValues = insertPaddingPointsForChart(windowValues, currentTime); + + if (sortedValues.length === 0) { + return null; + } + + // firstTimestamp is absolute over the full 15-day data (before N-day filtering), + // with the padding offset applied to match chart rendering + const firstTimestamp = roundTimestampToFiveMinutes( + sortedValues[0][0] - PROMETHEUS_QUERY_INTERVAL_SECONDS, + ); + + const srcProperties = getSrcProperties(incident.metric); + + return { + component: incident.metric.component, + componentList: incident.metric.componentList, + group_id: incident.metric.group_id, + layer: incident.metric.layer, + values: paddedValues, + x: groupIdToX.get(incident.metric.group_id), + resolved, + firing: !resolved, + firstTimestamp, + ...srcProperties, + metric: incident.metric, + } as ProcessedIncident; + }) + .filter((incident): incident is ProcessedIncident => incident !== null); + + return processedIncidents; +} + +/** + * Splits a sorted array of [timestamp, severity] tuples into consecutive segments + * where the severity value remains the same. + */ +function splitBySeverityChange( + sortedValues: Array<[number, string]>, +): Array> { + if (sortedValues.length === 0) return []; + + const segments: Array> = []; + let currentSegment: Array<[number, string]> = [sortedValues[0]]; + + for (let i = 1; i < sortedValues.length; i++) { + if (sortedValues[i][1] !== sortedValues[i - 1][1]) { + segments.push(currentSegment); + currentSegment = [sortedValues[i]]; + } else { + currentSegment.push(sortedValues[i]); + } + } + + segments.push(currentSegment); + return segments; } /** @@ -96,18 +163,37 @@ function getSrcProperties(metric: PrometheusLabels): Partial { * deduplicates timestamps (keeping only the highest severity per timestamp), * and combines components into componentList. * + * After merging and deduplication, each incident is split into separate entries + * when the severity (values[1]) changes between consecutive data points. + * This means a single group_id may produce multiple entries if its severity + * transitions over time (e.g., from warning '1' to critical '2'). + * * @param prometheusResults - Array of Prometheus query results to convert. - * @returns Array of incident objects with deduplicated values and combined properties. + * @returns Array of incident objects with deduplicated values and combined properties, + * split by severity changes. */ export function getIncidents( prometheusResults: PrometheusResult[], ): Array { const incidents = new Map(); + // Track which metric labels correspond to each severity value per group_id + const severityMetrics = new Map>(); for (const result of prometheusResults) { const groupId = result.metric.group_id; + // Track the metric for all severity values in this result (skip Watchdog) + if (result.values.length > 0 && result.metric.src_alertname !== 'Watchdog') { + if (!severityMetrics.has(groupId)) { + severityMetrics.set(groupId, new Map()); + } + const severityValues = new Set(result.values.map((v) => v[1])); + for (const sv of severityValues) { + severityMetrics.get(groupId).set(sv, result.metric); + } + } + const existingIncident = incidents.get(groupId); if (existingIncident) { @@ -151,7 +237,37 @@ export function getIncidents( } } - return Array.from(incidents.values()); + // Split each incident into separate entries when severity (values[1]) changes over time + const result: Array = []; + + for (const incident of incidents.values()) { + const groupId = incident.metric.group_id; + const sortedValues = [...incident.values].sort((a, b) => a[0] - b[0]); + const segments = splitBySeverityChange(sortedValues); + const groupSeverityMetrics = severityMetrics.get(groupId); + + for (const segmentValues of segments) { + const segmentSeverity = segmentValues[0][1]; // uniform within a segment + // Use the metric from the Prometheus result that contributed this severity, + // preserving shared properties (componentList, silenced) from the merged incident + const severitySpecificMetric = groupSeverityMetrics?.get(segmentSeverity); + + result.push({ + metric: { + ...incident.metric, + ...(severitySpecificMetric && { + src_alertname: severitySpecificMetric.src_alertname, + src_namespace: severitySpecificMetric.src_namespace, + src_severity: severitySpecificMetric.src_severity, + component: severitySpecificMetric.component, + }), + } as Metric, + values: segmentValues, + }); + } + } + + return result; } /** @@ -166,14 +282,13 @@ export const getIncidentsTimeRanges = ( timespan: number, currentTime: number, ): Array<{ endTime: number; duration: number }> => { - const ONE_DAY = 24 * 60 * 60 * 1000; // 24 hours in milliseconds const startTime = currentTime - timespan; - const timeRanges = [{ endTime: startTime + ONE_DAY, duration: ONE_DAY }]; + const timeRanges = [{ endTime: startTime + DAY_MS, duration: DAY_MS }]; while (timeRanges[timeRanges.length - 1].endTime < currentTime) { const lastRange = timeRanges[timeRanges.length - 1]; - const nextEndTime = lastRange.endTime + ONE_DAY; - timeRanges.push({ endTime: nextEndTime, duration: ONE_DAY }); + const nextEndTime = lastRange.endTime + DAY_MS; + timeRanges.push({ endTime: nextEndTime, duration: DAY_MS }); } return timeRanges; @@ -186,20 +301,20 @@ export const getIncidentsTimeRanges = ( * @param incidents - Array of Prometheus results containing incident data. * @returns Array of partial incident objects with silenced status as boolean and x position. */ -export const processIncidentsForAlerts = ( - incidents: Array, -): Array> => { +export const processIncidentsForAlerts = (incidents: Array) => { return incidents.map((incident, index) => { // Read silenced value from cluster_health_components_map metric label // If missing, default to false const silenced = incident.metric.silenced === 'true'; // Return the processed incident - return { + const retval = { ...incident.metric, values: incident.values, x: incidents.length - index, silenced, + firstTimestamp: 0, }; + return retval; }); }; diff --git a/web/src/components/Incidents/utils.spec.ts b/web/src/components/Incidents/utils.spec.ts index 97c55df8f..b0dcdf566 100644 --- a/web/src/components/Incidents/utils.spec.ts +++ b/web/src/components/Incidents/utils.spec.ts @@ -1,4 +1,94 @@ -import { insertPaddingPointsForChart, roundDateToInterval } from './utils'; +import { + getCurrentTime, + insertPaddingPointsForChart, + removeTrailingPaddingFromSeveritySegments, + roundDateToInterval, + roundTimestampToFiveMinutes, +} from './utils'; +import { Incident } from './model'; + +describe('getCurrentTime', () => { + it('should return current time rounded down to 5-minute boundary', () => { + const result = getCurrentTime(); + const now = Date.now(); + const intervalMs = 300 * 1000; // 5 minutes in milliseconds + const expected = Math.floor(now / intervalMs) * intervalMs; + + expect(result).toBe(expected); + expect(result % intervalMs).toBe(0); // Should be on 5-minute boundary + }); + + it('should round down to nearest 5-minute boundary', () => { + const mockTime = 1704067230 * 1000; // 2024-01-01 00:00:30 UTC (30 seconds past) + jest.spyOn(Date, 'now').mockReturnValue(mockTime); + + const result = getCurrentTime(); + const expected = 1704067200 * 1000; // 2024-01-01 00:00:00 UTC (rounded down) + + expect(result).toBe(expected); + + jest.restoreAllMocks(); + }); + + it('should return same value for times on 5-minute boundaries', () => { + const mockTime = 1704067200 * 1000; // 2024-01-01 00:00:00 UTC (on boundary) + jest.spyOn(Date, 'now').mockReturnValue(mockTime); + + const result = getCurrentTime(); + expect(result).toBe(mockTime); + + jest.restoreAllMocks(); + }); +}); + +describe('roundTimestampToFiveMinutes', () => { + it('should return same value when timestamp is already on 5-minute boundary', () => { + const timestamp = 1704067200; // 2024-01-01 00:00:00 UTC + const result = roundTimestampToFiveMinutes(timestamp); + expect(result).toBe(1704067200); + }); + + it('should round down timestamp that is 30 seconds past boundary', () => { + const timestamp = 1704067230; // 2024-01-01 00:00:30 UTC + const result = roundTimestampToFiveMinutes(timestamp); + expect(result).toBe(1704067200); // Rounded down to 00:00:00 + }); + + it('should round down timestamp that is 4 minutes 59 seconds past boundary', () => { + const timestamp = 1704067499; // 2024-01-01 00:04:59 UTC + const result = roundTimestampToFiveMinutes(timestamp); + expect(result).toBe(1704067200); // Rounded down to 00:00:00 + }); + + it('should return next boundary when timestamp is exactly on next boundary', () => { + const timestamp = 1704067500; // 2024-01-01 00:05:00 UTC + const result = roundTimestampToFiveMinutes(timestamp); + expect(result).toBe(1704067500); // Already on boundary + }); + + it('should round down timestamp that is 1 second past boundary', () => { + const timestamp = 1704067201; // 2024-01-01 00:00:01 UTC + const result = roundTimestampToFiveMinutes(timestamp); + expect(result).toBe(1704067200); // Rounded down + }); + + it('should handle timestamps with large values', () => { + const timestamp = 1735689600; // 2025-01-01 00:00:00 UTC + const result = roundTimestampToFiveMinutes(timestamp); + expect(result).toBe(1735689600); + }); + + it('should round down correctly for various offsets', () => { + const base = 1704067200; // 2024-01-01 00:00:00 UTC + + expect(roundTimestampToFiveMinutes(base + 0)).toBe(base); // On boundary + expect(roundTimestampToFiveMinutes(base + 1)).toBe(base); // 1 second + expect(roundTimestampToFiveMinutes(base + 60)).toBe(base); // 1 minute + expect(roundTimestampToFiveMinutes(base + 299)).toBe(base); // 4 min 59 sec + expect(roundTimestampToFiveMinutes(base + 300)).toBe(base + 300); // 5 minutes (next boundary) + expect(roundTimestampToFiveMinutes(base + 301)).toBe(base + 300); // 5 min 1 sec + }); +}); describe('insertPaddingPointsForChart', () => { describe('edge cases', () => { @@ -313,3 +403,226 @@ describe('roundDateToInterval', () => { }); }); }); + +describe('removeTrailingPaddingFromSeveritySegments', () => { + const makeIncident = ( + overrides: Partial & { values: Array<[number, string]> }, + ): Incident => ({ + component: 'test-component', + componentList: ['test-component'], + layer: 'compute', + firing: false, + group_id: 'group-1', + src_severity: 'critical', + src_alertname: 'TestAlert', + src_namespace: 'test-ns', + severity: 'critical', + silenced: false, + x: 1, + firstTimestamp: 1000, + metric: { group_id: 'group-1', component: 'test-component' }, + ...overrides, + }); + + it('should return the group unchanged when it has a single incident', () => { + const group = [ + makeIncident({ + values: [ + [1000, '2'], + [1300, '2'], + [1600, '2'], + ], + }), + ]; + + const result = removeTrailingPaddingFromSeveritySegments(group); + + expect(result).toEqual(group); + expect(result[0].values).toHaveLength(3); + }); + + it('should remove the trailing value from non-last segments in a multi-segment group', () => { + const group = [ + makeIncident({ + values: [ + [1000, '1'], + [1300, '1'], + [1600, '1'], // trailing padding — should be removed + ], + }), + makeIncident({ + values: [ + [1300, '2'], + [1600, '2'], + [1900, '2'], + ], + }), + ]; + + const result = removeTrailingPaddingFromSeveritySegments(group); + + // First segment (non-last): trailing value removed + expect(result[0].values).toEqual([ + [1000, '1'], + [1300, '1'], + ]); + // Last segment: unchanged + expect(result[1].values).toEqual([ + [1300, '2'], + [1600, '2'], + [1900, '2'], + ]); + }); + + it('should not remove the trailing value from the last segment', () => { + const group = [ + makeIncident({ + values: [ + [1000, '1'], + [1300, '1'], + [1600, '1'], + ], + }), + makeIncident({ + values: [ + [1600, '2'], + [1900, '2'], + [2200, '2'], + ], + }), + ]; + + const result = removeTrailingPaddingFromSeveritySegments(group); + + // Last segment should keep all values + const lastSegment = result[result.length - 1]; + expect(lastSegment.values).toHaveLength(3); + }); + + it('should not remove values from a non-last segment that has only one value', () => { + const group = [ + makeIncident({ + values: [[1000, '1']], // single value — nothing to trim + }), + makeIncident({ + values: [ + [1300, '2'], + [1600, '2'], + ], + }), + ]; + + const result = removeTrailingPaddingFromSeveritySegments(group); + + // Single-value segment should be unchanged + expect(result[0].values).toEqual([[1000, '1']]); + // Last segment unchanged + expect(result[1].values).toEqual([ + [1300, '2'], + [1600, '2'], + ]); + }); + + it('should sort segments by their first timestamp before processing', () => { + // Pass segments in reverse order to verify sorting + const group = [ + makeIncident({ + values: [ + [2000, '2'], + [2300, '2'], + [2600, '2'], + ], + }), + makeIncident({ + values: [ + [1000, '1'], + [1300, '1'], + [1600, '1'], + ], + }), + ]; + + const result = removeTrailingPaddingFromSeveritySegments(group); + + // After sorting, the segment starting at 1000 is first (non-last) and gets trimmed + expect(result[0].values).toEqual([ + [1000, '1'], + [1300, '1'], + ]); + // The segment starting at 2000 is last and stays unchanged + expect(result[1].values).toEqual([ + [2000, '2'], + [2300, '2'], + [2600, '2'], + ]); + }); + + it('should handle three severity segments, trimming only non-last ones', () => { + const group = [ + makeIncident({ + values: [ + [1000, '0'], + [1300, '0'], + [1600, '0'], + ], + }), + makeIncident({ + values: [ + [1600, '1'], + [1900, '1'], + [2200, '1'], + ], + }), + makeIncident({ + values: [ + [2200, '2'], + [2500, '2'], + [2800, '2'], + ], + }), + ]; + + const result = removeTrailingPaddingFromSeveritySegments(group); + + // First segment: trimmed + expect(result[0].values).toEqual([ + [1000, '0'], + [1300, '0'], + ]); + // Second segment: trimmed (also non-last) + expect(result[1].values).toEqual([ + [1600, '1'], + [1900, '1'], + ]); + // Third (last) segment: unchanged + expect(result[2].values).toEqual([ + [2200, '2'], + [2500, '2'], + [2800, '2'], + ]); + }); + + it('should not mutate the original incident objects', () => { + const original = makeIncident({ + values: [ + [1000, '1'], + [1300, '1'], + [1600, '1'], + ], + }); + const group = [ + original, + makeIncident({ + values: [ + [1600, '2'], + [1900, '2'], + ], + }), + ]; + + removeTrailingPaddingFromSeveritySegments(group); + + // Original incident should still have all 3 values + expect(original.values).toHaveLength(3); + }); +}); diff --git a/web/src/components/Incidents/utils.ts b/web/src/components/Incidents/utils.ts index 3b12d5b2c..e05305986 100644 --- a/web/src/components/Incidents/utils.ts +++ b/web/src/components/Incidents/utils.ts @@ -13,11 +13,15 @@ import { DaysFilters, Incident, IncidentFiltersCombined, - IncidentsDetailsAlert, SpanDates, Timestamps, } from './model'; +/** + * Number of milliseconds in a day + */ +export const DAY_MS = 24 * 60 * 60 * 1000; // 24 hours in milliseconds + /** * The Prometheus query step interval in seconds. * @@ -73,6 +77,19 @@ export const roundDateToInterval = (date: Date): Date => { return new Date(roundedMs); }; +/** + * Rounds a timestamp down to the nearest 5-minute boundary. + * + * @param timestampSeconds - Timestamp in seconds (Prometheus format) + * @returns Timestamp in seconds, rounded down to the nearest 5-minute boundary + */ +export const roundTimestampToFiveMinutes = (timestampSeconds: number): number => { + return ( + Math.floor(timestampSeconds / PROMETHEUS_QUERY_INTERVAL_SECONDS) * + PROMETHEUS_QUERY_INTERVAL_SECONDS + ); +}; + /** * Determines if an incident or alert is resolved based on the time elapsed since the last * data point. @@ -285,16 +302,38 @@ function createNodataInterval( } /** - * Creates an array of incident data for chart bars, ensuring that when - * two severities have the same time range, the lower severity is removed. + * Removes trailing padding values from non-last severity segments within a group. * - * @param {Object} incident - The incident data containing - * values with timestamps and severity levels. - * @returns {Array} - An array of incident objects with `y0`, - * `y`, `x`, and `name` fields representing the bars for the chart. + * When an incident changes severity, its segments share the same chart row. + * Non-last segments have a trailing padding point (+300s) that overlaps with the + * next segment's leading padding point (-300s). This function removes the trailing + * padding value from non-last segments to prevent visual overlap. + * + * @param group - Array of incidents sharing the same group_id (severity segments) + * @returns The group with trailing padding removed from non-last segments */ -export const createIncidentsChartBars = (incident: Incident, dateArray: SpanDates) => { - const groupedData = consolidateAndMergeIntervals(incident, dateArray); +export function removeTrailingPaddingFromSeveritySegments(group: Incident[]): Incident[] { + if (group.length <= 1) return group; + + const sorted = [...group].sort((a, b) => a.values[0][0] - b.values[0][0]); + + return sorted.map((incident, idx) => { + if (idx < sorted.length - 1 && incident.values.length > 1) { + return { ...incident, values: incident.values.slice(0, -1) }; + } + return incident; + }); +} + +export const createIncidentsChartBars = (incidents: Incident[], dateArray: SpanDates) => { + const getSeverityName = (value) => { + return value === '2' ? 'Critical' : value === '1' ? 'Warning' : 'Info'; + }; + const barChartColorScheme = { + critical: t_global_color_status_danger_default.var, + info: t_global_color_status_info_default.var, + warning: t_global_color_status_warning_default.var, + }; const data: { y0: Date; @@ -305,77 +344,91 @@ export const createIncidentsChartBars = (incident: Incident, dateArray: SpanDate componentList: string[]; group_id: string; nodata: boolean; + startDate: Date; fill: string; }[] = []; - const getSeverityName = (value) => { - return value === '2' ? 'Critical' : value === '1' ? 'Warning' : 'Info'; - }; - const barChartColorScheme = { - critical: t_global_color_status_danger_default.var, - info: t_global_color_status_info_default.var, - warning: t_global_color_status_warning_default.var, - }; - - for (let i = 0; i < groupedData.length; i++) { - const severity = getSeverityName(groupedData[i][2]); - const isLastElement = i === groupedData.length - 1; - data.push({ - y0: new Date(groupedData[i][0] * 1000), - y: new Date(groupedData[i][1] * 1000), - x: incident.x, - name: severity, - firing: isLastElement ? incident.firing : false, - componentList: incident.componentList || [], - group_id: incident.group_id, - nodata: groupedData[i][2] === 'nodata' ? true : false, - fill: - severity === 'Critical' - ? barChartColorScheme.critical - : severity === 'Warning' - ? barChartColorScheme.warning - : barChartColorScheme.info, - }); + for (const incident of incidents) { + const groupedData = consolidateAndMergeIntervals(incident, dateArray); + + for (let i = 0; i < groupedData.length; i++) { + const severity = getSeverityName(groupedData[i][2]); + const isLastElement = i === groupedData.length - 1; + + data.push({ + y0: new Date(groupedData[i][0] * 1000), + y: new Date(groupedData[i][1] * 1000), + x: incident.x, + name: severity, + firing: isLastElement ? incident.firing : false, + componentList: incident.componentList || [], + group_id: incident.group_id, + nodata: groupedData[i][2] === 'nodata', + startDate: new Date(incident.firstTimestamp * 1000), + fill: + severity === 'Critical' + ? barChartColorScheme.critical + : severity === 'Warning' + ? barChartColorScheme.warning + : barChartColorScheme.info, + }); + } } return data; }; -function consolidateAndMergeAlertIntervals(data: Alert) { - if (!data.values || data.values.length === 0) { - return []; - } +function consolidateAndMergeAlertIntervals(alerts: Array): Array { + if (alerts.length === 0) return []; + + // Tag each value with its source alert's firstTimestamp, then sort + const taggedValues = alerts.flatMap((alert) => + alert.values.map((v) => ({ timestamp: v[0], firstTimestamp: alert.firstTimestamp })), + ); - // Spread the array items to prevent sorting the original array - const sortedValues = [...data.values].sort((a, b) => a[0] - b[0]); + if (taggedValues.length === 0) return []; + + const sorted = [...taggedValues].sort((a, b) => a.timestamp - b.timestamp); const intervals: Array = []; - let currentStart = sortedValues[0][0]; + let currentStart = sorted[0].timestamp; + let currentFirstTimestamp = sorted[0].firstTimestamp; let previousTimestamp = currentStart; - for (let i = 1; i < sortedValues.length; i++) { - const currentTimestamp = sortedValues[i][0]; - const timeDifference = (currentTimestamp - previousTimestamp) / 60; // Convert to minutes + for (let i = 1; i < sorted.length; i++) { + const timeDifference = (sorted[i].timestamp - previousTimestamp) / 60; // Convert to minutes if (timeDifference > 5) { - intervals.push([currentStart, sortedValues[i - 1][0], 'data']); - intervals.push([previousTimestamp + 1, currentTimestamp - 1, 'nodata']); - currentStart = sortedValues[i][0]; + intervals.push([currentStart, sorted[i - 1].timestamp, 'data', currentFirstTimestamp]); + intervals.push([previousTimestamp + 1, sorted[i].timestamp - 1, 'nodata']); + currentStart = sorted[i].timestamp; + currentFirstTimestamp = sorted[i].firstTimestamp; } - previousTimestamp = currentTimestamp; + previousTimestamp = sorted[i].timestamp; } - intervals.push([currentStart, sortedValues[sortedValues.length - 1][0], 'data']); + intervals.push([ + currentStart, + sorted[sorted.length - 1].timestamp, + 'data', + currentFirstTimestamp, + ]); // For dynamic alerts timeline, we don't add padding gaps since the dateArray // is already calculated to fit the alert data with appropriate padding - // This allows the timeline to focus on the actual alert activity period return intervals; } -export const createAlertsChartBars = (alert: IncidentsDetailsAlert): AlertsChartBar[] => { - const groupedData = consolidateAndMergeAlertIntervals(alert); +export const createAlertsChartBars = (alerts: Array): AlertsChartBar[] => { + if (alerts.length === 0) return []; + + const groupedData = consolidateAndMergeAlertIntervals(alerts); + + // Use first interval for identity, last for alertstate + const firstAlert = alerts[0]; + const lastAlert = alerts[alerts.length - 1]; + const barChartColorScheme = { critical: t_global_color_status_danger_default.var, info: t_global_color_status_info_default.var, @@ -386,22 +439,25 @@ export const createAlertsChartBars = (alert: IncidentsDetailsAlert): AlertsChart for (let i = 0; i < groupedData.length; i++) { const isLastElement = i === groupedData.length - 1; + const intervalFirstTimestamp = groupedData[i][3] ?? firstAlert.firstTimestamp; + data.push({ y0: new Date(groupedData[i][0] * 1000), y: new Date(groupedData[i][1] * 1000), - x: alert.x, - severity: alert.severity[0].toUpperCase() + alert.severity.slice(1), - name: alert.alertname, - namespace: alert.namespace, - layer: alert.layer, - component: alert.component, - nodata: groupedData[i][2] === 'nodata' ? true : false, - alertstate: isLastElement ? alert.alertstate : 'resolved', - silenced: alert.silenced, + startDate: new Date(intervalFirstTimestamp * 1000), + x: firstAlert.x, + severity: firstAlert.severity[0].toUpperCase() + firstAlert.severity.slice(1), + name: firstAlert.alertname, + namespace: firstAlert.namespace, + layer: firstAlert.layer, + component: firstAlert.component, + nodata: groupedData[i][2] === 'nodata', + alertstate: isLastElement ? lastAlert.alertstate : 'resolved', + silenced: firstAlert.silenced, fill: - alert.severity === 'critical' + firstAlert.severity === 'critical' ? barChartColorScheme.critical - : alert.severity === 'warning' + : firstAlert.severity === 'warning' ? barChartColorScheme.warning : barChartColorScheme.info, }); @@ -453,7 +509,6 @@ export function generateDateArray(days: number, currentTime: number): Array Date: Wed, 22 Apr 2026 17:44:23 +0200 Subject: [PATCH 2/2] feat: address coderabbit issues on incidents --- .../Incidents/processIncidents.spec.ts | 165 +++++++++++++ .../components/Incidents/processIncidents.ts | 11 +- web/src/components/Incidents/utils.spec.ts | 220 +++++++++++++++++- web/src/components/Incidents/utils.ts | 19 +- 4 files changed, 407 insertions(+), 8 deletions(-) diff --git a/web/src/components/Incidents/processIncidents.spec.ts b/web/src/components/Incidents/processIncidents.spec.ts index 111599fc7..c1c3eeddd 100644 --- a/web/src/components/Incidents/processIncidents.spec.ts +++ b/web/src/components/Incidents/processIncidents.spec.ts @@ -1017,6 +1017,171 @@ describe('getIncidents', () => { }); }); + describe('severityMetrics per-segment correctness', () => { + it('should assign correct metric to each segment when severity repeats (W→C→W)', () => { + const t1 = 1000; + const t2 = 1300; + const t3 = 1600; + const t4 = 1900; + const t5 = 2200; + const t6 = 2500; + + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'g1', + component: 'comp-warn-early', + src_alertname: 'AlertWarnEarly', + src_namespace: 'ns-early', + src_severity: 'warning', + }, + values: [ + [t1, '1'], + [t2, '1'], + ], + }, + { + metric: { + group_id: 'g1', + component: 'comp-crit', + src_alertname: 'AlertCrit', + src_namespace: 'ns-crit', + src_severity: 'critical', + }, + values: [ + [t3, '2'], + [t4, '2'], + ], + }, + { + metric: { + group_id: 'g1', + component: 'comp-warn-late', + src_alertname: 'AlertWarnLate', + src_namespace: 'ns-late', + src_severity: 'warning', + }, + values: [ + [t5, '1'], + [t6, '1'], + ], + }, + ]; + + const result = getIncidents(data); + expect(result).toHaveLength(3); + + const segments = [...result].sort((a, b) => a.values[0][0] - b.values[0][0]); + + expect(segments[0].metric.src_alertname).toBe('AlertWarnEarly'); + expect(segments[0].metric.src_namespace).toBe('ns-early'); + + expect(segments[1].metric.src_alertname).toBe('AlertCrit'); + + expect(segments[2].metric.src_alertname).toBe('AlertWarnLate'); + expect(segments[2].metric.src_namespace).toBe('ns-late'); + }); + + it('should not let later results overwrite earlier metric for same severity', () => { + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'g1', + component: 'first', + src_alertname: 'First', + src_namespace: 'ns-first', + src_severity: 'warning', + }, + values: [[1000, '1']], + }, + { + metric: { + group_id: 'g1', + component: 'middle', + src_alertname: 'Middle', + src_namespace: 'ns-middle', + src_severity: 'critical', + }, + values: [[1300, '2']], + }, + { + metric: { + group_id: 'g1', + component: 'second', + src_alertname: 'Second', + src_namespace: 'ns-second', + src_severity: 'warning', + }, + values: [[1600, '1']], + }, + ]; + + const result = getIncidents(data); + const segments = [...result].sort((a, b) => a.values[0][0] - b.values[0][0]); + + const firstWarning = segments[0]; + const secondWarning = segments[2]; + + expect(firstWarning.metric.src_alertname).not.toBe(secondWarning.metric.src_alertname); + expect(firstWarning.metric.src_alertname).toBe('First'); + expect(secondWarning.metric.src_alertname).toBe('Second'); + }); + + it('should handle four segments with two severity repetitions', () => { + const data: PrometheusResult[] = [ + { + metric: { + group_id: 'g1', + component: 'c1', + src_alertname: 'A1', + src_namespace: 'ns1', + src_severity: 'critical', + }, + values: [[1000, '2']], + }, + { + metric: { + group_id: 'g1', + component: 'c2', + src_alertname: 'A2', + src_namespace: 'ns2', + src_severity: 'warning', + }, + values: [[1300, '1']], + }, + { + metric: { + group_id: 'g1', + component: 'c3', + src_alertname: 'A3', + src_namespace: 'ns3', + src_severity: 'critical', + }, + values: [[1600, '2']], + }, + { + metric: { + group_id: 'g1', + component: 'c4', + src_alertname: 'A4', + src_namespace: 'ns4', + src_severity: 'warning', + }, + values: [[1900, '1']], + }, + ]; + + const result = getIncidents(data); + const segments = [...result].sort((a, b) => a.values[0][0] - b.values[0][0]); + + expect(segments).toHaveLength(4); + expect(segments[0].metric.src_alertname).toBe('A1'); + expect(segments[1].metric.src_alertname).toBe('A2'); + expect(segments[2].metric.src_alertname).toBe('A3'); + expect(segments[3].metric.src_alertname).toBe('A4'); + }); + }); + describe('silenced status handling', () => { it('should use silenced value from most recent timestamp when merging', () => { // Use same severity to avoid severity splitting; test focuses on silenced status diff --git a/web/src/components/Incidents/processIncidents.ts b/web/src/components/Incidents/processIncidents.ts index 46d1c9b76..40b416a8c 100644 --- a/web/src/components/Incidents/processIncidents.ts +++ b/web/src/components/Incidents/processIncidents.ts @@ -178,7 +178,7 @@ export function getIncidents( ): Array { const incidents = new Map(); // Track which metric labels correspond to each severity value per group_id - const severityMetrics = new Map>(); + const severityMetrics = new Map>(); for (const result of prometheusResults) { const groupId = result.metric.group_id; @@ -188,9 +188,13 @@ export function getIncidents( if (!severityMetrics.has(groupId)) { severityMetrics.set(groupId, new Map()); } + const metricsForGroup = severityMetrics.get(groupId); const severityValues = new Set(result.values.map((v) => v[1])); for (const sv of severityValues) { - severityMetrics.get(groupId).set(sv, result.metric); + if (!metricsForGroup.has(sv)) { + metricsForGroup.set(sv, []); + } + metricsForGroup.get(sv).push(result.metric); } } @@ -250,7 +254,8 @@ export function getIncidents( const segmentSeverity = segmentValues[0][1]; // uniform within a segment // Use the metric from the Prometheus result that contributed this severity, // preserving shared properties (componentList, silenced) from the merged incident - const severitySpecificMetric = groupSeverityMetrics?.get(segmentSeverity); + const metricsArray = groupSeverityMetrics?.get(segmentSeverity); + const severitySpecificMetric = metricsArray?.shift(); result.push({ metric: { diff --git a/web/src/components/Incidents/utils.spec.ts b/web/src/components/Incidents/utils.spec.ts index b0dcdf566..567377b80 100644 --- a/web/src/components/Incidents/utils.spec.ts +++ b/web/src/components/Incidents/utils.spec.ts @@ -1,11 +1,12 @@ import { + createAlertsChartBars, getCurrentTime, insertPaddingPointsForChart, removeTrailingPaddingFromSeveritySegments, roundDateToInterval, roundTimestampToFiveMinutes, } from './utils'; -import { Incident } from './model'; +import { Alert, Incident } from './model'; describe('getCurrentTime', () => { it('should return current time rounded down to 5-minute boundary', () => { @@ -626,3 +627,220 @@ describe('removeTrailingPaddingFromSeveritySegments', () => { expect(original.values).toHaveLength(3); }); }); + +describe('createAlertsChartBars - padding boundary gap detection', () => { + const makeAlert = ( + overrides: Partial & { values: Array<[number, string]>; firstTimestamp: number }, + ): Alert => ({ + alertname: 'TestAlert', + alertsStartFiring: 0, + alertsEndFiring: 0, + alertstate: 'firing', + component: 'test-component', + layer: 'compute', + name: 'TestAlert', + namespace: 'test-ns', + resolved: false, + severity: 'critical', + silenced: false, + x: 1, + ...overrides, + }); + + it('should detect gap between alerts whose padded values would overlap', () => { + // Alert A: real data at [1000, 1300]. Padded: [700, 1000, 1300, 1600] + // Alert B: real data at [2200, 2500]. Padded: [1900, 2200, 2500, 2800] + // Real gap: 2200 - 1300 = 900s (15 min) — should be detected + // Without fix: padded timestamps 1600 and 1900 are only 300s (5 min) apart — no gap + const alertA = makeAlert({ + values: [ + [700, '2'], + [1000, '2'], + [1300, '2'], + [1600, '2'], + ], + firstTimestamp: 700, + }); + + const alertB = makeAlert({ + values: [ + [1900, '2'], + [2200, '2'], + [2500, '2'], + [2800, '2'], + ], + firstTimestamp: 1900, + }); + + const bars = createAlertsChartBars([alertA, alertB]); + + const dataBars = bars.filter((b) => !b.nodata); + const nodataBars = bars.filter((b) => b.nodata); + + expect(dataBars.length).toBe(2); + expect(nodataBars.length).toBe(1); + }); + + it('should merge alerts whose real data is within 5 minutes of each other', () => { + // Alert A: real data at [1000, 1300]. Padded: [700, 1000, 1300, 1600] + // Alert B: real data at [1500, 1800]. Padded: [1200, 1500, 1800, 2100] + // Real gap: 1500 - 1300 = 200s (3.3 min) — should merge + const alertA = makeAlert({ + values: [ + [700, '2'], + [1000, '2'], + [1300, '2'], + [1600, '2'], + ], + firstTimestamp: 700, + }); + + const alertB = makeAlert({ + values: [ + [1200, '2'], + [1500, '2'], + [1800, '2'], + [2100, '2'], + ], + firstTimestamp: 1200, + }); + + const bars = createAlertsChartBars([alertA, alertB]); + + const dataBars = bars.filter((b) => !b.nodata); + expect(dataBars.length).toBe(1); + }); + + it('should handle alerts with only 2 values (single real point + padding)', () => { + // Alert with 2 values: [pad, real]. Only the last (real) value is used. + const alertA = makeAlert({ + values: [ + [700, '2'], + [1000, '2'], + ], + firstTimestamp: 700, + }); + + const alertB = makeAlert({ + values: [ + [1900, '2'], + [2200, '2'], + ], + firstTimestamp: 1900, + }); + + const bars = createAlertsChartBars([alertA, alertB]); + + const dataBars = bars.filter((b) => !b.nodata); + const nodataBars = bars.filter((b) => b.nodata); + + // Real gap: 2200 - 1000 = 1200s (20 min) > 5 min — should detect gap + expect(dataBars.length).toBe(2); + expect(nodataBars.length).toBe(1); + }); + + it('should preserve firstTimestamp per interval after gap detection', () => { + const alertA = makeAlert({ + values: [ + [700, '2'], + [1000, '2'], + [1300, '2'], + [1600, '2'], + ], + firstTimestamp: 500, + }); + + const alertB = makeAlert({ + values: [ + [1900, '2'], + [2200, '2'], + [2500, '2'], + [2800, '2'], + ], + firstTimestamp: 1700, + }); + + const bars = createAlertsChartBars([alertA, alertB]) as any[]; + + const dataBars = bars.filter((b) => !b.nodata); + expect(dataBars.length).toBe(2); + + const startDates = dataBars.map((b) => b.startDate.getTime() / 1000); + expect(startDates).toContain(500); + expect(startDates).toContain(1700); + }); + + it('should preserve last real data point for firing alerts without trailing padding', () => { + // Firing alert where currentTime < lastReal + 300, so no trailing padding was added. + // Padded values: [700, 1000, 1100, 1200] (leading pad only, no trailing). + // The old slice(1, -1) would drop 1200. The fix keeps it. + const alertA = makeAlert({ + values: [ + [700, '2'], + [1000, '2'], + [1100, '2'], + [1200, '2'], + ], + firstTimestamp: 700, + resolved: false, + }); + + // Second alert starts 20 minutes later — should be a separate interval + const alertB = makeAlert({ + values: [ + [2100, '2'], + [2400, '2'], + [2500, '2'], + [2600, '2'], + ], + firstTimestamp: 2100, + resolved: false, + }); + + const bars = createAlertsChartBars([alertA, alertB]); + + const dataBars = bars.filter((b) => !b.nodata); + expect(dataBars.length).toBe(2); + + // Verify the first interval ends at 1200 (last real point), not 1100 + const firstInterval = dataBars[0] as any; + expect(firstInterval.y.getTime() / 1000).toBe(1200); + }); + + it('should strip trailing padding from resolved alerts for accurate gap detection', () => { + // Resolved alert A: real data [1000, 1300], trailing pad at 1600 + // Resolved alert B: real data [1900, 2200], trailing pad at 2500 + // Real gap: 1900 - 1300 = 600s = 10 min — should be detected + const alertA = makeAlert({ + values: [ + [700, '2'], + [1000, '2'], + [1300, '2'], + [1600, '2'], + ], + firstTimestamp: 700, + resolved: true, + alertstate: 'resolved', + }); + + const alertB = makeAlert({ + values: [ + [1600, '2'], + [1900, '2'], + [2200, '2'], + [2500, '2'], + ], + firstTimestamp: 1600, + resolved: true, + alertstate: 'resolved', + }); + + const bars = createAlertsChartBars([alertA, alertB]); + + const dataBars = bars.filter((b) => !b.nodata); + const nodataBars = bars.filter((b) => b.nodata); + + expect(dataBars.length).toBe(2); + expect(nodataBars.length).toBe(1); + }); +}); diff --git a/web/src/components/Incidents/utils.ts b/web/src/components/Incidents/utils.ts index e05305986..a3b202779 100644 --- a/web/src/components/Incidents/utils.ts +++ b/web/src/components/Incidents/utils.ts @@ -381,10 +381,21 @@ export const createIncidentsChartBars = (incidents: Incident[], dateArray: SpanD function consolidateAndMergeAlertIntervals(alerts: Array): Array { if (alerts.length === 0) return []; - // Tag each value with its source alert's firstTimestamp, then sort - const taggedValues = alerts.flatMap((alert) => - alert.values.map((v) => ({ timestamp: v[0], firstTimestamp: alert.firstTimestamp })), - ); + // Strip boundary padding from insertPaddingPointsForChart so gap detection + // uses real data timestamps and doesn't merge across alert boundaries. + // Leading padding (index 0) is always present. Trailing padding (last index) + // is only guaranteed for resolved alerts (last data point >= 600s old, so the + // >= 300s condition for after-padding is always met). Firing alerts may lack + // trailing padding when their last data point is very recent, so keep the + // last value to avoid dropping real data. + const taggedValues = alerts.flatMap((alert) => { + const values = alert.values; + if (values.length <= 1) { + return values.map((v) => ({ timestamp: v[0], firstTimestamp: alert.firstTimestamp })); + } + const coreValues = values.length >= 3 && alert.resolved ? values.slice(1, -1) : values.slice(1); + return coreValues.map((v) => ({ timestamp: v[0], firstTimestamp: alert.firstTimestamp })); + }); if (taggedValues.length === 0) return [];