Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve localstack test performance #184

Merged
merged 4 commits into from Jun 2, 2022
Merged

fix: improve localstack test performance #184

merged 4 commits into from Jun 2, 2022

Conversation

thantos
Copy link
Collaborator

@thantos thantos commented Jun 1, 2022

Improving the performance of the localstack tests.

Currently seeing 240s to fun the function.localstack tests with a 12m build time.

Current Improvements:

  • Simplify the stack deployment logic saw a 1+ minute improvement on function deployment, hoping for 1-3 minute overall improvement.

    • Clean up
  • Write up detailed performance breakdown

  • Remove the performance measuring code added

  • tsc + functionless - 50 seconds

  • Add CDK, bundle, and run serializer - 160 seconds

    • Bundle?
  • CdnCompile - 80 seconds

  • Deploy Stack - 30 seconds

@netlify
Copy link

netlify bot commented Jun 1, 2022

Deploy Preview for effortless-malabi-1c3e77 ready!

Name Link
🔨 Latest commit 5811b9a
🔍 Latest deploy log https://app.netlify.com/sites/effortless-malabi-1c3e77/deploys/6298a373a9ca68000860106e
😎 Deploy Preview https://deploy-preview-184--effortless-malabi-1c3e77.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Comment on lines +62 to +64
stack: cloudAssembly.getStackArtifact(
stack.artifactId
) as unknown as cxapi.CloudFormationStackArtifact,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work? What is cloudAssembly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cloud assembly is what CDK calls the output of a stack synth.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original inspiration: aws/aws-cdk#18667 (comment)

@sam-goodwin
Copy link
Collaborator

Build failed after 13 minutes. Sure it's an optimization?

@thantos
Copy link
Collaborator Author

thantos commented Jun 2, 2022

Small Writeup

CDK exports two libraries, @aws-cdk/cx-api and aws-cdk-lib/cx-api

the Stack synth produces aws-cdk-lib/cx-api CloudArtifact, but to deploy programmatically, we needed @aws-cdk/cx-api CloudArtifact.

To do this, (copied code from a github issue) we load the assembly output from aws-cdk-lib/cx-api into a new @aws-cdk/cx-api.

This worked, but was taking 80+ seconds.

Before

// synth
const cloudAssembly = await asyncSynth(app);
const sdkProvider = await SdkProvider.withAwsCliCompatibleDefaults({
  httpOptions: clientConfig as any,
});

// config hacking...
// @ts-ignore
sdkProvider.sdkOptions = {
  // @ts-ignore
  ...sdkProvider.sdkOptions,
  endpoint: clientConfig.endpoint,
  s3ForcePathStyle: clientConfig.s3ForcePathStyle,
  accessKeyId: clientConfig.credentials.accessKeyId,
  secretAccessKey: clientConfig.credentials.secretAccessKey,
  credentials: clientConfig.credentials,
};

// convert the aws-cdk-lib/cx-api CloudAssembly into a @aws-cdk/cx-api CloudAssembly
const assembly = new cxapi.CloudAssembly(cloudAssembly.directory);
const stackArtifact = cxapi.CloudFormationStackArtifact.fromManifest(
	assembly,
	stack.artifactId,
	assembly.getStackArtifact(stack.artifactId).manifest
) as cxapi.CloudFormationStackArtifact

// deploy using 
await cfn.deployStack({
    stack: stackArtifact,
	force: true,
});

Deeper Dive

Why did we need to do this?

Well... trying to use the output of the stack form aws-cdk-lib produced typescript issues.

Pasted image 20220601141858

Why are the types different?

Two different npm packages.

Where does the code come form?

From the same source. https://github.com/aws/aws-cdk/tree/7eda25625ec1d15fe610097b0456438d6806d9c9/packages/@aws-cdk/cx-api

Why are the bundles different?

CDK Strips deprecated fields, methods, and classes out for the v2 release of CDK (aws-cdk-lib) but they do not do this for aws-cdk/cx-api.

The types be coerced cloudAssembly.getStackArtifact(stack.artifactId) as unknown as cxapi.CloudFormationStackArtifact from the aws-cdk-lib type to the aws-cdk/cx-api form as long as neither side expect use the of deprecated fields.

After

const cloudAssembly = await asyncSynth(app);
const sdkProvider = await SdkProvider.withAwsCliCompatibleDefaults({
  httpOptions: clientConfig as any,
});

// @ts-ignore
sdkProvider.sdkOptions = {
  // @ts-ignore
  ...sdkProvider.sdkOptions,
  endpoint: clientConfig.endpoint,
  s3ForcePathStyle: clientConfig.s3ForcePathStyle,
  accessKeyId: clientConfig.credentials.accessKeyId,
  secretAccessKey: clientConfig.credentials.secretAccessKey,
  credentials: clientConfig.credentials,
};

await cfn.deployStack({
    stack: cloudAssembly.getStackArtifact(stack.artifactId) as unknown as cxapi.CloudFormationStackArtifact,
	force: true,
});

@thantos
Copy link
Collaborator Author

thantos commented Jun 2, 2022

Build failed after 13 minutes. Sure it's an optimization?

I THINK that jest doesn't like the performance observer and console logs which are now removed. We'll see. Its running faster in general on my machine.

@thantos
Copy link
Collaborator Author

thantos commented Jun 2, 2022

Looks like the performance improvement is minor on github actions.

Locally I see either 240s run on the function.localstack or 180s runs.

On localstack it appears PASS test/function.localstack.test.ts (225.047 s) 220+s runs are more common.

I'd say lets push this change and I'll cut a ticket to further improve.

Major timing right now looks like:

  1. Stack Synth - 131s - 200s (seen here)
  2. Deploy Stack - 30s

The delay is likely driving by lambda bundling as we can see stack synth for the event bus tests is closers to 7s

@thantos thantos marked this pull request as ready for review June 2, 2022 12:08
stack.artifactId,
cloudAssembly.getStackArtifact(stack.artifactId).manifest
) as cxapi.CloudFormationStackArtifact;

await cfn.deployStack({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the problem that localstack uses CDK 1.x ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, there is no localstack here (except for SDK props hack above). This is all CDK.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right but presumably we only do this to meet localstacks expected CDK 1.x input?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh, no, cdk is running locally and localstack only sees the cfn. This is all because CDK doesn't support (aka makes it hard as fuck) to synth and deploy programmatically.

@thantos thantos merged commit a421632 into main Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants