Skip to content

Commit

Permalink
For slack triage notifications, use business hours left to notify hou…
Browse files Browse the repository at this point in the history 
…rly (#735)

* use business hours left for act fact queue
  • Loading branch information
@hubertdeng123
hubertdeng123 authored Dec 27, 2023
1 parent 90c9b6e commit f9457ee
Show file tree
Hide file tree
Showing 6 changed files with 352 additions and 46 deletions.
4 changes: 2 additions & 2 deletions src/brain/issueLabelHandler/followups.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,12 +234,12 @@ export async function ensureOneWaitingForLabel({
let timeToRespondBy;
if (labelName === WAITING_FOR_PRODUCT_OWNER_LABEL) {
timeToRespondBy =
(await calculateSLOViolationTriage(
calculateSLOViolationTriage(
WAITING_FOR_PRODUCT_OWNER_LABEL,
issue.labels,
repo,
org.slug
)) || moment().toISOString();
) || moment().toISOString();
} else if (labelName === WAITING_FOR_SUPPORT_LABEL) {
timeToRespondBy =
(await calculateSLOViolationRoute(
Expand Down
124 changes: 94 additions & 30 deletions src/utils/businessHours.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
calculateSLOViolationRoute,
calculateSLOViolationTriage,
calculateTimeToRespondBy,
getBusinessHoursLeft,
getNextAvailableBusinessHourWindow,
} from './businessHours';

Expand Down Expand Up @@ -285,8 +286,8 @@ describe('businessHours tests', function () {
});

describe('calculateSLOViolationTriage', function () {
it('should calculate SLO violation when product area is not defined', async function () {
const result = await calculateSLOViolationTriage(
it('should calculate SLO violation when product area is not defined', function () {
const result = calculateSLOViolationTriage(
WAITING_FOR_PRODUCT_OWNER_LABEL,
[{ name: 'Test' }],
'routing-repo',
Expand All @@ -295,8 +296,8 @@ describe('businessHours tests', function () {
expect(result).not.toEqual(null);
});

it('should not calculate SLO violation if label is not untriaged', async function () {
const result = await calculateSLOViolationTriage(
it('should not calculate SLO violation if label is not untriaged', function () {
const result = calculateSLOViolationTriage(
'Status: Test',
[{ name: 'Product Area: Test' }],
'routing-repo',
Expand All @@ -305,8 +306,8 @@ describe('businessHours tests', function () {
expect(result).toEqual(null);
});

it('should not calculate SLO violation if label is unrouted', async function () {
const result = await calculateSLOViolationTriage(
it('should not calculate SLO violation if label is unrouted', function () {
const result = calculateSLOViolationTriage(
WAITING_FOR_SUPPORT_LABEL,
[{ name: 'Product Area: Test' }],
'routing-repo',
Expand All @@ -315,8 +316,8 @@ describe('businessHours tests', function () {
expect(result).toEqual(null);
});

it('should calculate SLO violation if label is untriaged', async function () {
const result = await calculateSLOViolationTriage(
it('should calculate SLO violation if label is untriaged', function () {
const result = calculateSLOViolationTriage(
WAITING_FOR_PRODUCT_OWNER_LABEL,
[{ name: 'Product Area: Test' }],
'routing-repo',
Expand All @@ -325,8 +326,8 @@ describe('businessHours tests', function () {
expect(result).not.toEqual(null);
});

it('should calculate SLO violation if label is waiting for product owner', async function () {
const result = await calculateSLOViolationTriage(
it('should calculate SLO violation if label is waiting for product owner', function () {
const result = calculateSLOViolationTriage(
WAITING_FOR_PRODUCT_OWNER_LABEL,
[{ name: 'Product Area: Test' }],
'routing-repo',
Expand All @@ -337,8 +338,8 @@ describe('businessHours tests', function () {
});

describe('getNextAvailableBusinessHourWindow', function () {
it('should get open source product area timezones if product area does not have offices', async function () {
const { start, end } = await getNextAvailableBusinessHourWindow(
it('should get open source product area timezones if product area does not have offices', function () {
const { start, end } = getNextAvailableBusinessHourWindow(
'Does not exist',
moment('2022-12-08T12:00:00.000Z').utc(),
'routing-repo',
Expand All @@ -352,8 +353,8 @@ describe('businessHours tests', function () {
);
});

it('should get sfo timezones for repo with no offices defined', async function () {
const { start, end } = await getNextAvailableBusinessHourWindow(
it('should get sfo timezones for repo with no offices defined', function () {
const { start, end } = getNextAvailableBusinessHourWindow(
'',
moment('2022-12-08T12:00:00.000Z').utc(),
'test-null',
Expand All @@ -367,8 +368,8 @@ describe('businessHours tests', function () {
);
});

it('should get sfo timezones for Test', async function () {
const { start, end } = await getNextAvailableBusinessHourWindow(
it('should get sfo timezones for Test', function () {
const { start, end } = getNextAvailableBusinessHourWindow(
'Test',
moment('2022-12-08T12:00:00.000Z').utc(),
'routing-repo',
Expand All @@ -382,8 +383,8 @@ describe('businessHours tests', function () {
);
});

it('should get vie timezone for Test if it has the closest business hours available', async function () {
const { start, end } = await getNextAvailableBusinessHourWindow(
it('should get vie timezone for Test if it has the closest business hours available', function () {
const { start, end } = getNextAvailableBusinessHourWindow(
'Non-Overlapping Timezone',
moment('2022-12-08T12:00:00.000Z').utc(),
'routing-repo',
Expand All @@ -397,8 +398,8 @@ describe('businessHours tests', function () {
);
});

it('should get sfo timezone for Test if it has the closest business hours available', async function () {
const { start, end } = await getNextAvailableBusinessHourWindow(
it('should get sfo timezone for Test if it has the closest business hours available', function () {
const { start, end } = getNextAvailableBusinessHourWindow(
'Non-Overlapping Timezone',
moment('2022-12-08T16:30:00.000Z').utc(),
'routing-repo',
Expand All @@ -412,8 +413,8 @@ describe('businessHours tests', function () {
);
});

it('should get yyz timezone for Test if it has the closest business hours available', async function () {
const { start, end } = await getNextAvailableBusinessHourWindow(
it('should get yyz timezone for Test if it has the closest business hours available', function () {
const { start, end } = getNextAvailableBusinessHourWindow(
'All-Timezones',
moment('2022-12-08T16:30:00.000Z').utc(),
'routing-repo',
Expand All @@ -427,8 +428,8 @@ describe('businessHours tests', function () {
);
});

it('should return vie hours for Christmas for product area subscribed to vie, yyz, sfo', async function () {
const { start, end } = await getNextAvailableBusinessHourWindow(
it('should return vie hours for Christmas for product area subscribed to vie, yyz, sfo', function () {
const { start, end } = getNextAvailableBusinessHourWindow(
'All-Timezones',
moment('2023-12-23T12:00:00.000Z').utc(),
'routing-repo',
Expand All @@ -442,8 +443,8 @@ describe('businessHours tests', function () {
);
});

it('should return vie hours for Saturday for product area subscribed to vie, yyz, sfo', async function () {
const { start, end } = await getNextAvailableBusinessHourWindow(
it('should return vie hours for Saturday for product area subscribed to vie, yyz, sfo', function () {
const { start, end } = getNextAvailableBusinessHourWindow(
'All-Timezones',
moment('2022-12-17T12:00:00.000Z').utc(),
'routing-repo',
Expand All @@ -457,8 +458,8 @@ describe('businessHours tests', function () {
);
});

it('should return vie hours for Sunday for product area subscribed to vie, yyz, sfo', async function () {
const { start, end } = await getNextAvailableBusinessHourWindow(
it('should return vie hours for Sunday for product area subscribed to vie, yyz, sfo', function () {
const { start, end } = getNextAvailableBusinessHourWindow(
'All-Timezones',
moment('2022-12-18T12:00:00.000Z').utc(),
'routing-repo',
Expand All @@ -472,8 +473,8 @@ describe('businessHours tests', function () {
);
});

it('should return yyz hours for Saturday for product area subscribed to yyz, sfo', async function () {
const { start, end } = await getNextAvailableBusinessHourWindow(
it('should return yyz hours for Saturday for product area subscribed to yyz, sfo', function () {
const { start, end } = getNextAvailableBusinessHourWindow(
'Overlapping Timezone',
moment('2022-12-17T12:00:00.000Z').utc(),
'routing-repo',
Expand All @@ -487,4 +488,67 @@ describe('businessHours tests', function () {
);
});
});

describe('getBusinessHoursLeft', function () {
it('should correctly calculate business hours left overnight', function () {
const triageBy = '2023-12-22T18:00:00.000Z';
const now = moment('2023-12-22T01:00:00.000Z');
const repo = 'test-ttt-simple';
const org = 'getsentry';
const productArea = 'Other';
expect(
getBusinessHoursLeft(triageBy, now, repo, org, productArea)
).toEqual(1);
});
it('should correctly calculate business hours over multiple days', function () {
const triageBy = '2023-12-22T18:00:00.000Z';
const now = moment('2023-12-21T01:00:00.000Z');
const repo = 'test-ttt-simple';
const org = 'getsentry';
const productArea = 'Other';
expect(
getBusinessHoursLeft(triageBy, now, repo, org, productArea)
).toEqual(9);
});
it('should correctly calculate business hours over holiday', function () {
const triageBy = '2024-01-02T17:00:00.000Z';
const now = moment('2023-12-23T00:00:00.000Z');
const repo = 'test-ttt-simple';
const org = 'getsentry';
const productArea = 'Other';
expect(
getBusinessHoursLeft(triageBy, now, repo, org, productArea)
).toEqual(1);
});
it('should correctly account for weekends when calculating business hours', function () {
const triageBy = '2023-01-17T18:00:00.000Z';
const now = moment('2023-01-14T00:00:00.000Z');
const repo = 'test-ttt-simple';
const org = 'getsentry';
const productArea = 'Other';
expect(
getBusinessHoursLeft(triageBy, now, repo, org, productArea)
).toEqual(2);
});
it('should correctly calculate business hours for issues with non overlapping timezones', function () {
const triageBy = '2023-12-22T18:00:00.000Z';
const now = moment('2023-12-21T01:00:00.000Z');
const repo = 'routing-repo';
const org = 'getsentry';
const productArea = 'Non-Overlapping Timezone';
expect(
getBusinessHoursLeft(triageBy, now, repo, org, productArea)
).toEqual(25);
});
it('should correctly calculate business hours for issues with overlapping timezones', function () {
const triageBy = '2023-12-22T18:00:00.000Z';
const now = moment('2023-12-21T01:00:00.000Z');
const repo = 'routing-repo';
const org = 'getsentry';
const productArea = 'Overlapping Timezone';
expect(
getBusinessHoursLeft(triageBy, now, repo, org, productArea)
).toEqual(15);
});
});
});
56 changes: 47 additions & 9 deletions src/utils/businessHours.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,18 @@ interface BusinessHourWindow {
end: moment.Moment;
}

export async function calculateTimeToRespondBy(
export function calculateTimeToRespondBy(
numDays: number,
productArea: string,
repo: string,
org: string,
testTimestamp?: string
) {
): string {
let cursor =
testTimestamp !== undefined ? moment(testTimestamp).utc() : moment().utc();
let msRemaining = numDays * BUSINESS_DAY_IN_MS;
while (msRemaining > 0) {
const nextBusinessHours = await getNextAvailableBusinessHourWindow(
const nextBusinessHours = getNextAvailableBusinessHourWindow(
productArea,
cursor,
repo,
Expand All @@ -52,12 +52,12 @@ export async function calculateTimeToRespondBy(
return cursor.toISOString();
}

export async function calculateSLOViolationTriage(
export function calculateSLOViolationTriage(
target_name: string,
labels: any,
repo: string,
org: string
) {
): string | null {
// calculate time to triage for issues that come in with untriaged label
if (target_name === WAITING_FOR_PRODUCT_OWNER_LABEL) {
const productArea = labels
Expand All @@ -80,11 +80,11 @@ export async function calculateSLOViolationTriage(
return null;
}

export async function calculateSLOViolationRoute(
export function calculateSLOViolationRoute(
target_name: string,
repo: string,
org: string
) {
): string | null {
if (target_name === WAITING_FOR_SUPPORT_LABEL) {
return calculateTimeToRespondBy(MAX_ROUTE_DAYS, 'Unknown', repo, org);
}
Expand All @@ -109,12 +109,16 @@ export const isTimeInBusinessHours = (time: moment.Moment, office: string) => {
return false;
};

export async function getNextAvailableBusinessHourWindow(
/*
This function returns the current business hour window, if the time given is within business
hours. Otherwise, returns the next business hour window an issue should be triaged at.
*/
export function getNextAvailableBusinessHourWindow(
productArea: string,
momentTime: moment.Moment,
repo: string,
org: string
): Promise<BusinessHourWindow> {
): BusinessHourWindow {
let offices: Set<string> = new Set<string>();
const teams = getTeams(repo, org, productArea);
// TODO(getsentry/team-ospo#200): Add codecov support
Expand Down Expand Up @@ -177,3 +181,37 @@ export async function getNextAvailableBusinessHourWindow(
);
return businessHourWindows[0];
}

export function getBusinessHoursLeft(
triageBy: string,
now: moment.Moment,
repo: string,
org: string,
productArea: string
): number {
let businessHoursLeft = 0;
let momentIterator = moment(now.format());
const triageByMoment = moment(triageBy);
while (momentIterator.diff(triageBy, 'hours') < 0) {
const businessHourWindow = getNextAvailableBusinessHourWindow(
productArea,
momentIterator,
repo,
org
);
// Two cases to handle.
// 1. If moment iterator is currently in business hours, we take the minimum of hours left in a day or the hours left until triage by date given.
// 2. If moment iterator is out of business hours, we set the iterator to the next time issue should be accounted in business hours.
if (momentIterator >= businessHourWindow.start) {
businessHoursLeft +=
momentIterator.diff(
moment.min(triageByMoment, businessHourWindow.end),
'hours'
) * -1;
momentIterator = businessHourWindow.end;
} else {
momentIterator = businessHourWindow.start;
}
}
return businessHoursLeft;
}
4 changes: 2 additions & 2 deletions src/utils/metrics.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,12 +288,12 @@ export async function insertOss(
data.product_area &&
data.product_area.slice(PRODUCT_AREA_LABEL_PREFIX.length);
if (data.action === 'labeled') {
data.timeToRouteBy = await calculateSLOViolationRoute(
data.timeToRouteBy = calculateSLOViolationRoute(
data.target_name,
payload.repository.name,
payload.organization.login
);
data.timeToTriageBy = await calculateSLOViolationTriage(
data.timeToTriageBy = calculateSLOViolationTriage(
data.target_name,
issue.labels,
payload.repository.name,
Expand Down
Loading

0 comments on commit f9457ee

Please sign in to comment.