Improve NodeJS Lambda Layer cold start time #163
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ezhang6811
changed the title
ezhang6811
force-pushed
the
zhaez/cold-start-improvements
branch
from
8ecd37a to
b5c9e6f
Compare
ezhang6811
force-pushed
the
zhaez/cold-start-improvements
branch
from
b5c9e6f to
cab9e49
Compare
ezhang6811
force-pushed
the
zhaez/cold-start-improvements
branch
from
0512cd2 to
6df8880
Compare
ezhang6811
force-pushed
the
zhaez/cold-start-improvements
branch
from
6df8880 to
c6b2d64
Compare
mxiamxia
reviewed
Mar 12, 2025
| @@ -22,7 +22,9 @@ | |||
| "repository": "aws-observability/aws-otel-js-instrumentation", | |||
| "scripts": { | |||
| "clean": "rimraf build/*", | |||
| "compile": "tsc -p .", | |||
| "compile:tsc": "tsc -p .", | |||
There was a problem hiding this comment.
who will run "compile:tsc" if we shift "compile" to "compile:webpack" as code compiling entry point?
There was a problem hiding this comment.
It probably won't be run unless someone wanted to see the previous package structure.
mxiamxia
reviewed
Mar 12, 2025
Comment on lines
+4
to
+7
| "module": "es2020", | ||
| "target": "es2020", | ||
| "moduleResolution": "node", | ||
| }, |
There was a problem hiding this comment.
is this the same target being used by otel lambda upstream?
mxiamxia
reviewed
Mar 12, 2025
Comment on lines
+209
to
+211
| private getPropagator(): TextMapPropagator { | ||
| if (process.env.OTEL_PROPAGATORS == null || process.env.OTEL_PROPAGATORS.trim() === '') { | ||
| return new CompositePropagator({ |
There was a problem hiding this comment.
is there a better option to not rewrite this function by copying it from upstream? can we just patch propagatorMap for auto-configuration-propagators/utils file?
There was a problem hiding this comment.
See #171, we revert to using the upstream dependency
jj22ee
reviewed
Mar 17, 2025
| @@ -119,8 +124,8 @@ | |||
| }, | |||
| "files": [ | |||
| "build/src/**/*.js", | |||
| "build/src/**/*.js.map", | |||
There was a problem hiding this comment.
If we keep the compile:tsc command, there's no harm in keeping "build/src/**/*.js.map".
| @@ -190,6 +206,32 @@ export class AwsOpentelemetryConfigurator { | |||
| return autoResource; | |||
| } | |||
|
|
|||
| private getPropagator(): TextMapPropagator { | |||
There was a problem hiding this comment.
nit: This can be a regular function outside of this class.
ezhang6811
added a commit
that referenced
this pull request
Mar 22, 2025
*Description of changes:* This PR is a rehash of #163 with some minor changes reverted after several autoinstrumentation package dependencies were upgraded in #168. The changes suggested by the unaddressed comments in the previous PR are addressed here. The primary change made to reduce the cold start time was to bundle the ADOT autoinstrumentation package with webpack instead of using the TS compiler. Using the same build targets as the upstream (see [this PR](https://github.com/open-telemetry/opentelemetry-lambda/pull/1679/files)), the cold start performance of the layer was improved by about 50%, from ~765ms to ~385ms. By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes:
Change 1: Using webpack to bundle the autoinstrumentation package improves tree-shaking, which significantly shortens the cold-start time for the lambda layer. Inspired by open-telemetry/opentelemetry-lambda#1679.
To test the cold-start improvement, a sample Lambda function instrumented with the Lambda layer was deployed, and a script was run which repeatedly invokes the Lambda function. The initialization time averages are then queried on Cloudwatch logs. The process is then repeated with the lambda layer removed from the Lambda function, and the times are subtracted to get the Lambda layer cold start time.
From my tests, the average cold start time for the Lambda layer improved from ~770ms to ~350ms, an almost 55% reduction. This could be further improved in the future by resolving any conflicting subdependency versions and providing more ESM compatible dependencies, which can further reduce the output bundle.
Change 2: Remove the auto-configuration-propagators dependency from autoinstrumentation package, which eliminates the B3 and Jaeger propagator transitive dependencies. This reduces the size of the zipped layer from 15MB 11.6 MB, approximately 23% decrease. Will update with performance improvements once I finish testing this change.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.