chore(lambda-nodejs): do not add unused environment variable by default for sdk v3 lambda runtimes #30117

scanlonp · 2024-05-08T23:30:50Z

Closes #29497
Related to #29538

Reason for this change

The AWS_NODEJS_CONNECTION_REUSE_ENABLED does not exist in SDK v3. Including it in the environment can add to cold start times.

Description of changes

For Lambda runtimes >= Node 18, do not set the variable by default. If set explicitly, give the user a warning.

We can plan to simplify this logic & deprecate the property after we deprecate Node 16 and remove it as the default runtime for this construct.

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

vinayak-kukreja · 2024-05-09T22:11:45Z

packages/aws-cdk-lib/aws-lambda-nodejs/README.md

-when working with the AWS SDK for JavaScript. Set the `awsSdkConnectionReuse` prop to `false` to disable it.
+when working with the AWS SDK v2 for JavaScript. Set the `awsSdkConnectionReuse` prop to `false` to disable it.
+
+The AWS SDK v3 for JavaScript does not include the environment variable set by `awsSdkConnectionReuse`. See [this guide](https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/node-reusing-connections.html) for information about reusing connections. Therefore, for runtimes >= Node 18, which include SDK v3, the prop defaults to `false`, and must be explicitly set to `true` in order for the environment variable to be set.


Would setting it to true do anything in sdk v3?

No, the property does not exist. I believe it has no effect whatsoever.

vinayak-kukreja · 2024-05-09T22:12:40Z

packages/aws-cdk-lib/aws-lambda-nodejs/lib/function.ts

