feat(synthetics): add activeTracing, memory and timeout property to Canary class by mazyu36 · Pull Request #30556 · aws/aws-cdk

mazyu36 · 2024-06-15T12:46:05Z

Issue # (if applicable)

Closes #9300, #14086, #28152

Reason for this change

In order to make the properties in the runConfig of the L1 Construct configurable in the L2 Construct.

At the moment, the L2 Canary Class only supports environmentVariables in the runConfig.

Description of changes

Add missing properties.

activeTracing
memory
timeout

Note: The following is stated in #9300, but when tested, it was possible to set only memoryInMb (memory).

The difficulty is that timeoutInSeconds is required if runConfig is set, so one cannot only specify memoryInMb.

Test code is here.

class TestStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    new synthetics.Canary(this, 'Canary', {
      canaryName: 'next',
      runtime: synthetics.Runtime.SYNTHETICS_NODEJS_PUPPETEER_7_0,
      test: synthetics.Test.custom({
        handler: 'index.handler',
        code: synthetics.Code.fromInline(`
          exports.handler = async () => {
            console.log(\'hello world\');
          };`),
      }),
      cleanup: synthetics.Cleanup.LAMBDA,
      memory: Size.mebibytes(2048),
    });
  }
}

Description of how you validated changes

Add unit tests and integ tests.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

mazyu36 · 2024-06-15T12:55:17Z

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

+      !props.timeout) {
      return undefined;
    }
+    const activeTracingNotSupportedRuntime = [


The error messages are taken directly from the documentation.
However, since syn-nodejs-2.0 and earlier versions are no longer available, I am only setting it for the currently available runtimes that do not support active tracing (see also docs).

I was unsure about including this validation, so if you have any thoughts or feedback, please let me know.

What about checking props.runtime.family !== RuntimeFamily.NODEJS like in the other PR?

Thank you. I made the correction since checking the runtime is better.
(Even though I did it myself in another PR...lol)

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

lpizzinidev

Thanks 👍 Left some comments for adjustments

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

lpizzinidev · 2024-06-16T15:48:05Z

packages/aws-cdk-lib/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 Mib.
+   *
+   * @default Size.mebibytes(5)


Are you sure about this default? 🤔

Thank you. I made the correction since 1024MB is the default.

lpizzinidev · 2024-06-16T15:52:48Z

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

+      !props.timeout) {
      return undefined;
    }
+    const activeTracingNotSupportedRuntime = [


What about checking props.runtime.family !== RuntimeFamily.NODEJS like in the other PR?

packages/aws-cdk-lib/aws-synthetics/README.md

Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

mazyu36 · 2024-06-17T09:53:54Z

@lpizzinidev
Thank you for always reviewing. I have addressed all comments.

lpizzinidev · 2024-06-17T19:26:29Z

@mazyu36 Thanks for the changes 👍 Answered https://github.com/aws/aws-cdk/pull/30556/files#r1643336431

mazyu36 · 2024-06-18T02:18:13Z

@lpizzinidev
Thanks.
I've added a validation, and answered https://github.com/aws/aws-cdk/pull/30556/files#r1643641842

lpizzinidev

Nice 👍

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

paulhcsun

Nice work @mazyu36! Thanks for the contribution!

Just have one minor wording change for the error message and I'd be happy to approve.

paulhcsun · 2024-07-30T23:02:28Z

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

+
+    // Only check runtime family is nodejs because versions prior to syn-nodejs-2.0 are deprecated and can no longer be configured.
+    if (props.activeTracing && !cdk.Token.isUnresolved(props.runtime.family) && props.runtime.family !== RuntimeFamily.NODEJS) {
+      throw new Error('You can enable active tracing only for canaries that use version `syn-nodejs-2.0` or later for their canary runtime.');


Suggested change

throw new Error('You can enable active tracing only for canaries that use version `syn-nodejs-2.0` or later for their canary runtime.');

throw new Error('You can only enable active tracing for canaries that use canary runtime version `syn-nodejs-2.0` or later.');

@paulhcsun
Thanks.
I've updated error message and unit tests.

paulhcsun · 2024-07-31T20:39:32Z

@Mergifyio update

mergify · 2024-07-31T20:39:58Z

update

☑️ Nothing to do

Details

#commits-behind>0 [📌 update requirement]
-closed [📌 update requirement]
-conflict [📌 update requirement]
queue-position=-1 [📌 update requirement]

mergify · 2024-07-31T21:09:55Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2024-07-31T21:40:37Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: ee9f60d
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2024-07-31T21:40:55Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions · 2024-07-31T21:41:09Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

github-actions bot added feature-request A feature should be added or improved. p2 labels Jun 15, 2024

aws-cdk-automation requested a review from a team June 15, 2024 12:46

github-actions bot added the star-contributor [Pilot] contributed between 25-49 PRs to the CDK label Jun 15, 2024

mazyu36 commented Jun 15, 2024

View reviewed changes

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

mazyu36 marked this pull request as draft June 15, 2024 13:09

mazyu36 force-pushed the synthetics-runconfig branch from 1a13629 to e3d9b92 Compare June 15, 2024 13:27

mazyu36 marked this pull request as ready for review June 15, 2024 13:47

mazyu36 force-pushed the synthetics-runconfig branch from e3d9b92 to a40be25 Compare June 15, 2024 14:00

feat: add activeTracing, memory and timeout property to Canary class

18d4100

mazyu36 force-pushed the synthetics-runconfig branch from af745ad to 18d4100 Compare June 15, 2024 15:46

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jun 15, 2024

lpizzinidev suggested changes Jun 16, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jun 16, 2024

mazyu36 and others added 12 commits June 17, 2024 16:45

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

f6f7b51

Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

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

011e4f9

Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

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

ba7dc65

Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

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

819536a

Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

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

104d50c

Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

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

2f0668a

Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

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

1aa8741

Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

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

7714123

Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

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

ff54d26

Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

Update packages/aws-cdk-lib/aws-synthetics/README.md

d858093

Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

Update packages/aws-cdk-lib/aws-synthetics/README.md

65c298b

Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

Update packages/aws-cdk-lib/aws-synthetics/README.md

25f0197

Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>

mazyu36 force-pushed the synthetics-runconfig branch 3 times, most recently from 6ec1bdc to ae0b174 Compare June 17, 2024 08:57

Merge branch 'main' into synthetics-runconfig

0174c8b

github-actions bot added the effort/medium Medium work item – several days of effort label Jun 18, 2024

mazyu36 force-pushed the synthetics-runconfig branch from c2badc8 to 16e9e26 Compare June 18, 2024 02:14

fix: add validation

9d08774

mazyu36 force-pushed the synthetics-runconfig branch from 16e9e26 to 9d08774 Compare June 18, 2024 03:17

lpizzinidev approved these changes Jun 18, 2024

View reviewed changes

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

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jun 18, 2024

chore: minor update

c32f70c

github-actions bot mentioned this pull request Jul 1, 2024

Monthly PR metrics report #30715

Closed

mazyu36 added 2 commits July 24, 2024 20:53

Merge branch 'main' into synthetics-runconfig

4354235

update integ test

0cba11c

paulhcsun self-assigned this Jul 29, 2024

paulhcsun reviewed Jul 30, 2024

View reviewed changes

update error message

eee4cb0

mazyu36 requested a review from paulhcsun July 31, 2024 00:02

Merge branch 'main' into synthetics-runconfig

07cdc78

paulhcsun approved these changes Jul 31, 2024

View reviewed changes

Merge branch 'main' into synthetics-runconfig

ee9f60d

mergify bot merged commit 9295a85 into aws:main Jul 31, 2024

github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2024

mazyu36 deleted the synthetics-runconfig branch July 31, 2024 22:03

	throw new Error('You can enable active tracing only for canaries that use version `syn-nodejs-2.0` or later for their canary runtime.');
	throw new Error('You can only enable active tracing for canaries that use canary runtime version `syn-nodejs-2.0` or later.');

Conversation

mazyu36 commented Jun 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lpizzinidev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mazyu36 commented Jun 17, 2024

Uh oh!

lpizzinidev commented Jun 17, 2024

Uh oh!

mazyu36 commented Jun 18, 2024

Uh oh!

lpizzinidev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paulhcsun left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulhcsun commented Jul 31, 2024

Uh oh!

mergify bot commented Jul 31, 2024

☑️ Nothing to do

Uh oh!

mergify bot commented Jul 31, 2024

Uh oh!

aws-cdk-automation commented Jul 31, 2024

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Jul 31, 2024

Uh oh!

github-actions bot commented Jul 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mazyu36 commented Jun 15, 2024 •

edited

Loading

paulhcsun left a comment •

edited

Loading