Generalize alert notifications

brave · Dec 6, 2024 · 224c5fc · 224c5fc
1 parent f3f88bd
commit 224c5fc
Show file tree

Hide file tree

Showing 3 changed files with 114 additions and 56 deletions.
diff --git a/src/config.js b/src/config.js
@@ -33,18 +33,49 @@ module.exports = {
 
   channelId: 'C05S50MFHPE', // #finch-updates
 
-  // Add your slack ID to get notifications about new kill switches.
+  // Add your slack ID to get alerts about changing features.
+  // Please note that alerts are only sent for changes in stable channel,
+  // not in beta or dev.
   // To retrive it use Slack profile => Copy Member ID.
-  killSwitchNotificationIds: [
-    'U02DG0ATML3', // @matuchin
-    'UE87NRK2A', // @iefremov
-    'UB9PF4X5K', // @Terry
-  ],
-  processingErrorNotificationIds: [
-    'U02DG0ATML3', // @matuchin
-    'UE87NRK2A', // @iefremov
-  ],
-  gpuRelatedNotificationIds: [
-    'U0D73ULKD', // @serg
+  alerts: [
+    {
+      description: 'Kill switches changes detected',
+      killSwitch: true, // matches to any kill switch change
+      ids: [
+        'U02DG0ATML3', // @matuchin
+        'UE87NRK2A', // @iefremov
+        'UB9PF4X5K', // @Terry
+      ],
+    },
+    {
+      description: ':x: Processing errors detected',
+      processingError: true, // matches to any processing error
+      ids: [
+        'U02DG0ATML3', // @matuchin
+        'UE87NRK2A', // @iefremov
+      ],
+    },
+    {
+      description: 'GPU related changes detected',
+      features: [
+        'DefaultANGLEVulkan',
+        'DefaultPassthroughCommandDecoder',
+        'EnableDrDcVulkan',
+        'Vulkan',
+        'VulkanFromANGLE',
+        'VulkanV2',
+        'VulkanVMALargeHeapBlockSizeExperiment',
+      ],
+      ids: [
+        'U0D73ULKD', // @serg
+      ],
+    },
+    {
+      description: 'WebUSBBlocklist changes detected',
+      features: ['WebUSBBlocklist'],
+      ids: [
+        'U02031KK8SY', // @shivan
+      ],
+    },
   ],
 };
diff --git a/src/core/summary.ts b/src/core/summary.ts
@@ -270,14 +270,21 @@ class MrkdwnMessage {
   }
 }
 