@@ -43,15 +43,19 @@ export interface NodejsFunctionProps extends lambda.FunctionOptions {
  readonly runtime?: lambda.Runtime;

  /**
+   * The `AWS_NODEJS_CONNECTION_REUSE_ENABLED` environment variable does not exist in the AWS SDK for JavaScript v3.
+   *
+   * This prop will be deprecated when the Lambda Node16 runtime is deprecated on June 12, 2024.


Can we also add a @see and the link to where this deadline is mentioned

vinayak-kukreja · 2024-05-09T22:13:56Z

packages/aws-cdk-lib/aws-lambda-nodejs/lib/function.ts

+        Annotations.of(scope).addWarningV2('aws-cdk-lib/aws-lambda-nodejs:unusedSdkEvironmentVariable', 'The AWS_NODEJS_CONNECTION_REUSE_ENABLED environment variable does not exist in SDK v3. You have explicitly set `awsSdkConnectionReuse`; please make sure this is intentional.');
+        this.addEnvironment('AWS_NODEJS_CONNECTION_REUSE_ENABLED', '1', { removeInEdge: true });


Would this env variable do anything in sdk v3? Is this for backward compatibility?

I left this so users can set it if need be (they can also configure their environment variables directly). I figured some users may define >= Node 18 runtimes, but bundle the sdk v2 into their v2 handlers, and this would be for ease of use / backwards compatibility.

scanlonp · 2024-05-14T23:40:41Z

@Mergifyio update

mergify · 2024-05-14T23:40:59Z

update

✅ Branch has been successfully updated

scanlonp · 2024-05-20T17:07:37Z

@Mergifyio update

mergify · 2024-05-20T17:08:01Z

update

❌ Mergify doesn't have permission to update

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/github-merit-badger.yml without workflows permission

colifran

Nice job!

aws-cdk-automation · 2024-05-20T17:38:19Z

AWS CodeBuild CI Report

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

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

mergify · 2024-05-20T17:38:40Z

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).

…lt for sdk v3 lambda runtimes (aws#30117) Closes aws#29497 Related to aws#29538 ### Reason for this change The `AWS_NODEJS_CONNECTION_REUSE_ENABLED` does not exist in SDK v3. Including it in the environment can add to cold start times. ### Description of changes For Lambda runtimes >= Node 18, do not set the variable by default. If set explicitly, give the user a warning. We can plan to simplify this logic & deprecate the property after we deprecate Node 16 and remove it as the default runtime for this construct. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

aws-cdk-automation · 2024-07-25T21:10:08Z

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.

stop adding removed environment variable in sdk v3

e5695f5

github-actions bot added bug This issue is a bug. effort/medium Medium work item – several days of effort p1 labels May 8, 2024

aws-cdk-automation requested a review from a team May 8, 2024 23:31

scanlonp changed the title ~~chore(lambda-nodejs): do not add unused environment variable by default in js sdk v3 lambda runtimes~~ chore(lambda-nodejs): do not add unused environment variable by default for sdk v3 lambda runtimes May 8, 2024

mergify bot added the contribution/core This is a PR that came from AWS. label May 8, 2024

update readme

af2be71

vinayak-kukreja reviewed May 9, 2024

View reviewed changes

scanlonp added 3 commits May 9, 2024 15:54

add lambda node deprecation link

7c19a3d

add link for v3 reusing connections

a7bd120

integ snapshots

9c192fe

github-actions bot mentioned this pull request May 13, 2024

Weekly PR metrics report #30176

Closed

Merge branch 'main' into scanlonp/unused-env-var

510f49b

scanlonp added 3 commits May 15, 2024 11:25

stepfunctions integ snapshot

6a5865b

amplify snapshot

39cf5d6

kinesis snapshot

fc40cc6

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

Merge branch 'main' into scanlonp/unused-env-var

db24795

colifran approved these changes May 20, 2024

View reviewed changes

mergify bot merged commit cf3599b into aws:main May 20, 2024
14 checks passed

scanlonp mentioned this pull request May 20, 2024

chore(lambda-nodejs): set default value of awsSdkConnectionReuse to false (under feature flag) #29538

Closed

1 task

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

Monthly PR metrics report #30414

Closed

aws locked as resolved and limited conversation to collaborators Jul 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(lambda-nodejs): do not add unused environment variable by default for sdk v3 lambda runtimes #30117

chore(lambda-nodejs): do not add unused environment variable by default for sdk v3 lambda runtimes #30117

scanlonp commented May 8, 2024 •

edited

Loading

vinayak-kukreja May 9, 2024

scanlonp May 9, 2024

vinayak-kukreja May 9, 2024

vinayak-kukreja May 9, 2024

scanlonp May 9, 2024

scanlonp commented May 14, 2024

mergify bot commented May 14, 2024

scanlonp commented May 20, 2024

mergify bot commented May 20, 2024

colifran left a comment

aws-cdk-automation commented May 20, 2024

mergify bot commented May 20, 2024

aws-cdk-automation commented Jul 25, 2024

		Annotations.of(scope).addWarningV2('aws-cdk-lib/aws-lambda-nodejs:unusedSdkEvironmentVariable', 'The AWS_NODEJS_CONNECTION_REUSE_ENABLED environment variable does not exist in SDK v3. You have explicitly set `awsSdkConnectionReuse`; please make sure this is intentional.');
		this.addEnvironment('AWS_NODEJS_CONNECTION_REUSE_ENABLED', '1', { removeInEdge: true });

chore(lambda-nodejs): do not add unused environment variable by default for sdk v3 lambda runtimes #30117

chore(lambda-nodejs): do not add unused environment variable by default for sdk v3 lambda runtimes #30117

Conversation

scanlonp commented May 8, 2024 • edited Loading

Reason for this change

Description of changes

vinayak-kukreja May 9, 2024

Choose a reason for hiding this comment

scanlonp May 9, 2024

Choose a reason for hiding this comment

vinayak-kukreja May 9, 2024

Choose a reason for hiding this comment

vinayak-kukreja May 9, 2024

Choose a reason for hiding this comment

scanlonp May 9, 2024

Choose a reason for hiding this comment

scanlonp commented May 14, 2024

mergify bot commented May 14, 2024

✅ Branch has been successfully updated

scanlonp commented May 20, 2024

mergify bot commented May 20, 2024

❌ Mergify doesn't have permission to update

colifran left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented May 20, 2024

AWS CodeBuild CI Report

mergify bot commented May 20, 2024

aws-cdk-automation commented Jul 25, 2024

scanlonp commented May 8, 2024 •

edited

Loading