From de488df5406ae04cfb4fb18d569d12672efe0c9c Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Wed, 21 Nov 2018 11:39:16 +0100 Subject: [PATCH] fix(aws-elasticloadbalancingv2): target group metrics (#1226) TargetGroup metrics used to use ${TargetGroup.LoadBalancerArns} to find the load balancer's full name, but that introduces a deployment-time ordering dependency on the creation of the Listener object. Instead, use the Listener ARN to get the load balancer name. We now have an ordering requirement in the CDK code but that can be detected early and solved by the user. Fixes #1213. --- .../test/integ.asg-w-elbv2.expected.json | 30 +--- .../aws-autoscaling/test/test.scaling.ts | 5 +- .../lib/alb/application-target-group.ts | 13 +- .../lib/nlb/network-target-group.ts | 18 +- .../lib/shared/base-target-group.ts | 23 ++- .../test/alb/test.listener.ts | 14 +- .../test/integ.alb.expected.json | 164 +++++++++++++++++- .../test/integ.alb.ts | 14 +- 8 files changed, 228 insertions(+), 53 deletions(-) diff --git a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json index 2fdd04c3a8a11..a2f3320e35c4e 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json +++ b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json @@ -495,15 +495,7 @@ "Fn::Split": [ "/", { - "Fn::Select": [ - 0, - { - "Fn::GetAtt": [ - "LBListenerTargetGroupF04FCF6D", - "LoadBalancerArns" - ] - } - ] + "Ref": "LBListener49E825B4" } ] } @@ -517,15 +509,7 @@ "Fn::Split": [ "/", { - "Fn::Select": [ - 0, - { - "Fn::GetAtt": [ - "LBListenerTargetGroupF04FCF6D", - "LoadBalancerArns" - ] - } - ] + "Ref": "LBListener49E825B4" } ] } @@ -539,15 +523,7 @@ "Fn::Split": [ "/", { - "Fn::Select": [ - 0, - { - "Fn::GetAtt": [ - "LBListenerTargetGroupF04FCF6D", - "LoadBalancerArns" - ] - } - ] + "Ref": "LBListener49E825B4" } ] } diff --git a/packages/@aws-cdk/aws-autoscaling/test/test.scaling.ts b/packages/@aws-cdk/aws-autoscaling/test/test.scaling.ts index c80464f02d474..ac2c3e5bddde3 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/test.scaling.ts +++ b/packages/@aws-cdk/aws-autoscaling/test/test.scaling.ts @@ -94,10 +94,7 @@ export = { const arnParts = { "Fn::Split": [ "/", - { "Fn::Select": [ - 0, - { "Fn::GetAtt": [ "ALBListenerTargetsGroup01D7716A", "LoadBalancerArns" ] } - ]} + { Ref: "ALBListener3B99FF85" } ]}; expect(stack).to(haveResource('AWS::AutoScaling::ScalingPolicy', { diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-target-group.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-target-group.ts index 7969c81ea1a72..d579fab83d5a6 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-target-group.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-target-group.ts @@ -1,7 +1,8 @@ import cloudwatch = require('@aws-cdk/aws-cloudwatch'); import ec2 = require('@aws-cdk/aws-ec2'); import cdk = require('@aws-cdk/cdk'); -import { BaseTargetGroup, BaseTargetGroupProps, ITargetGroup, LoadBalancerTargetProps, TargetGroupRefProps } from '../shared/base-target-group'; +import { BaseTargetGroup, BaseTargetGroupProps, ITargetGroup, loadBalancerNameFromListenerArn, + LoadBalancerTargetProps, TargetGroupRefProps } from '../shared/base-target-group'; import { ApplicationProtocol } from '../shared/enums'; import { BaseImportedTargetGroup } from '../shared/imported'; import { determineProtocolAndPort, LazyDependable } from '../shared/util'; @@ -149,6 +150,16 @@ export class ApplicationTargetGroup extends BaseTargetGroup { this.loadBalancerAssociationDependencies.push(dependable || listener); } + /** + * Full name of first load balancer + */ + public get firstLoadBalancerFullName(): string { + if (this.listeners.length === 0) { + throw new Error('The TargetGroup needs to be attached to a LoadBalancer before you can call this method'); + } + return loadBalancerNameFromListenerArn(this.listeners[0].listenerArn); + } + /** * Return the given named metric for this Application Load Balancer Target Group * diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-target-group.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-target-group.ts index 65060e9f8a647..4f9dfd2251591 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-target-group.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-target-group.ts @@ -1,5 +1,6 @@ import cdk = require('@aws-cdk/cdk'); -import { BaseTargetGroup, BaseTargetGroupProps, ITargetGroup, LoadBalancerTargetProps, TargetGroupRefProps } from '../shared/base-target-group'; +import { BaseTargetGroup, BaseTargetGroupProps, ITargetGroup, loadBalancerNameFromListenerArn, + LoadBalancerTargetProps, TargetGroupRefProps } from '../shared/base-target-group'; import { Protocol } from '../shared/enums'; import { BaseImportedTargetGroup } from '../shared/imported'; import { LazyDependable } from '../shared/util'; @@ -42,12 +43,16 @@ export class NetworkTargetGroup extends BaseTargetGroup { return new ImportedNetworkTargetGroup(parent, id, props); } + private readonly listeners: INetworkListener[]; + constructor(parent: cdk.Construct, id: string, props: NetworkTargetGroupProps) { super(parent, id, props, { protocol: Protocol.Tcp, port: props.port, }); + this.listeners = []; + if (props.proxyProtocolV2) { this.setAttribute('proxy_protocol_v2.enabled', 'true'); } @@ -72,6 +77,17 @@ export class NetworkTargetGroup extends BaseTargetGroup { */ public registerListener(listener: INetworkListener) { this.loadBalancerAssociationDependencies.push(listener); + this.listeners.push(listener); + } + + /** + * Full name of first load balancer + */ + public get firstLoadBalancerFullName(): string { + if (this.listeners.length === 0) { + throw new Error('The TargetGroup needs to be attached to a LoadBalancer before you can call this method'); + } + return loadBalancerNameFromListenerArn(this.listeners[0].listenerArn); } } diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts index 2a8207a27020d..d433af0ae2c63 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts @@ -160,7 +160,7 @@ export abstract class BaseTargetGroup extends cdk.Construct implements ITargetGr * * @example app/my-load-balancer/123456789 */ - public readonly firstLoadBalancerFullName: string; + public abstract readonly firstLoadBalancerFullName: string; /** * Health check for the members of this target group @@ -240,11 +240,6 @@ export abstract class BaseTargetGroup extends cdk.Construct implements ITargetGr this.loadBalancerArns = this.resource.targetGroupLoadBalancerArns.toString(); this.targetGroupName = this.resource.targetGroupName; this.defaultPort = `${additionalProps.port}`; - - const firstLoadBalancerArn = new cdk.FnSelect(0, this.targetGroupLoadBalancerArns); - // arn:aws:elasticloadbalancing:us-west-2:123456789012:loadbalancer/app/my-internal-load-balancer/50dc6c495c0c9188 - const arnParts = new cdk.FnSplit('/', firstLoadBalancerArn); - this.firstLoadBalancerFullName = `${new cdk.FnSelect(1, arnParts)}/${new cdk.FnSelect(2, arnParts)}/${new cdk.FnSelect(3, arnParts)}`; } /** @@ -365,3 +360,19 @@ export interface LoadBalancerTargetProps { */ targetJson?: any; } + +/** + * Extract the full load balancer name (used for metrics) from the listener ARN: + * + * Turns + * + * arn:aws:elasticloadbalancing:us-west-2:123456789012:listener/app/my-load-balancer/50dc6c495c0c9188/f2f7dc8efc522ab2 + * + * Into + * + * app/my-load-balancer/50dc6c495c0c9188 + */ +export function loadBalancerNameFromListenerArn(listenerArn: string) { + const arnParts = new cdk.FnSplit('/', listenerArn); + return `${new cdk.FnSelect(1, arnParts)}/${new cdk.FnSelect(2, arnParts)}/${new cdk.FnSelect(3, arnParts)}`; +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts index b7881e4c31110..94d66146bcb60 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts @@ -389,7 +389,12 @@ export = { // GIVEN const stack = new cdk.Stack(); const vpc = new ec2.VpcNetwork(stack, 'VPC'); + const lb = new elbv2.ApplicationLoadBalancer(stack, 'LB', { vpc }); const group = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', { vpc, port: 80 }); + lb.addListener('SomeListener', { + port: 80, + defaultTargetGroups: [group] + }); // WHEN const metrics = []; @@ -404,16 +409,17 @@ export = { for (const metric of metrics) { test.equal('AWS/ApplicationELB', metric.namespace); - const firstArn = { "Fn::Select": [0, { "Fn::GetAtt": ["TargetGroup3D7CD9B8", "LoadBalancerArns"] }] }; + const loadBalancerArn = { Ref: "LBSomeListenerCA01F1A0" }; + test.deepEqual(cdk.resolve(metric.dimensions), { TargetGroup: { 'Fn::GetAtt': [ 'TargetGroup3D7CD9B8', 'TargetGroupFullName' ] }, LoadBalancer: { 'Fn::Join': [ '', - [ { 'Fn::Select': [ 1, { 'Fn::Split': [ '/', firstArn ] } ] }, + [ { 'Fn::Select': [ 1, { 'Fn::Split': [ '/', loadBalancerArn ] } ] }, '/', - { 'Fn::Select': [ 2, { 'Fn::Split': [ '/', firstArn ] } ] }, + { 'Fn::Select': [ 2, { 'Fn::Split': [ '/', loadBalancerArn ] } ] }, '/', - { 'Fn::Select': [ 3, { 'Fn::Split': [ '/', firstArn ] } ] } + { 'Fn::Select': [ 3, { 'Fn::Split': [ '/', loadBalancerArn ] } ] } ] ] } diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json index 9473d122acc7e..6aa99e1919368 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json @@ -67,9 +67,6 @@ }, "VPCPublicSubnet1DefaultRoute91CEF279": { "Type": "AWS::EC2::Route", - "DependsOn": [ - "VPCVPCGW99B986DC" - ], "Properties": { "RouteTableId": { "Ref": "VPCPublicSubnet1RouteTableFEE4B781" @@ -78,7 +75,10 @@ "GatewayId": { "Ref": "VPCIGWB7E252D3" } - } + }, + "DependsOn": [ + "VPCVPCGW99B986DC" + ] }, "VPCPublicSubnet1EIP6AD938E8": { "Type": "AWS::EC2::EIP", @@ -158,9 +158,6 @@ }, "VPCPublicSubnet2DefaultRouteB7481BBA": { "Type": "AWS::EC2::Route", - "DependsOn": [ - "VPCVPCGW99B986DC" - ], "Properties": { "RouteTableId": { "Ref": "VPCPublicSubnet2RouteTable6F1A15F1" @@ -169,7 +166,10 @@ "GatewayId": { "Ref": "VPCIGWB7E252D3" } - } + }, + "DependsOn": [ + "VPCVPCGW99B986DC" + ] }, "VPCPublicSubnet2EIP4947BC00": { "Type": "AWS::EC2::EIP", @@ -471,6 +471,154 @@ }, "Priority": 10 } + }, + "ResponseTimeHigh1D16E109F": { + "Type": "AWS::CloudWatch::Alarm", + "Properties": { + "ComparisonOperator": "GreaterThanOrEqualToThreshold", + "EvaluationPeriods": 2, + "MetricName": "TargetResponseTime", + "Namespace": "AWS/ApplicationELB", + "Period": 300, + "Threshold": 5, + "Dimensions": [ + { + "Name": "TargetGroup", + "Value": { + "Fn::GetAtt": [ + "LBListenerTargetGroupF04FCF6D", + "TargetGroupFullName" + ] + } + }, + { + "Name": "LoadBalancer", + "Value": { + "Fn::Join": [ + "", + [ + { + "Fn::Select": [ + 1, + { + "Fn::Split": [ + "/", + { + "Ref": "LBListener49E825B4" + } + ] + } + ] + }, + "/", + { + "Fn::Select": [ + 2, + { + "Fn::Split": [ + "/", + { + "Ref": "LBListener49E825B4" + } + ] + } + ] + }, + "/", + { + "Fn::Select": [ + 3, + { + "Fn::Split": [ + "/", + { + "Ref": "LBListener49E825B4" + } + ] + } + ] + } + ] + ] + } + } + ], + "Statistic": "Average" + } + }, + "ResponseTimeHigh2FFCF1FE1": { + "Type": "AWS::CloudWatch::Alarm", + "Properties": { + "ComparisonOperator": "GreaterThanOrEqualToThreshold", + "EvaluationPeriods": 2, + "MetricName": "TargetResponseTime", + "Namespace": "AWS/ApplicationELB", + "Period": 300, + "Threshold": 5, + "Dimensions": [ + { + "Name": "TargetGroup", + "Value": { + "Fn::GetAtt": [ + "LBListenerConditionalTargetGroupA75CCCD9", + "TargetGroupFullName" + ] + } + }, + { + "Name": "LoadBalancer", + "Value": { + "Fn::Join": [ + "", + [ + { + "Fn::Select": [ + 1, + { + "Fn::Split": [ + "/", + { + "Ref": "LBListener49E825B4" + } + ] + } + ] + }, + "/", + { + "Fn::Select": [ + 2, + { + "Fn::Split": [ + "/", + { + "Ref": "LBListener49E825B4" + } + ] + } + ] + }, + "/", + { + "Fn::Select": [ + 3, + { + "Fn::Split": [ + "/", + { + "Ref": "LBListener49E825B4" + } + ] + } + ] + } + ] + ] + } + } + ], + "Statistic": "Average" + } } } } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.ts index 2c4f5bc9c1af9..604ea4f8522c6 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.ts @@ -19,16 +19,26 @@ const listener = lb.addListener('Listener', { port: 80, }); -listener.addTargets('Target', { +const group1 = listener.addTargets('Target', { port: 80, targets: [new elbv2.IpTarget('10.0.1.1')] }); -listener.addTargets('ConditionalTarget', { +const group2 = listener.addTargets('ConditionalTarget', { priority: 10, hostHeader: 'example.com', port: 80, targets: [new elbv2.IpTarget('10.0.1.2')] }); +group1.metricTargetResponseTime().newAlarm(stack, 'ResponseTimeHigh1', { + threshold: 5, + evaluationPeriods: 2, +}); + +group2.metricTargetResponseTime().newAlarm(stack, 'ResponseTimeHigh2', { + threshold: 5, + evaluationPeriods: 2, +}); + app.run();