+class Alert {
+  description: string;
+  ids: string[];
+  killSwitch?: boolean;
+  processingError?: boolean;
+  features?: string[];
+}
+
 export function summaryToJson(
   summary: Map<StudyPriority, SummaryItem[]>,
   newGitSha1?: string,
 ): string | undefined {
   const output = new MrkdwnMessage();
-  let hasKillSwitchImportantUpdate = false;
-  let hasBadStudies = false;
-  let gpuRelatedFeaturesDetected = false;
+
+  const alerts = new Set<Alert>();
 
   // From the highest to the lowest priority:
   const priorityList = Object.values(StudyPriority)
@@ -296,13 +303,21 @@ export function summaryToJson(
     itemList.sort((a, b) => a.action - b.action);
     for (const e of itemList) {
       for (const f of e.affectedFeatures) {
-        if (config.gpuRelatedFeatures.includes(f)) {
-          gpuRelatedFeaturesDetected = true;
-          break;
-        }
+        config.alerts.forEach((alert) => {
+          if (alert.features?.includes(f)) {
+            alerts.add(alert);
+          }
+        });
       }
-      hasKillSwitchImportantUpdate ||= e.isKillSwitchImportantUpdate();
-      hasBadStudies ||= e.hasBadStudies;
+      config.alerts.forEach((alert) => {
+        if (alert.killSwitch && e.isKillSwitchImportantUpdate()) {
+          alerts.add(alert);
+        }
+        if (alert.processingError && e.hasBadStudies) {
+          alerts.add(alert);
+        }
+      });
+
       const block = new TextBlock(e.actionToText());
       block.addLink(url_utils.getGriffinUiUrl(e.studyName), e.studyName);
       block.addLink(
@@ -337,31 +352,13 @@ export function summaryToJson(
   }
   if (output.toString() === '') return undefined;
 
-  if (gpuRelatedFeaturesDetected) {
+  alerts.forEach((alert) => {
     output.addBlockToTop(
       new TextBlock(
-        'GPU related changes detected, cc ' +
-          config.gpuRelatedNotificationIds.map((i) => `<@${i}>`).join(),
+        alert.description + ', cc ' + alert.ids.map((i) => `<@${i}>`).join(),
       ),
     );
-  }
-
-  if (hasKillSwitchImportantUpdate) {
-    output.addBlockToTop(
-      new TextBlock(
-        'Kill switches changes detected, cc ' +
-          config.killSwitchNotificationIds.map((i) => `<@${i}>`).join(),
-      ),
-    );
-  }
-  if (hasBadStudies) {
-    output.addBlock(
-      new TextBlock(
-        ':x: Processing ERRORS detected.\\n cc ' +
-          config.processingErrorNotificationIds.map((i) => `<@${i}>`).join(),
-      ),
-    );
-  }
+  });
   return `{
     "channel" : "${config.channelId}",
     "text": "New finch changes detected",

diff --git a/src/finch_tracker/tracker_lib.test.ts b/src/finch_tracker/tracker_lib.test.ts
@@ -57,8 +57,7 @@ test('seed serialization', async () => {
 });
 
 describe('summary', () => {
-  const common = {
-    name: 'TestStudy',
+  const commonFilters = {
     filter: {
       channel: [Study_Channel.STABLE],
       platform: [Study_Platform.WINDOWS],
@@ -67,7 +66,7 @@ describe('summary', () => {
     },
   };
   const oldStudy = Study.create({
-    ...common,
+    name: 'TestStudy',
     experiment: [
       {
         name: 'Enabled',
@@ -79,10 +78,11 @@ describe('summary', () => {
         probability_weight: 80,
       },
     ],
+    ...commonFilters,
   });
 
   const newStudy = Study.create({
-    ...common,
+    name: 'TestStudy',
     experiment: [
       {
         name: 'Enabled',
@@ -92,16 +92,28 @@ describe('summary', () => {
       {
         name: 'DisableAnother',
         probability_weight: 70,
-        feature_association: { disable_feature: ['BadFeature'] },
+        feature_association: { disable_feature: ['WebUSBBlocklist'] },
       },
       {
         name: 'Default',
         probability_weight: 10,
       },
     ],
+    ...commonFilters,
+  });
+  const killSwitchStudy = Study.create({
+    name: 'StudyBrokenFeature_KillSwitch',
+    experiment: [
+      {
+        name: 'BrokenFeature_KillSwitch',
+        probability_weight: 100,
+        feature_association: { disable_feature: ['BrokenFeature'] },
+      },
+    ],
+    ...commonFilters,
   });
   const oldSeed = VariationsSeed.create({ study: [oldStudy] });
-  const newSeed = VariationsSeed.create({ study: [newStudy] });
+  const newSeed = VariationsSeed.create({ study: [newStudy, killSwitchStudy] });
 
   const summary = makeSummary(
     oldSeed,
@@ -111,7 +123,7 @@ describe('summary', () => {
   );
 
   test('verify content', () => {
-    expect(summary.size).toBe(1);
+    expect(summary.size).toBe(2);
     const itemList = summary.get(StudyPriority.STABLE_ALL);
     expect(itemList?.length).toBe(1);
     const item = itemList?.at(0);
@@ -127,16 +139,34 @@ describe('summary', () => {
 
     expect(item?.affectedFeatures.size).toBe(2);
     expect(item?.affectedFeatures).toContain('GoodFeature');
-    expect(item?.affectedFeatures).toContain('BadFeature');
+    expect(item?.affectedFeatures).toContain('WebUSBBlocklist');
+
+    const killSwitchItemList = summary.get(
+      StudyPriority.STABLE_EMERGENCY_KILL_SWITCH,
+    );
+    expect(killSwitchItemList?.length).toBe(1);
+    const killSwitchItem = killSwitchItemList?.at(0);
+
+    expect(killSwitchItem?.studyName).toBe('StudyBrokenFeature_KillSwitch');
+    expect(killSwitchItem?.affectedFeatures.size).toBe(1);
+    expect(killSwitchItem?.affectedFeatures).toContain('BrokenFeature');
+    expect(killSwitchItem?.newPriority).toBe(
+      StudyPriority.STABLE_EMERGENCY_KILL_SWITCH,
+    );
+    expect(killSwitchItem?.action).toBe(ItemAction.New);
   });
 
-  test('serialization', () => {
-    const payloadJSON = summaryToJson(summary, undefined);
-    expect(payloadJSON).toBeDefined();
-    const payload =
-      payloadJSON !== undefined ? JSON.parse(payloadJSON) : undefined;
+  test('summaryToSlackJson', () => {
+    const payloadString = summaryToJson(summary, undefined);
+    expect(payloadString).toBeDefined();
+
+    expect(payloadString).toContain(
+      'Kill switches changes detected, cc <@U02DG0ATML3>',
+    );
+
+    expect(payloadString).toContain('WebUSBBlocklist changes detected');
 
     // Check that payload is valid JSON.
-    expect(payload).toBeInstanceOf(Object);
+    expect(JSON.parse(payloadString!)).toBeInstanceOf(Object);
   });
 });