feat(synthetics): add RunConfig parameters to Cloudwatch Synthetics Canary L2 construct #11865

flochaz · 2020-12-04T07:54:36Z

Cloudwatch Synthetics recently added Environment Variables injection feature.

This PR add all new RunConfig properties (https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-synthetics-canary-runconfig.html) to the Canary L2 Construct.

Those new properties are all about lambda configuration so used same naming and docs as lambda equivalent.

Fixes #10515

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2020-12-04T07:54:39Z

mergify · 2020-12-04T07:55:14Z

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

mergify · 2020-12-04T07:57:26Z

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

…cdk into flochaz/syntheticsRunConfig

packages/@aws-cdk/aws-synthetics/lib/canary.ts

danataws

Suggest using the defaults set by the service.

NetaNir · 2020-12-04T22:27:09Z

Thanks for submitting this, before I review, have you seen this issue: #9300, which discuss adding the RunConfig. We had some issues with the implementation, wondering if you considered them

NetaNir · 2020-12-04T22:42:26Z

Im also wondering about the impact of the added Lambda config to the function being updated. For example, if the environment variables are modified, will the function be redeployed? We have a lot of logic that handles this edge cases in the aws lambda construct. Wondering how we can avoid duplicating this logic

flochaz · 2020-12-05T07:13:16Z

Im also wondering about the impact of the added Lambda config to the function being updated. For example, if the environment variables are modified, will the function be redeployed? We have a lot of logic that handles this edge cases in the aws lambda construct. Wondering how we can avoid duplicating this logic

Since those properties are not set to the lambda but to the Canary CFN resource I expect this change to be handled directly in the service. But good point, I will make a test to make sure of it.

NetaNir · 2020-12-06T20:28:45Z

I would actually assume that all Lambda properties are passed to the function as is by the Synthetics service, if that's the case we need to think how we can reuse the logic which handles these properties updates from the aws-lambda module.

flochaz · 2020-12-07T10:19:12Z

I would actually assume that all Lambda properties are passed to the function as is by the Synthetics service, if that's the case we need to think how we can reuse the logic which handles these properties updates from the aws-lambda module.

I did some test using raw cloud Formation and creation, deletion and update of environment variables are properly propagated on stack update. Do you mind to point me to the logic in lambda construct ?

…haz/aws-cdk into flochaz/syntheticsRunConfig

danataws · 2020-12-08T16:57:09Z

also wondering about the impact of the added Lambda config to the function being updated. For example, if the environment variables are modified, will the function be redeployed? We have a lot of logic that handles this edge cases in the aws lambda construct. Wondering how we can avoid duplicating this

If the env vars are modified, CloudWatch Synthetics updates the function.

colvint · 2021-01-21T15:37:30Z

@colvint you can escape hatches in the meantime

Right, thanks @NetaNir -- we're gonna hold off on canaries until this lands. Take your time. Thanks for all the work here gang!

flochaz · 2021-01-26T13:34:48Z

@NetaNir @colvint Should be good now

markcarroll · 2021-02-09T01:19:31Z

Hey guys, this looks very helpful. We are deploying some canaries right now and would love to be able to ditch the L1 escape hatch code. Did this just drop off the radar?

aws-cdk-automation · 2021-03-01T21:01:55Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: 9634d78
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

NetaNir · 2021-03-01T21:04:49Z

We have seen some odd behavior on function update when using the Function resource. Mainly around the update of properties such as Environment Variables. Since this has been a source of bad user experience (hard to debug), before we add this I want to think about a group of integration tests we can add that will verify the behavior, this will allow us to track the behavior and document it clearly.

Sorry for the inconvenience, in the meantime please use escape hatches

jonny-rimek · 2021-05-02T12:04:40Z

adding an env var looks like this:

        const cfnCanary = canary.node.defaultChild as synthetics.CfnCanary
        cfnCanary.addPropertyOverride('RunConfig.EnvironmentVariables', {
            API_URL: "someurl"
        })

