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
Merged
18 changes: 18 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,21 @@ More information can be found [here](https://docs.aws.amazon.com/cli/latest/user
cd packages/aws-lambda/layer/bin/
REGIONS=<region> SKIP_DOCKER_IMAGE=true BUILD_LAYER_WITH=local LAYER_NAME=experimental-instana-nodejs-with-extension ./publish-layer.sh
```
## ESM Support

We have added the ESM support for all Node.js versions, Since version 20.6, [ESM loaders are off-thread](https://github.com/nodejs/node/pull/44710), loaded separately, a shift from previous setups where the Instana collector was loaded within the loader, leading to a disruption in existing implementation. To resolve this, we've replaced the deprecated `--experimental-loader` with `--import`, facilitating the loading of the collector in the main thread. However, note that `--import` is only compatible with Node.js v18.19 and later, necessitating the maintenance of both styles for different Node.js versions.

Efforts are ongoing to integrate native ESM support, detailed in ref INSTA-807.

Use the following command to enable experimental ESM support:
kirrg001 marked this conversation as resolved.
Show resolved Hide resolved

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

```sh
node --import /path/to/instana/node_modules/@instana/collector/esm-register.mjs entry-point
```
- For Node.js versions less than 18.19:

```sh
node --experimental-loader /path/to/instana/node_modules/@instana/collector/esm-loader.mjs entry-point
```
23 changes: 8 additions & 15 deletions packages/aws-fargate/esm-loader.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,22 @@
'use strict';

/**
* We currently only instrument CJS modules. As soon as we want
* to instrument ES modules (such as `got` v12), the requireHook will
* no longer work. Therefor we would need to wrap the target ES module
* with our instrumentations using the resolve & load hook.
* IMPORTANT NOTE: From Node.js version 18.19 and above, the ESM loaders operate off-thread.
* Consequently, ESM instrumentation using '--experimental-loader' becomes deprecated.
* Instead, we are using '--import' for loading instrumentation and relocated the Instana collector
* loading to './esm-register' file.
* Please note that '--import' flag is unavailable in earlier versions, hence we maintain both setups.
* In future we will incorporate the native ESM support with register method.
*
* Usage:
* ENV NODE_OPTIONS='--experimental-loader=/instana/node_modules/
* @instana/aws-fargate/esm-loader.mjs'
*
* NOTE: When using ESM, the customers have the option to set up the aws-fargate
* by configuring ENV variable within the docker file. Given that aws-fargate
* by configuring ENV variable within the docker file. Given that aws-fargate
* self-initializes, its current utilization remains unchanged.
* However, there's potential for future integration where the init() method
* However, there's potential for future integration where the init() method
* can be invoked from this context.
*/

import './src/index.js';
/*
export async function resolve(specifier, context, nextResolve) {
return nextResolve(specifier, context, nextResolve);
}

export async function load(url, context, nextLoad) {
return nextLoad(url, context, nextLoad);
}
*/
24 changes: 24 additions & 0 deletions packages/aws-fargate/esm-register.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* (c) Copyright IBM Corp. 2024
*/

/**
* As of Node.js version 18.19 and above, ESM loaders (--experimental-loader)
* are executed in a dedicated thread, separate from the main thread.
* see https://github.com/nodejs/node/pull/44710.
* Previously, loading the Instana collector within the loader and after the update ESM support
* no longer working with v18.19 and above. To address this, we've opted to load the Instana
* collector in the main thread using --import.
* Additionally, we aim to incorporate native ESM support by utilizing the node register method,
* enabling customization of the ESM loader with 'import-in-the-middle'.
*
* Usage:
* ENV NODE_OPTIONS='--import /instana/node_modules/@instana/aws-fargate/esm-register.mjs
*/

// Importing the Instana trace initialization module here, as this is executed in the main thread.
import './src/index.js';

// We plan to utilize this for adding native ESM support in the near future
// import { register } from 'node:module';
// register(./loader.mjs, import.meta.url);
3 changes: 2 additions & 1 deletion packages/aws-fargate/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"main": "src/index.js",
"files": [
"src",
"esm-loader.mjs"
"esm-loader.mjs",
aryamohanan marked this conversation as resolved.
Show resolved Hide resolved
"esm-register.mjs"
],
"publishConfig": {
"access": "public"
Expand Down
8 changes: 7 additions & 1 deletion packages/aws-fargate/test/Control.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const config = require('@instana/core/test/config');
const AbstractServerlessControl = require('../../serverless/test/util/AbstractServerlessControl');
const portfinder = require('../../collector/test/test_util/portfinder');
const PATH_TO_INSTANA_FARGATE_PACKAGE = path.join(__dirname, '..');
const isLatestEsmSupportedVersion = require('@instana/core').tracing.isLatestEsmSupportedVersion;
let execArg;

function Control(opts) {
Expand Down Expand Up @@ -84,9 +85,14 @@ Control.prototype.startMonitoredProcess = function startMonitoredProcess() {
env.INSTANA_ENDPOINT_URL = this.backendBaseUrl;
env.INSTANA_AGENT_KEY = this.instanaAgentKey;
}

const loaderPath = isLatestEsmSupportedVersion(process.versions.node)
? ['--import', `${path.join(__dirname, '..', 'esm-register.mjs')}`]
: [`--experimental-loader=${path.join(__dirname, '..', 'esm-loader.mjs')}`];

if (this.opts.containerAppPath && this.opts.env && this.opts.env.ESM_TEST) {
if (this.opts.containerAppPath.endsWith('.mjs')) {
execArg = [`--experimental-loader=${path.join(__dirname, '..', 'esm-loader.mjs')}`];
execArg = loaderPath;
} else {
execArg = ['--require', PATH_TO_INSTANA_FARGATE_PACKAGE];
}
Expand Down
19 changes: 6 additions & 13 deletions packages/azure-container-services/esm-loader.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
'use strict';

/**
* We currently only instrument CJS modules. As soon as we want
* to instrument ES modules (such as `got` v12), the requireHook will
* no longer work. Therefor we would need to wrap the target ES module
* with our instrumentations using the resolve & load hook.
* IMPORTANT NOTE: From Node.js version 18.19 and above, the ESM loaders operate off-thread.
* Consequently, ESM instrumentation using '--experimental-loader' becomes deprecated.
* Instead, we are using '--import' for loading instrumentation and relocated the Instana collector
* loading to './esm-register' file.
* Please note that '--import' flag is unavailable in earlier versions, hence we maintain both setups.
* We will incorporate the native ESM support by using 'import-in-the-middle' ith register method.
*
* Usage:
* ENV NODE_OPTIONS='--experimental-loader=/instana/node_modules/
Expand All @@ -22,12 +24,3 @@
*/

import './src/index.js';
/*
export async function resolve(specifier, context, nextResolve) {
return nextResolve(specifier, context, nextResolve);
}

export async function load(url, context, nextLoad) {
return nextLoad(url, context, nextLoad);
}
*/
24 changes: 24 additions & 0 deletions packages/azure-container-services/esm-register.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* (c) Copyright IBM Corp. 2024
*/

/**
* As of Node.js version 18.19 and above, ESM loaders (--experimental-loader)
* are executed in a dedicated thread, separate from the main thread.
* see https://github.com/nodejs/node/pull/44710.
* Previously, loading the Instana collector within the loader and after the update ESM support
* no longer working with v18.19 and above. To address this, we've opted to load the Instana
* collector in the main thread using --import. Additionally, we aim to incorporate native ESM
* support by utilizing the node register method, enabling customization of the ESM loader
* with 'import-in-the-middle'.
*
* Usage:
* ENV NODE_OPTIONS='--import /instana/node_modules/@instana/azure-container-services/esm-register.mjs server.js
*/

// Importing the Instana trace initialization module here, as this is executed in the main thread.
import './src/index.js';

// We plan to utilize this for adding native ESM support in the near future
// import { register } from 'node:module';
// register(./loader.mjs, import.meta.url);
3 changes: 2 additions & 1 deletion packages/azure-container-services/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"main": "src/index.js",
"files": [
"src",
"esm-loader.mjs"
"esm-loader.mjs",
"esm-register.mjs"
],
"publishConfig": {
"access": "public"
Expand Down
9 changes: 6 additions & 3 deletions packages/azure-container-services/test/Control.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const fetch = require('node-fetch');
const portfinder = require('@instana/collector/test/test_util/portfinder');
const config = require('@instana/core/test/config');
const AbstractServerlessControl = require('../../serverless/test/util/AbstractServerlessControl');
const isLatestEsmSupportedVersion = require('@instana/core').tracing.isLatestEsmSupportedVersion;

const PATH_TO_INSTANA_AZURE_PACKAGE = path.join(__dirname, '..');
let execArg;
Expand Down Expand Up @@ -50,10 +51,12 @@ class Control extends AbstractServerlessControl {
env.INSTANA_AGENT_KEY = this.instanaAgentKey;
}

const loaderPath = isLatestEsmSupportedVersion(process.versions.node)
? ['--import', `${path.join(__dirname, '..', 'esm-register.mjs')}`]
: [`--experimental-loader=${path.join(__dirname, '..', 'esm-loader.mjs')}`];

if (this.opts.containerAppPath && this.opts.env && this.opts.env.ESM_TEST) {
execArg = this.opts.containerAppPath.endsWith('.mjs')
? [`--experimental-loader=${path.join(__dirname, '..', 'esm-loader.mjs')}`]
: ['--require', PATH_TO_INSTANA_AZURE_PACKAGE];
execArg = this.opts.containerAppPath.endsWith('.mjs') ? loaderPath : ['--require', PATH_TO_INSTANA_AZURE_PACKAGE];
} else {
execArg = ['--require', PATH_TO_INSTANA_AZURE_PACKAGE];
}
Expand Down
2 changes: 1 addition & 1 deletion packages/azure-container-services/test/esm/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ if (esmSupportedVersion(process.versions.node)) {
});
} else {
// Skip the tests for unsupported Node.js version
describe('Azure Container Service', function () {
describe('[ESM] Azure Container Service', function () {
it('should skip tests for unsupported Node.js version', function () {
// eslint-disable-next-line no-console
console.log(`Skipping tests. Node.js version ${process.versions.node} is not supported.`);
Expand Down
21 changes: 6 additions & 15 deletions packages/collector/esm-loader.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
'use strict';

/**
* We currently only instrument CJS modules. As soon as we want
* to instrument ES modules (such as `got` v12), the requireHook will
* no longer work. Therefor we would need to wrap the target ES module
* with our instrumentations using the resolve & load hook.
*
* IMPORTANT NOTE: From Node.js version 18.19 and above, the ESM loaders operate off-thread.
* Consequently, ESM instrumentation using '--experimental-loader' becomes deprecated.
* Instead, we are using '--import' for loading instrumentation and relocated the Instana collector
* loading to './esm-register' file.
* Please note that '--import' flag is unavailable in earlier versions, hence we maintain both setups.
* We will incorporate the native ESM support by using 'import-in-the-middle' ith register method.
* Usage:
* node --experimental-loader=@instana/collector/esm-loader.mjs server.js
*
Expand All @@ -19,13 +20,3 @@

import instana from './src/index.js';
instana();

/*
export async function resolve(specifier, context, nextResolve) {
return nextResolve(specifier, context, nextResolve);
}

export async function load(url, context, nextLoad) {
return nextLoad(url, context, nextLoad);
}
*/
25 changes: 25 additions & 0 deletions packages/collector/esm-register.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* (c) Copyright IBM Corp. 2024
*/

/**
* As of Node.js version 18.19 and above, ESM loaders (--experimental-loader)
* are executed in a dedicated thread, separate from the main thread.
* see https://github.com/nodejs/node/pull/44710.
* Previously, loading the Instana collector within the loader and after the update ESM support
* no longer working with v18.19 and above. To address this, we've opted to load the Instana
* collector in the main thread using --import. Additionally, we aim to incorporate native ESM
* support by utilizing the node register method, enabling customization of the ESM loader
* with 'import-in-the-middle'.
*
* Usage:
* node --import @instana/collector/esm-register.mjs server.js
*/

// Importing the Instana trace initialization module here, as this is executed in the main thread.
import instana from './src/index.js';
aryamohanan marked this conversation as resolved.
Show resolved Hide resolved
instana();

// We plan to utilize this for adding native ESM support in the near future
// import { register } from 'node:module';
// register(./loader.mjs, import.meta.url);
3 changes: 2 additions & 1 deletion packages/collector/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
"files": [
"src",
"CHANGELOG.md",
"esm-loader.mjs"
"esm-loader.mjs",
"esm-register.mjs"
],
"publishConfig": {
"access": "public"
Expand Down
6 changes: 4 additions & 2 deletions packages/collector/test/test_util/ProcessControls.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const globalAgent = require('../globalAgent');
const portFinder = require('./portfinder');
const sslDir = path.join(__dirname, '..', 'apps', 'ssl');
const cert = fs.readFileSync(path.join(sslDir, 'cert'));

const isLatestEsmSupportedVersion = require('@instana/core').tracing.isLatestEsmSupportedVersion;
class ProcessControls {
/**
* @typedef {Object} ProcessControlsOptions
Expand Down Expand Up @@ -61,7 +61,9 @@ class ProcessControls {
const esmApp = files.find(f => f.indexOf('.mjs') !== -1);

if (esmApp) {
opts.execArgv = [`--experimental-loader=${path.join(__dirname, '..', '..', 'esm-loader.mjs')}`];
opts.execArgv = isLatestEsmSupportedVersion(process.versions.node)
aryamohanan marked this conversation as resolved.
Show resolved Hide resolved
? [`--import=${path.join(__dirname, '..', '..', 'esm-register.mjs')}`]
: [`--experimental-loader=${path.join(__dirname, '..', '..', 'esm-loader.mjs')}`];
opts.appPath = path.join(opts.dirname, 'app.mjs');
}
} catch (err) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const config = require('@instana/core/test/config');
const testUtils = require('@instana/core/test/test_util');
const ProcessControls = require('../../../../test_util/ProcessControls');
const globalAgent = require('../../../../globalAgent');

const isLatestEsmSupportedVersion = require('@instana/core').tracing.isLatestEsmSupportedVersion;
const mochaSuiteFn =
supportedVersion(process.versions.node) && semver.gte(process.versions.node, '18.0.0') ? describe : describe.skip;

Expand All @@ -42,12 +42,15 @@ mochaSuiteFn('[ESM] tracing/sdk/multiple_installations', function () {
let controls;

before(async () => {
const nodeOptions = isLatestEsmSupportedVersion(process.versions.node)
? '--import ./load-instana.mjs'
: '--experimental-loader ./load-instana.mjs';
controls = new ProcessControls({
useGlobalAgent: true,
cwd: path.join(__dirname, 'src'),
appPath: path.join(__dirname, 'src', 'app.mjs'),
env: {
NODE_OPTIONS: '--experimental-loader ./load-instana.mjs',
NODE_OPTIONS: nodeOptions,
INSTANA_COLLECTOR_PATH: pathToSeparateInstanaCollector
}
});
Expand Down
23 changes: 19 additions & 4 deletions packages/core/src/tracing/esmSupportedVersion.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,23 @@

const semver = require('semver');

/** @type {(version: string) => boolean} */
module.exports = exports = function esmSupportedVersion(version) {
// https://github.com/nodejs/node/pull/44710
return semver.satisfies(version, '>=14.0.0 <18.19');
/**
* Check if the given Node.js version supports ESM.
* @param {string} version - The Node.js version to check.
* @returns {boolean} - True if ESM is supported, false otherwise.
*/
exports.esmSupportedVersion = function esmSupportedVersion(version) {
return semver.gte(version, '14.0.0');
};

/**
* Check if the given Node.js version is the latest version that supports ESM.
* @param {string} version - The Node.js version to check.
* @returns {boolean} - True if the version is the latest that supports ESM, false otherwise.
*/
exports.isLatestEsmSupportedVersion = function isLatestEsmSupportedVersion(version) {
// Reference: https://nodejs.org/en/blog/release/v18.19.0#esm-and-customization-hook-changes
// Node.js v18.19 and above the loaders are off threaded
// https://instana.slack.com/archives/G0118PFNN20/p1708556683665099
return semver.gte(version, '18.19.0');
};
3 changes: 2 additions & 1 deletion packages/core/src/tracing/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const tracingUtil = require('./tracingUtil');
const spanBuffer = require('./spanBuffer');
const supportedVersion = require('./supportedVersion');
const { otelInstrumentations } = require('./opentelemetry-instrumentations');
const esmSupportedVersion = require('./esmSupportedVersion');
const { esmSupportedVersion, isLatestEsmSupportedVersion } = require('./esmSupportedVersion');

let tracingEnabled = false;
let tracingActivated = false;
Expand Down Expand Up @@ -121,6 +121,7 @@ exports.spanBuffer = spanBuffer;
exports.supportedVersion = supportedVersion;
exports.util = tracingUtil;
exports.esmSupportedVersion = esmSupportedVersion;
exports.isLatestEsmSupportedVersion = isLatestEsmSupportedVersion;

/**
* @param {import('../util/normalizeConfig').InstanaConfig} cfg
Expand Down
9 changes: 7 additions & 2 deletions packages/core/src/util/applicationUnderMonitoring.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,18 @@ function getMainPackageJsonPathStartingAtDirectory(startDirectory, cb) {
if (
// @ts-ignore
(process._preload_modules && process._preload_modules.length > 0) ||
(process.env.NODE_OPTIONS && process.env.NODE_OPTIONS.indexOf('--experimental-loader') !== -1) ||
(process.env.NODE_OPTIONS &&
(process.env.NODE_OPTIONS.indexOf('--experimental-loader') !== -1 ||
process.env.NODE_OPTIONS.indexOf('--import') !== -1)) ||
(process.execArgv &&
process.execArgv.length > 0 &&
((process.execArgv[0].indexOf('--experimental-loader') !== -1 &&
process.execArgv[0].indexOf('esm-loader.mjs')) !== -1 ||
(process.execArgv[0].indexOf('--experimental-loader') !== -1 &&
process.execArgv[1].indexOf('esm-loader.mjs') !== -1)))
process.execArgv[1].indexOf('esm-loader.mjs') !== -1) ||
(process.execArgv[0].indexOf('--import') !== -1 && process.execArgv[0].indexOf('esm-register.mjs')) !==
-1 ||
(process.execArgv[0].indexOf('--import') !== -1 && process.execArgv[1].indexOf('esm-register.mjs') !== -1)))
) {
// @ts-ignore
mainModule = {
Expand Down
Loading