diff --git a/README.md b/README.md index 78ceb5f..aa13de6 100644 --- a/README.md +++ b/README.md @@ -1,12 +1,13 @@ # Tracing Hooks -This repository contains a ESM loader for injecting tracing channel hooks into Node.js modules. It also has a patch for Module to be used to patch CJS modules. +This repository contains a ESM loader for injecting tracing channel hooks into +Node.js modules. It also has a patch for Module to be used to patch CJS modules. ## Usage Note: the module loading hooks API in Node.js has changed as of v26. To support all active Node.js versions with -forward-compatibility, create a combined loader as an ESM module. +forward-compatibility, create a combined loader as an ES Module (ESM). This can be done for any CommonJS _or_ ES Module application, but the loader itself must use ESM. @@ -138,4 +139,4 @@ On this path, diagnostics for ES modules are posted from the loader thread and therefore arrive asynchronously, some time after the module was transformed; diagnostics for CommonJS modules come from the `_compile` patch and are emitted synchronously as before. The port does not keep the process alive, so -diagnostics still in flight when the process exits are dropped. \ No newline at end of file +diagnostics still in flight when the process exits are dropped. diff --git a/hook-sync.mjs b/hook-sync.mjs index 979f9b8..401d94f 100644 --- a/hook-sync.mjs +++ b/hook-sync.mjs @@ -1 +1,7 @@ -export { initializeSync as initialize, loadSync as load, resolveSync as resolve, setDiagnosticsHook, createDiagnosticsPort } from './hook.mjs' +export { + initializeSync as initialize, + loadSync as load, + resolveSync as resolve, + setDiagnosticsHook, + createDiagnosticsPort +} from './hook.mjs' diff --git a/hook.mjs b/hook.mjs index d8b121e..de95d65 100644 --- a/hook.mjs +++ b/hook.mjs @@ -1,12 +1,15 @@ 'use strict' -import createDebug from 'debug' -import { create } from '@apm-js-collab/code-transformer' -import parse from 'module-details-from-path' + +import { readFile } from 'node:fs/promises' +import { readFileSync } from 'node:fs' import { fileURLToPath } from 'node:url' import { MessageChannel } from 'node:worker_threads' -import getPackageVersion from './lib/get-package-version.js' +import { create as defaultCreate } from '@apm-js-collab/code-transformer' +import createDebug from 'debug' +import parse from 'module-details-from-path' import { setDiagnosticsHook, emitDiagnostics } from './lib/diagnostics.js' -import { readFileSync } from 'node:fs' +import getPackageVersion from './lib/get-package-version.js' + const debug = createDebug('@apm-js-collab/tracing-hooks:esm-hook') let transformers = null let packages = null @@ -34,10 +37,10 @@ export function createDiagnosticsPort() { return port2 } -export async function initialize(data = {}) { - return initializeSync(data) +export async function initialize(data = {}, { create = defaultCreate } = {}) { + return initializeSync(data, { create }) } -export function initializeSync(data = {}) { +export function initializeSync(data = {}, { create = defaultCreate } = {}) { const instrumentations = data?.instrumentations || [] instrumentator = create(instrumentations) packages = new Set(instrumentations.map(i => i.module.name)) @@ -152,4 +155,4 @@ export function loadResult(url, result) { } return result -} \ No newline at end of file +} diff --git a/index.js b/index.js index 985e20b..f17756d 100644 --- a/index.js +++ b/index.js @@ -1,5 +1,5 @@ 'use strict' -const { create } = require('@apm-js-collab/code-transformer') +const { create: defaultCreate } = require('@apm-js-collab/code-transformer') const Module = require('node:module') const parse = require('module-details-from-path') const { pathToFileURL } = require('node:url') @@ -8,7 +8,7 @@ const { emitDiagnostics } = require('./lib/diagnostics') const debug = require('debug')('@apm-js-collab/tracing-hooks:module-patch') class ModulePatch { - constructor({ instrumentations = [] } = {}) { + constructor({ instrumentations = [], create = defaultCreate } = {}) { this.packages = new Set(instrumentations.map(i => i.module.name)) this.instrumentator = create(instrumentations) this.compile = Module.prototype._compile diff --git a/test/create-injection.test.js b/test/create-injection.test.js new file mode 100644 index 0000000..454173f --- /dev/null +++ b/test/create-injection.test.js @@ -0,0 +1,343 @@ +'use strict' + +const test = require('node:test') +const path = require('node:path') +const { readFileSync } = require('node:fs') +const Module = require('node:module') + +const ModulePatch = require('../index.js') + +test('create function receives instrumentations array', (t) => { + t.plan(2) + + const mockCreate = (instrumentations) => { + t.assert.ok(Array.isArray(instrumentations), 'instrumentations should be an array') + t.assert.strictEqual(instrumentations.length, 1, 'should receive one instrumentation') + return { + getTransformer: () => null + } + } + + const instrumentations = [ + { + channelName: 'testChannel', + module: { name: 'test-pkg', versionRange: '>=1.0.0', filePath: 'index.js' }, + functionQuery: { className: 'TestClass', methodName: 'testMethod' } + } + ] + + new ModulePatch({ instrumentations, create: mockCreate }) +}) + +test('getTransformer is called with correct arguments', (t) => { + t.plan(3) + t.after(() => { + modulePatch.unpatch() + }) + + const mockCreate = () => { + return { + getTransformer: (name, version, filePath) => { + t.assert.strictEqual(name, 'pkg-1', 'package name should be pkg-1') + t.assert.ok(version, 'version should be provided') + t.assert.strictEqual(filePath, 'foo.js', 'filePath should be foo.js') + return null + } + } + } + + const instrumentations = [ + { + channelName: 'testChannel', + module: { name: 'pkg-1', versionRange: '>=1', filePath: 'foo.js' }, + functionQuery: { className: 'Foo', methodName: 'doStuff' } + } + ] + + const modulePatch = new ModulePatch({ instrumentations, create: mockCreate }) + modulePatch.patch() + + const modulePath = path.join(__dirname, './example-deps/lib/node_modules/pkg-1/foo.js') + const resolvedPath = Module._resolveFilename(modulePath, null, false) + const data = readFileSync(resolvedPath, 'utf8') + const testModule = new Module(resolvedPath) + testModule._compile(data, resolvedPath) +}) + +test('transformer.transform is called with correct arguments', (t) => { + t.plan(3) + t.after(() => { + modulePatch.unpatch() + }) + + const mockCreate = () => { + return { + getTransformer: () => { + return { + transform: (content, format) => { + t.assert.ok(typeof content === 'string', 'content should be a string') + t.assert.ok(content.length > 0, 'content should not be empty') + t.assert.strictEqual(format, 'cjs', 'format should be cjs') + return { code: content } + }, + free: () => {} + } + } + } + } + + const instrumentations = [ + { + channelName: 'testChannel', + module: { name: 'pkg-1', versionRange: '>=1', filePath: 'foo.js' }, + functionQuery: { className: 'Foo', methodName: 'doStuff' } + } + ] + + const modulePatch = new ModulePatch({ instrumentations, create: mockCreate }) + modulePatch.patch() + + const modulePath = path.join(__dirname, './example-deps/lib/node_modules/pkg-1/foo.js') + const resolvedPath = Module._resolveFilename(modulePath, null, false) + const data = readFileSync(resolvedPath, 'utf8') + const testModule = new Module(resolvedPath) + testModule._compile(data, resolvedPath) +}) + +test('transformer.free is called after transform', (t) => { + t.plan(2) + t.after(() => { + modulePatch.unpatch() + }) + + let transformCalled = false + + const mockCreate = () => { + return { + getTransformer: () => { + return { + transform: (content, format) => { + transformCalled = true + return { code: content } + }, + free: () => { + t.assert.ok(transformCalled, 'transform should be called before free') + t.assert.ok(true, 'free should be called') + } + } + } + } + } + + const instrumentations = [ + { + channelName: 'testChannel', + module: { name: 'pkg-1', versionRange: '>=1', filePath: 'foo.js' }, + functionQuery: { className: 'Foo', methodName: 'doStuff' } + } + ] + + const modulePatch = new ModulePatch({ instrumentations, create: mockCreate }) + modulePatch.patch() + + const modulePath = path.join(__dirname, './example-deps/lib/node_modules/pkg-1/foo.js') + const resolvedPath = Module._resolveFilename(modulePath, null, false) + const data = readFileSync(resolvedPath, 'utf8') + const testModule = new Module(resolvedPath) + testModule._compile(data, resolvedPath) +}) + +test('transformer.free is called even when transform throws', (t) => { + t.plan(2) + t.after(() => { + modulePatch.unpatch() + }) + + const mockCreate = () => { + return { + getTransformer: () => { + return { + transform: () => { + t.assert.ok(true, 'transform should be called') + throw new Error('Transform error') + }, + free: () => { + t.assert.ok(true, 'free should be called even when transform throws') + } + } + } + } + } + + const instrumentations = [ + { + channelName: 'testChannel', + module: { name: 'pkg-1', versionRange: '>=1', filePath: 'foo.js' }, + functionQuery: { className: 'Foo', methodName: 'doStuff' } + } + ] + + const modulePatch = new ModulePatch({ instrumentations, create: mockCreate }) + modulePatch.patch() + + const modulePath = path.join(__dirname, './example-deps/lib/node_modules/pkg-1/foo.js') + const resolvedPath = Module._resolveFilename(modulePath, null, false) + const data = readFileSync(resolvedPath, 'utf8') + const testModule = new Module(resolvedPath) + // Should not throw - error is caught internally + testModule._compile(data, resolvedPath) +}) + +test('getTransformer not called for non-instrumented packages', (t) => { + t.plan(1) + t.after(() => { + modulePatch.unpatch() + }) + + const mockCreate = () => { + return { + getTransformer: () => { + t.assert.fail('getTransformer should not be called for non-instrumented packages') + return null + } + } + } + + const instrumentations = [ + { + channelName: 'testChannel', + module: { name: 'pkg-1', versionRange: '>=1', filePath: 'foo.js' }, + functionQuery: { className: 'Foo', methodName: 'doStuff' } + } + ] + + const modulePatch = new ModulePatch({ instrumentations, create: mockCreate }) + modulePatch.patch() + + // Try to compile a different package + const modulePath = path.join(__dirname, './example-deps/lib/node_modules/pkg-2/index.js') + const resolvedPath = Module._resolveFilename(modulePath, null, false) + const data = readFileSync(resolvedPath, 'utf8') + const testModule = new Module(resolvedPath) + testModule._compile(data, resolvedPath) + + t.assert.ok(testModule.exports, 'module should compile successfully') +}) + +test('mock create function can return transformed code', (t) => { + t.plan(3) + t.after(() => { + modulePatch.unpatch() + }) + + const mockCreate = () => { + return { + getTransformer: () => { + return { + transform: (content) => { + t.assert.ok(content.includes('class Foo'), 'original content should contain Foo class') + const transformed = `/* TRANSFORMED */\n${content}` + t.assert.ok(transformed.startsWith('/* TRANSFORMED */'), 'transformed code should have comment') + return { code: transformed } + }, + free: () => {} + } + } + } + } + + const instrumentations = [ + { + channelName: 'testChannel', + module: { name: 'pkg-1', versionRange: '>=1', filePath: 'foo.js' }, + functionQuery: { className: 'Foo', methodName: 'doStuff' } + } + ] + + const modulePatch = new ModulePatch({ instrumentations, create: mockCreate }) + modulePatch.patch() + + const modulePath = path.join(__dirname, './example-deps/lib/node_modules/pkg-1/foo.js') + const resolvedPath = Module._resolveFilename(modulePath, null, false) + const data = readFileSync(resolvedPath, 'utf8') + const testModule = new Module(resolvedPath) + testModule._compile(data, resolvedPath) + + t.assert.ok(testModule.exports, 'module should export successfully') +}) + +test('getTransformer returns null for non-matching transformer', (t) => { + t.plan(2) + t.after(() => { + modulePatch.unpatch() + }) + + const mockCreate = () => { + return { + getTransformer: (name, version, filePath) => { + t.assert.strictEqual(name, 'pkg-1', 'should be called with pkg-1') + t.assert.strictEqual(filePath, 'foo.js', 'should be called with foo.js') + // Return null to simulate no matching transformer + return null + } + } + } + + const instrumentations = [ + { + channelName: 'testChannel', + module: { name: 'pkg-1', versionRange: '>=1', filePath: 'foo.js' }, + functionQuery: { className: 'NonExistent', methodName: 'nonExistent' } + } + ] + + const modulePatch = new ModulePatch({ instrumentations, create: mockCreate }) + modulePatch.patch() + + const modulePath = path.join(__dirname, './example-deps/lib/node_modules/pkg-1/foo.js') + const resolvedPath = Module._resolveFilename(modulePath, null, false) + const data = readFileSync(resolvedPath, 'utf8') + const testModule = new Module(resolvedPath) + testModule._compile(data, resolvedPath) +}) + +test('transform receives exact file content', (t) => { + t.plan(2) + t.after(() => { + modulePatch.unpatch() + }) + + const modulePath = path.join(__dirname, './example-deps/lib/node_modules/pkg-1/foo.js') + const resolvedPath = Module._resolveFilename(modulePath, null, false) + const expectedContent = readFileSync(resolvedPath, 'utf8') + + const mockCreate = () => { + return { + getTransformer: () => { + return { + transform: (content, format) => { + t.assert.strictEqual(content, expectedContent, 'content should match original file') + t.assert.strictEqual(format, 'cjs', 'format should be cjs') + return { code: content } + }, + free: () => {} + } + } + } + } + + const instrumentations = [ + { + channelName: 'testChannel', + module: { name: 'pkg-1', versionRange: '>=1', filePath: 'foo.js' }, + functionQuery: { className: 'Foo', methodName: 'doStuff' } + } + ] + + const modulePatch = new ModulePatch({ instrumentations, create: mockCreate }) + modulePatch.patch() + + const data = readFileSync(resolvedPath, 'utf8') + const testModule = new Module(resolvedPath) + testModule._compile(data, resolvedPath) +}) diff --git a/test/hook-with-injected-dep.test.js b/test/hook-with-injected-dep.test.js new file mode 100644 index 0000000..e02efa3 --- /dev/null +++ b/test/hook-with-injected-dep.test.js @@ -0,0 +1,266 @@ +'use strict' + +const test = require('node:test') +const path = require('node:path') +const { readFile } = require('node:fs/promises') + +test('hook.mjs accepts custom create function via initialize options', async (t) => { + t.plan(2) + + const mockCreate = (instrumentations) => { + return { + getTransformer(name, version, filePath) { + if (name === 'esm-pkg' && filePath === 'foo.js') { + return { + transform(content, format) { + return { code: '/* CUSTOM TRANSFORMER */\n' + content } + }, + free() {}, + moduleName: name + } + } + return null + } + } + } + + const hook = await import('../hook.mjs?' + Date.now()) + + hook.initialize({ + instrumentations: [ + { + channelName: 'createTest', + module: { name: 'esm-pkg', versionRange: '>=1', filePath: 'foo.js' }, + functionQuery: { + className: 'Foo', + methodName: 'doStuff', + kind: 'Async' + } + } + ] + }, { create: mockCreate }) + + const esmPath = path.join(__dirname, './example-deps/lib/node_modules/esm-pkg/foo.js') + async function resolveFn() { + return { url: `file://${esmPath}` } + } + async function nextLoad() { + const data = await readFile(esmPath, 'utf8') + return { + format: 'module', + source: data + } + } + + const url = await hook.resolve('esm-pkg', {}, resolveFn) + const result = await hook.load(url.url, {}, nextLoad) + + t.assert.ok(result.source.includes('/* CUSTOM TRANSFORMER */'), 'should use custom create function') + t.assert.strictEqual(result.shortCircuit, true, 'should short circuit') +}) + +test('hook.mjs defaults to @apm-js-collab/code-transformer when create not provided', async (t) => { + t.plan(2) + + const hook = await import('../hook.mjs?' + Date.now()) + + hook.initialize({ + instrumentations: [ + { + channelName: 'defaultTest', + module: { name: 'esm-pkg', versionRange: '>=1', filePath: 'foo.js' }, + functionQuery: { + className: 'Foo', + methodName: 'doStuff', + kind: 'Async' + } + } + ] + }) + + const esmPath = path.join(__dirname, './example-deps/lib/node_modules/esm-pkg/foo.js') + async function resolveFn() { + return { url: `file://${esmPath}` } + } + async function nextLoad() { + const data = await readFile(esmPath, 'utf8') + return { + format: 'module', + source: data + } + } + + const url = await hook.resolve('esm-pkg', {}, resolveFn) + const result = await hook.load(url.url, {}, nextLoad) + + t.assert.strictEqual(result.shortCircuit, true, 'should transform using default transformer') + t.assert.ok(result.source.includes('diagnostics_channel'), 'should include diagnostics_channel from default transformer') +}) + +test('custom create function exercises getTransformer with correct args', async (t) => { + t.plan(4) + + const mockCreate = (instrumentations) => { + t.assert.strictEqual(instrumentations.length, 1, 'should receive one instrumentation') + return { + getTransformer(name, version, filePath) { + t.assert.strictEqual(name, 'esm-pkg', 'name should be esm-pkg') + t.assert.ok(version, 'version should be provided') + t.assert.strictEqual(filePath, 'foo.js', 'filePath should be foo.js') + + return { + transform(content, format) { + if (format !== 'esm') throw new Error('Expected format to be esm') + return { code: content } + }, + free() {}, + moduleName: name + } + } + } + } + + const hook = await import('../hook.mjs?' + Date.now()) + + hook.initialize({ + instrumentations: [ + { + channelName: 'argsTest', + module: { name: 'esm-pkg', versionRange: '>=1', filePath: 'foo.js' }, + functionQuery: { + className: 'Foo', + methodName: 'doStuff', + kind: 'Async' + } + } + ] + }, { create: mockCreate }) + + const esmPath = path.join(__dirname, './example-deps/lib/node_modules/esm-pkg/foo.js') + async function resolveFn() { + return { url: `file://${esmPath}` } + } + async function nextLoad() { + const data = await readFile(esmPath, 'utf8') + return { + format: 'module', + source: data + } + } + + const url = await hook.resolve('esm-pkg', {}, resolveFn) + await hook.load(url.url, {}, nextLoad) +}) + +test('custom create function exercises free method', async (t) => { + t.plan(2) + + let freeCalls = 0 + + const mockCreate = () => { + return { + getTransformer(name, version, filePath) { + return { + transform(content, format) { + return { code: content } + }, + free() { + freeCalls++ + }, + moduleName: name + } + } + } + } + + const hook = await import('../hook.mjs?' + Date.now()) + + hook.initialize({ + instrumentations: [ + { + channelName: 'freeTest', + module: { name: 'esm-pkg', versionRange: '>=1', filePath: 'foo.js' }, + functionQuery: { + className: 'Foo', + methodName: 'doStuff' + } + } + ] + }, { create: mockCreate }) + + const esmPath = path.join(__dirname, './example-deps/lib/node_modules/esm-pkg/foo.js') + async function resolveFn() { + return { url: `file://${esmPath}` } + } + async function nextLoad() { + return { + format: 'module', + source: await readFile(esmPath, 'utf8') + } + } + + const url = await hook.resolve('esm-pkg', {}, resolveFn) + await hook.load(url.url, {}, nextLoad) + + t.assert.strictEqual(freeCalls, 1, 'free should be called once') + + await hook.load(url.url, {}, nextLoad) + t.assert.strictEqual(freeCalls, 2, 'free should be called again on second load') +}) + +test('custom create function handles transform errors', async (t) => { + t.plan(2) + + let freeCalled = false + + const mockCreate = () => { + return { + getTransformer(name, version, filePath) { + return { + transform(content, format) { + throw new Error('Transform failed intentionally') + }, + free() { + freeCalled = true + }, + moduleName: name + } + } + } + } + + const hook = await import('../hook.mjs?' + Date.now()) + + hook.initialize({ + instrumentations: [ + { + channelName: 'errorTest', + module: { name: 'esm-pkg', versionRange: '>=1', filePath: 'foo.js' }, + functionQuery: { + className: 'Foo', + methodName: 'doStuff' + } + } + ] + }, { create: mockCreate }) + + const esmPath = path.join(__dirname, './example-deps/lib/node_modules/esm-pkg/foo.js') + async function resolveFn() { + return { url: `file://${esmPath}` } + } + async function nextLoad() { + return { + format: 'module', + source: await readFile(esmPath, 'utf8') + } + } + + const url = await hook.resolve('esm-pkg', {}, resolveFn) + + await t.assert.doesNotReject( + async () => await hook.load(url.url, {}, nextLoad), + 'should handle transform errors gracefully' + ) + + t.assert.strictEqual(freeCalled, true, 'free should still be called when transform throws') +})