Skip to content

Commit 289237d

Browse files
committed
feat: address coderabbit issues on incidents
1 parent d9fe8a0 commit 289237d

4 files changed

Lines changed: 407 additions & 8 deletions

File tree

web/src/components/Incidents/processIncidents.spec.ts

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,6 +1017,171 @@ describe('getIncidents', () => {
10171017
});
10181018
});
10191019

1020+
describe('severityMetrics per-segment correctness', () => {
1021+
it('should assign correct metric to each segment when severity repeats (W→C→W)', () => {
1022+
const t1 = 1000;
1023+
const t2 = 1300;
1024+
const t3 = 1600;
1025+
const t4 = 1900;
1026+
const t5 = 2200;
1027+
const t6 = 2500;
1028+
1029+
const data: PrometheusResult[] = [
1030+
{
1031+
metric: {
1032+
group_id: 'g1',
1033+
component: 'comp-warn-early',
1034+
src_alertname: 'AlertWarnEarly',
1035+
src_namespace: 'ns-early',
1036+
src_severity: 'warning',
1037+
},
1038+
values: [
1039+
[t1, '1'],
1040+
[t2, '1'],
1041+
],
1042+
},
1043+
{
1044+
metric: {
1045+
group_id: 'g1',
1046+
component: 'comp-crit',
1047+
src_alertname: 'AlertCrit',
1048+
src_namespace: 'ns-crit',
1049+
src_severity: 'critical',
1050+
},
1051+
values: [
1052+
[t3, '2'],
1053+
[t4, '2'],
1054+
],
1055+
},
1056+
{
1057+
metric: {
1058+
group_id: 'g1',
1059+
component: 'comp-warn-late',
1060+
src_alertname: 'AlertWarnLate',
1061+
src_namespace: 'ns-late',
1062+
src_severity: 'warning',
1063+
},
1064+
values: [
1065+
[t5, '1'],
1066+
[t6, '1'],
1067+
],
1068+
},
1069+
];
1070+
1071+
const result = getIncidents(data);
1072+
expect(result).toHaveLength(3);
1073+
1074+
const segments = [...result].sort((a, b) => a.values[0][0] - b.values[0][0]);
1075+
1076+
expect(segments[0].metric.src_alertname).toBe('AlertWarnEarly');
1077+
expect(segments[0].metric.src_namespace).toBe('ns-early');
1078+
1079+
expect(segments[1].metric.src_alertname).toBe('AlertCrit');
1080+
1081+
expect(segments[2].metric.src_alertname).toBe('AlertWarnLate');
1082+
expect(segments[2].metric.src_namespace).toBe('ns-late');
1083+
});
1084+
1085+
it('should not let later results overwrite earlier metric for same severity', () => {
1086+
const data: PrometheusResult[] = [
1087+
{
1088+
metric: {
1089+
group_id: 'g1',
1090+
component: 'first',
1091+
src_alertname: 'First',
1092+
src_namespace: 'ns-first',
1093+
src_severity: 'warning',
1094+
},
1095+
values: [[1000, '1']],
1096+
},
1097+
{
1098+
metric: {
1099+
group_id: 'g1',
1100+
component: 'middle',
1101+
src_alertname: 'Middle',
1102+
src_namespace: 'ns-middle',
1103+
src_severity: 'critical',
1104+
},
1105+
values: [[1300, '2']],
1106+
},
1107+
{
1108+
metric: {
1109+
group_id: 'g1',
1110+
component: 'second',
1111+
src_alertname: 'Second',
1112+
src_namespace: 'ns-second',
1113+
src_severity: 'warning',
1114+
},
1115+
values: [[1600, '1']],
1116+
},
1117+
];
1118+
1119+
const result = getIncidents(data);
1120+
const segments = [...result].sort((a, b) => a.values[0][0] - b.values[0][0]);
1121+
1122+
const firstWarning = segments[0];
1123+
const secondWarning = segments[2];
1124+
1125+
expect(firstWarning.metric.src_alertname).not.toBe(secondWarning.metric.src_alertname);
1126+
expect(firstWarning.metric.src_alertname).toBe('First');
1127+
expect(secondWarning.metric.src_alertname).toBe('Second');
1128+
});
1129+
1130+
it('should handle four segments with two severity repetitions', () => {
1131+
const data: PrometheusResult[] = [
1132+
{
1133+
metric: {
1134+
group_id: 'g1',
1135+
component: 'c1',
1136+
src_alertname: 'A1',
1137+
src_namespace: 'ns1',
1138+
src_severity: 'critical',
1139+
},
1140+
values: [[1000, '2']],
1141+
},
1142+
{
1143+
metric: {
1144+
group_id: 'g1',
1145+
component: 'c2',
1146+
src_alertname: 'A2',
1147+
src_namespace: 'ns2',
1148+
src_severity: 'warning',
1149+
},
1150+
values: [[1300, '1']],
1151+
},
1152+
{
1153+
metric: {
1154+
group_id: 'g1',
1155+
component: 'c3',
1156+
src_alertname: 'A3',
1157+
src_namespace: 'ns3',
1158+
src_severity: 'critical',
1159+
},
1160+
values: [[1600, '2']],
1161+
},
1162+
{
1163+
metric: {
1164+
group_id: 'g1',
1165+
component: 'c4',
1166+
src_alertname: 'A4',
1167+
src_namespace: 'ns4',
1168+
src_severity: 'warning',
1169+
},
1170+
values: [[1900, '1']],
1171+
},
1172+
];
1173+
1174+
const result = getIncidents(data);
1175+
const segments = [...result].sort((a, b) => a.values[0][0] - b.values[0][0]);
1176+
1177+
expect(segments).toHaveLength(4);
1178+
expect(segments[0].metric.src_alertname).toBe('A1');
1179+
expect(segments[1].metric.src_alertname).toBe('A2');
1180+
expect(segments[2].metric.src_alertname).toBe('A3');
1181+
expect(segments[3].metric.src_alertname).toBe('A4');
1182+
});
1183+
});
1184+
10201185
describe('silenced status handling', () => {
10211186
it('should use silenced value from most recent timestamp when merging', () => {
10221187
// Use same severity to avoid severity splitting; test focuses on silenced status

web/src/components/Incidents/processIncidents.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ export function getIncidents(
178178
): Array<PrometheusResult & { metric: Metric }> {
179179
const incidents = new Map<string, PrometheusResult & { metric: Metric }>();
180180
// Track which metric labels correspond to each severity value per group_id
181-
const severityMetrics = new Map<string, Map<string, PrometheusLabels>>();
181+
const severityMetrics = new Map<string, Map<string, PrometheusLabels[]>>();
182182

183183
for (const result of prometheusResults) {
184184
const groupId = result.metric.group_id;
@@ -188,9 +188,13 @@ export function getIncidents(
188188
if (!severityMetrics.has(groupId)) {
189189
severityMetrics.set(groupId, new Map());
190190
}
191+
const metricsForGroup = severityMetrics.get(groupId);
191192
const severityValues = new Set(result.values.map((v) => v[1]));
192193
for (const sv of severityValues) {
193-
severityMetrics.get(groupId).set(sv, result.metric);
194+
if (!metricsForGroup.has(sv)) {
195+
metricsForGroup.set(sv, []);
196+
}
197+
metricsForGroup.get(sv).push(result.metric);
194198
}
195199
}
196200

@@ -250,7 +254,8 @@ export function getIncidents(
250254
const segmentSeverity = segmentValues[0][1]; // uniform within a segment
251255
// Use the metric from the Prometheus result that contributed this severity,
252256
// preserving shared properties (componentList, silenced) from the merged incident
253-
const severitySpecificMetric = groupSeverityMetrics?.get(segmentSeverity);
257+
const metricsArray = groupSeverityMetrics?.get(segmentSeverity);
258+
const severitySpecificMetric = metricsArray?.shift();
254259

255260
result.push({
256261
metric: {

0 commit comments

Comments
 (0)