Skip to content
7 changes: 6 additions & 1 deletion web/src/components/Incidents/AlertsChart/AlertsChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
generateAlertsDateArray,
getCurrentTime,
roundDateToInterval,
roundTimestampToFiveMinutes,
} from '../utils';
import { dateTimeFormatter, timeFormatter } from '../../console/utils/datetime';
import { useTranslation } from 'react-i18next';
Expand Down Expand Up @@ -161,7 +162,11 @@ 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(
new Date(
roundTimestampToFiveMinutes(datum.startDate.getTime() / 1000) * 1000,
),
);
const endDate =
datum.alertstate === 'firing'
? '---'
Expand Down
26 changes: 24 additions & 2 deletions web/src/components/Incidents/IncidentsChart/IncidentsChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ import {
} from '@patternfly/react-tokens';
import '../incidents-styles.css';
import { IncidentsTooltip } from '../IncidentsTooltip';
import { Incident } from '../model';
import { Incident, IncidentsTimestamps } from '../model';
import {
calculateIncidentsChartDomain,
createIncidentsChartBars,
generateDateArray,
matchTimestampMetricForIncident,
roundDateToInterval,
roundTimestampToFiveMinutes,
} from '../utils';
import { dateTimeFormatter, timeFormatter } from '../../console/utils/datetime';
import { useTranslation } from 'react-i18next';
Expand All @@ -57,6 +59,7 @@ const formatComponentList = (componentList: string[] | undefined): string => {

const IncidentsChart = ({
incidentsData,
incidentsTimestamps,
chartDays,
theme,
selectedGroupId,
Expand All @@ -65,6 +68,7 @@ const IncidentsChart = ({
lastRefreshTime,
}: {
incidentsData: Array<Incident>;
incidentsTimestamps: IncidentsTimestamps;
chartDays: number;
theme: 'light' | 'dark';
selectedGroupId: string;
Expand All @@ -80,6 +84,20 @@ const IncidentsChart = ({
[chartDays, currentTime],
);

// enrich incidentsData with first_timestamp from timestamp metric
incidentsData = incidentsData.map((incident) => {
Comment thread
PeterYurkovich marked this conversation as resolved.
Outdated
// find the matched timestamp for the incident
const matchedMinTimestamp = matchTimestampMetricForIncident(
incident,
incidentsTimestamps.minOverTime,
);

return {
...incident,
firstTimestamp: parseInt(matchedMinTimestamp?.value?.[1] ?? '0'),
};
});

const { t, i18n } = useTranslation(process.env.I18N_NAMESPACE);

const chartData = useMemo(() => {
Expand Down Expand Up @@ -176,7 +194,11 @@ const IncidentsChart = ({
if (datum.nodata) {
return '';
}
const startDate = dateTimeFormatter(i18n.language).format(new Date(datum.y0));
const startDate = dateTimeFormatter(i18n.language).format(
new Date(
roundTimestampToFiveMinutes(datum.startDate.getTime() / 1000) * 1000,
),
);
const endDate = datum.firing
? '---'
: dateTimeFormatter(i18n.language).format(
Expand Down
28 changes: 22 additions & 6 deletions web/src/components/Incidents/IncidentsDetailsRowTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { Alert, IncidentsDetailsAlert } from './model';
import { IncidentAlertStateIcon } from './IncidentAlertStateIcon';
import { useMemo } from 'react';
import { DataTestIDs } from '../data-test';
import { roundTimestampToFiveMinutes } from './utils';

interface IncidentsDetailsRowTableProps {
alerts: Alert[];
Expand All @@ -24,10 +25,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 +47,27 @@ const IncidentsDetailsRowTable = ({ alerts }: IncidentsDetailsRowTableProps) =>
<SeverityBadge severity={alertDetails.severity} />
</Td>
<Td dataLabel="expanded-details-firingstart">
<Timestamp timestamp={alertDetails.alertsStartFiring * 1000} />
<Timestamp
timestamp={
roundTimestampToFiveMinutes(
alertDetails.firstTimestamp > 0
? alertDetails.firstTimestamp
: alertDetails.alertsStartFiring,
) * 1000
}
/>
</Td>
<Td dataLabel="expanded-details-firingend">
{!alertDetails.resolved ? (
'---'
) : (
<Timestamp timestamp={alertDetails.alertsEndFiring * 1000} />
<Timestamp
Comment thread
PeterYurkovich marked this conversation as resolved.
timestamp={
(alertDetails.lastTimestamp > 0
? alertDetails.lastTimestamp
: alertDetails.alertsEndFiring) * 1000
}
/>
)}
</Td>
<Td dataLabel="expanded-details-alertstate">
Expand Down
147 changes: 101 additions & 46 deletions web/src/components/Incidents/IncidentsPage.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable react-hooks/exhaustive-deps */
import { useMemo, useState, useEffect, useCallback } from 'react';
import { useSafeFetch } from '../console/utils/safe-fetch-hook';
import { createAlertsQuery, fetchDataForIncidentsAndAlerts } from './api';
import { createAlertsQuery, fetchDataForIncidentsAndAlerts, fetchInstantData } from './api';
import { useTranslation } from 'react-i18next';
import {
Bullseye,
Expand Down Expand Up @@ -46,7 +46,9 @@ import {
setAlertsAreLoading,
setAlertsData,
setAlertsTableData,
setAlertsTimestamps,
setFilteredIncidentsData,
setIncidentsTimestamps,
setIncidentPageFilterType,
setIncidents,
setIncidentsActiveFilters,
Expand Down Expand Up @@ -146,6 +148,10 @@ const IncidentsPage = () => {
(state: MonitoringState) => state.plugins.mcp.incidentsData.filteredIncidentsData,
);

const incidentsTimestamps = useSelector(
(state: MonitoringState) => state.plugins.mcp.incidentsData.incidentsTimestamps,
);

const selectedGroupId = incidentsActiveFilters.groupId?.[0] ?? undefined;

const incidentPageFilterTypeSelected = useSelector(
Expand Down Expand Up @@ -229,49 +235,74 @@ 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;

// Fetch timestamps and alerts in parallel, but wait for both before processing
const timestampsPromise = Promise.all(
Comment thread
PeterYurkovich marked this conversation as resolved.
Outdated
['min_over_time(timestamp(ALERTS{alertstate="firing"})[15d:5m])'].map(async (query) => {
const response = await fetchInstantData(safeFetch, query);
return response.data.result;
}),
);

const alertsPromise = Promise.all(
timeRanges.map(async (range) => {
const response = await fetchDataForIncidentsAndAlerts(
safeFetch,
range,
createAlertsQuery(incidentForAlertProcessing),
);
return response.data.result;
}),
);

Promise.all([timestampsPromise, alertsPromise])
.then(([timestampsResults, alertsResults]) => {
// Dispatch timestamps to store
const fetchedAlertsTimestamps = {
minOverTime: timestampsResults[0],
};
dispatch(
setAlertsTimestamps({
alertsTimestamps: fetchedAlertsTimestamps,
}),
);

const prometheusResults = alertsResults.flat();
const alerts = convertToAlerts(
prometheusResults,
incidentForAlertProcessing,
currentTime,
fetchedAlertsTimestamps,
);
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 }));
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
})
.catch((err) => {
// eslint-disable-next-line no-console
console.log(err);
Comment thread
PeterYurkovich marked this conversation as resolved.
Outdated
});
}, [incidentForAlertProcessing, timeRanges, rules]);
Comment on lines 232 to +283
Copy link
Copy Markdown

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 @@ -294,14 +325,34 @@ const IncidentsPage = () => {
? `cluster_health_components_map{group_id='${selectedGroupId}'}`
: 'cluster_health_components_map';

Promise.all(
// Fetch timestamps and incidents in parallel, but wait for both before processing
const timestampsPromise = Promise.all(
Comment thread
PeterYurkovich marked this conversation as resolved.
Outdated
['min_over_time(timestamp(cluster_health_components_map)[15d:5m])'].map(async (query) => {
Comment thread
PeterYurkovich marked this conversation as resolved.
Outdated
const response = await fetchInstantData(safeFetch, query);
return response.data.result;
}),
);

const incidentsPromise = Promise.all(
calculatedTimeRanges.map(async (range) => {
const response = await fetchDataForIncidentsAndAlerts(safeFetch, range, incidentsQuery);
return response.data.result;
}),
)
.then((results) => {
const prometheusResults = results.flat();
);

Promise.all([timestampsPromise, incidentsPromise])
.then(([timestampsResults, incidentsResults]) => {
// Dispatch timestamps to store
const fetchedTimestamps = {
minOverTime: timestampsResults[0],
};
dispatch(
setIncidentsTimestamps({
incidentsTimestamps: fetchedTimestamps,
}),
);

const prometheusResults = incidentsResults.flat();
const incidents = convertToIncidents(prometheusResults, currentTime);

// Update the raw, unfiltered incidents state
Expand All @@ -317,7 +368,10 @@ const IncidentsPage = () => {
setIncidentsAreLoading(false);

if (isGroupSelected) {
setIncidentForAlertProcessing(processIncidentsForAlerts(prometheusResults));
// Use fetchedTimestamps directly instead of stale closure value
setIncidentForAlertProcessing(
processIncidentsForAlerts(prometheusResults, fetchedTimestamps),
);
dispatch(setAlertsAreLoading({ alertsAreLoading: true }));
} else {
closeDropDownFilters();
Expand Down Expand Up @@ -638,6 +692,7 @@ const IncidentsPage = () => {
<StackItem>
<IncidentsChart
incidentsData={filteredData}
incidentsTimestamps={incidentsTimestamps}
chartDays={timeRanges.length}
theme={theme}
selectedGroupId={selectedGroupId}
Expand Down
7 changes: 5 additions & 2 deletions web/src/components/Incidents/IncidentsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { GroupedAlertStateIcon } from './IncidentAlertStateIcon';

import { GroupedAlert } from './model';
import { DataTestIDs } from '../data-test';
import { roundTimestampToFiveMinutes } from './utils';

export const IncidentsTable = () => {
const { t } = useTranslation(process.env.I18N_NAMESPACE);
Expand Down Expand Up @@ -95,7 +96,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 Expand Up @@ -180,7 +181,9 @@ export const IncidentsTable = () => {
)}
</Td>
<Td dataLabel={columnNames.startDate}>
<Timestamp timestamp={getMinStartDate(alert) * 1000} />
<Timestamp
timestamp={roundTimestampToFiveMinutes(getMinStartDate(alert)) * 1000}
Comment thread
PeterYurkovich marked this conversation as resolved.
Outdated
/>
</Td>
<Td
dataLabel={columnNames.state}
Expand Down
Loading