Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize finch tracker alerts #1278

Merged
merged 1 commit into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 43 additions & 12 deletions src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
],
},
],
};
57 changes: 27 additions & 30 deletions src/core/summary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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(
Expand Down Expand Up @@ -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",
Expand Down
58 changes: 44 additions & 14 deletions src/finch_tracker/tracker_lib.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -67,7 +66,7 @@ describe('summary', () => {
},
};
const oldStudy = Study.create({
...common,
name: 'TestStudy',
experiment: [
{
name: 'Enabled',
Expand All @@ -79,10 +78,11 @@ describe('summary', () => {
probability_weight: 80,
},
],
...commonFilters,
});

const newStudy = Study.create({
...common,
name: 'TestStudy',
experiment: [
{
name: 'Enabled',
Expand All @@ -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,
Expand All @@ -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);
Expand All @@ -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);
});
});
Loading