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

trivikr commented 1 year ago

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
[X] This feature might incur a breaking change

CDK version used

2.76.0

Environment details (OS name and version, etc.)

N/A

pahud commented 1 year ago

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 commented 1 year ago

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 commented 5 months ago

Looks like this issue was fixed in https://github.com/aws/aws-cdk/pull/29207

Can it be closed @jonife ?

jonife commented 5 months ago

yes we can close this issue

github-actions[bot] commented 5 months ago

⚠️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 commented 4 months ago

@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: https://github.com/aws/aws-cdk/blob/247aa35676b3b1ead91a56fd00e819c67a9a7285/packages/aws-cdk-lib/aws-lambda-nodejs/lib/types.ts#L179 Comparing it with the current code: https://github.com/aws/aws-cdk/blob/247aa35676b3b1ead91a56fd00e819c67a9a7285/packages/aws-cdk-lib/aws-lambda-nodejs/lib/bundling.ts#L131-L136

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/*",

aws / aws-cdk