From f91c1373672ee7458286bb89be2ccae208b5ce60 Mon Sep 17 00:00:00 2001 From: Robert Kenny Date: Mon, 9 Oct 2023 12:23:52 +0100 Subject: [PATCH] feat: Add 4xx alarms as an optional parameter to ec2 base alarm config Being alerted to high volumes of 404 requests is arguably a common pattern that is useful to extend to CDK. We've added this to another project where it would have allowed us to catch a production issue, so contributing upstream to gauge if useful for others. This change adds an optional http4xxAlarm to the `Alarms` type on the `GuEc2AppProps` interface, as it is perhaps not as critical to consider as the 5xx events, but still hopefully a useful affordance. --- .../__snapshots__/ec2-alarms.test.ts.snap | 216 ++++++++++++++++++ src/constructs/cloudwatch/alarm.ts | 9 + src/constructs/cloudwatch/ec2-alarms.test.ts | 57 ++++- src/constructs/cloudwatch/ec2-alarms.ts | 37 ++- src/patterns/ec2-app/base.test.ts | 29 +++ src/patterns/ec2-app/base.ts | 22 +- 6 files changed, 365 insertions(+), 5 deletions(-) diff --git a/src/constructs/cloudwatch/__snapshots__/ec2-alarms.test.ts.snap b/src/constructs/cloudwatch/__snapshots__/ec2-alarms.test.ts.snap index 4913dd5620..23efbc83b2 100644 --- a/src/constructs/cloudwatch/__snapshots__/ec2-alarms.test.ts.snap +++ b/src/constructs/cloudwatch/__snapshots__/ec2-alarms.test.ts.snap @@ -1,5 +1,221 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`The GuAlb4xxPercentageAlarm construct should create the correct alarm resource with minimal config 1`] = ` +{ + "Metadata": { + "gu:cdk:constructs": [ + "GuStack", + "GuApplicationLoadBalancer", + "GuAlb4xxPercentageAlarm", + ], + "gu:cdk:version": "TEST", + }, + "Outputs": { + "ApplicationLoadBalancerTestingDnsName": { + "Description": "DNS entry for ApplicationLoadBalancerTesting", + "Value": { + "Fn::GetAtt": [ + "ApplicationLoadBalancerTesting172A253B", + "DNSName", + ], + }, + }, + }, + "Resources": { + "ApplicationLoadBalancerTesting172A253B": { + "Properties": { + "LoadBalancerAttributes": [ + { + "Key": "deletion_protection.enabled", + "Value": "true", + }, + ], + "Scheme": "internal", + "SecurityGroups": [ + { + "Fn::GetAtt": [ + "ApplicationLoadBalancerTestingSecurityGroup883A01A4", + "GroupId", + ], + }, + ], + "Subnets": [ + "", + ], + "Tags": [ + { + "Key": "App", + "Value": "testing", + }, + { + "Key": "gu:cdk:version", + "Value": "TEST", + }, + { + "Key": "gu:repo", + "Value": "guardian/cdk", + }, + { + "Key": "Stack", + "Value": "test-stack", + }, + { + "Key": "Stage", + "Value": "TEST", + }, + ], + "Type": "application", + }, + "Type": "AWS::ElasticLoadBalancingV2::LoadBalancer", + }, + "ApplicationLoadBalancerTestingSecurityGroup883A01A4": { + "Properties": { + "GroupDescription": "Automatically created Security Group for ELB TestApplicationLoadBalancerTesting8F9EA5A8", + "SecurityGroupEgress": [ + { + "CidrIp": "255.255.255.255/32", + "Description": "Disallow all traffic", + "FromPort": 252, + "IpProtocol": "icmp", + "ToPort": 86, + }, + ], + "Tags": [ + { + "Key": "App", + "Value": "testing", + }, + { + "Key": "gu:cdk:version", + "Value": "TEST", + }, + { + "Key": "gu:repo", + "Value": "guardian/cdk", + }, + { + "Key": "Stack", + "Value": "test-stack", + }, + { + "Key": "Stage", + "Value": "TEST", + }, + ], + "VpcId": "test", + }, + "Type": "AWS::EC2::SecurityGroup", + }, + "High4xxPercentageAlarmTestingE21E0AD7": { + "Properties": { + "ActionsEnabled": true, + "AlarmActions": [ + { + "Fn::Join": [ + "", + [ + "arn:aws:sns:", + { + "Ref": "AWS::Region", + }, + ":", + { + "Ref": "AWS::AccountId", + }, + ":test-topic", + ], + ], + }, + ], + "AlarmDescription": "testing exceeded 1% error rate", + "AlarmName": "High 4XX error percentage from testing in TEST", + "ComparisonOperator": "GreaterThanThreshold", + "EvaluationPeriods": 1, + "Metrics": [ + { + "Expression": "100*(m1+m2)/m3", + "Id": "expr_1", + "Label": "% of 4XX responses served for testing (load balancer and instances combined)", + }, + { + "Id": "m1", + "MetricStat": { + "Metric": { + "Dimensions": [ + { + "Name": "LoadBalancer", + "Value": { + "Fn::GetAtt": [ + "ApplicationLoadBalancerTesting172A253B", + "LoadBalancerFullName", + ], + }, + }, + ], + "MetricName": "HTTPCode_ELB_4XX_Count", + "Namespace": "AWS/ApplicationELB", + }, + "Period": 60, + "Stat": "Sum", + }, + "ReturnData": false, + }, + { + "Id": "m2", + "MetricStat": { + "Metric": { + "Dimensions": [ + { + "Name": "LoadBalancer", + "Value": { + "Fn::GetAtt": [ + "ApplicationLoadBalancerTesting172A253B", + "LoadBalancerFullName", + ], + }, + }, + ], + "MetricName": "HTTPCode_Target_4XX_Count", + "Namespace": "AWS/ApplicationELB", + }, + "Period": 60, + "Stat": "Sum", + }, + "ReturnData": false, + }, + { + "Id": "m3", + "MetricStat": { + "Metric": { + "Dimensions": [ + { + "Name": "LoadBalancer", + "Value": { + "Fn::GetAtt": [ + "ApplicationLoadBalancerTesting172A253B", + "LoadBalancerFullName", + ], + }, + }, + ], + "MetricName": "RequestCount", + "Namespace": "AWS/ApplicationELB", + }, + "Period": 60, + "Stat": "Sum", + }, + "ReturnData": false, + }, + ], + "Threshold": 1, + "TreatMissingData": "notBreaching", + }, + "Type": "AWS::CloudWatch::Alarm", + }, + }, +} +`; + exports[`The GuAlb5xxPercentageAlarm construct should create the correct alarm resource with minimal config 1`] = ` { "Metadata": { diff --git a/src/constructs/cloudwatch/alarm.ts b/src/constructs/cloudwatch/alarm.ts index 8e71c5d43b..0db663f135 100644 --- a/src/constructs/cloudwatch/alarm.ts +++ b/src/constructs/cloudwatch/alarm.ts @@ -10,6 +10,15 @@ export interface GuAlarmProps extends AlarmProps, AppIdentity { okAction?: boolean; } +export interface Http4xxAlarmProps + extends Omit< + GuAlarmProps, + "snsTopicName" | "evaluationPeriods" | "metric" | "period" | "threshold" | "treatMissingData" | "app" + > { + tolerated4xxPercentage: number; + numberOfMinutesAboveThresholdBeforeAlarm?: number; +} + export interface Http5xxAlarmProps extends Omit< GuAlarmProps, diff --git a/src/constructs/cloudwatch/ec2-alarms.test.ts b/src/constructs/cloudwatch/ec2-alarms.test.ts index 40fd994414..ad7a331cf1 100644 --- a/src/constructs/cloudwatch/ec2-alarms.test.ts +++ b/src/constructs/cloudwatch/ec2-alarms.test.ts @@ -5,7 +5,7 @@ import { ApplicationListener, ApplicationProtocol } from "aws-cdk-lib/aws-elasti import { simpleGuStackForTesting } from "../../utils/test"; import type { AppIdentity } from "../core"; import { GuApplicationLoadBalancer, GuApplicationTargetGroup } from "../loadbalancing"; -import { GuAlb5xxPercentageAlarm, GuUnhealthyInstancesAlarm } from "./ec2-alarms"; +import { GuAlb4xxPercentageAlarm, GuAlb5xxPercentageAlarm, GuUnhealthyInstancesAlarm } from "./ec2-alarms"; const vpc = Vpc.fromVpcAttributes(new Stack(), "VPC", { vpcId: "test", @@ -72,6 +72,61 @@ describe("The GuAlb5xxPercentageAlarm construct", () => { }); }); +describe("The GuAlb4xxPercentageAlarm construct", () => { + it("should create the correct alarm resource with minimal config", () => { + const stack = simpleGuStackForTesting(); + const alb = new GuApplicationLoadBalancer(stack, "ApplicationLoadBalancer", { ...app, vpc }); + const props = { + tolerated4xxPercentage: 1, + snsTopicName: "test-topic", + }; + new GuAlb4xxPercentageAlarm(stack, { ...app, loadBalancer: alb, ...props }); + expect(Template.fromStack(stack).toJSON()).toMatchSnapshot(); + }); + + it("should use a custom description if one is provided", () => { + const stack = simpleGuStackForTesting(); + const alb = new GuApplicationLoadBalancer(stack, "ApplicationLoadBalancer", { ...app, vpc }); + const props = { + alarmDescription: "test-custom-alarm-description", + tolerated4xxPercentage: 1, + snsTopicName: "test-topic", + }; + new GuAlb4xxPercentageAlarm(stack, { ...app, loadBalancer: alb, ...props }); + Template.fromStack(stack).hasResourceProperties("AWS::CloudWatch::Alarm", { + AlarmDescription: "test-custom-alarm-description", + }); + }); + + it("should use a custom alarm name if one is provided", () => { + const stack = simpleGuStackForTesting(); + const alb = new GuApplicationLoadBalancer(stack, "ApplicationLoadBalancer", { ...app, vpc }); + const props = { + alarmName: "test-custom-alarm-name", + tolerated4xxPercentage: 1, + snsTopicName: "test-topic", + }; + new GuAlb4xxPercentageAlarm(stack, { ...app, loadBalancer: alb, ...props }); + Template.fromStack(stack).hasResourceProperties("AWS::CloudWatch::Alarm", { + AlarmName: "test-custom-alarm-name", + }); + }); + + it("should adjust the number of evaluation periods if a custom value is provided", () => { + const stack = simpleGuStackForTesting(); + const alb = new GuApplicationLoadBalancer(stack, "ApplicationLoadBalancer", { ...app, vpc }); + const props = { + tolerated4xxPercentage: 1, + numberOfMinutesAboveThresholdBeforeAlarm: 3, + snsTopicName: "test-topic", + }; + new GuAlb4xxPercentageAlarm(stack, { ...app, loadBalancer: alb, ...props }); + Template.fromStack(stack).hasResourceProperties("AWS::CloudWatch::Alarm", { + EvaluationPeriods: 3, + }); + }); +}); + describe("The GuUnhealthyInstancesAlarm construct", () => { it("should create the correct alarm resource with minimal config", () => { const stack = simpleGuStackForTesting(); diff --git a/src/constructs/cloudwatch/ec2-alarms.ts b/src/constructs/cloudwatch/ec2-alarms.ts index c46e91c4e3..192064f928 100644 --- a/src/constructs/cloudwatch/ec2-alarms.ts +++ b/src/constructs/cloudwatch/ec2-alarms.ts @@ -5,8 +5,11 @@ import type { GuStack } from "../core"; import { AppIdentity } from "../core"; import type { GuApplicationLoadBalancer, GuApplicationTargetGroup } from "../loadbalancing"; import { GuAlarm } from "./alarm"; -import type { GuAlarmProps, Http5xxAlarmProps } from "./alarm"; +import type { GuAlarmProps, Http4xxAlarmProps, Http5xxAlarmProps } from "./alarm"; +interface GuAlb4xxPercentageAlarmProps extends Pick, Http4xxAlarmProps, AppIdentity { + loadBalancer: GuApplicationLoadBalancer; +} interface GuAlb5xxPercentageAlarmProps extends Pick, Http5xxAlarmProps, AppIdentity { loadBalancer: GuApplicationLoadBalancer; } @@ -47,6 +50,38 @@ export class GuAlb5xxPercentageAlarm extends GuAlarm { } } +/** + * Creates an alarm which is triggered whenever the percentage of requests with a 4xx response code exceeds + * the specified threshold. + */ +export class GuAlb4xxPercentageAlarm extends GuAlarm { + constructor(scope: GuStack, props: GuAlb4xxPercentageAlarmProps) { + const mathExpression = new MathExpression({ + expression: "100*(m1+m2)/m3", + usingMetrics: { + m1: props.loadBalancer.metrics.httpCodeElb(HttpCodeElb.ELB_4XX_COUNT), + m2: props.loadBalancer.metrics.httpCodeTarget(HttpCodeTarget.TARGET_4XX_COUNT), + m3: props.loadBalancer.metrics.requestCount(), + }, + label: `% of 4XX responses served for ${props.app} (load balancer and instances combined)`, + period: Duration.minutes(1), + }); + const defaultAlarmName = `High 4XX error percentage from ${props.app} in ${scope.stage}`; + const defaultDescription = `${props.app} exceeded ${props.tolerated4xxPercentage}% error rate`; + const alarmProps = { + ...props, + metric: mathExpression, + treatMissingData: TreatMissingData.NOT_BREACHING, + threshold: props.tolerated4xxPercentage, + comparisonOperator: ComparisonOperator.GREATER_THAN_THRESHOLD, + alarmName: props.alarmName ?? defaultAlarmName, + alarmDescription: props.alarmDescription ?? defaultDescription, + evaluationPeriods: props.numberOfMinutesAboveThresholdBeforeAlarm ?? 1, + }; + super(scope, AppIdentity.suffixText(props, "High4xxPercentageAlarm"), alarmProps); + } +} + /** * Creates an alarm which is triggered whenever there have been several healthcheck failures within a single hour. */ diff --git a/src/patterns/ec2-app/base.test.ts b/src/patterns/ec2-app/base.test.ts index b38cabe959..25497e5be8 100644 --- a/src/patterns/ec2-app/base.test.ts +++ b/src/patterns/ec2-app/base.test.ts @@ -153,6 +153,7 @@ describe("the GuEC2App pattern", function () { }, monitoringConfiguration: { snsTopicName: "test-topic", + // Note: http4xxAlarm is not a required parameter http5xxAlarm: { tolerated5xxPercentage: 5, }, @@ -164,6 +165,34 @@ describe("the GuEC2App pattern", function () { GuTemplate.fromStack(stack).hasResourceWithLogicalId("AWS::CloudWatch::Alarm", /^High5xxPercentageAlarm.+/); }); + it("creates a High4xxPercentageAlarm if the relevant monitoringConfiguration is provided", function () { + const stack = simpleGuStackForTesting(); + const app = "test-gu-ec2-app"; + new GuEc2App(stack, { + applicationPort: 3000, + app, + access: { scope: AccessScope.PUBLIC }, + instanceType: InstanceType.of(InstanceClass.T4G, InstanceSize.MEDIUM), + certificateProps: { + domainName: "domain-name-for-your-application.example", + }, + scaling: { + minimumInstances: 1, + }, + monitoringConfiguration: { + snsTopicName: "test-topic", + http5xxAlarm: false, + http4xxAlarm: { + tolerated4xxPercentage: 5, + }, + unhealthyInstancesAlarm: false, + }, + userData: "", + }); + //The shape of this alarm is tested at construct level + GuTemplate.fromStack(stack).hasResourceWithLogicalId("AWS::CloudWatch::Alarm", /^High4xxPercentageAlarm.+/); + }); + it("creates an UnhealthyInstancesAlarm if the user enables it", function () { const stack = simpleGuStackForTesting(); const app = "test-gu-ec2-app"; diff --git a/src/patterns/ec2-app/base.ts b/src/patterns/ec2-app/base.ts index 8117480ac4..51cc79a2aa 100644 --- a/src/patterns/ec2-app/base.ts +++ b/src/patterns/ec2-app/base.ts @@ -23,8 +23,12 @@ import { AccessScope, MetadataKeys, NAMED_SSM_PARAMETER_PATHS } from "../../cons import { GuCertificate } from "../../constructs/acm"; import type { GuUserDataProps } from "../../constructs/autoscaling"; import { GuAutoScalingGroup, GuUserData } from "../../constructs/autoscaling"; -import type { Http5xxAlarmProps, NoMonitoring } from "../../constructs/cloudwatch"; -import { GuAlb5xxPercentageAlarm, GuUnhealthyInstancesAlarm } from "../../constructs/cloudwatch"; +import type { Http4xxAlarmProps, Http5xxAlarmProps, NoMonitoring } from "../../constructs/cloudwatch"; +import { + GuAlb4xxPercentageAlarm, + GuAlb5xxPercentageAlarm, + GuUnhealthyInstancesAlarm, +} from "../../constructs/cloudwatch"; import type { GuStack } from "../../constructs/core"; import { AppIdentity, GuLoggingStreamNameParameter, GuStringParameter } from "../../constructs/core"; import { GuHttpsEgressSecurityGroup, GuSecurityGroup, GuVpc, SubnetType } from "../../constructs/ec2"; @@ -109,6 +113,10 @@ export interface Alarms { * Enable the 5xx alarm with settings. */ http5xxAlarm: false | Http5xxAlarmProps; + /** + * Enable the 4xx alarm with settings. + */ + http4xxAlarm?: false | Http4xxAlarmProps; /** * Enable the unhealthy instances alarm. */ @@ -460,8 +468,16 @@ export class GuEc2App extends Construct { } if (!monitoringConfiguration.noMonitoring) { - const { http5xxAlarm, snsTopicName, unhealthyInstancesAlarm } = monitoringConfiguration; + const { http5xxAlarm, http4xxAlarm, snsTopicName, unhealthyInstancesAlarm } = monitoringConfiguration; + if (http4xxAlarm) { + new GuAlb4xxPercentageAlarm(scope, { + app, + loadBalancer, + snsTopicName, + ...http4xxAlarm, + }); + } if (http5xxAlarm) { new GuAlb5xxPercentageAlarm(scope, { app,