chore(lambda-nodejs): stop bundling lambda provided sdk by default

[fynnfluegge]

This PR disables bundling of Lambda provided SDK by default which leads to lower cold starts since aws-sdk is removed from asset bundle as extensively explained and benchmarked in the related issue.

Breaking change. An opt im property is added to bundle the aws-sdk as it has been the default before.

Closes #25492.

---

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

[gitpod-io]

[corymhall]

@fynnfluegge thanks for the PR! In general I like this change, I think we should be bundling everything by default. I'm worried about making this change the default without a feature flag though.

I tested the existing integration tests and the aws-sdk v2 test timed out until I increased the memorySize of the function. This is just one breaking point, but it makes me think that there could be more that we are not thinking of.

1. Lets put this behind a feature flag so that we don't change the behavior for existing users
2. Is there a minimum required memory setting for aws-sdk?

[corymhall]

@fynnfluegge thanks for the PR! In general I like this change, I think we should be bundling everything by default. I'm worried about making this change the default without a feature flag though.

I tested the existing integration tests and the aws-sdk v2 test timed out until I increased the memorySize of the function. This is just one breaking point, but it makes me think that there could be more that we are not thinking of.

1. Lets put this behind a feature flag so that we don't change the behavior for existing users
2. Is there a minimum required memory setting for aws-sdk?

[corymhall]

Can you remove this so that the integ test will test the new behavior?

Can you also add diffAssets: true to the IntegTest

[fynnfluegge]

Sure, modified it to test both behaviours ✅
However, the test that uses the default bundled aws-sdk doesn't pass since it cannot resolve the import?

Usually it should be bundled as default, any idea how to test this?

[corymhall]

I think it may be because there is no lock file. I would try adding a new sub folder for this handler with a minimal package.json and package-lock.json.

[fynnfluegge]

That's exactly what I did 🤔

[corymhall]

Ok I think for this test we have to have the bundling actually perform an npm install.

    this.lambdaFunction = new lambda.NodejsFunction(this, 'external-sdk-v3', {
      entry: path.join(__dirname, 'integ-handlers/v3/dependencies-sdk-v3.ts'),
      runtime: Runtime.NODEJS_18_X,
      depsLockFilePath: path.join(__dirname, 'integ-handlers/v3/package-lock.json'),
      bundling: {
        bundleLambdaProvidedAwsSdk: true,
        commandHooks: {
          beforeInstall() {
            return [];
          },
          afterBundling() {
            return [];
          },
          beforeBundling(inputDir, _outputDir) {
            return [`cd ${inputDir}`, 'npm install --ci'];
          },
        },
      },
    });

[fynnfluegge]

Okay that seems to work ✅
So I want to create the new snapshots now but that leads me into a rabbit hole 😄
When running yarn integ --update-on-failed aws-lambda-nodejs/test/integ.dependencies.js it says for the test with the bundled sdk:

It is the Response object is too long error which relates to several issues i.e. #2825. Any idea how to do a workaround for this?

Pull request has been modified.

[fynnfluegge]

@fynnfluegge thanks for the PR! In general I like this change, I think we should be bundling everything by default. I'm worried about making this change the default without a feature flag though.

I tested the existing integration tests and the aws-sdk v2 test timed out until I increased the memorySize of the function. This is just one breaking point, but it makes me think that there could be more that we are not thinking of.

1. Lets put this behind a feature flag so that we don't change the behavior for existing users
2. Is there a minimum required memory setting for aws-sdk?

Hey @corymhall thanks for reviewing!

1. Feature flag sounds reasonable, will put the changes behind it ✅
2. Not sure 🤔 Will take a look at this

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 21070da
• Result: FAILED
• Build Logs (available for 30 days)

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

[aws-cdk-automation]

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

[aws-cdk-automation]

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.