aws_lambda_nodejs: Using Lambda Provided SDK by default in NodejsFunction leads to higher cold starts

[trivikr]

Describe the feature

The BundlingOptions in NodejsFunction construct removes AWS SDK dependencies by default.

This uses Lambda Provided SDK in the resulting function. This has higher cold start than a bundled function with AWS SDK dependencies included.

This happens, because the Node.js runtime has to do module resolution and go through multiple files while reading dependency code in the bundled function which uses Lambda Provided SDK. When SDK in bundled with the function code, the cold starts are lower as the as Node.js runtime has to read single file without any module resolution.

Use Case

The default NodeJsFunction created by CDK Lambda should have low cold starts.

Lambda customers are usually sensitive to cold starts. The bundling of the source code already reduces the function size, which is way below the Lambda Quotas limit of 50 MB.

For repro, refer README of https://github.com/trivikr/aws-cdk-lambda-nodejs-function-cold-start-latency

Here are the benchmark numbers in ms:

{
  'NodejsFunction default (uses Lambda Provided SDK)': 1227.1435,
  'NodejsFunction custom (uses Customer Deployed SDK)': 929.441
}

Notes:

• The benchmarks were run on count 1000 to ensure they’re reliable.
• As of 2023-05-09, it ensured that the SDK version is same in both cases, i.e. 3.188.0
• The numbers show median value for all cold start invocations. For other statistics, refer to output of measure-cold-start Step Function execution.

Proposed Solution

The default NodeJsFunction created by CDK Lambda should not exclude AWS SDK dependencies.

Other Information

No response

Acknowledgements

• I may be able to implement this feature request
• This feature might incur a breaking change

CDK version used

2.76.0

Environment details (OS name and version, etc.)

N/A

[pahud]

Nice callout and thanks for the detailed report. Maybe we should allow users to opt in with an optional property but I am not sure. But yes, the cold start is a pain we can't ignore.

[trivikr]

Maybe we should allow users to opt in with an optional property but I am not sure

Users can pass empty array in externalModules to bundle the AWS SDK dependencies.

The feature request is whether CDK should bundle SDK dependencies by default, as it has lower cold starts.

[trivikr]

Looks like this issue was fixed in #29207

Can it be closed @jonife ?

[jonife]

yes we can close this issue

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

[WtfJoke]

@jonife I am not sure if this should have been closed.

The new variable bundleAwsSDK defaults to false, e.g. slow cold starts.
At least the doc changes to externalModules are not correct, since with the default value false of bundleAwsSDK externalModulesReplacement are still made.

Mentioned docs:

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

Line 179 in 247aa35

* @default - no replacements are made

Comparing it with the current code:

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

Lines 131 to 136 in 247aa35

	// Don't automatically externalize any dependencies when using a `latest` runtime which may
	// update versions in the future.
	// Don't automatically externalize aws sdk if `bundleAwsSDK` is true so it can be
	// include in the bundle asset
	const defaultExternals = props.runtime?.isVariable || props.bundleAwsSDK ? [] : versionedExternals;
	const externals = props.externalModules ?? defaultExternals;

Following (failing) test confirms that behaviour:

test('esbuild bundling includes aws-sdk by default', () => {
  Bundling.bundle(stack, {
    entry,
    projectRoot,
    depsLockFilePath,
    runtime: Runtime.NODEJS_18_X,
    architecture: Architecture.X86_64,
  });

  // Correctly bundles with esbuild
  expect(Code.fromAsset).toHaveBeenCalledWith(path.dirname(depsLockFilePath), {
    assetHashType: AssetHashType.OUTPUT,
    bundling: expect.objectContaining({
      command: [
        'bash', '-c',
        `esbuild --bundle "/asset-input/lib/handler.ts" --target=${STANDARD_TARGET} --platform=node --outfile="/asset-output/index.js"`,
      ],
    }),
  });
});

    -       "esbuild --bundle \"/asset-input/lib/handler.ts\" --target=node18 --platform=node --outfile=\"/asset-output/index.js\"",
    +       "esbuild --bundle \"/asset-input/lib/handler.ts\" --target=node18 --platform=node --outfile=\"/asset-output/index.js\" --external:@aws-sdk/*",