Skip to content

feat(nodejs): bundling with webpack #1679

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

Merged

Conversation

serkan-ozal
Copy link
Contributor

@serkan-ozal serkan-ozal commented Jan 25, 2025

With this PR, core (wrapper.js and most) of the OTEL AWS Lambda Node.js layer is bundled (tree-shaken and minified) with Webpack. So, this significantly reduces AWS Lambda coldstart delay of the OTEL AWS Lambda Node.js layer.

Additionally, with this PR, users can also disable metrics and logs by OTEL_METRICS_EXPORTER=none and OTEL_LOGS_EXPORTER=none to cut some more (~30 ms) coldstart delay if they don't need metrics and logs, but just traces.

  • ✅ All tests are passing
  • ✅ Tested with CJS and ESM based Lambda handlers

According to my experiments,

  • the coldstart delay reduces to ~220 ms from ~550 ms: 60% coldstart reduction (190 ms if metrics and logs are disabled as described above)
  • the Lambda layer artifact size reduces to 471 KB from 8.9 MB: 95% package size reduction

There are still improvements we can do over this:

  • There are improvements have been merged into the opentelemetr-js and opentelemetry-js-contrib repositories, but have not released and used in the Lambda layer yet.
  • There are still pending improvement PRs to the opentelemetr-js and opentelemetry-js-contrib repositories waiting for review.
  • Cache compiled V8 bytecode of the core (wrapper.js) and reuse it without compiling during coldstart.

Hopefully, with all these improvements, OTEL AWS Lambda Node.js layer coldstart overhead will be down to two digit ms (< 100 ms).

Besides that, we also need some improvements in the OTEL Lambda collector extension as it adds additional 270-280 ms delay to the coldstart.

@serkan-ozal serkan-ozal force-pushed the feat/nodejs/bundling-with-webpack branch 2 times, most recently from d3d5182 to 3c93842 Compare January 25, 2025 10:26
@serkan-ozal serkan-ozal marked this pull request as ready for review January 25, 2025 10:30
@serkan-ozal serkan-ozal requested a review from a team as a code owner January 25, 2025 10:30
@tylerbenson
Copy link
Member

@pragmaticivan @martinkuba would you mind taking a look at this please?

Copy link
Member

@pragmaticivan pragmaticivan left a comment

Choose a reason for hiding this comment

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

Could not spot anything wrong/major and synced with the PR author in slack with some questions.

@tylerbenson tylerbenson added the javascript Pull requests that update Javascript code label Feb 4, 2025
@serkan-ozal serkan-ozal force-pushed the feat/nodejs/bundling-with-webpack branch from 40e93da to f6c4b35 Compare February 5, 2025 18:49
@serkan-ozal
Copy link
Contributor Author

@tylerbenson @pragmaticivan I am done with this

rm -rf ./build/workspace/node_modules

# Space separated list of external NPM packages
EXTERNAL_PACKAGES=( "import-in-the-middle" )
Copy link
Contributor

Choose a reason for hiding this comment

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

js newbie here :)

Instead of having this separated script file with a list of external dependencies, would it work if we had a separated node package with package.json where you define all the dependencies. This would be a "noop package", but it would allow you to explicitly select what version of the external dependencies you want to use. and then you install it with just npm install --prefix ./build/workspace --production --ignore-scripts.

Copy link
Contributor Author

@serkan-ozal serkan-ozal Feb 7, 2025

Choose a reason for hiding this comment

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

The reason why I have used bash script to install external packages is that I want to use the same version of import-in-the-middle coming from OTEL packages. So, I have to resolve its version dynamically. That is why I first resolve its version by npm query below and then install it with that specific version.

@serkan-ozal serkan-ozal merged commit 80d9105 into open-telemetry:main Feb 10, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants