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(synthetics): add RunConfig parameters to Cloudwatch Synthetics Canary L2 construct #11865

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
214a0f0
Add RunConfig parameters to Cloudwatch Synthetics Canary L2 construct
flochaz Dec 4, 2020
7805f75
remove duplicated test
flochaz Dec 4, 2020
7cf20a6
Merge branch 'master' into flochaz/syntheticsRunConfig
flochaz Dec 4, 2020
4071407
Add environment variables example in README
flochaz Dec 4, 2020
61b059c
Merge branch 'flochaz/syntheticsRunConfig' of github.com:flochaz/aws-…
flochaz Dec 4, 2020
7eb7e19
Merge branch 'master' into flochaz/syntheticsRunConfig
flochaz Dec 7, 2020
4ec6f25
Add RunConfig parameters to Cloudwatch Synthetics Canary L2 construct
flochaz Dec 4, 2020
99870c0
remove duplicated test
flochaz Dec 4, 2020
3573348
Add environment variables example in README
flochaz Dec 4, 2020
696b194
Merge branch 'flochaz/syntheticsRunConfig' of https://github.com/floc…
flochaz Dec 8, 2020
42bb7e0
Default canary timeout to schedule interval if lower than max timeout
flochaz Dec 8, 2020
ff7405e
Merge branch 'master' into flochaz/syntheticsRunConfig
flochaz Dec 8, 2020
9bc867f
Merge branch 'master' into flochaz/syntheticsRunConfig
flochaz Dec 14, 2020
e769a67
Merge branch 'master' into flochaz/syntheticsRunConfig
flochaz Dec 29, 2020
7994724
Merge branch 'master' into flochaz/syntheticsRunConfig
flochaz Jan 4, 2021
a3c8504
Merge branch 'master' into flochaz/syntheticsRunConfig
flochaz Jan 8, 2021
42867b8
Fix doc, default and missing permission for tracing
flochaz Jan 20, 2021
cc9db36
Merge branch 'master' into flochaz/syntheticsRunConfig
flochaz Jan 20, 2021
0ec44ad
Add test for xray permission
flochaz Jan 20, 2021
92c60e1
Merge branch 'master' into flochaz/syntheticsRunConfig
flochaz Jan 20, 2021
4ad2928
Merge branch 'master' into flochaz/syntheticsRunConfig
flochaz Jan 26, 2021
d1ce352
Merge branch 'master' into flochaz/syntheticsRunConfig
flochaz Feb 16, 2021
9634d78
Merge branch 'master' into flochaz/syntheticsRunConfig
flochaz Mar 1, 2021
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
5 changes: 4 additions & 1 deletion packages/@aws-cdk/aws-synthetics/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ const canary = new synthetics.Canary(this, 'MyCanary', {
handler: 'index.handler',
}),
runtime: synthetics.Runtime.SYNTHETICS_NODEJS_PUPPETEER_3_0,
environment: {
URL: 'https://api.example.com/user/books/topbook/'
}
});
```

Expand All @@ -57,7 +60,7 @@ const log = require('SyntheticsLogger');
const pageLoadBlueprint = async function () {

// INSERT URL here
const URL = "https://api.example.com/user/books/topbook/";
const URL =process.env.URL;

let page = await synthetics.getPage();
const response = await page.goto(URL, {waitUntil: 'domcontentloaded', timeout: 30000});
Expand Down
79 changes: 76 additions & 3 deletions packages/@aws-cdk/aws-synthetics/lib/canary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,46 @@ export interface CanaryProps {
*/
readonly test: Test;

/**
* Key-value pairs that the Synthetics caches and makes available for your canary
* scripts. Use environment variables to apply configuration changes, such
* as test and production environment configurations, without changing your
* Canary script source code.
*
* @default - No environment variables.
*/
readonly environment?: { [key: string]: string };
Copy link
Contributor

Choose a reason for hiding this comment

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

remove when merging from master


/**
* How long the canary is allowed to run before it must stop.
* You can't set this time to be longer than the frequency of the runs of this canary.
* If you omit this field, the frequency of the canary is used as this value, up to a maximum of 900 seconds.
*
* @default cdk.Duration.seconds(840)
*/
readonly timeout?: cdk.Duration;
Copy link
Contributor

Choose a reason for hiding this comment

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

There are important caveats mentioned in the cloudformation documentation
You can't set this time to be longer than the frequency of the runs of this canary.
If you omit this field, the frequency of the canary is used as this value, up to a maximum of 900 seconds.
See: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-synthetics-canary-runconfig.html#cfn-synthetics-canary-runconfig-timeoutinseconds

Please reflect this in your documentation

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be in the documentation as well as in the code, if the provided value is not valid we should throw an error. There has been a lot of discussion about this feature in #9300

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added to doc and commented in the code


/**
* The maximum amount of memory that the canary can use while running. This value
* must be a multiple of 64. The range is 960 to 3008.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* must be a multiple of 64. The range is 960 to 3008.
* must be a multiple of 64. The range is 960 to 3008.

*
* @default 960
*/
readonly memorySize?: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it to the doc but not sure if I should check the % 64 === 0 and throw. I haven't seen such things being done anywhere else in cdk source code. Let me know if I should add the check and throw.

Copy link
Contributor

Choose a reason for hiding this comment

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

this should use Size from @aws-cdk/core and also please add checks for the multiple of 64 and the min and max values


/**
* Specifies whether this canary is to use active AWS X-Ray tracing when it runs.
* Active tracing enables this canary run to be displayed in the ServiceLens and X-Ray service maps
Comment on lines +266 to +267
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Specifies whether this canary is to use active AWS X-Ray tracing when it runs.
* Active tracing enables this canary run to be displayed in the ServiceLens and X-Ray service maps
* Specifies whether this canary is to use active AWS X-Ray tracing when it runs.
*
* Active tracing enables this canary run to be displayed in the ServiceLens and X-Ray service maps

* even if the canary does not hit an endpoint that has X-ray tracing enabled.
*
* You can enable active tracing only for canaries that use version syn-nodejs-2.0 or later for their canary runtime.
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a check for this? it should be version != SYNTHETICS_1_0

*
* Enabling tracing increases canary run time by 2.5% to 7%.
*
* @default false
*/
readonly tracing?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to give the canary permissions described here: https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch_Synthetics_Canaries_tracing.html
Also would be good to mention in your documentation that enabling this option increases canary run time by 2.5% to 7%.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

Copy link
Contributor

Choose a reason for hiding this comment

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

please note that if a role is provided, it must also have permissions documented here: https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch_Synthetics_Canaries_tracing.html


}

/**
Expand Down Expand Up @@ -284,18 +324,21 @@ export class Canary extends cdk.Resource {
encryption: s3.BucketEncryption.KMS_MANAGED,
});

this.role = props.role ?? this.createDefaultRole(props.artifactsBucketLocation?.prefix);
this.role = props.role ?? this.createDefaultRole(props.artifactsBucketLocation?.prefix, props.tracing);

const schedule = this.createSchedule(props);

const resource: CfnCanary = new CfnCanary(this, 'Resource', {
artifactS3Location: this.artifactsBucket.s3UrlForObject(props.artifactsBucketLocation?.prefix),
executionRoleArn: this.role.roleArn,
startCanaryAfterCreation: props.startAfterCreation ?? true,
runtimeVersion: props.runtime.name,
name: this.physicalName,
schedule: this.createSchedule(props),
schedule: schedule,
failureRetentionPeriod: props.failureRetentionPeriod?.toDays(),
successRetentionPeriod: props.successRetentionPeriod?.toDays(),
code: this.createCode(props),
runConfig: this.createRunConfig(props, schedule),
});

this.canaryId = resource.attrId;
Expand Down Expand Up @@ -339,7 +382,7 @@ export class Canary extends cdk.Resource {
/**
* Returns a default role for the canary
*/
private createDefaultRole(prefix?: string): iam.IRole {
private createDefaultRole(prefix?: string, tracingEnabled?: boolean): iam.IRole {
const { partition } = cdk.Stack.of(this);
// Created role will need these policies to run the Canary.
// https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-synthetics-canary.html#cfn-synthetics-canary-executionrolearn
Expand All @@ -365,6 +408,15 @@ export class Canary extends cdk.Resource {
],
});

if (tracingEnabled) {
policy.addStatements(
new iam.PolicyStatement({
resources: ['*'],
actions: ['xray:PutTraceSegments'],
}),
);
}

return new iam.Role(this, 'ServiceRole', {
assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
inlinePolicies: {
Expand Down Expand Up @@ -400,6 +452,27 @@ export class Canary extends cdk.Resource {
};
}

/**
* Retruns a runConfig object
*/
private createRunConfig(props:CanaryProps, schedule: CfnCanary.ScheduleProperty): CfnCanary.RunConfigProperty {
// Cloudformation implementation made TimeoutInSeconds a required field where it should not (see links below).
// So here is a workaround to fix https://github.com/aws/aws-cdk/issues/9300 and still follow documented behavior.
// https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-synthetics-canary-runconfig.html#cfn-synthetics-canary-runconfig-timeoutinseconds
// https://github.com/aws-cloudformation/aws-cloudformation-resource-providers-synthetics/issues/31

const MAX_CANARY_TIMEOUT = 840;
const RATE_IN_SECONDS = Schedule.expressionToRateInSeconds(schedule.expression);
const DEFAULT_CANARY_TIMEOUT_IN_SECONDS = RATE_IN_SECONDS <= MAX_CANARY_TIMEOUT ? RATE_IN_SECONDS : MAX_CANARY_TIMEOUT;
Comment on lines +464 to +466
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for all caps here


return {
timeoutInSeconds: props.timeout?.toSeconds() ?? DEFAULT_CANARY_TIMEOUT_IN_SECONDS,
Copy link
Contributor

Choose a reason for hiding this comment

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

needs to check this is smaller than the rate

activeTracing: props.tracing,
environmentVariables: props.environment,
memoryInMb: props.memorySize,
};
}

/**
* Creates a unique name for the canary. The generated name is the physical ID of the canary.
*/
Expand Down
27 changes: 27 additions & 0 deletions packages/@aws-cdk/aws-synthetics/lib/schedule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,33 @@ export class Schedule {
return new Schedule(`rate(${minutes} minutes)`);
}

/**
* Convert a Schedule expression in to a number of seconds
*
* @param expression Schedule expression such as 'rate(2 minutes)'
*/
public static expressionToRateInSeconds(expression: string): number {
Copy link
Contributor

Choose a reason for hiding this comment

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

convert to a Duration

// extract data isolating the number and units of the rate set
const regularExpression = /^rate\(([0-9]*) ([a-z]*)\)/;
const split = regularExpression.exec(expression);
Comment on lines +52 to +53
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer using just string checks instead of a regular expression

const number = Number(split ? split[1] : '0');
const unit = split ? split[2] : 'minutes';

switch (unit) {
case 'second':
case 'seconds':
return number;
case 'minute':
case 'minutes':
return 60 * number;
case 'hour':
case 'hours':
return 3600 * number;
default:
throw new Error('Unit not supported');
}
}

private constructor(
/**
* The Schedule expression
Expand Down
165 changes: 164 additions & 1 deletion packages/@aws-cdk/aws-synthetics/test/canary.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import '@aws-cdk/assert/jest';
import { objectLike } from '@aws-cdk/assert';
import { objectLike, Capture } from '@aws-cdk/assert';
import * as iam from '@aws-cdk/aws-iam';
import * as s3 from '@aws-cdk/aws-s3';
import { App, Duration, Lazy, Stack } from '@aws-cdk/core';
Expand Down Expand Up @@ -170,6 +170,130 @@ test('Runtime can be specified', () => {
});
});

test('RunConfig attributes can be specified', () => {
// GIVEN
const stack = new Stack(new App(), 'canaries');
const environmentVariables = {
test_key_1: 'TEST_VALUE_1',
test_key_2: 'TEST_VALUE_2',
};
const timeout = 10;
const memorySize = 256;
const activateTracing = true;

// WHEN
new synthetics.Canary(stack, 'Canary', {
runtime: synthetics.Runtime.SYNTHETICS_1_0,
test: synthetics.Test.custom({
handler: 'index.handler',
code: synthetics.Code.fromInline('/* Synthetics handler code */'),
}),
environment: environmentVariables,
timeout: Duration.seconds(timeout),
memorySize: memorySize,
tracing: activateTracing,
});

// THEN
expect(stack).toHaveResourceLike('AWS::Synthetics::Canary', {
RunConfig: {
EnvironmentVariables: environmentVariables,
TimeoutInSeconds: timeout,
MemoryInMB: memorySize,
ActiveTracing: activateTracing,
},
});
});

test('If timeout not provided it default to schedule set with rate', () => {
// GIVEN
const stack = new Stack(new App(), 'canaries');
const scheduledRate = Duration.minutes(3);
// WHEN
new synthetics.Canary(stack, 'Canary', {
runtime: synthetics.Runtime.SYNTHETICS_1_0,
test: synthetics.Test.custom({
handler: 'index.handler',
code: synthetics.Code.fromInline('/* Synthetics handler code */'),
}),
schedule: synthetics.Schedule.rate(scheduledRate),
});

// THEN
expect(stack).toHaveResourceLike('AWS::Synthetics::Canary', {
RunConfig: {
TimeoutInSeconds: scheduledRate.toSeconds(),
},
});
});

test('If timeout not provided it default to schedule set with expressionString', () => {
// GIVEN
const stack = new Stack(new App(), 'canaries');
// WHEN
new synthetics.Canary(stack, 'Canary', {
runtime: synthetics.Runtime.SYNTHETICS_1_0,
test: synthetics.Test.custom({
handler: 'index.handler',
code: synthetics.Code.fromInline('/* Synthetics handler code */'),
}),
schedule: {
expressionString: 'rate(2 minutes)',
},
});

// THEN
expect(stack).toHaveResourceLike('AWS::Synthetics::Canary', {
RunConfig: {
TimeoutInSeconds: 120,
},
});
});


test('If timeout not provided it default to default schedule if schedule is not set', () => {
// GIVEN
const stack = new Stack(new App(), 'canaries');
// WHEN
new synthetics.Canary(stack, 'Canary', {
runtime: synthetics.Runtime.SYNTHETICS_1_0,
test: synthetics.Test.custom({
handler: 'index.handler',
code: synthetics.Code.fromInline('/* Synthetics handler code */'),
}),
});

// THEN
expect(stack).toHaveResourceLike('AWS::Synthetics::Canary', {
RunConfig: {
TimeoutInSeconds: 300,
},
});
});

test('If timeout not provided it default to MAX timeout if schedule is higher than max', () => {
// GIVEN
const stack = new Stack(new App(), 'canaries');
const scheduledRate = Duration.hours(1);

// WHEN
new synthetics.Canary(stack, 'Canary', {
runtime: synthetics.Runtime.SYNTHETICS_1_0,
test: synthetics.Test.custom({
handler: 'index.handler',
code: synthetics.Code.fromInline('/* Synthetics handler code */'),
}),
schedule: synthetics.Schedule.rate(scheduledRate),
});

// THEN
expect(stack).toHaveResourceLike('AWS::Synthetics::Canary', {
RunConfig: {
TimeoutInSeconds: 840,
},
});
});

test('Runtime can be customized', () => {
// GIVEN
const stack = new Stack(new App(), 'canaries');
Expand Down Expand Up @@ -269,6 +393,45 @@ test('Schedule can be set to run once', () => {
});
});

test('On tracing enabled, the generated role will have xray PutTraceSegments permission', () => {
// GIVEN
const stack = new Stack(new App(), 'canaries');


// WHEN
new synthetics.Canary(stack, 'Canary', {
test: synthetics.Test.custom({
handler: 'index.handler',
code: synthetics.Code.fromInline('/* Synthetics handler code */'),
}),
runtime: synthetics.Runtime.SYNTHETICS_NODEJS_2_0,
tracing: true,
});

// THEN
const policyStatements = Capture.anyType();
expect(stack).toHaveResourceLike('AWS::IAM::Role', {
Policies: [
{
PolicyDocument: {
Statement: policyStatements.capture(),
},
},
],
});

expect(policyStatements.capturedValue).toEqual(
expect.arrayContaining([
expect.objectContaining({
Action: 'xray:PutTraceSegments',
Effect: 'Allow',
Resource: '*',
}),
]),
);

});

test('Throws when rate above 60 minutes', () => {
// GIVEN
const stack = new Stack(new App(), 'canaries');
Expand Down
15 changes: 12 additions & 3 deletions packages/@aws-cdk/aws-synthetics/test/integ.canary.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,10 @@
"DurationInSeconds": "0",
"Expression": "rate(1 minute)"
},
"StartCanaryAfterCreation": true
"StartCanaryAfterCreation": true,
"RunConfig": {
"TimeoutInSeconds":60
}
}
},
"MyCanaryOneArtifactsBucketDF4A487D": {
Expand Down Expand Up @@ -286,7 +289,10 @@
"DurationInSeconds": "0",
"Expression": "rate(5 minutes)"
},
"StartCanaryAfterCreation": true
"StartCanaryAfterCreation": true,
"RunConfig": {
"TimeoutInSeconds":300
}
}
},
"MyCanaryTwoArtifactsBucket79B179B6": {
Expand Down Expand Up @@ -453,7 +459,10 @@
"DurationInSeconds": "0",
"Expression": "rate(5 minutes)"
},
"StartCanaryAfterCreation": true
"StartCanaryAfterCreation": true,
"RunConfig": {
"TimeoutInSeconds":300
}
}
}
},
Expand Down