Skip to content
15 changes: 13 additions & 2 deletions web/src/components/Incidents/AlertsChart/AlertsChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, typeof alertsData>();
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(() => {
Expand Down Expand Up @@ -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'
? '---'
Expand Down
27 changes: 22 additions & 5 deletions web/src/components/Incidents/IncidentsChart/IncidentsChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
calculateIncidentsChartDomain,
createIncidentsChartBars,
generateDateArray,
removeTrailingPaddingFromSeveritySegments,
roundDateToInterval,
} from '../utils';
import { dateTimeFormatter, timeFormatter } from '../../console/utils/datetime';
Expand Down Expand Up @@ -89,10 +90,27 @@ 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<string, typeof filteredIncidents>();
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));
chartBars.sort((a, b) => a[0].x - b[0].x);

// Reassign consecutive x values to eliminate gaps between bars
Expand All @@ -102,7 +120,6 @@ const IncidentsChart = ({
useEffect(() => {
setIsLoading(false);
}, [incidentsData]);

useEffect(() => {
setChartContainerHeight(chartData?.length < 5 ? 300 : chartData?.length * 60);
setChartHeight(chartData?.length < 5 ? 250 : chartData?.length * 55);
Expand Down Expand Up @@ -176,7 +193,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(
Expand Down
25 changes: 19 additions & 6 deletions web/src/components/Incidents/IncidentsDetailsRowTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<Tr key={rowIndex}>
Expand All @@ -45,13 +46,25 @@ const IncidentsDetailsRowTable = ({ alerts }: IncidentsDetailsRowTableProps) =>
<SeverityBadge severity={alertDetails.severity} />
</Td>
<Td dataLabel="expanded-details-firingstart">
<Timestamp timestamp={alertDetails.alertsStartFiring * 1000} />
<Timestamp
timestamp={
(alertDetails.firstTimestamp > 0
? alertDetails.firstTimestamp
: alertDetails.alertsStartFiring) * 1000
}
/>
</Td>
<Td dataLabel="expanded-details-firingend">
{!alertDetails.resolved ? (
'---'
) : (
<Timestamp timestamp={alertDetails.alertsEndFiring * 1000} />
<Timestamp
timestamp={
(alertDetails.lastTimestamp > 0
? alertDetails.lastTimestamp
: alertDetails.alertsEndFiring) * 1000
}
/>
)}
</Td>
<Td dataLabel="expanded-details-alertstate">
Expand Down
105 changes: 59 additions & 46 deletions web/src/components/Incidents/IncidentsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
onIncidentFiltersSelect,
parseUrlParams,
updateBrowserUrl,
DAY_MS,
} from './utils';
import { groupAlertsForTable, convertToAlerts } from './processAlerts';
import { CompressArrowsAltIcon, CompressIcon, FilterIcon } from '@patternfly/react-icons';
Expand Down Expand Up @@ -229,49 +230,57 @@ 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,
);
// Guard: don't process if no incidents selected or timeRanges not ready
if (incidentForAlertProcessing.length === 0 || timeRanges.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);
});
})();
}, [incidentForAlertProcessing]);
}
if (!isEmpty(filteredData)) {
dispatch(setAlertsAreLoading({ alertsAreLoading: false }));
} else {
dispatch(setAlertsAreLoading({ alertsAreLoading: true }));
}
})
.catch((err) => {
// eslint-disable-next-line no-console
console.error(err);
});
}, [incidentForAlertProcessing, timeRanges, rules]);
Comment on lines 232 to +283
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

daysSpan used inside effect but missing from dependency array — stale closure risk.

Line 259 passes daysSpan to convertToAlerts, but daysSpan is not listed in the dependency array at line 283 ([incidentForAlertProcessing, timeRanges, rules]). When the user changes the days filter, daysSpan updates via a separate useEffect (line 222), but this effect won't re-run for that change alone. The captured daysSpan in the closure may be stale, causing alerts to be filtered against the wrong time window.

Proposed fix
-  }, [incidentForAlertProcessing, timeRanges, rules]);
+  }, [incidentForAlertProcessing, timeRanges, rules, daysSpan]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/Incidents/IncidentsPage.tsx` around lines 232 - 283, The
effect calls convertToAlerts(..., daysSpan) but daysSpan is not in the useEffect
dependency array, causing a stale closure; update the dependency array for the
useEffect that contains convertToAlerts (the effect that maps fetchTimeRanges
and dispatches setAlertsData / setAlertsTableData) to include daysSpan so the
effect re-runs when the days filter changes.


useEffect(() => {
if (!isInitialized) return;
Expand All @@ -287,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,
}),
);

Expand Down
2 changes: 1 addition & 1 deletion web/src/components/Incidents/IncidentsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
6 changes: 5 additions & 1 deletion web/src/components/Incidents/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export type Timestamps = [number, string];

export type SpanDates = Array<number>;

export type AlertsIntervalsArray = [number, number, 'data' | 'nodata'];
export type AlertsIntervalsArray = [number, number, 'data' | 'nodata', number?];

export type Incident = {
component: string;
Expand All @@ -15,10 +15,12 @@ export type Incident = {
src_severity: string;
src_alertname: string;
src_namespace: string;
severity: any;
silenced: boolean;
x: number;
values: Array<Timestamps>;
metric: Metric;
firstTimestamp: number;
};

// Define the interface for Metric
Expand Down Expand Up @@ -47,6 +49,7 @@ export type Alert = {
severity: Severity;
silenced: boolean;
x: number;
firstTimestamp: number;
values: Array<Timestamps>;
alertsExpandedRowData?: Array<Alert>;
};
Expand Down Expand Up @@ -101,6 +104,7 @@ export type IncidentsDetailsAlert = {
resolved: boolean;
severity: Severity;
x: number;
firstTimestamp: number;
values: Array<Timestamps>;
silenced: boolean;
rule: {
Expand Down
Loading