Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ec2): add vpn metrics #1979

Merged
merged 3 commits into from
Mar 12, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-ec2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -341,3 +341,5 @@ vpc.addVpnConnection('Dynamic', {
```

Routes will be propagated on the route tables associated with the private subnets.

VPN connections expose metric across all tunnels and per connection.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

example/snippet on how to use?

2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-ec2/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@ export * from './vpn';

// AWS::EC2 CloudFormation Resources:
export * from './ec2.generated';

import './ec2-augmentations.generated';
39 changes: 39 additions & 0 deletions packages/@aws-cdk/aws-ec2/lib/vpn.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import cloudwatch = require('@aws-cdk/aws-cloudwatch');
import cdk = require('@aws-cdk/cdk');
import net = require('net');
import { CfnCustomerGateway, CfnVPNConnection, CfnVPNConnectionRoute } from './ec2.generated';
Expand Down Expand Up @@ -98,6 +99,44 @@ export enum VpnConnectionType {
}

export class VpnConnection extends cdk.Construct implements IVpnConnection {
/**
* Return the given named metric for all VPN connections.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"all VPC connections in the account/region"

*/
public static metricAll(metricName: string, props?: cloudwatch.MetricCustomization): cloudwatch.Metric {
return new cloudwatch.Metric({
namespace: 'AWS/VPN',
metricName,
...props
});
}

/**
* Metric for the tunnel state of all VPN connections.
*
* @default average over 5 minutes
*/
public static metricAllTunnelState(props?: cloudwatch.MetricCustomization): cloudwatch.Metric {
return this.metricAll('TunnelSate', { statistic: 'avg', ...props });
}

/**
* Metric for the tunnel data in of all VPN connections.
*
* @default sum over 5 minutes
*/
public static metricAllTunnelDataIn(props?: cloudwatch.MetricCustomization): cloudwatch.Metric {
return this.metricAll('TunnelDataIn', { statistic: 'sum', ...props });
}

/**
* Metric for the tunnel data out of all VPN connections.
*
* @default sum over 5 minutes
*/
public static metricAllTunnelDataOut(props?: cloudwatch.MetricCustomization): cloudwatch.Metric {
return this.metricAll('TunnelDataOut', { statistic: 'sum', ...props });
}

public readonly vpnId: string;
public readonly customerGatewayId: string;
public readonly customerGatewayIp: string;
Expand Down
4 changes: 3 additions & 1 deletion packages/@aws-cdk/aws-ec2/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,13 @@
},
"dependencies": {
"@aws-cdk/aws-iam": "^0.25.2",
"@aws-cdk/aws-cloudwatch": "^0.25.2",
"@aws-cdk/cdk": "^0.25.2",
"@aws-cdk/cx-api": "^0.25.2"
},
"homepage": "https://github.com/awslabs/aws-cdk",
"peerDependencies": {
"@aws-cdk/aws-cloudwatch": "^0.25.2",
"@aws-cdk/cdk": "^0.25.2",
"@aws-cdk/cx-api": "^0.25.2"
},
Expand All @@ -78,4 +80,4 @@
"resource-attribute:@aws-cdk/aws-ec2.ISecurityGroup.securityGroupVpcId"
]
}
}
}
41 changes: 40 additions & 1 deletion packages/@aws-cdk/aws-ec2/test/test.vpn.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { expect, haveResource, } from '@aws-cdk/assert';
import { Stack } from '@aws-cdk/cdk';
import { Test } from 'nodeunit';
import { VpcNetwork } from '../lib';
import { VpcNetwork, VpnConnection } from '../lib';

export = {
'can add a vpn connection to a vpc with a vpn gateway'(test: Test) {
Expand Down Expand Up @@ -258,6 +258,45 @@ export = {
}
}), /`tunnelInsideCidr`.+size/);

test.done();
},

'can use metricTunnelState on a vpn connection'(test: Test) {
// GIVEN
const stack = new Stack();

const vpc = new VpcNetwork(stack, 'VpcNetwork', {
vpnGateway: true
});

const vpn = vpc.addVpnConnection('Vpn', {
ip: '192.0.2.1'
});

// THEN
test.deepEqual(stack.node.resolve(vpn.metricTunnelState()), {
dimensions: { VpnId: { Ref: 'VpcNetworkVpnA476C58D' } },
namespace: 'AWS/VPN',
metricName: 'TunnelState',
periodSec: 300,
statistic: 'Average'
});

test.done();
},

'can use metricAllTunnelDataOut'(test: Test) {
// GIVEN
const stack = new Stack();

// THEN
test.deepEqual(stack.node.resolve(VpnConnection.metricAllTunnelDataOut()), {
namespace: 'AWS/VPN',
metricName: 'TunnelDataOut',
periodSec: 300,
statistic: 'Sum'
});

test.done();
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"overrides": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't call these "overrides". We should think about them simply as configuration of the augmentation system that has some default values, so you don't have to supply them but you can, and I would avoid attaching any heuristics to them (i.e. "Base" suffix).

So, maybe:

{
  "baseClass": "VpnConnectionBase",
  "baseClassFile": "vpn",
  "interface": "IVpnConnection"
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will rename these. The current implementation doesn't attach anything to these overrides.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does:

  • baseClass is the resource name + "Base" suffix
  • baseClassFile is the l2-base
  • interface is I + resource name

Copy link
Contributor Author

@jogold jogold Mar 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I'm following here.

const baseClassName = (overrides && overrides.class) || l2ClassName + 'Base';
const interfaceName = (overrides && overrides.interface) || 'I' + l2ClassName;
const baseClassModule = `./${(overrides && overrides.module) || `${kebabL2ClassName}-base`}`;

From above, if there's an override it's taken as is, no?

Regarding the names in the JSON file:

  • baseClass or class? for the VpnConnection it's not a base class because there's no VpnConnectionBase in this case
  • ...file or ...module?

Copy link
Contributor

@eladb eladb Mar 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yes, you are right. I guess class makes sense (and the doc should say that by default it uses the "Base" class). I would go with file and not module because we mostly (in our project) "module" is interchangeable with "package" (or "npm module").

"interface": "IVpnConnection",
"class": "VpnConnection",
"module": "vpn"
},
"metrics": {
"namespace": "AWS/VPN",
"dimensions": { "VpnId": "this.vpnId" },
"metrics": [
{
"name": "TunnelState",
"documentation": "The state of the tunnel. 0 indicates DOWN and 1 indicates UP."
},
{
"name": "TunnelDataIn",
"documentation": "The bytes received through the VPN tunnel.",
"type": "count"
},
{
"name": "TunnelDataOut",
"documentation": "The bytes sent through the VPN tunnel.",
"type": "count"
}
]
}
}
24 changes: 23 additions & 1 deletion packages/@aws-cdk/cfnspec/lib/schema/augmentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,28 @@ export interface ResourceAugmentation {
* Metric augmentations for this resource type
*/
metrics?: ResourceMetricAugmentations;

/**
* Overrides for this resource augmentation
*/
overrides?: ResourceOverrides;
}

export interface ResourceOverrides {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to AugmentationOptions and line up with the schema above, specify the @defaults

/**
* The name of the resource class
*/
class?: string;

/**
* The name of the resource interface
*/
interface?: string;

/**
* The name of the module
*/
module?: string;
}

export interface ResourceMetricAugmentations {
Expand Down Expand Up @@ -57,4 +79,4 @@ export enum MetricType {
* property. The most useful aggregate of this type of metric is "Max".
*/
Gauge = 'gauge'
}
}
11 changes: 6 additions & 5 deletions tools/cfn2ts/lib/augmentation-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class AugmentationGenerator {

if (aug.metrics) {
this.code.line('import cloudwatch = require("@aws-cdk/aws-cloudwatch");');
this.emitMetricAugmentations(resourceTypeName, aug.metrics);
this.emitMetricAugmentations(resourceTypeName, aug.metrics, aug.overrides);
hadAugmentations = true;
}
}
Expand All @@ -39,14 +39,15 @@ export class AugmentationGenerator {
return await this.code.save(dir);
}

private emitMetricAugmentations(resourceTypeName: string, metrics: schema.ResourceMetricAugmentations) {
private emitMetricAugmentations(resourceTypeName: string, metrics: schema.ResourceMetricAugmentations, overrides?: schema.ResourceOverrides) {
const cfnName = SpecName.parse(resourceTypeName);
const resourceName = genspec.CodeName.forCfnResource(cfnName, this.affix);
const l2ClassName = resourceName.className.replace(/^Cfn/, '');
const kebabL2ClassName = l2ClassName.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase();

const baseClassName = l2ClassName + 'Base';
const interfaceName = 'I' + l2ClassName;
const baseClassModule = `./${l2ClassName.toLowerCase()}-base`;
const baseClassName = (overrides && overrides.class) || l2ClassName + 'Base';
const interfaceName = (overrides && overrides.interface) || 'I' + l2ClassName;
const baseClassModule = `./${(overrides && overrides.module) || `${kebabL2ClassName}-base`}`;

this.code.line(`import { ${baseClassName} } from "${baseClassModule}";`);

Expand Down