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

[badmintoncryer]

Issue # (if applicable)

Closes #29497 .

Reason for this change

Reference to #29497

The AWS SDK for JavaScript v3 is included by default in AWS Lambda Node.js 18 and Node.js 20 runtimes.

NodejsFunction construct comes with an awsSdkConnectionReuse optional prop, which is set to true by default. This default automatically sets the AWS_NODEJS_CONNECTION_REUSE_ENABLED environment variable to 1 in the Lambda function's configuration.

However, it has been identified that the AWS_NODEJS_CONNECTION_REUSE_ENABLED environment variable aws/aws-sdk-js-v3#5883, rendering the variable obsolete for those utilizing the more recent Lambda runtimes. This situation not only leads to unnecessary configuration, but it also has a performance impact. The inclusion of this redundant environment variable is associated with a 20ms increase in cold start times, highlighting an opportunity for performance improvement by removing this default setting for those who do not require it.

Description of changes

Change default value of awsSdkConnectionReuse from true to false

Description of how you validated changes

I have added both unit 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

[aws-cdk-automation]

AWS CodeBuild CI Report

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

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

[TheRealAmazonKendra]

We will actually be removing v2 altogether in the next few weeks or so. Given that, I don't think we should add an additional feature flag or update the default here.

ETA: sdk v2, not cdk v2

[badmintoncryer]

@TheRealAmazonKendra Does this mean that CDK v3 will be launched in a few weeks?!

[TheRealAmazonKendra]

@TheRealAmazonKendra Does this mean that CDK v3 will be launched in a few weeks?!

Sdk v3, not cdk. I should have specified more clearly.

[badmintoncryer]

@TheRealAmazonKendra I see. I'm sorry for the misunderstanding.

I think the current default values are for SDK v2, and this PR will optimize the default settings for SDK v3.
If SDK v2 becomes obsolete, I believe it would be even more appropriate to apply this PR. What do you think?

[shikha372]

Hi @badmintoncryer ,
To confirm the expected behaviour when we move to SDK v3, if its expected to be set to false by default, we wouldn't need this feature flag in which case might need to remove this code altogether ?

[badmintoncryer]

Hi @shikha372.
SDK V3 does not utilize AWS_NODEJS_CONNECTION_REUSE_ENABLED option but AWS CDK generates this environment value by default. Original issue says that this redundant environment value affects to lambda's latency.

Therefore, I think it is necessary to update the default value of awsSdkConnectionReuse option in
NodejsFunctionProps.
If it's okay to just make a modification to change the default value of awsSdkConnectionReuse, I believe the feature flag is not necessary. However, I think that would be a breaking change, which is why I've implemented it using a feature flag.

I might be misunderstanding various things. Please don't hesitate to point out any issues.

[TheRealAmazonKendra]

When we move from sdk v2 to v3, we'll probably just deprecate this field as it will no longer have any relevance to this construct.

I'm not precisely sure of what the path forward on this construct library as a whole will be but, in contrast to our normal practice of not pushing breaking changes or forcing upgrades, we will have to. Anyone still on sdkv2 will also still be on the node16 Lambda runtime (this is the most recent it supports).

On July 15th, no new Lambda functions will be able to be created with this runtime and then on August 15th, all updates to existing Lambda functions will be blocked. See: https://docs.aws.amazon.com/lambda/latest/dg/lambda-runtimes.html

Since the consequences of still being on that runtime are pretty major, this might be a case where we do use a breaking change and force an update.

THAT BEING SAID, we haven't settled on an exact plan here so don't take this as the absolute path forward. Just my thoughts as we begin to plan this upgrade.

[badmintoncryer]

@TheRealAmazonKendra
Thank you for your detailed response; I now have a clear understanding of the situation.

For now, I'll leave this PR as it is. I would appreciate it if you could reply here once a decision has been made.
Of course, I am also prepared to update the PR to a breaking change without the use of feature flags if that becomes necessary. Please just let me know.

[scanlonp]

Hey @badmintoncryer, wanted to give you an update on the path we took with this.

Along the lines of what Kendra was saying, we want to keep some of the decisions that straddle the breaking change line to be made by the team itself. In that vein, I made this change in #30117 in a similar manner without the feature flag at all. There is also the possibility that when we deprecate the Node 16 runtime, we will deprecate the prop altogether.

I think we will want to take that approach for most of the Node 16 / SDK v2 upgrades. I don't want you to feel like we took this PR from you, and I certainly consulted it when making mine, but we wanted to have full control on this type of change.

Again, greatly appreciate you taking the time to submit this PR.