Skip to content

Commit

Permalink
fix(aws-elasticloadbalancingv2): target group metrics (#1226)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rix0rrr authored Nov 21, 2018
1 parent 542bee4 commit de488df
Show file tree
Hide file tree
Showing 8 changed files with 228 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -495,15 +495,7 @@
"Fn::Split": [
"/",
{
"Fn::Select": [
0,
{
"Fn::GetAtt": [
"LBListenerTargetGroupF04FCF6D",
"LoadBalancerArns"
]
}
]
"Ref": "LBListener49E825B4"
}
]
}
Expand All @@ -517,15 +509,7 @@
"Fn::Split": [
"/",
{
"Fn::Select": [
0,
{
"Fn::GetAtt": [
"LBListenerTargetGroupF04FCF6D",
"LoadBalancerArns"
]
}
]
"Ref": "LBListener49E825B4"
}
]
}
Expand All @@ -539,15 +523,7 @@
"Fn::Split": [
"/",
{
"Fn::Select": [
0,
{
"Fn::GetAtt": [
"LBListenerTargetGroupF04FCF6D",
"LoadBalancerArns"
]
}
]
"Ref": "LBListener49E825B4"
}
]
}
Expand Down
5 changes: 1 addition & 4 deletions packages/@aws-cdk/aws-autoscaling/test/test.scaling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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
*
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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');
}
Expand All @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)}`;
}

/**
Expand Down Expand Up @@ -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)}`;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
Expand All @@ -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 ] } ] }
]
]
}
Expand Down
Loading

0 comments on commit de488df

Please sign in to comment.