BenChaimberg · 2021-06-03T16:01:57Z

@flochaz thank you for the work you've done so far on this feature! We would like to continue this work but in a more segmented manner – would you be willing to split out just the environment variable portion of this PR? If not, I am happy to modify your changes and cite you as a collaborator :)

I'll have more feedback on the implementation itself after the split

flochaz · 2021-06-04T07:06:48Z

Sure, I can do that next week.

…

On Thu, 3 Jun 2021 at 18:02, Ben Chaimberg ***@***.***> wrote: @flochaz <https://github.com/flochaz> thank you for the work you've done so far on this feature! We would like to continue this work but in a more segmented manner – would you be willing to split out just the environment variable portion of this PR? If not, I am happy to modify your changes and cite you as a collaborator :) I'll have more feedback on the implementation itself after the split — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11865 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJLIULAKZDVQ4YYRGRLE5TTQ6RQLANCNFSM4UNCXO7Q> .

-- Florian Chazal

BenChaimberg · 2021-06-11T04:40:29Z

@flochaz split out your changes into #15082 – thanks again!

BenChaimberg · 2021-06-16T23:11:02Z

packages/@aws-cdk/aws-synthetics/lib/canary.ts

+   *
+   * @default 128
+   */
+  readonly memorySize?: number;


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

BenChaimberg · 2021-06-16T23:11:25Z

packages/@aws-cdk/aws-synthetics/lib/canary.ts

+   * 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


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

BenChaimberg · 2021-06-16T23:13:38Z

packages/@aws-cdk/aws-synthetics/lib/canary.ts

+   *
+   * @default false
+   */
+  readonly tracing?: boolean;


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

BenChaimberg · 2021-06-16T23:14:45Z

packages/@aws-cdk/aws-synthetics/lib/canary.ts

+   * 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.


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

BenChaimberg · 2021-06-16T23:15:34Z

packages/@aws-cdk/aws-synthetics/lib/canary.ts

+   *
+   * @default - No environment variables.
+   */
+  readonly environment?: { [key: string]: string };


remove when merging from master

BenChaimberg · 2021-06-16T23:16:05Z

packages/@aws-cdk/aws-synthetics/lib/canary.ts

+    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;


no need for all caps here

BenChaimberg · 2021-06-16T23:17:23Z

packages/@aws-cdk/aws-synthetics/lib/canary.ts

