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: resolved esm loader issue for manual instrumentation node v18.19.0 and above #1063

Merged
merged 15 commits into from
Apr 10, 2024

Conversation

aryamohanan
Copy link
Contributor

@aryamohanan aryamohanan commented Mar 4, 2024

Starting from version 20.6, ESM loaders are off-threaded, loaded in separate thread, whereas previously we were loading our Instana collector in loader, so the change broke existing implementation. To resolve this, we used the --import flag, replacing the deprecated --experimental-loader and allowing the collector to load within the main thread.

Note that --import is supported only in Node.js versions 18.19 and later. Thus, we maintain both loading styles for compatibility across different Node.js versions.

To enable experimental ESM support, use the following command:

  • For Node.js versions greater than or equal to 18.19:

    node --import /path/to/instana/node_modules/@instana/collector/esm-register.mjs entry-point

  • For Node.js versions less than 18.19:

    node --experimental-loader /path/to/instana/node_modules/@instana/collector/esm-loader.mjs entry-point

We're actively working on implementing native ESM support, targeted for resolution in INSTA-807.

References:

Move ESM loaders off-thread

nodejs/node#47880

Notes:

  • Node 18.19 is also affected due to back-porting.
  • --import flag is still on experimental phase.

Tasks

  • loaders are off threaded!
  • experimental-loader flag deprecated
  • support all Node.js versions above 18.19
  • run all unit testcases

Post Release

  • Update the build version 18.18 to the latest 18.x.x(18.20.1)
  • Consider changing the development version to v20 from 18
  • Automated instrumentation update

@aryamohanan aryamohanan force-pushed the fix-esm-loader branch 4 times, most recently from 4da6bbe to 4b83d46 Compare March 8, 2024 12:40
@aryamohanan aryamohanan marked this pull request as ready for review March 11, 2024 10:34
@aryamohanan aryamohanan requested a review from a team as a code owner March 11, 2024 10:34
@aryamohanan aryamohanan marked this pull request as draft March 13, 2024 09:22
@aryamohanan aryamohanan force-pushed the fix-esm-loader branch 2 times, most recently from faa88a9 to 722f0a6 Compare March 20, 2024 05:24
@aryamohanan aryamohanan force-pushed the fix-esm-loader branch 2 times, most recently from 029c460 to d4c4d23 Compare April 2, 2024 15:15
@aryamohanan aryamohanan marked this pull request as ready for review April 3, 2024 04:27
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/aws-fargate/register.mjs Outdated Show resolved Hide resolved
packages/google-cloud-run/test/esm/test.js Outdated Show resolved Hide resolved
packages/collector/package.json Outdated Show resolved Hide resolved
packages/aws-fargate/package.json Show resolved Hide resolved
@aryamohanan aryamohanan changed the title fix: fixed esm loader issue for node v18.19.0 and above fix: fixed esm loader issue for manual instrumentation node v18.19.0 and above Apr 9, 2024
@aryamohanan aryamohanan force-pushed the fix-esm-loader branch 3 times, most recently from 83e46b5 to 198c762 Compare April 10, 2024 10:13
@kirrg001
Copy link
Contributor

Question: the PR pipeline runs with Node 18.18. How do we know that the tests are green for aws-fargate & 18.19?

@aryamohanan
Copy link
Contributor Author

Question: the PR pipeline runs with Node 18.18. How do we know that the tests are green for aws-fargate & 18.19?

Tested with v20 in a seprate pipeline, thanks again for pointing this ❤️

@aryamohanan aryamohanan changed the title fix: fixed esm loader issue for manual instrumentation node v18.19.0 and above fix: resolved esm loader issue for manual instrumentation node v18.19.0 and above Apr 10, 2024
Copy link
Contributor

@kirrg001 kirrg001 left a comment

Choose a reason for hiding this comment

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

Great work 👹

@aryamohanan aryamohanan merged commit d69aff8 into main Apr 10, 2024
2 checks passed
@aryamohanan aryamohanan deleted the fix-esm-loader branch April 10, 2024 16:33
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

2 participants