From f66cfc0db2733d396fdfe0a3d7627f19a55d38e4 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Fri, 20 Dec 2024 16:42:52 -0800 Subject: [PATCH 1/2] feat(opentelemetry-node): improved ESM instrumentation support This updates to usage of IITM's support for only hooking modules intended to be hooked (added in IITM 1.11.0, see https://github.com/nodejs/import-in-the-middle/pull/146). This helps workaround cases where IITM hooking breaks some modules. The openai-chat.mjs is one such example. --- .gitignore | 1 + examples/openai-chat.mjs | 33 +++++++++++ packages/opentelemetry-node/import.mjs | 59 ++++++------------- packages/opentelemetry-node/package-lock.json | 14 ++--- 4 files changed, 60 insertions(+), 47 deletions(-) create mode 100644 examples/openai-chat.mjs diff --git a/.gitignore b/.gitignore index dae2806f..ec6d34c8 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ build .DS_Store tmp /packages/mockotlpserver/db +/openai.env diff --git a/examples/openai-chat.mjs b/examples/openai-chat.mjs new file mode 100644 index 00000000..efe59452 --- /dev/null +++ b/examples/openai-chat.mjs @@ -0,0 +1,33 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you 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 + * + * http://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. + */ + +// An ES Modules (ESM) version of "openai-chat.js". +// +// Usage: +// OPENAI_API_KEY=sk-... \ +// node --import @elastic/opentelemetry-node openai-chat.mjs + +import {OpenAI} from 'openai'; + +const openai = new OpenAI(); +const result = await openai.chat.completions.create({ + model: 'gpt-4o-mini', + messages: [{role: 'user', content: 'Why is the sky blue? Answer briefly.'}], +}); +console.log(result.choices[0]?.message?.content); diff --git a/packages/opentelemetry-node/import.mjs b/packages/opentelemetry-node/import.mjs index f3d014b2..b2073e35 100644 --- a/packages/opentelemetry-node/import.mjs +++ b/packages/opentelemetry-node/import.mjs @@ -20,52 +20,31 @@ // Register ESM hook and start the SDK. // This is called for `--import @elastic/opentelemetry-node`. -import * as module from 'node:module'; +import { register } from 'node:module'; import {isMainThread} from 'node:worker_threads'; +// TODO: @opentelemetry/instrumentation should re-export this IITM method. +// XXX add explicit IITM dep? +// XXX can we have a guard that there is only one install of IITM in play? +import { createAddHookMessageChannel } from 'import-in-the-middle'; + import {log} from './lib/logging.js'; -/** - * Return true iff it looks like the `@elastic/opentelemetry-node/hook.mjs` - * was loaded via node's `--experimental-loader` option. - * - * Dev Note: keep this in sync with "require.js". - */ -function haveHookFromExperimentalLoader() { - const USED_LOADER_OPT = - /--(experimental-)?loader(\s+|=)@elastic\/opentelemetry-node\/hook.mjs/; - for (let i = 0; i < process.execArgv.length; i++) { - const arg = process.execArgv[i]; - const nextArg = process.execArgv[i + 1]; - if ( - (arg === '--loader' || arg === '--experimental-loader') && - nextArg === '@elastic/opentelemetry-node/hook.mjs' - ) { - log.trace('bootstrap-import: --loader hook args used'); - return true; - } else if (USED_LOADER_OPT.test(arg)) { - log.trace('bootstrap-import: --loader hook arg used'); - return true; - } - } - if ( - process.env.NODE_OPTIONS && - USED_LOADER_OPT.test(process.env.NODE_OPTIONS) - ) { - log.trace('bootstrap-import: --loader arg used in NODE_OPTIONS'); - return true; - } - return false; -} +// Note: If there are *multiple* installations of import-in-the-middle, then +// only those instrumentations using this same one will be hooked. +const { registerOptions, waitForAllMessagesAcknowledged } = + createAddHookMessageChannel(); if (isMainThread) { - if ( - typeof module.register === 'function' && - !haveHookFromExperimentalLoader() - ) { - log.trace('bootstrap-import: registering module hook'); - module.register('./hook.mjs', import.meta.url); - } + log.trace('bootstrap-import: registering module hook'); + // XXX module.register('./hook.mjs', import.meta.url); + // TODO: `@opentelemetry/instrumentation/hook.mjs` needs to re-export initialize + // register('@opentelemetry/instrumentation/hook.mjs', import.meta.url, registerOptions); + register('import-in-the-middle/hook.mjs', import.meta.url, registerOptions); await import('./lib/start.js'); + + // Ensure that the loader has acknowledged all the modules before we allow + // execution to continue. + await waitForAllMessagesAcknowledged(); } diff --git a/packages/opentelemetry-node/package-lock.json b/packages/opentelemetry-node/package-lock.json index 7e0771cc..2e5a8b8b 100644 --- a/packages/opentelemetry-node/package-lock.json +++ b/packages/opentelemetry-node/package-lock.json @@ -95,7 +95,7 @@ }, "../mockotlpserver": { "name": "@elastic/mockotlpserver", - "version": "0.6.1", + "version": "0.6.2", "dev": true, "license": "Apache-2.0", "dependencies": { @@ -6397,9 +6397,9 @@ ] }, "node_modules/import-in-the-middle": { - "version": "1.9.0", - "resolved": "https://registry.npmjs.org/import-in-the-middle/-/import-in-the-middle-1.9.0.tgz", - "integrity": "sha512-Ng1SJINJDBzyUEkx9Mj32XD8G0TQCUb5TMoL9V91CTn6F3wYZLygLuhNFrv0cNMBZaeptnL1zecV6XrIdHJ+xQ==", + "version": "1.12.0", + "resolved": "https://registry.npmjs.org/import-in-the-middle/-/import-in-the-middle-1.12.0.tgz", + "integrity": "sha512-yAgSE7GmtRcu4ZUSFX/4v69UGXwugFFSdIQJ14LHPOPPQrWv8Y7O9PHsw8Ovk7bKCLe4sjXMbZFqGFcLHpZ89w==", "dependencies": { "acorn": "^8.8.2", "acorn-import-attributes": "^1.9.5", @@ -13823,9 +13823,9 @@ "dev": true }, "import-in-the-middle": { - "version": "1.9.0", - "resolved": "https://registry.npmjs.org/import-in-the-middle/-/import-in-the-middle-1.9.0.tgz", - "integrity": "sha512-Ng1SJINJDBzyUEkx9Mj32XD8G0TQCUb5TMoL9V91CTn6F3wYZLygLuhNFrv0cNMBZaeptnL1zecV6XrIdHJ+xQ==", + "version": "1.12.0", + "resolved": "https://registry.npmjs.org/import-in-the-middle/-/import-in-the-middle-1.12.0.tgz", + "integrity": "sha512-yAgSE7GmtRcu4ZUSFX/4v69UGXwugFFSdIQJ14LHPOPPQrWv8Y7O9PHsw8Ovk7bKCLe4sjXMbZFqGFcLHpZ89w==", "requires": { "acorn": "^8.8.2", "acorn-import-attributes": "^1.9.5", From 084285dd8fb0c64767e2084ae9f24e2becfe0d79 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Mon, 23 Dec 2024 07:58:23 -0800 Subject: [PATCH 2/2] lint fix --- packages/opentelemetry-node/import.mjs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/opentelemetry-node/import.mjs b/packages/opentelemetry-node/import.mjs index b2073e35..6aa25783 100644 --- a/packages/opentelemetry-node/import.mjs +++ b/packages/opentelemetry-node/import.mjs @@ -20,20 +20,20 @@ // Register ESM hook and start the SDK. // This is called for `--import @elastic/opentelemetry-node`. -import { register } from 'node:module'; +import {register} from 'node:module'; import {isMainThread} from 'node:worker_threads'; // TODO: @opentelemetry/instrumentation should re-export this IITM method. // XXX add explicit IITM dep? // XXX can we have a guard that there is only one install of IITM in play? -import { createAddHookMessageChannel } from 'import-in-the-middle'; +import {createAddHookMessageChannel} from 'import-in-the-middle'; import {log} from './lib/logging.js'; // Note: If there are *multiple* installations of import-in-the-middle, then // only those instrumentations using this same one will be hooked. -const { registerOptions, waitForAllMessagesAcknowledged } = - createAddHookMessageChannel(); +const {registerOptions, waitForAllMessagesAcknowledged} = + createAddHookMessageChannel(); if (isMainThread) { log.trace('bootstrap-import: registering module hook');