+
+  /**
+   * 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.


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.

BenChaimberg · 2021-06-16T23:17:59Z

packages/@aws-cdk/aws-synthetics/lib/canary.ts

+    const DEFAULT_CANARY_TIMEOUT_IN_SECONDS = RATE_IN_SECONDS <= MAX_CANARY_TIMEOUT ? RATE_IN_SECONDS : MAX_CANARY_TIMEOUT;
+
+    return {
+      timeoutInSeconds: props.timeout?.toSeconds() ?? DEFAULT_CANARY_TIMEOUT_IN_SECONDS,


needs to check this is smaller than the rate

BenChaimberg · 2021-06-16T23:18:33Z

packages/@aws-cdk/aws-synthetics/lib/schedule.ts

+    const regularExpression = /^rate\(([0-9]*) ([a-z]*)\)/;
+    const split = regularExpression.exec(expression);


prefer using just string checks instead of a regular expression

BenChaimberg · 2021-06-16T23:18:54Z

packages/@aws-cdk/aws-synthetics/lib/schedule.ts

+   *
+   * @param expression Schedule expression such as 'rate(2 minutes)'
+   */
+  public static expressionToRateInSeconds(expression: string): number {


convert to a Duration

RichiCoder1 · 2022-01-12T17:05:49Z

Is this PR still actively moving forward? Is this something I could help pick up again?

flochaz · 2022-01-12T20:46:51Z

if you can please do. I didn't ended up doing the split requested due to lack of time ...

…

On Wed, Jan 12, 2022, 18:06 Richard Simpson ***@***.***> wrote: Is this PR still actively moving forward? Is this something I could help pick up again? — Reply to this email directly, view it on GitHub <#11865 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJLIUID3HEDIYNPRQZCZLLUVWYHTANCNFSM4UNCXO7Q> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

This PR is a continuation of aws#11865 with the intention of filling out the remaining RunConfig values as well as adding missing VpcConfig properties. I wasn't sure whether VPC settings should be top level (like Lambda) or in their own configuration object, so I made them seperate for now.

RichiCoder1 · 2022-01-19T00:46:46Z

I've created a follow up here: #18447

kaizencc · 2022-03-11T19:20:38Z

Closing this PR since it looks like it's getting picked up in separate pieces by other members of the community

This PR adds vpc support to synthetics and is a continuation of #11865. See [Running a canary on a vpc](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch_Synthetics_Canaries_VPC.html). Fixes #9954 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Add RunConfig parameters to Cloudwatch Synthetics Canary L2 construct

214a0f0

remove duplicated test

7805f75

flochaz changed the title ~~Add RunConfig parameters to Cloudwatch Synthetics Canary L2 construct~~ feat: Add RunConfig parameters to Cloudwatch Synthetics Canary L2 construct Dec 4, 2020

Merge branch 'master' into flochaz/syntheticsRunConfig

7cf20a6

flochaz changed the title ~~feat: Add RunConfig parameters to Cloudwatch Synthetics Canary L2 construct~~ feat(synthetics): add RunConfig parameters to Cloudwatch Synthetics Canary L2 construct Dec 4, 2020

github-actions bot assigned NetaNir Dec 4, 2020

github-actions bot added the @aws-cdk/aws-synthetics Related to Amazon CloudWatch Synthetics label Dec 4, 2020

flochaz added 2 commits December 4, 2020 08:57

Add environment variables example in README

4071407

Merge branch 'flochaz/syntheticsRunConfig' of github.com:flochaz/aws-…

61b059c

…cdk into flochaz/syntheticsRunConfig

danataws reviewed Dec 4, 2020

View reviewed changes

packages/@aws-cdk/aws-synthetics/lib/canary.ts Outdated Show resolved Hide resolved

danataws reviewed Dec 4, 2020

View reviewed changes

flochaz added 7 commits December 7, 2020 11:19

Merge branch 'master' into flochaz/syntheticsRunConfig

7eb7e19

Add RunConfig parameters to Cloudwatch Synthetics Canary L2 construct

4ec6f25

remove duplicated test

99870c0

Add environment variables example in README

3573348

Merge branch 'flochaz/syntheticsRunConfig' of https://github.com/floc…

696b194

…haz/aws-cdk into flochaz/syntheticsRunConfig

Default canary timeout to schedule interval if lower than max timeout

42bb7e0

Merge branch 'master' into flochaz/syntheticsRunConfig

ff7405e

flochaz added 3 commits December 14, 2020 14:18

Merge branch 'master' into flochaz/syntheticsRunConfig

9bc867f

Merge branch 'master' into flochaz/syntheticsRunConfig

e769a67

Merge branch 'master' into flochaz/syntheticsRunConfig

7994724

flochaz added 2 commits January 20, 2021 17:05

Add test for xray permission

0ec44ad

Merge branch 'master' into flochaz/syntheticsRunConfig

92c60e1

Merge branch 'master' into flochaz/syntheticsRunConfig

4ad2928

flochaz added 2 commits February 16, 2021 20:29

Merge branch 'master' into flochaz/syntheticsRunConfig

d1ce352

Merge branch 'master' into flochaz/syntheticsRunConfig

9634d78

thomasklinger1234 mentioned this pull request Apr 9, 2021

(synthetics): add tracing #14086

Open

2 tasks

NetaNir removed their assignment Apr 30, 2021

NGL321 assigned NetaNir May 17, 2021

BenChaimberg self-assigned this Jun 3, 2021

BenChaimberg suggested changes Jun 16, 2021

View reviewed changes

NetaNir unassigned BenChaimberg and NetaNir Jun 22, 2021

RomainMuller force-pushed the master branch from 924c117 to ebfd5f2 Compare September 29, 2021 13:07

RichiCoder1 mentioned this pull request Jan 14, 2022

feat(synthetics): add vpc configuration #18447

Merged

kaizencc closed this Mar 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(synthetics): add RunConfig parameters to Cloudwatch Synthetics Canary L2 construct #11865

feat(synthetics): add RunConfig parameters to Cloudwatch Synthetics Canary L2 construct #11865

flochaz commented Dec 4, 2020 •

edited

Loading

gitpod-io bot commented Dec 4, 2020 •

edited

Loading

mergify bot commented Dec 4, 2020

mergify bot commented Dec 4, 2020

danataws left a comment

NetaNir commented Dec 4, 2020

NetaNir commented Dec 4, 2020

flochaz commented Dec 5, 2020 •

edited

Loading

NetaNir commented Dec 6, 2020

flochaz commented Dec 7, 2020

danataws commented Dec 8, 2020

colvint commented Jan 21, 2021

flochaz commented Jan 26, 2021

markcarroll commented Feb 9, 2021

aws-cdk-automation commented Mar 1, 2021

NetaNir commented Mar 1, 2021

jonny-rimek commented May 2, 2021

BenChaimberg commented Jun 3, 2021

flochaz commented Jun 4, 2021 via email

BenChaimberg commented Jun 11, 2021

BenChaimberg Jun 16, 2021

BenChaimberg Jun 16, 2021

BenChaimberg Jun 16, 2021

BenChaimberg Jun 16, 2021

BenChaimberg Jun 16, 2021

BenChaimberg Jun 16, 2021

BenChaimberg Jun 16, 2021

BenChaimberg Jun 16, 2021

BenChaimberg Jun 16, 2021

BenChaimberg Jun 16, 2021

RichiCoder1 commented Jan 12, 2022

flochaz commented Jan 12, 2022 via email

RichiCoder1 commented Jan 19, 2022

kaizencc commented Mar 11, 2022

		* 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

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

		const regularExpression = /^rate\(([0-9]) ([a-z])\)/;
		const split = regularExpression.exec(expression);

feat(synthetics): add RunConfig parameters to Cloudwatch Synthetics Canary L2 construct #11865

feat(synthetics): add RunConfig parameters to Cloudwatch Synthetics Canary L2 construct #11865

Conversation

flochaz commented Dec 4, 2020 • edited Loading

gitpod-io bot commented Dec 4, 2020 • edited Loading

mergify bot commented Dec 4, 2020

mergify bot commented Dec 4, 2020

danataws left a comment

Choose a reason for hiding this comment

NetaNir commented Dec 4, 2020

NetaNir commented Dec 4, 2020

flochaz commented Dec 5, 2020 • edited Loading

NetaNir commented Dec 6, 2020

flochaz commented Dec 7, 2020

danataws commented Dec 8, 2020

colvint commented Jan 21, 2021

flochaz commented Jan 26, 2021

markcarroll commented Feb 9, 2021

aws-cdk-automation commented Mar 1, 2021

AWS CodeBuild CI Report

NetaNir commented Mar 1, 2021

jonny-rimek commented May 2, 2021

BenChaimberg commented Jun 3, 2021

flochaz commented Jun 4, 2021 via email

BenChaimberg commented Jun 11, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RichiCoder1 commented Jan 12, 2022

flochaz commented Jan 12, 2022 via email

RichiCoder1 commented Jan 19, 2022

kaizencc commented Mar 11, 2022

flochaz commented Dec 4, 2020 •

edited

Loading

gitpod-io bot commented Dec 4, 2020 •

edited

Loading

flochaz commented Dec 5, 2020 •

edited

Loading