From 425687282696f9ba1654c2ed98293fb6f8ea40d7 Mon Sep 17 00:00:00 2001 From: vmarchaud Date: Sun, 20 Mar 2022 10:30:12 +0100 Subject: [PATCH] feat(instrumentation): add support for esm module --- CHANGELOG.md | 2 + .../opentelemetry-instrumentation/README.md | 4 ++ .../package.json | 5 +- .../node/RequireInTheMiddleSingleton.ts | 41 +++++++++------- .../src/platform/node/instrumentation.ts | 19 ++++---- .../test/node/esmInstrumentation.test.mjs | 47 +++++++++++++++++++ .../test/node/node_modules/.gitkeep | 0 .../node_modules/my-esm-module/package.json | 6 +++ .../node_modules/my-esm-module/src/index.mjs | 6 +++ 9 files changed, 102 insertions(+), 28 deletions(-) create mode 100644 experimental/packages/opentelemetry-instrumentation/test/node/esmInstrumentation.test.mjs create mode 100644 experimental/packages/opentelemetry-instrumentation/test/node/node_modules/.gitkeep create mode 100644 experimental/packages/opentelemetry-instrumentation/test/node/node_modules/my-esm-module/package.json create mode 100644 experimental/packages/opentelemetry-instrumentation/test/node/node_modules/my-esm-module/src/index.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 79881b78dd8..c881c026e16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ All notable changes to this project will be documented in this file. * feat(sdk-trace): re-export sdk-trace-base in sdk-trace-node and web [#3319](https://github.com/open-telemetry/opentelemetry-js/pull/3319) @legendecas * feat: enable tree shaking [#3329](https://github.com/open-telemetry/opentelemetry-js/pull/3329) @pkanal +* feat(instrumentation): add support for esm module [#2846](https://github.com/open-telemetry/opentelemetry-js/pull/2846) @vmarchaud + ### :bug: (Bug Fix) diff --git a/experimental/packages/opentelemetry-instrumentation/README.md b/experimental/packages/opentelemetry-instrumentation/README.md index 26e9f8c6962..74756af7856 100644 --- a/experimental/packages/opentelemetry-instrumentation/README.md +++ b/experimental/packages/opentelemetry-instrumentation/README.md @@ -263,6 +263,10 @@ If nothing is specified the global registered provider is used. Usually this is There might be usecase where someone has the need for more providers within an application. Please note that special care must be takes in such setups to avoid leaking information from one provider to the other because there are a lot places where e.g. the global `ContextManager` or `Propagator` is used. +## ESM within Node.JS + +As the module loading mechanism for ESM is different than CJS, you need to select a custom loader so instrumentation can load hook on the esm module it want to patch, to do so you need to provide `--loader=import-in-the-middle/hook.mjs` to the `node` binary. This only works for Node.JS > 12. + ## License Apache 2.0 - See [LICENSE][license-url] for more information. diff --git a/experimental/packages/opentelemetry-instrumentation/package.json b/experimental/packages/opentelemetry-instrumentation/package.json index abc09deb239..f2c0fc7cf2e 100644 --- a/experimental/packages/opentelemetry-instrumentation/package.json +++ b/experimental/packages/opentelemetry-instrumentation/package.json @@ -47,7 +47,9 @@ "tdd": "npm run tdd:node", "tdd:node": "npm run test -- --watch-extensions ts --watch", "tdd:browser": "karma start", - "test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts' --exclude 'test/browser/**/*.ts'", + "test:cjs": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts' --exclude 'test/browser/**/*.ts'", + "test:esm": "nyc node --loader=import-in-the-middle/hook.mjs node_modules/.bin/_mocha test/node/*.test.mjs", + "test": "npm run test:cjs && npm run test:esm", "test:browser": "nyc karma start --single-run", "version": "node ../../../scripts/version-update.js", "watch": "tsc --build --watch tsconfig.all.json", @@ -69,6 +71,7 @@ }, "dependencies": { "@opentelemetry/api-metrics": "0.33.0", + "import-in-the-middle": "^1.3.4", "require-in-the-middle": "^5.0.3", "semver": "^7.3.2", "shimmer": "^1.2.1" diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/RequireInTheMiddleSingleton.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/RequireInTheMiddleSingleton.ts index 812db52b683..1f03fdec059 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/node/RequireInTheMiddleSingleton.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/RequireInTheMiddleSingleton.ts @@ -15,6 +15,7 @@ */ import * as RequireInTheMiddle from 'require-in-the-middle'; +import * as ImportInTheMiddle from 'import-in-the-middle'; import * as path from 'path'; import { ModuleNameTrie, ModuleNameSeparator } from './ModuleNameTrie'; @@ -23,6 +24,12 @@ export type Hooked = { onRequire: RequireInTheMiddle.OnRequireFn }; +/** + * We are forced to re-type there because ImportInTheMiddle is exported as normal CJS + * in the JS files but transpiled ESM (with a default export) in its typing. + */ + const ESMHook = ImportInTheMiddle as unknown as typeof ImportInTheMiddle.default; + /** * Whether Mocha is running in this process * Inspired by https://github.com/AndreasPizsa/detect-mocha @@ -35,12 +42,12 @@ const isMocha = ['afterEach','after','beforeEach','before','describe','it'].ever }); /** - * Singleton class for `require-in-the-middle` + * Singleton class for `require-in-the-middle` and `import-in-the-middle` * Allows instrumentation plugins to patch modules with only a single `require` patch - * WARNING: Because this class will create its own `require-in-the-middle` (RITM) instance, + * WARNING: Because this class will create its own RITM and IITM instance, * we should minimize the number of new instances of this class. * Multiple instances of `@opentelemetry/instrumentation` (e.g. multiple versions) in a single process - * will result in multiple instances of RITM, which will have an impact + * will result in multiple instances of RITM/ITTM, which will have an impact * on the performance of instrumentation hooks being applied. */ export class RequireInTheMiddleSingleton { @@ -52,23 +59,23 @@ export class RequireInTheMiddleSingleton { } private _initialize() { - RequireInTheMiddle( - // Intercept all `require` calls; we will filter the matching ones below - null, - { internals: true }, - (exports, name, basedir) => { - // For internal files on Windows, `name` will use backslash as the path separator - const normalizedModuleName = normalizePathSeparators(name); + // + const onHook = (exports: any, name: string, basedir: string | undefined | void) => { + // For internal files on Windows, `name` will use backslash as the path separator + const normalizedModuleName = normalizePathSeparators(name); - const matches = this._moduleNameTrie.search(normalizedModuleName, { maintainInsertionOrder: true }); + const matches = this._moduleNameTrie.search(normalizedModuleName, { maintainInsertionOrder: true }); - for (const { onRequire } of matches) { - exports = onRequire(exports, name, basedir); - } - - return exports; + for (const { onRequire } of matches) { + exports = onRequire(exports, name, basedir ? basedir : undefined); } - ); + + return exports; + } + // Intercept all `require` calls; we will filter the matching ones below + RequireInTheMiddle(null, { internals: true }, onHook); + // We can give no module to patch but this signature isn't exposed in typings + new ESMHook(null as any, { internals: true }, onHook) } /** diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts index d80985431ce..6d62a49a9da 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts @@ -76,16 +76,14 @@ export abstract class InstrumentationBase }); } - private _extractPackageVersion(baseDir: string): string | undefined { + private _extractPackage(baseDir: string): { name?: string, version?: string, main?: string } { try { // eslint-disable-next-line @typescript-eslint/no-var-requires - const version = require(path.join(baseDir, 'package.json')).version; - return typeof version === 'string' ? version : undefined; + return require(path.join(baseDir, 'package.json')); } catch (error) { diag.warn('Failed extracting version', baseDir); } - - return undefined; + return {}; } private _onRequire( @@ -104,12 +102,13 @@ export abstract class InstrumentationBase return exports; } - const version = this._extractPackageVersion(baseDir); - module.moduleVersion = version; - if (module.name === name) { + const pkg = this._extractPackage(baseDir); + module.moduleVersion = pkg.version; + // if the targeted module is an esm, the name will be the name of its entrypoint + if (module.name === name || (pkg.main && path.normalize(`${pkg.name}/${pkg.main}`) === name)) { // main module if ( - isSupported(module.supportedVersions, version, module.includePrerelease) + isSupported(module.supportedVersions, pkg.version, module.includePrerelease) ) { if (typeof module.patch === 'function') { module.moduleExports = exports; @@ -124,7 +123,7 @@ export abstract class InstrumentationBase const files = module.files ?? []; const supportedFileInstrumentations = files .filter(f => f.name === name) - .filter(f => isSupported(f.supportedVersions, version, module.includePrerelease)); + .filter(f => isSupported(f.supportedVersions, pkg.version, module.includePrerelease)); return supportedFileInstrumentations.reduce( (patchedExports, file) => { file.moduleExports = patchedExports; diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/esmInstrumentation.test.mjs b/experimental/packages/opentelemetry-instrumentation/test/node/esmInstrumentation.test.mjs new file mode 100644 index 00000000000..a80341368d0 --- /dev/null +++ b/experimental/packages/opentelemetry-instrumentation/test/node/esmInstrumentation.test.mjs @@ -0,0 +1,47 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as assert from 'assert' +import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '../../build/src/index.js'; + +describe('when loading esm module', () => { + it('should patch module file', async () => { + class TestInstrumentation extends InstrumentationBase { + constructor(onPatch, onUnpatch) { + super('my-esm-instrumentation', '0.1.0'); + } + + init() { + return [ + new InstrumentationNodeModuleDefinition( + 'my-esm-module', + ['*'], + (exports, version) => { + exports.myConstant = 43; + exports.myFunction = () => 'another'; + } + ) + ]; + } + } + + const instrumentation = new TestInstrumentation(); + instrumentation.enable(); + const exported = await import('my-esm-module'); + assert.deepEqual(exported.myConstant, 43); + assert.deepEqual(exported.myFunction(), 'another'); + }); +}); diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/node_modules/.gitkeep b/experimental/packages/opentelemetry-instrumentation/test/node/node_modules/.gitkeep new file mode 100644 index 00000000000..e69de29bb2d diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/node_modules/my-esm-module/package.json b/experimental/packages/opentelemetry-instrumentation/test/node/node_modules/my-esm-module/package.json new file mode 100644 index 00000000000..70e551be04c --- /dev/null +++ b/experimental/packages/opentelemetry-instrumentation/test/node/node_modules/my-esm-module/package.json @@ -0,0 +1,6 @@ +{ + "name": "my-esm-module", + "version": "0.1.0", + "main": "./src/index.mjs", + "type": "module" +} diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/node_modules/my-esm-module/src/index.mjs b/experimental/packages/opentelemetry-instrumentation/test/node/node_modules/my-esm-module/src/index.mjs new file mode 100644 index 00000000000..9e383818280 --- /dev/null +++ b/experimental/packages/opentelemetry-instrumentation/test/node/node_modules/my-esm-module/src/index.mjs @@ -0,0 +1,6 @@ + +export const myFunction = () => { + return 'test'; +}; + +export const myConstant = 42;