From bb74ce2682be130b3a805cbdff05c288500f0781 Mon Sep 17 00:00:00 2001 From: Thomas King <102960825+tkgnm@users.noreply.github.com> Date: Thu, 9 Nov 2023 14:58:48 +0000 Subject: [PATCH 1/5] Correct test name --- notificationworkerlambda/cdk/lib/senderworker.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notificationworkerlambda/cdk/lib/senderworker.test.ts b/notificationworkerlambda/cdk/lib/senderworker.test.ts index ea5f7e784..c51b80954 100644 --- a/notificationworkerlambda/cdk/lib/senderworker.test.ts +++ b/notificationworkerlambda/cdk/lib/senderworker.test.ts @@ -2,7 +2,7 @@ import { Template } from "aws-cdk-lib/assertions"; import { App } from "aws-cdk-lib" import {SenderWorkerStack} from "./senderworker"; -describe('The MobileAppsRendering stack', () => { +describe('The Sender Worker stack', () => { it('matches the snapshot', () => { const app = new App(); const stack = new SenderWorkerStack(app, 'SenderWorkerStack', { From f2fdf378fd11b2b162a9866ff34ee95788b80bfa Mon Sep 17 00:00:00 2001 From: Thomas King <102960825+tkgnm@users.noreply.github.com> Date: Thu, 9 Nov 2023 14:59:32 +0000 Subject: [PATCH 2/5] Change alarm periods Update snap Disable alarm on CODE increase alarm period Once a day for editions as they only send one notification a day. Once every 30 minutes for others as we often times go more than 30 minutes without sending a notification. Reinstate on CODE remove line test with short period --- .../__snapshots__/senderworker.test.ts.snap | 22 +++++++++---------- .../cdk/lib/senderworker.ts | 18 ++++++++++----- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/notificationworkerlambda/cdk/lib/__snapshots__/senderworker.test.ts.snap b/notificationworkerlambda/cdk/lib/__snapshots__/senderworker.test.ts.snap index 7d5f48a27..97271f951 100644 --- a/notificationworkerlambda/cdk/lib/__snapshots__/senderworker.test.ts.snap +++ b/notificationworkerlambda/cdk/lib/__snapshots__/senderworker.test.ts.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`The MobileAppsRendering stack matches the snapshot 1`] = ` +exports[`The Sender Worker stack matches the snapshot 1`] = ` Object { "Metadata": Object { "gu:cdk:constructs": Array [], @@ -512,7 +512,7 @@ Object { }, "androidSenderTooFewInvocationsAlarmC306ECD5": Object { "Properties": Object { - "ActionsEnabled": false, + "ActionsEnabled": true, "AlarmActions": Array [ Object { "Ref": "AlarmTopicArn", @@ -536,7 +536,7 @@ Object { "Ref": "AlarmTopicArn", }, ], - "Period": 360, + "Period": 60, "Statistic": "Sum", "Threshold": 0, "TreatMissingData": "breaching", @@ -982,7 +982,7 @@ Object { }, "androidbetaSenderTooFewInvocationsAlarm7B50CD4B": Object { "Properties": Object { - "ActionsEnabled": false, + "ActionsEnabled": true, "AlarmActions": Array [ Object { "Ref": "AlarmTopicArn", @@ -1006,7 +1006,7 @@ Object { "Ref": "AlarmTopicArn", }, ], - "Period": 360, + "Period": 60, "Statistic": "Sum", "Threshold": 0, "TreatMissingData": "breaching", @@ -1452,7 +1452,7 @@ Object { }, "androideditionSenderTooFewInvocationsAlarmE4AB00FB": Object { "Properties": Object { - "ActionsEnabled": false, + "ActionsEnabled": true, "AlarmActions": Array [ Object { "Ref": "AlarmTopicArn", @@ -1476,7 +1476,7 @@ Object { "Ref": "AlarmTopicArn", }, ], - "Period": 360, + "Period": 60, "Statistic": "Sum", "Threshold": 0, "TreatMissingData": "breaching", @@ -1922,7 +1922,7 @@ Object { }, "iosSenderTooFewInvocationsAlarm14117FE9": Object { "Properties": Object { - "ActionsEnabled": false, + "ActionsEnabled": true, "AlarmActions": Array [ Object { "Ref": "AlarmTopicArn", @@ -1946,7 +1946,7 @@ Object { "Ref": "AlarmTopicArn", }, ], - "Period": 360, + "Period": 60, "Statistic": "Sum", "Threshold": 0, "TreatMissingData": "breaching", @@ -2392,7 +2392,7 @@ Object { }, "ioseditionSenderTooFewInvocationsAlarmD32BB28C": Object { "Properties": Object { - "ActionsEnabled": false, + "ActionsEnabled": true, "AlarmActions": Array [ Object { "Ref": "AlarmTopicArn", @@ -2416,7 +2416,7 @@ Object { "Ref": "AlarmTopicArn", }, ], - "Period": 360, + "Period": 60, "Statistic": "Sum", "Threshold": 0, "TreatMissingData": "breaching", diff --git a/notificationworkerlambda/cdk/lib/senderworker.ts b/notificationworkerlambda/cdk/lib/senderworker.ts index 4923415e8..50028a1fc 100644 --- a/notificationworkerlambda/cdk/lib/senderworker.ts +++ b/notificationworkerlambda/cdk/lib/senderworker.ts @@ -25,6 +25,7 @@ interface SenderWorkerOpts { platform: string, paramPrefix: string, isBatchingSqsMessages: boolean, + dailyAlarmPeriod: boolean } class SenderWorker extends cdkcore.Construct { @@ -152,9 +153,11 @@ class SenderWorker extends cdkcore.Construct { comparisonOperator: cloudwatch.ComparisonOperator.LESS_THAN_OR_EQUAL_TO_THRESHOLD, evaluationPeriods: 1, threshold: 0, - metric: senderLambdaCtr.metricInvocations({period: cdk.Duration.seconds(360), statistic: "Sum"}), + // whole day for editions, 60 minutes for others + // metric: senderLambdaCtr.metricInvocations({period: cdk.Duration.seconds(props.dailyAlarmPeriod ? 60 * 60 * 24 : 60 * 60), statistic: "Sum"}), + metric: senderLambdaCtr.metricInvocations({period: cdk.Duration.seconds(60), statistic: "Sum"}), treatMissingData: cloudwatch.TreatMissingData.BREACHING, - actionsEnabled: false // isEnabled + actionsEnabled: true // isEnabled }) senderTooFewInvocationsAlarm.addAlarmAction(snsTopicAction) senderTooFewInvocationsAlarm.addOkAction(snsTopicAction) @@ -217,14 +220,15 @@ export class SenderWorkerStack extends GuStack { let workerQueueArns: string[] = [] - const addWorker = (workerName: string, paramPrefix: string, handler: string, isBatchingSqsMessages: boolean = false) => { + const addWorker = (workerName: string, paramPrefix: string, handler: string, isBatchingSqsMessages: boolean = false, dailyAlarmPeriod: boolean = false) => { let worker = new SenderWorker(this, workerName, { ...props, platform: workerName, paramPrefix: paramPrefix, handler: handler, isBatchingSqsMessages, - ...sharedOpts + ...sharedOpts, + dailyAlarmPeriod: dailyAlarmPeriod }) workerQueueArns.push(worker.senderSqs.queueArn) } @@ -236,8 +240,10 @@ export class SenderWorkerStack extends GuStack { addWorker("ios", "iosLive", "com.gu.notifications.worker.IOSSender::handleChunkTokens") addWorker("android", "androidLive", "com.gu.notifications.worker.AndroidSender::handleChunkTokens", true) - addWorker("ios-edition", "iosEdition", "com.gu.notifications.worker.IOSSender::handleChunkTokens") - addWorker("android-edition", "androidEdition", "com.gu.notifications.worker.AndroidSender::handleChunkTokens") + // edition apps only send one notification a day in order to get content for that day + addWorker("ios-edition", "iosEdition", "com.gu.notifications.worker.IOSSender::handleChunkTokens", false, true) + addWorker("android-edition", "androidEdition", "com.gu.notifications.worker.AndroidSender::handleChunkTokens", false, true) + addWorker("android-beta", "androidBeta", "com.gu.notifications.worker.AndroidSender::handleChunkTokens") /* From f4b4e1eb698dac5078d69923046e3d4ebb3a97d7 Mon Sep 17 00:00:00 2001 From: Thomas King <102960825+tkgnm@users.noreply.github.com> Date: Fri, 10 Nov 2023 11:40:00 +0000 Subject: [PATCH 3/5] Change alarm periods --- .../cdk/lib/__snapshots__/senderworker.test.ts.snap | 10 +++++----- notificationworkerlambda/cdk/lib/senderworker.ts | 3 +-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/notificationworkerlambda/cdk/lib/__snapshots__/senderworker.test.ts.snap b/notificationworkerlambda/cdk/lib/__snapshots__/senderworker.test.ts.snap index 97271f951..4fdc0780a 100644 --- a/notificationworkerlambda/cdk/lib/__snapshots__/senderworker.test.ts.snap +++ b/notificationworkerlambda/cdk/lib/__snapshots__/senderworker.test.ts.snap @@ -536,7 +536,7 @@ Object { "Ref": "AlarmTopicArn", }, ], - "Period": 60, + "Period": 3600, "Statistic": "Sum", "Threshold": 0, "TreatMissingData": "breaching", @@ -1006,7 +1006,7 @@ Object { "Ref": "AlarmTopicArn", }, ], - "Period": 60, + "Period": 3600, "Statistic": "Sum", "Threshold": 0, "TreatMissingData": "breaching", @@ -1476,7 +1476,7 @@ Object { "Ref": "AlarmTopicArn", }, ], - "Period": 60, + "Period": 86400, "Statistic": "Sum", "Threshold": 0, "TreatMissingData": "breaching", @@ -1946,7 +1946,7 @@ Object { "Ref": "AlarmTopicArn", }, ], - "Period": 60, + "Period": 3600, "Statistic": "Sum", "Threshold": 0, "TreatMissingData": "breaching", @@ -2416,7 +2416,7 @@ Object { "Ref": "AlarmTopicArn", }, ], - "Period": 60, + "Period": 86400, "Statistic": "Sum", "Threshold": 0, "TreatMissingData": "breaching", diff --git a/notificationworkerlambda/cdk/lib/senderworker.ts b/notificationworkerlambda/cdk/lib/senderworker.ts index 50028a1fc..56b1fb7f0 100644 --- a/notificationworkerlambda/cdk/lib/senderworker.ts +++ b/notificationworkerlambda/cdk/lib/senderworker.ts @@ -154,8 +154,7 @@ class SenderWorker extends cdkcore.Construct { evaluationPeriods: 1, threshold: 0, // whole day for editions, 60 minutes for others - // metric: senderLambdaCtr.metricInvocations({period: cdk.Duration.seconds(props.dailyAlarmPeriod ? 60 * 60 * 24 : 60 * 60), statistic: "Sum"}), - metric: senderLambdaCtr.metricInvocations({period: cdk.Duration.seconds(60), statistic: "Sum"}), + metric: senderLambdaCtr.metricInvocations({period: cdk.Duration.seconds(props.dailyAlarmPeriod ? 60 * 60 * 24 : 60 * 60), statistic: "Sum"}), treatMissingData: cloudwatch.TreatMissingData.BREACHING, actionsEnabled: true // isEnabled }) From c21d959b3175717c90d139e77149020fe014e090 Mon Sep 17 00:00:00 2001 From: Thomas King <102960825+tkgnm@users.noreply.github.com> Date: Fri, 10 Nov 2023 11:45:41 +0000 Subject: [PATCH 4/5] Add mss-admins as codeowners --- .github/CODEOWNERS | 1 + 1 file changed, 1 insertion(+) create mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 000000000..518c5200b --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1 @@ +* @guardian/mss-admins From 3c7e8b49d8e80381e9241793136e0511bf0c61d4 Mon Sep 17 00:00:00 2001 From: Thomas King <102960825+tkgnm@users.noreply.github.com> Date: Fri, 10 Nov 2023 14:28:19 +0000 Subject: [PATCH 5/5] Increase evaluation period for http code alarms to 10 --- notification/conf/notification.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/notification/conf/notification.yaml b/notification/conf/notification.yaml index 9083f70ee..98da9b287 100644 --- a/notification/conf/notification.yaml +++ b/notification/conf/notification.yaml @@ -380,7 +380,7 @@ Resources: Dimensions: - Name: LoadBalancerName Value: !Ref LoadBalancerToPrivateASG - EvaluationPeriods: 1 + EvaluationPeriods: 10 MetricName: HTTPCode_Backend_5XX Namespace: AWS/ELB Period: 60 @@ -398,7 +398,7 @@ Resources: Dimensions: - Name: LoadBalancerName Value: !Ref LoadBalancerToPrivateASG - EvaluationPeriods: 1 + EvaluationPeriods: 10 MetricName: HTTPCode_ELB_5XX Namespace: AWS/ELB Period: 60