-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Changes from 5 commits
214a0f0
7805f75
7cf20a6
4071407
61b059c
7eb7e19
4ec6f25
99870c0
3573348
696b194
42bb7e0
ff7405e
9bc867f
e769a67
7994724
a3c8504
42867b8
cc9db36
0ec44ad
92c60e1
4ad2928
d1ce352
9634d78
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -203,6 +203,45 @@ 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 }; | ||
|
||
/** | ||
* The function execution time (in seconds) after which Lambda terminates | ||
* the function. Because the execution time affects cost, set this value | ||
* based on the function's expected execution time. | ||
* | ||
* @default cdk.Duration.seconds(900) | ||
*/ | ||
readonly timeout?: cdk.Duration; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are important caveats mentioned in the cloudformation documentation Please reflect this in your documentation There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added to doc and commented in the code |
||
|
||
/** | ||
* The amount of memory, in MB, that is allocated to your Lambda function. | ||
* Lambda uses this value to proportionally allocate the amount of CPU | ||
* power. For more information, see Resource Model in the AWS Lambda | ||
* Developer Guide. | ||
* | ||
* @default 128 | ||
*/ | ||
readonly memorySize?: number; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This value must be a multiple of 64. The range is 960 to 3008. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 even if the canary does not hit an endpoint that has | ||
* X-ray tracing enabled. | ||
* | ||
* @default false | ||
*/ | ||
readonly tracing?: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
||
} | ||
|
||
/** | ||
|
@@ -264,6 +303,7 @@ export class Canary extends cdk.Resource { | |
failureRetentionPeriod: props.failureRetentionPeriod?.toDays(), | ||
successRetentionPeriod: props.successRetentionPeriod?.toDays(), | ||
code: this.createCode(props), | ||
runConfig: this.createRunConfig(props), | ||
}); | ||
|
||
this.canaryId = resource.attrId; | ||
|
@@ -384,6 +424,19 @@ export class Canary extends cdk.Resource { | |
}; | ||
} | ||
|
||
/** | ||
* Retruns a runConfig object | ||
*/ | ||
private createRunConfig(props:CanaryProps): CfnCanary.RunConfigProperty { | ||
const DEFAULT_CANARY_TIMEOUT_IN_SECONDS = 900; | ||
flochaz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return { | ||
timeoutInSeconds: props.timeout?.toSeconds() ?? DEFAULT_CANARY_TIMEOUT_IN_SECONDS, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
*/ | ||
|
There was a problem hiding this comment.
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