From 154b699e3a772aaa2eb205ad2079b91654c1ad29 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Thu, 4 Apr 2024 15:47:25 -0700 Subject: [PATCH 1/8] chore(instr-undici): updates/tweaks for new undici instrumentation - 'codecov' npm script was copypasta from core repo - update to same test-all-versions ver as other pkgs in the workspace - update undici to latest (it is a security update) - add label-prs workflow support pkg:instrumentation-undici --- .github/component-label-map.yml | 4 + package-lock.json | 282 +----------------- plugins/node/instrumentation-undici/README.md | 10 +- .../node/instrumentation-undici/package.json | 7 +- 4 files changed, 24 insertions(+), 279 deletions(-) diff --git a/.github/component-label-map.yml b/.github/component-label-map.yml index de2ff8c3ef..d49f02ac12 100644 --- a/.github/component-label-map.yml +++ b/.github/component-label-map.yml @@ -225,6 +225,10 @@ pkg:instrumentation-tedious: - any-glob-to-any-file: - plugins/node/instrumentation-tedious/** - packages/opentelemetry-test-utils/** +pkg:instrumentation-undici: + - changed-files: + - any-glob-to-any-file: + - plugins/node/instrumentation-undici/** pkg:instrumentation-user-interaction: - changed-files: - any-glob-to-any-file: diff --git a/package-lock.json b/package-lock.json index 9a502dc61d..f86e86670d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -34429,9 +34429,9 @@ } }, "node_modules/undici": { - "version": "6.10.1", - "resolved": "https://registry.npmjs.org/undici/-/undici-6.10.1.tgz", - "integrity": "sha512-kSzmWrOx3XBKTgPm4Tal8Hyl3yf+hzlA00SAf4goxv8LZYafKmS6gJD/7Fe5HH/DMNiFTRXvkwhLo7mUn5fuQQ==", + "version": "6.11.1", + "resolved": "https://registry.npmjs.org/undici/-/undici-6.11.1.tgz", + "integrity": "sha512-KyhzaLJnV1qa3BSHdj4AZ2ndqI0QWPxYzaIOio0WzcEJB9gvuysprJSLtpvc2D9mhR9jPDUk7xlJlZbH2KR5iw==", "dev": true, "engines": { "node": ">=18.0" @@ -37382,7 +37382,7 @@ "license": "Apache-2.0", "dependencies": { "@opentelemetry/core": "^1.8.0", - "@opentelemetry/instrumentation": "^0.49.1" + "@opentelemetry/instrumentation": "^0.50.0" }, "devDependencies": { "@opentelemetry/api": "^1.7.0", @@ -37396,10 +37396,10 @@ "rimraf": "5.0.5", "semver": "^7.6.0", "superagent": "8.0.9", - "test-all-versions": "6.0.0", + "test-all-versions": "6.1.0", "ts-mocha": "10.0.0", "typescript": "4.4.4", - "undici": "6.10.1" + "undici": "6.11.1" }, "engines": { "node": ">=14" @@ -37408,137 +37408,6 @@ "@opentelemetry/api": "^1.7.0" } }, - "plugins/node/instrumentation-undici/node_modules/ansi-styles": { - "version": "4.3.0", - "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-4.3.0.tgz", - "integrity": "sha512-zbB9rCJAT1rbjiVDb2hqKFHNYLxgtk8NURxZ3IZwD3F6NtxbXZQCnnSi1Lkx+IDohdPlFp222wVALIheZJQSEg==", - "dev": true, - "dependencies": { - "color-convert": "^2.0.1" - }, - "engines": { - "node": ">=8" - }, - "funding": { - "url": "https://github.com/chalk/ansi-styles?sponsor=1" - } - }, - "plugins/node/instrumentation-undici/node_modules/chalk": { - "version": "4.1.2", - "resolved": "https://registry.npmjs.org/chalk/-/chalk-4.1.2.tgz", - "integrity": "sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA==", - "dev": true, - "dependencies": { - "ansi-styles": "^4.1.0", - "supports-color": "^7.1.0" - }, - "engines": { - "node": ">=10" - }, - "funding": { - "url": "https://github.com/chalk/chalk?sponsor=1" - } - }, - "plugins/node/instrumentation-undici/node_modules/cli-spinners": { - "version": "2.9.2", - "resolved": "https://registry.npmjs.org/cli-spinners/-/cli-spinners-2.9.2.tgz", - "integrity": "sha512-ywqV+5MmyL4E7ybXgKys4DugZbX0FC6LnwrhjuykIjnK9k8OQacQ7axGKnjDXWNhns0xot3bZI5h55H8yo9cJg==", - "dev": true, - "engines": { - "node": ">=6" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, - "plugins/node/instrumentation-undici/node_modules/color-convert": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/color-convert/-/color-convert-2.0.1.tgz", - "integrity": "sha512-RRECPsj7iu/xb5oKYcsFHSppFNnsj/52OVTRKb4zP5onXwVF3zVmmToNcOfGC+CRDpfK/U584fMg38ZHCaElKQ==", - "dev": true, - "dependencies": { - "color-name": "~1.1.4" - }, - "engines": { - "node": ">=7.0.0" - } - }, - "plugins/node/instrumentation-undici/node_modules/color-name": { - "version": "1.1.4", - "resolved": "https://registry.npmjs.org/color-name/-/color-name-1.1.4.tgz", - "integrity": "sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==", - "dev": true - }, - "plugins/node/instrumentation-undici/node_modules/is-ci": { - "version": "3.0.1", - "resolved": "https://registry.npmjs.org/is-ci/-/is-ci-3.0.1.tgz", - "integrity": "sha512-ZYvCgrefwqoQ6yTyYUbQu64HsITZ3NfKX1lzaEYdkTDcfKzzCI/wthRRYKkdjHKFVgNiXKAKm65Zo1pk2as/QQ==", - "dev": true, - "dependencies": { - "ci-info": "^3.2.0" - }, - "bin": { - "is-ci": "bin.js" - } - }, - "plugins/node/instrumentation-undici/node_modules/log-symbols": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/log-symbols/-/log-symbols-4.1.0.tgz", - "integrity": "sha512-8XPvpAA8uyhfteu8pIvQxpJZ7SYYdpUivZpGy6sFsBuKRY/7rQGavedeB8aK+Zkyq6upMFVL/9AW6vOYzfRyLg==", - "dev": true, - "dependencies": { - "chalk": "^4.1.0", - "is-unicode-supported": "^0.1.0" - }, - "engines": { - "node": ">=10" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, - "plugins/node/instrumentation-undici/node_modules/supports-color": { - "version": "7.2.0", - "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-7.2.0.tgz", - "integrity": "sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw==", - "dev": true, - "dependencies": { - "has-flag": "^4.0.0" - }, - "engines": { - "node": ">=8" - } - }, - "plugins/node/instrumentation-undici/node_modules/test-all-versions": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/test-all-versions/-/test-all-versions-6.0.0.tgz", - "integrity": "sha512-/9wVTBRa7+arvItGinCYy/8+z7sHTsrs9cwEY/xAnzrkSEM7Tp2Cz49ewYZYuO1YYMLqxEaQp2g7Dnns7n7BGA==", - "dev": true, - "dependencies": { - "after-all-results": "^2.0.0", - "ansi-diff-stream": "^1.2.1", - "cli-spinners": "^2.9.2", - "deepmerge": "^4.3.1", - "import-fresh": "^3.3.0", - "is-ci": "^3.0.1", - "js-yaml": "^4.1.0", - "log-symbols": "^4.1.0", - "minimist": "^1.2.8", - "npm-package-versions": "^1.0.1", - "once": "^1.4.0", - "parse-env-string": "^1.0.1", - "resolve": "^1.22.8", - "semver": "^7.5.4", - "spawn-npm-install": "^1.2.0", - "which": "^2.0.2" - }, - "bin": { - "tav": "index.js" - }, - "engines": { - "node": ">=14" - } - }, "plugins/node/opentelemetry-instrumentation-aws-lambda": { "name": "@opentelemetry/instrumentation-aws-lambda", "version": "0.40.0", @@ -38798,39 +38667,6 @@ "@opentelemetry/api": "^1.3.0" } }, - "plugins/node/opentelemetry-instrumentation-undici": { - "name": "@opentelemetry/instrumentation-undici", - "version": "0.33.0", - "extraneous": true, - "license": "Apache-2.0", - "dependencies": { - "@opentelemetry/core": "^1.8.0", - "@opentelemetry/instrumentation": "^0.49.1" - }, - "devDependencies": { - "@opentelemetry/api": "^1.3.0", - "@opentelemetry/sdk-metrics": "^1.8.0", - "@opentelemetry/sdk-trace-base": "^1.8.0", - "@opentelemetry/sdk-trace-node": "^1.8.0", - "@types/mocha": "7.0.2", - "@types/node": "18.6.5", - "mocha": "7.2.0", - "nyc": "15.1.0", - "rimraf": "5.0.5", - "semver": "^7.6.0", - "superagent": "8.0.9", - "test-all-versions": "6.0.0", - "ts-mocha": "10.0.0", - "typescript": "4.4.4", - "undici": "^6.7.0" - }, - "engines": { - "node": ">=14" - }, - "peerDependencies": { - "@opentelemetry/api": "^1.3.0" - } - }, "plugins/node/opentelemetry-instrumentation-winston": { "name": "@opentelemetry/instrumentation-winston", "version": "0.36.0", @@ -47392,7 +47228,7 @@ "requires": { "@opentelemetry/api": "^1.7.0", "@opentelemetry/core": "^1.8.0", - "@opentelemetry/instrumentation": "^0.49.1", + "@opentelemetry/instrumentation": "^0.50.0", "@opentelemetry/sdk-metrics": "^1.8.0", "@opentelemetry/sdk-trace-base": "^1.8.0", "@opentelemetry/sdk-trace-node": "^1.8.0", @@ -47403,104 +47239,10 @@ "rimraf": "5.0.5", "semver": "^7.6.0", "superagent": "8.0.9", - "test-all-versions": "6.0.0", + "test-all-versions": "6.1.0", "ts-mocha": "10.0.0", "typescript": "4.4.4", - "undici": "6.10.1" - }, - "dependencies": { - "ansi-styles": { - "version": "4.3.0", - "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-4.3.0.tgz", - "integrity": "sha512-zbB9rCJAT1rbjiVDb2hqKFHNYLxgtk8NURxZ3IZwD3F6NtxbXZQCnnSi1Lkx+IDohdPlFp222wVALIheZJQSEg==", - "dev": true, - "requires": { - "color-convert": "^2.0.1" - } - }, - "chalk": { - "version": "4.1.2", - "resolved": "https://registry.npmjs.org/chalk/-/chalk-4.1.2.tgz", - "integrity": "sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA==", - "dev": true, - "requires": { - "ansi-styles": "^4.1.0", - "supports-color": "^7.1.0" - } - }, - "cli-spinners": { - "version": "2.9.2", - "resolved": "https://registry.npmjs.org/cli-spinners/-/cli-spinners-2.9.2.tgz", - "integrity": "sha512-ywqV+5MmyL4E7ybXgKys4DugZbX0FC6LnwrhjuykIjnK9k8OQacQ7axGKnjDXWNhns0xot3bZI5h55H8yo9cJg==", - "dev": true - }, - "color-convert": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/color-convert/-/color-convert-2.0.1.tgz", - "integrity": "sha512-RRECPsj7iu/xb5oKYcsFHSppFNnsj/52OVTRKb4zP5onXwVF3zVmmToNcOfGC+CRDpfK/U584fMg38ZHCaElKQ==", - "dev": true, - "requires": { - "color-name": "~1.1.4" - } - }, - "color-name": { - "version": "1.1.4", - "resolved": "https://registry.npmjs.org/color-name/-/color-name-1.1.4.tgz", - "integrity": "sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==", - "dev": true - }, - "is-ci": { - "version": "3.0.1", - "resolved": "https://registry.npmjs.org/is-ci/-/is-ci-3.0.1.tgz", - "integrity": "sha512-ZYvCgrefwqoQ6yTyYUbQu64HsITZ3NfKX1lzaEYdkTDcfKzzCI/wthRRYKkdjHKFVgNiXKAKm65Zo1pk2as/QQ==", - "dev": true, - "requires": { - "ci-info": "^3.2.0" - } - }, - "log-symbols": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/log-symbols/-/log-symbols-4.1.0.tgz", - "integrity": "sha512-8XPvpAA8uyhfteu8pIvQxpJZ7SYYdpUivZpGy6sFsBuKRY/7rQGavedeB8aK+Zkyq6upMFVL/9AW6vOYzfRyLg==", - "dev": true, - "requires": { - "chalk": "^4.1.0", - "is-unicode-supported": "^0.1.0" - } - }, - "supports-color": { - "version": "7.2.0", - "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-7.2.0.tgz", - "integrity": "sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw==", - "dev": true, - "requires": { - "has-flag": "^4.0.0" - } - }, - "test-all-versions": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/test-all-versions/-/test-all-versions-6.0.0.tgz", - "integrity": "sha512-/9wVTBRa7+arvItGinCYy/8+z7sHTsrs9cwEY/xAnzrkSEM7Tp2Cz49ewYZYuO1YYMLqxEaQp2g7Dnns7n7BGA==", - "dev": true, - "requires": { - "after-all-results": "^2.0.0", - "ansi-diff-stream": "^1.2.1", - "cli-spinners": "^2.9.2", - "deepmerge": "^4.3.1", - "import-fresh": "^3.3.0", - "is-ci": "^3.0.1", - "js-yaml": "^4.1.0", - "log-symbols": "^4.1.0", - "minimist": "^1.2.8", - "npm-package-versions": "^1.0.1", - "once": "^1.4.0", - "parse-env-string": "^1.0.1", - "resolve": "^1.22.8", - "semver": "^7.5.4", - "spawn-npm-install": "^1.2.0", - "which": "^2.0.2" - } - } + "undici": "6.11.1" } }, "@opentelemetry/instrumentation-user-interaction": { @@ -68501,9 +68243,9 @@ } }, "undici": { - "version": "6.10.1", - "resolved": "https://registry.npmjs.org/undici/-/undici-6.10.1.tgz", - "integrity": "sha512-kSzmWrOx3XBKTgPm4Tal8Hyl3yf+hzlA00SAf4goxv8LZYafKmS6gJD/7Fe5HH/DMNiFTRXvkwhLo7mUn5fuQQ==", + "version": "6.11.1", + "resolved": "https://registry.npmjs.org/undici/-/undici-6.11.1.tgz", + "integrity": "sha512-KyhzaLJnV1qa3BSHdj4AZ2ndqI0QWPxYzaIOio0WzcEJB9gvuysprJSLtpvc2D9mhR9jPDUk7xlJlZbH2KR5iw==", "dev": true }, "undici-types": { diff --git a/plugins/node/instrumentation-undici/README.md b/plugins/node/instrumentation-undici/README.md index 38b6fd6e9e..7ab919047a 100644 --- a/plugins/node/instrumentation-undici/README.md +++ b/plugins/node/instrumentation-undici/README.md @@ -46,11 +46,11 @@ Undici instrumentation has few options available to choose from. You can set the | Options | Type | Description | | ------- | ---- | ----------- | -| [`ignoreRequestHook`](https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-undici/src/types.ts#63) | `IgnoreRequestFunction` | Undici instrumentation will not trace all incoming requests that matched with custom function. | -| [`requestHook`](https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-undici/src/types.ts#65) | `RequestHookFunction` | Function for adding custom attributes before request is handled. | -| [`startSpanHook`](https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-undici/src/types.ts#67) | `StartSpanHookFunction` | Function for adding custom attributes before a span is started. | -| [`requireParentforSpans`](https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-undici/src/types.ts#69) | `Boolean` | Require a parent span is present to create new span for outgoing requests. | -| [`headersToSpanAttributes`](https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-undici/src/types.ts#71) | `Object` | List of case insensitive HTTP headers to convert to span attributes. Headers will be converted to span attributes in the form of `http.{request\|response}.header.header-name` where the name is only lowercased, e.g. `http.response.header.content-length`| +| [`ignoreRequestHook`](./src/types.ts#63) | `IgnoreRequestFunction` | Undici instrumentation will not trace all incoming requests that matched with custom function. | +| [`requestHook`](./src/types.ts#65) | `RequestHookFunction` | Function for adding custom attributes before request is handled. | +| [`startSpanHook`](./src/types.ts#67) | `StartSpanHookFunction` | Function for adding custom attributes before a span is started. | +| [`requireParentforSpans`](./src/types.ts#69) | `Boolean` | Require a parent span is present to create new span for outgoing requests. | +| [`headersToSpanAttributes`](./src/types.ts#71) | `Object` | List of case insensitive HTTP headers to convert to span attributes. Headers will be converted to span attributes in the form of `http.{request\|response}.header.header-name` where the name is only lowercased, e.g. `http.response.header.content-length`| ### Observations diff --git a/plugins/node/instrumentation-undici/package.json b/plugins/node/instrumentation-undici/package.json index b235be769a..fd21b60b47 100644 --- a/plugins/node/instrumentation-undici/package.json +++ b/plugins/node/instrumentation-undici/package.json @@ -1,7 +1,7 @@ { "name": "@opentelemetry/instrumentation-undici", "version": "0.1.0", - "description": "OpenTelemetry undici/fetch automatic instrumentation package.", + "description": "OpenTelemetry instrumentation for undici and Node.js fetch().", "main": "build/src/index.js", "types": "build/src/index.d.ts", "repository": "open-telemetry/opentelemetry-js-contrib", @@ -14,7 +14,6 @@ "clean": "rimraf build/*", "lint": "eslint . --ext .ts", "lint:fix": "eslint . --ext .ts --fix", - "codecov": "nyc report --reporter=json && codecov -f coverage/*.json -p ../../../", "watch": "tsc -w", "precompile": "tsc --version && lerna run version:update --scope @opentelemetry/instrumentation-undici --include-dependencies", "prewatch": "npm run precompile", @@ -53,10 +52,10 @@ "rimraf": "5.0.5", "semver": "^7.6.0", "superagent": "8.0.9", - "test-all-versions": "6.0.0", + "test-all-versions": "6.1.0", "ts-mocha": "10.0.0", "typescript": "4.4.4", - "undici": "6.10.1" + "undici": "6.11.1" }, "peerDependencies": { "@opentelemetry/api": "^1.7.0" From 7d06b0ef687071940b3fb82a4ac3dabcf5baff86 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Thu, 4 Apr 2024 15:55:25 -0700 Subject: [PATCH 2/8] use tav 6.1.0 facility to reduce the number of test-all-versions ver tested --- plugins/node/instrumentation-undici/.tav.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/plugins/node/instrumentation-undici/.tav.yml b/plugins/node/instrumentation-undici/.tav.yml index 9ef69e2462..11064c2591 100644 --- a/plugins/node/instrumentation-undici/.tav.yml +++ b/plugins/node/instrumentation-undici/.tav.yml @@ -1,8 +1,12 @@ undici: jobs: - - versions: ">=5 <6" + - versions: + include: ">=5 <6" + mode: max-7 node: '>=14' commands: npm run test - - versions: ">=6 <7" + - versions: + include: ">=6 <7" + mode: max-7 node: '>=18' commands: npm run test From b42ad9fdab8a20e40914dfff8c1bfb21850e6f2e Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Thu, 4 Apr 2024 16:43:15 -0700 Subject: [PATCH 3/8] skip this test case on old undici (< 5.12.0) --- .../node/instrumentation-undici/test/undici.test.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/plugins/node/instrumentation-undici/test/undici.test.ts b/plugins/node/instrumentation-undici/test/undici.test.ts index fd48803a3f..d1a0914332 100644 --- a/plugins/node/instrumentation-undici/test/undici.test.ts +++ b/plugins/node/instrumentation-undici/test/undici.test.ts @@ -29,6 +29,7 @@ import { SimpleSpanProcessor, } from '@opentelemetry/sdk-trace-base'; import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node'; +import * as semver from 'semver'; import { UndiciInstrumentation } from '../src/undici'; @@ -40,6 +41,8 @@ import type { fetch, stream, request, Client, Dispatcher } from 'undici'; type PromisedValue = T extends Promise ? R : never; +const UNDICI_VERSION = require('undici/package.json').version; + const instrumentation = new UndiciInstrumentation(); instrumentation.enable(); instrumentation.disable(); @@ -207,7 +210,13 @@ describe('UndiciInstrumentation `undici` tests', function () { assert.ok(spans.length === 0, 'ignoreRequestHook is filtering requests'); }); - it('should create valid spans for different request methods', async function () { + it.only('should create valid spans for different request methods', async function () { + if (semver.lt(UNDICI_VERSION, '5.12.0')) { + // undici versions <5.12.0 fail on a request with a weird request + // method in a way that is not worth debugging just for this test case. + this.skip(); + } + let spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 0); From 2c3ce0ce384d14b0a20900643efd2a140ad99802 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Fri, 5 Apr 2024 11:45:43 -0700 Subject: [PATCH 4/8] option 1: min-supported undici version in 5.12.0 We could support earlier versions of 5.x, but there is little point because those releases are so old. --- plugins/node/instrumentation-undici/.tav.yml | 2 +- plugins/node/instrumentation-undici/README.md | 5 ++ .../test/undici.test.ts | 53 ++++++------------- 3 files changed, 21 insertions(+), 39 deletions(-) diff --git a/plugins/node/instrumentation-undici/.tav.yml b/plugins/node/instrumentation-undici/.tav.yml index 11064c2591..88b92a7898 100644 --- a/plugins/node/instrumentation-undici/.tav.yml +++ b/plugins/node/instrumentation-undici/.tav.yml @@ -1,7 +1,7 @@ undici: jobs: - versions: - include: ">=5 <6" + include: ">=5.12.0 <6" mode: max-7 node: '>=14' commands: npm run test diff --git a/plugins/node/instrumentation-undici/README.md b/plugins/node/instrumentation-undici/README.md index 7ab919047a..0f92995347 100644 --- a/plugins/node/instrumentation-undici/README.md +++ b/plugins/node/instrumentation-undici/README.md @@ -14,6 +14,11 @@ If you're looking the instrumentation for browser's `fetch` API it is located at npm install --save @opentelemetry/instrumentation-undici ``` +## Supported Versions + +- `undici@>=5.12.0` + + ## Usage OpenTelemetry Undici/fetch Instrumentation allows the user to automatically collect trace data and export them to their backend of choice, to give observability to distributed systems. diff --git a/plugins/node/instrumentation-undici/test/undici.test.ts b/plugins/node/instrumentation-undici/test/undici.test.ts index d1a0914332..e0c5bfb142 100644 --- a/plugins/node/instrumentation-undici/test/undici.test.ts +++ b/plugins/node/instrumentation-undici/test/undici.test.ts @@ -210,13 +210,7 @@ describe('UndiciInstrumentation `undici` tests', function () { assert.ok(spans.length === 0, 'ignoreRequestHook is filtering requests'); }); - it.only('should create valid spans for different request methods', async function () { - if (semver.lt(UNDICI_VERSION, '5.12.0')) { - // undici versions <5.12.0 fail on a request with a weird request - // method in a way that is not worth debugging just for this test case. - this.skip(); - } - + it('should create valid spans for different request methods', async function () { let spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 0); @@ -226,37 +220,20 @@ describe('UndiciInstrumentation `undici` tests', function () { 'foo-client': 'bar', }; - // In version v5 if `undici` you get the following error when requesting with a method - // that is not one of the known ones in uppercase. Using - // - // SocketError: other side closed - // at Socket.onSocketEnd (node_modules/undici/lib/client.js:1118:22) - // at endReadableNT (internal/streams/readable.js:1333:12) - // at processTicksAndRejections (internal/process/task_queues.js:82:21) - let firstQueryResponse: PromisedValue>; - let secondQueryResponse: PromisedValue>; - try { - const queryRequestUrl = `${protocol}://${hostname}:${mockServer.port}/?query=test`; - firstQueryResponse = await undici.request(queryRequestUrl, { - headers, - // @ts-expect-error - method type expects in uppercase - method: 'get', - }); - await consumeResponseBody(firstQueryResponse.body); - - secondQueryResponse = await undici.request(queryRequestUrl, { - headers, - // @ts-expect-error - method type expects known HTTP method (GET, POST, PUT, ...) - method: 'custom', - }); - await consumeResponseBody(secondQueryResponse.body); - } catch (undiciErr) { - const { stack } = undiciErr as Error; - - if (stack?.startsWith('SocketError: other side closed')) { - this.skip(); - } - } + const queryRequestUrl = `${protocol}://${hostname}:${mockServer.port}/?query=test`; + const firstQueryResponse = await undici.request(queryRequestUrl, { + headers, + // @ts-expect-error - method type expects in uppercase + method: 'get', + }); + await consumeResponseBody(firstQueryResponse.body); + + const secondQueryResponse = await undici.request(queryRequestUrl, { + headers, + // @ts-expect-error - method type expects known HTTP method (GET, POST, PUT, ...) + method: 'custom', + }); + await consumeResponseBody(secondQueryResponse.body); assert.ok( firstQueryResponse!.headers['propagation-error'] === undefined, From 50b09660286cc9836cc89f0885aeda1d493439c9 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Fri, 5 Apr 2024 12:22:10 -0700 Subject: [PATCH 5/8] fix for undici@6.11.0 breakage, adapted from @david-luna patch in comments --- plugins/node/instrumentation-undici/src/undici.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/plugins/node/instrumentation-undici/src/undici.ts b/plugins/node/instrumentation-undici/src/undici.ts index 262bea42cd..0fa07f97a3 100644 --- a/plugins/node/instrumentation-undici/src/undici.ts +++ b/plugins/node/instrumentation-undici/src/undici.ts @@ -270,10 +270,13 @@ export class UndiciInstrumentation extends InstrumentationBase { for (let i = 0; i < headerEntries.length; i++) { const [k, v] = headerEntries[i]; - if (typeof request.headers === 'string') { - request.headers += `${k}: ${v}\r\n`; - } else { + if (typeof request.addHeader === 'function') { request.addHeader(k, v); + } else if (typeof request.headers === 'string') { + request.headers += `${k}: ${v}\r\n`; + } else if (Array.isArray(request.headers)) { + // undici@6.11.0 accidentally, briefly removed `request.addHeader()`. + request.headers.push(k, v); } } this._recordFromReq.set(request, { span, attributes, startTime }); From 34b1f737aa491312d05bc0174c6f90df8b4829ae Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Fri, 5 Apr 2024 12:27:15 -0700 Subject: [PATCH 6/8] fix markdown lint --- plugins/node/instrumentation-undici/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/node/instrumentation-undici/README.md b/plugins/node/instrumentation-undici/README.md index 0f92995347..cb5cd74772 100644 --- a/plugins/node/instrumentation-undici/README.md +++ b/plugins/node/instrumentation-undici/README.md @@ -18,7 +18,6 @@ npm install --save @opentelemetry/instrumentation-undici - `undici@>=5.12.0` - ## Usage OpenTelemetry Undici/fetch Instrumentation allows the user to automatically collect trace data and export them to their backend of choice, to give observability to distributed systems. From 54b11cb4e1e086d0d26d1f7b93cc08f6148f3801 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Fri, 5 Apr 2024 13:39:02 -0700 Subject: [PATCH 7/8] make ts compile happy --- plugins/node/instrumentation-undici/test/undici.test.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/plugins/node/instrumentation-undici/test/undici.test.ts b/plugins/node/instrumentation-undici/test/undici.test.ts index e0c5bfb142..aea2f036ce 100644 --- a/plugins/node/instrumentation-undici/test/undici.test.ts +++ b/plugins/node/instrumentation-undici/test/undici.test.ts @@ -29,7 +29,6 @@ import { SimpleSpanProcessor, } from '@opentelemetry/sdk-trace-base'; import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node'; -import * as semver from 'semver'; import { UndiciInstrumentation } from '../src/undici'; @@ -39,10 +38,6 @@ import { assertSpan } from './utils/assertSpan'; import type { fetch, stream, request, Client, Dispatcher } from 'undici'; -type PromisedValue = T extends Promise ? R : never; - -const UNDICI_VERSION = require('undici/package.json').version; - const instrumentation = new UndiciInstrumentation(); instrumentation.enable(); instrumentation.disable(); From 705b83d0201fd001a81d0c4409eba4d7880a69e4 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Wed, 10 Apr 2024 08:45:57 -0700 Subject: [PATCH 8/8] revert this part of the change, because the relative links don't work for the README hosted on npmjs.com --- plugins/node/instrumentation-undici/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/node/instrumentation-undici/README.md b/plugins/node/instrumentation-undici/README.md index cb5cd74772..08a7128404 100644 --- a/plugins/node/instrumentation-undici/README.md +++ b/plugins/node/instrumentation-undici/README.md @@ -50,11 +50,11 @@ Undici instrumentation has few options available to choose from. You can set the | Options | Type | Description | | ------- | ---- | ----------- | -| [`ignoreRequestHook`](./src/types.ts#63) | `IgnoreRequestFunction` | Undici instrumentation will not trace all incoming requests that matched with custom function. | -| [`requestHook`](./src/types.ts#65) | `RequestHookFunction` | Function for adding custom attributes before request is handled. | -| [`startSpanHook`](./src/types.ts#67) | `StartSpanHookFunction` | Function for adding custom attributes before a span is started. | -| [`requireParentforSpans`](./src/types.ts#69) | `Boolean` | Require a parent span is present to create new span for outgoing requests. | -| [`headersToSpanAttributes`](./src/types.ts#71) | `Object` | List of case insensitive HTTP headers to convert to span attributes. Headers will be converted to span attributes in the form of `http.{request\|response}.header.header-name` where the name is only lowercased, e.g. `http.response.header.content-length`| +| [`ignoreRequestHook`](https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-undici/src/types.ts#63) | `IgnoreRequestFunction` | Undici instrumentation will not trace all incoming requests that matched with custom function. | +| [`requestHook`](https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-undici/src/types.ts#65) | `RequestHookFunction` | Function for adding custom attributes before request is handled. | +| [`startSpanHook`](https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-undici/src/types.ts#67) | `StartSpanHookFunction` | Function for adding custom attributes before a span is started. | +| [`requireParentforSpans`](https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-undici/src/types.ts#69) | `Boolean` | Require a parent span is present to create new span for outgoing requests. | +| [`headersToSpanAttributes`](https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-undici/src/types.ts#71) | `Object` | List of case insensitive HTTP headers to convert to span attributes. Headers will be converted to span attributes in the form of `http.{request\|response}.header.header-name` where the name is only lowercased, e.g. `http.response.header.content-length`| ### Observations