From 137c78ca14f55004d68660c5832dc5e8e91a2a1d Mon Sep 17 00:00:00 2001 From: David Luna Date: Mon, 30 Sep 2024 18:16:16 +0200 Subject: [PATCH 1/5] fix(instr-undici): avoid use of deprecated APIs for diagnostic channels --- .../src/internal-types.ts | 13 ++++++-- .../node/instrumentation-undici/src/undici.ts | 32 ++++++++++++++++--- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/plugins/node/instrumentation-undici/src/internal-types.ts b/plugins/node/instrumentation-undici/src/internal-types.ts index fee0e7294a..a6d7604375 100644 --- a/plugins/node/instrumentation-undici/src/internal-types.ts +++ b/plugins/node/instrumentation-undici/src/internal-types.ts @@ -13,14 +13,21 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import type { Channel } from 'diagnostics_channel'; + +import type { Channel, ChannelListener } from 'diagnostics_channel'; import { UndiciRequest, UndiciResponse } from './types'; +export interface DiagnosticChannelApi { + subscribe?: (channel: string, listener: ChannelListener) => void; + unsubscribe?: (channel: string, listener: ChannelListener) => void; + channel: (name: string) => Channel; +} + export interface ListenerRecord { name: string; - channel: Channel; - onMessage: (message: any, name: string) => void; + onMessage: (message: any, name: string | symbol) => void; + unsubscribe: () => void; } export interface RequestMessage { diff --git a/plugins/node/instrumentation-undici/src/undici.ts b/plugins/node/instrumentation-undici/src/undici.ts index 0d8ebedc73..04e341f9b0 100644 --- a/plugins/node/instrumentation-undici/src/undici.ts +++ b/plugins/node/instrumentation-undici/src/undici.ts @@ -37,6 +37,7 @@ import { import { PACKAGE_NAME, PACKAGE_VERSION } from './version'; import { + DiagnosticChannelApi, ListenerRecord, RequestHeadersMessage, RequestMessage, @@ -77,7 +78,7 @@ export class UndiciInstrumentation extends InstrumentationBase sub.channel.unsubscribe(sub.onMessage)); + this._channelSubs.forEach(sub => sub.unsubscribe()); this._channelSubs.length = 0; } @@ -139,12 +140,35 @@ export class UndiciInstrumentation extends InstrumentationBase Number(n)); + const isCompatible = major > 18 || (major === 18 && minor >= 19); + + let unsubscribe: () => void; + if (typeof diagchApi.subscribe === 'function' && isCompatible) { + diagchApi.subscribe(diagnosticChannel, onMessage); + unsubscribe = () => diagchApi.unsubscribe?.(diagnosticChannel, onMessage); + } else { + const channel = diagchApi.channel(diagnosticChannel); + channel.subscribe(onMessage); + unsubscribe = () => channel.unsubscribe(onMessage); + } + this._channelSubs.push({ name: diagnosticChannel, - channel, onMessage, + unsubscribe, }); } From 765dbc982b1745cbf79344beee15027152ffa4b7 Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Oct 2024 11:42:39 +0200 Subject: [PATCH 2/5] chore(instr-undici): add suggestions --- .../node/instrumentation-undici/package.json | 2 +- .../src/internal-types.ts | 9 ------- .../node/instrumentation-undici/src/undici.ts | 25 ++++++------------- 3 files changed, 9 insertions(+), 27 deletions(-) diff --git a/plugins/node/instrumentation-undici/package.json b/plugins/node/instrumentation-undici/package.json index c6200cacb4..7092cb0d62 100644 --- a/plugins/node/instrumentation-undici/package.json +++ b/plugins/node/instrumentation-undici/package.json @@ -46,7 +46,7 @@ "@opentelemetry/sdk-trace-base": "^1.8.0", "@opentelemetry/sdk-trace-node": "^1.8.0", "@types/mocha": "7.0.2", - "@types/node": "18.6.5", + "@types/node": "18.18", "mocha": "7.2.0", "nyc": "15.1.0", "rimraf": "5.0.10", diff --git a/plugins/node/instrumentation-undici/src/internal-types.ts b/plugins/node/instrumentation-undici/src/internal-types.ts index a6d7604375..e6b2fff535 100644 --- a/plugins/node/instrumentation-undici/src/internal-types.ts +++ b/plugins/node/instrumentation-undici/src/internal-types.ts @@ -14,19 +14,10 @@ * limitations under the License. */ -import type { Channel, ChannelListener } from 'diagnostics_channel'; - import { UndiciRequest, UndiciResponse } from './types'; -export interface DiagnosticChannelApi { - subscribe?: (channel: string, listener: ChannelListener) => void; - unsubscribe?: (channel: string, listener: ChannelListener) => void; - channel: (name: string) => Channel; -} - export interface ListenerRecord { name: string; - onMessage: (message: any, name: string | symbol) => void; unsubscribe: () => void; } diff --git a/plugins/node/instrumentation-undici/src/undici.ts b/plugins/node/instrumentation-undici/src/undici.ts index 04e341f9b0..ba7e088a35 100644 --- a/plugins/node/instrumentation-undici/src/undici.ts +++ b/plugins/node/instrumentation-undici/src/undici.ts @@ -37,7 +37,6 @@ import { import { PACKAGE_NAME, PACKAGE_VERSION } from './version'; import { - DiagnosticChannelApi, ListenerRecord, RequestHeadersMessage, RequestMessage, @@ -138,36 +137,28 @@ export class UndiciInstrumentation extends InstrumentationBase void, ) { - // NOTE: later versions of `@types/node` include proper type definitions - // for diagnostics channel. However updating it breaks compilation :( - // Hence we have a helper interface to deal with it - const diagchApi = diagch as unknown as DiagnosticChannelApi; - - // NOTE: diagnostic channel had an issue tht was solved in v18.19.0 so - // instrumentation is going to use the new API when the process version is - // greater or equal - // ref: https://github.com/nodejs/node/pull/47520 + // `diagnostics_channel` had a ref counting bug until v18.19.0. + // https://github.com/nodejs/node/pull/47520 const [major, minor] = process.version .replace('v', '') .split('.') .map(n => Number(n)); - const isCompatible = major > 18 || (major === 18 && minor >= 19); + const useNewSubscribe = major > 18 || (major === 18 && minor >= 19); let unsubscribe: () => void; - if (typeof diagchApi.subscribe === 'function' && isCompatible) { - diagchApi.subscribe(diagnosticChannel, onMessage); - unsubscribe = () => diagchApi.unsubscribe?.(diagnosticChannel, onMessage); + if (useNewSubscribe) { + diagch.subscribe?.(diagnosticChannel, onMessage); + unsubscribe = () => diagch.unsubscribe?.(diagnosticChannel, onMessage); } else { - const channel = diagchApi.channel(diagnosticChannel); + const channel = diagch.channel(diagnosticChannel); channel.subscribe(onMessage); unsubscribe = () => channel.unsubscribe(onMessage); } this._channelSubs.push({ name: diagnosticChannel, - onMessage, unsubscribe, }); } From dca4b02508f7a2f4916b78b16f8326be0f46a079 Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Oct 2024 11:51:59 +0200 Subject: [PATCH 3/5] chore: update package lock file --- package-lock.json | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 211cfa0f3c..f19fc99418 100644 --- a/package-lock.json +++ b/package-lock.json @@ -38475,7 +38475,7 @@ "@opentelemetry/sdk-trace-base": "^1.8.0", "@opentelemetry/sdk-trace-node": "^1.8.0", "@types/mocha": "7.0.2", - "@types/node": "18.6.5", + "@types/node": "18.18", "mocha": "7.2.0", "nyc": "15.1.0", "rimraf": "5.0.10", @@ -38492,6 +38492,15 @@ "@opentelemetry/api": "^1.7.0" } }, + "plugins/node/instrumentation-undici/node_modules/@types/node": { + "version": "18.18.14", + "resolved": "https://registry.npmjs.org/@types/node/-/node-18.18.14.tgz", + "integrity": "sha512-iSOeNeXYNYNLLOMDSVPvIFojclvMZ/HDY2dU17kUlcsOsSQETbWIslJbYLZgA+ox8g2XQwSHKTkght1a5X26lQ==", + "dev": true, + "dependencies": { + "undici-types": "~5.26.4" + } + }, "plugins/node/opentelemetry-instrumentation-aws-lambda": { "name": "@opentelemetry/instrumentation-aws-lambda", "version": "0.44.0", @@ -52346,7 +52355,7 @@ "@opentelemetry/sdk-trace-base": "^1.8.0", "@opentelemetry/sdk-trace-node": "^1.8.0", "@types/mocha": "7.0.2", - "@types/node": "18.6.5", + "@types/node": "18.18", "mocha": "7.2.0", "nyc": "15.1.0", "rimraf": "5.0.10", @@ -52355,6 +52364,17 @@ "ts-mocha": "10.0.0", "typescript": "4.4.4", "undici": "6.11.1" + }, + "dependencies": { + "@types/node": { + "version": "18.18.14", + "resolved": "https://registry.npmjs.org/@types/node/-/node-18.18.14.tgz", + "integrity": "sha512-iSOeNeXYNYNLLOMDSVPvIFojclvMZ/HDY2dU17kUlcsOsSQETbWIslJbYLZgA+ox8g2XQwSHKTkght1a5X26lQ==", + "dev": true, + "requires": { + "undici-types": "~5.26.4" + } + } } }, "@opentelemetry/instrumentation-user-interaction": { From 81c32bda4da2673a4ad105fc69b0eb4901990763 Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Oct 2024 12:16:26 +0200 Subject: [PATCH 4/5] chore: pin @types/node version --- package-lock.json | 4 ++-- plugins/node/instrumentation-undici/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index f19fc99418..21c8c38db5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -38475,7 +38475,7 @@ "@opentelemetry/sdk-trace-base": "^1.8.0", "@opentelemetry/sdk-trace-node": "^1.8.0", "@types/mocha": "7.0.2", - "@types/node": "18.18", + "@types/node": "18.18.14", "mocha": "7.2.0", "nyc": "15.1.0", "rimraf": "5.0.10", @@ -52355,7 +52355,7 @@ "@opentelemetry/sdk-trace-base": "^1.8.0", "@opentelemetry/sdk-trace-node": "^1.8.0", "@types/mocha": "7.0.2", - "@types/node": "18.18", + "@types/node": "18.18.14", "mocha": "7.2.0", "nyc": "15.1.0", "rimraf": "5.0.10", diff --git a/plugins/node/instrumentation-undici/package.json b/plugins/node/instrumentation-undici/package.json index 7092cb0d62..18fcb7ad20 100644 --- a/plugins/node/instrumentation-undici/package.json +++ b/plugins/node/instrumentation-undici/package.json @@ -46,7 +46,7 @@ "@opentelemetry/sdk-trace-base": "^1.8.0", "@opentelemetry/sdk-trace-node": "^1.8.0", "@types/mocha": "7.0.2", - "@types/node": "18.18", + "@types/node": "18.18.14", "mocha": "7.2.0", "nyc": "15.1.0", "rimraf": "5.0.10", From 668e424acfbdeb52166b8cbadd7d6417b8ec3f00 Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Oct 2024 13:09:04 +0200 Subject: [PATCH 5/5] chore: fix lint issues --- plugins/node/instrumentation-undici/src/undici.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/node/instrumentation-undici/src/undici.ts b/plugins/node/instrumentation-undici/src/undici.ts index ba7e088a35..1755b7417b 100644 --- a/plugins/node/instrumentation-undici/src/undici.ts +++ b/plugins/node/instrumentation-undici/src/undici.ts @@ -137,7 +137,7 @@ export class UndiciInstrumentation extends InstrumentationBase void, + onMessage: (message: any, name: string | symbol) => void ) { // `diagnostics_channel` had a ref counting bug until v18.19.0. // https://github.com/nodejs/node/pull/47520 @@ -145,7 +145,7 @@ export class UndiciInstrumentation extends InstrumentationBase Number(n)); - const useNewSubscribe = major > 18 || (major === 18 && minor >= 19); + const useNewSubscribe = major > 18 || (major === 18 && minor >= 19); let unsubscribe: () => void; if (useNewSubscribe) {