From 762a65a6e4760f3da758fb766cbfd6069c6b92bd Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 29 Nov 2023 16:12:03 -0500 Subject: [PATCH 1/4] fix(instrumentation-http): resume responses when there is no response listener Fixes a memory leak where unhandled response bodies pile up in node 20 --- experimental/CHANGELOG.md | 1 + .../packages/opentelemetry-instrumentation-http/src/http.ts | 3 +++ 2 files changed, 4 insertions(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index b9935e26c58..90dedfcf6fc 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -13,6 +13,7 @@ All notable changes to experimental packages in this project will be documented * fix(sdk-logs): avoid map attribute set when count limit exceeded * fix(instrumentation-fetch): only access navigator if it is defined [#4063](https://github.com/open-telemetry/opentelemetry-js/pull/4063) * allows for experimental usage of this instrumentation with non-browser runtimes +* fix(instrumentation-http): memory leak when responses are not resumed ### :books: (Refine Doc) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index 9422bbc9efd..a76f1a918da 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -330,6 +330,9 @@ export class HttpInstrumentation extends InstrumentationBase { 'response', (response: http.IncomingMessage & { aborted?: boolean }) => { this._diag.debug('outgoingRequest on response()'); + if (request.listenerCount('request') <= 0) { + response.resume(); + } const responseAttributes = utils.getOutgoingRequestAttributesOnResponse(response); span.setAttributes(responseAttributes); From f6546365029a8f700a0096af396914d6b703c824 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 30 Nov 2023 08:50:29 +0100 Subject: [PATCH 2/4] feat: add script to update changelogs on release preparation (#4315) * feat: add script to update changelogs on releases * fix: address comments * Apply suggestions from code review Co-authored-by: Trent Mick * fix: apply suggestions from code review * fix: use packageJson.version instead of version --------- Co-authored-by: Trent Mick --- CHANGELOG.md | 12 ++----- experimental/CHANGELOG.md | 14 -------- package.json | 12 ++++--- scripts/update-changelog.js | 65 +++++++++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 28 deletions(-) create mode 100644 scripts/update-changelog.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 336e211d6f1..ebc5731da61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -365,16 +365,8 @@ There are no changes between 1.0.0 and the previous 0.33.0 version. * fix(sdk-web): parse url with relative url string [#2972](https://github.com/open-telemetry/opentelemetry-js/pull/2972) @legendecas -### :books: (Refine Doc) - -### :house: (Internal) - ## 1.2.0 -### :boom: Breaking Change - -### :rocket: (Enhancement) - ### :bug: (Bug Fix) * fix: sanitize attributes inputs [#2881](https://github.com/open-telemetry/opentelemetry-js/pull/2881) @legendecas @@ -2290,7 +2282,9 @@ Released 2020-03-19 Released 2020-03-16 -### This is a first official beta release, which provides almost fully complete metrics, tracing, and context propagation functionality but makes no promises around breaking changes +### First official beta release + +* provides almost fully complete metrics, tracing, and context propagation functionality but makes **no promises** around breaking changes ### :boom: Breaking Change diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index b9935e26c58..52fb1a7ab2b 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -14,10 +14,6 @@ All notable changes to experimental packages in this project will be documented * fix(instrumentation-fetch): only access navigator if it is defined [#4063](https://github.com/open-telemetry/opentelemetry-js/pull/4063) * allows for experimental usage of this instrumentation with non-browser runtimes -### :books: (Refine Doc) - -### :house: (Internal) - ## 0.45.1 ### :bug: (Bug Fix) @@ -183,8 +179,6 @@ All notable changes to experimental packages in this project will be documented * doc(instrumentation): add limitiations section to readme [#3786](https://github.com/open-telemetry/opentelemetry-js/pull/3786) @flarna -### :house: (Internal) - ## 0.38.0 ### :boom: Breaking Change @@ -401,10 +395,6 @@ All notable changes to experimental packages in this project will be documented * fix(histogram): fix maximum when only values < -1 are provided [#3086](https://github.com/open-telemetry/opentelemetry-js/pull/3086) @pichlermarc * fix(instrumentation-grpc): always set grpc semcov status code attribute with numeric value [#3076](https://github.com/open-telemetry/opentelemetry-js/pull/3076) @blumamir -### :books: (Refine Doc) - -### :house: (Internal) - ## 0.30.0 ### :boom: Breaking Change @@ -479,10 +469,6 @@ All notable changes to experimental packages in this project will be documented * fix(metrics): specification compliant default metric unit [#2983](https://github.com/open-telemetry/opentelemetry-js/pull/2983) @andyfleming * fix(opentelemetry-instrumentation): use all provided patches for the same file [#2963](https://github.com/open-telemetry/opentelemetry-js/pull/2963) @Ugzuzg -### :books: (Refine Doc) - -### :house: (Internal) - ## 0.28.0 ### :boom: Breaking Change diff --git a/package.json b/package.json index 4ea5fa489aa..a2d38285839 100644 --- a/package.json +++ b/package.json @@ -37,11 +37,11 @@ "comment_prepare_1": "echo scripts in this section automatically prepare releases. Intended for use by maintainers only.", "comment_prepare_2": "echo experimental preparation scripts only prepare experimental packages", - "prepare_release:experimental:patch": "npm run _check:no_changes && npm run _backup:package-json && npm run _lerna:remove_api && npm run _lerna:remove_stable && npm run _lerna:version_patch && npm run _restore:package-json", - "prepare_release:experimental:minor": "npm run _check:no_changes && npm run _backup:package-json && npm run _lerna:remove_api && npm run _lerna:remove_stable && npm run _lerna:version_minor && npm run _restore:package-json", + "prepare_release:experimental:patch": "npm run _check:no_changes && npm run _backup:package-json && npm run _lerna:remove_api && npm run _lerna:remove_stable && npm run _lerna:version_patch && npm run _restore:package-json && npm run _changelog:prepare_experimental", + "prepare_release:experimental:minor": "npm run _check:no_changes && npm run _backup:package-json && npm run _lerna:remove_api && npm run _lerna:remove_stable && npm run _lerna:version_minor && npm run _restore:package-json && npm run _changelog:prepare_experimental", "comment_prepare_3": "echo sdk preparation scripts prepare all stable and experimental packages", - "prepare_release:sdk:patch": "npm run _check:no_changes && npm run _backup:package-json && npm run _lerna:remove_api && npm run _lerna:version_patch && npm run _restore:package-json", - "prepare_release:sdk:minor": "npm run _check:no_changes && npm run _backup:package-json && npm run _lerna:remove_api && npm run _lerna:version_minor && npm run _restore:package-json", + "prepare_release:sdk:patch": "npm run _check:no_changes && npm run _backup:package-json && npm run _lerna:remove_api && npm run _lerna:version_patch && npm run _restore:package-json && npm run _changelog:prepare_experimental && npm run _changelog:prepare_stable", + "prepare_release:sdk:minor": "npm run _check:no_changes && npm run _backup:package-json && npm run _lerna:remove_api && npm run _lerna:version_minor && npm run _restore:package-json && npm run _changelog:prepare_experimental && npm run _changelog:prepare_stable", "release:publish": "lerna publish from-package --no-push --no-private --no-git-tag-version --no-verify-access", "comment_internal": "echo scripts below this line are for internal use", @@ -51,7 +51,9 @@ "_lerna:remove_api": "node -e 'var fs=require(\"fs\");var p=require(\"./package.json\");p.workspaces=p.workspaces.filter(p=>p!==\"api\");fs.writeFileSync(\"package.json\",JSON.stringify(p,null,2))'", "_lerna:remove_stable": "node -e 'var fs=require(\"fs\");var p=require(\"./package.json\");p.workspaces=p.workspaces.filter(p=>p!==\"packages/*\");fs.writeFileSync(\"package.json\",JSON.stringify(p,null,2))'", "_lerna:version_patch": "npx lerna version patch --exact --no-git-tag-version --no-push --yes", - "_lerna:version_minor": "npx lerna version minor --exact --no-git-tag-version --no-push --yes" + "_lerna:version_minor": "npx lerna version minor --exact --no-git-tag-version --no-push --yes", + "_changelog:prepare_experimental": "node scripts/update-changelog.js ./experimental/CHANGELOG.md ./experimental/packages/", + "_changelog:prepare_stable": "node scripts/update-changelog.js ./CHANGELOG.md ./packages/" }, "repository": "open-telemetry/opentelemetry-js", "keywords": [ diff --git a/scripts/update-changelog.js b/scripts/update-changelog.js new file mode 100644 index 00000000000..f131afc0ceb --- /dev/null +++ b/scripts/update-changelog.js @@ -0,0 +1,65 @@ +/** + * This script updates changelogs after lerna has updated versions in the respective areas (packages/*, experimental/packages/*) + * - removes all empty subsections (bugs, enhancements, etc.) in the changelog. + * - replaces the "Unreleased"-header with the version from the first non-private package in the directory (versions are expected to be uniform across a changelog) + * - adds a new "Unreleased"-header with empty subsections at the top + * + * Usage (from project root): + * - node scripts/update-changelog.js [PATH TO CHANGELOG] [DIRECTORY CONTAINING ASSOCIATED PACKAGES] + * Examples: + * - node scripts/update-changelog.js ./CHANGELOG.md ./packages + * - node scripts/update-changelog.js ./experimental/CHANGELOG.md ./experimental/packages + */ + +const fs = require('fs'); +const path = require("path"); + +const EMPTY_UNRELEASED_SECTION = `## Unreleased + +### :boom: Breaking Change + +### :rocket: (Enhancement) + +### :bug: (Bug Fix) + +### :books: (Refine Doc) + +### :house: (Internal) + +` + +function findFirstPackageVersion(basePath){ + const packageDirs = fs.readdirSync(basePath); + for(const packageDir of packageDirs){ + const packageJsonPath = path.join(basePath, packageDir, 'package.json'); + try { + const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, 'utf-8')); + + if(packageJson.private === true || packageJson.private === 'true'){ + console.log('Skipping version from private package at', packageJsonPath); + continue; + } + + if(packageJson.version != null){ + return packageJson.version; + } + + console.log('Version in', packageJsonPath, 'was null or undefined, skipping'); + } catch (err) { + console.log('Could not get package JSON', packageJsonPath, err); + } + } + throw new Error('Unable to extract version from packages in ' + basePath); +} + +// no special handling for bad args as this is only intended for use via predefined npm scripts. +const changelogPath = path.resolve(process.argv[2]); +const version = findFirstPackageVersion(path.resolve(process.argv[3])); + +const changelog = fs.readFileSync(changelogPath, 'utf8').toString() + // replace all empty sections + .replace(new RegExp('^###.*\n*(?=^##)', 'gm'), '') + // replace unreleased header with new unreleased section and a version header for the former unreleased section + .replace(RegExp('## Unreleased'), EMPTY_UNRELEASED_SECTION + '## ' + version); + +fs.writeFileSync(changelogPath, changelog); From bf8ee62a1ecb3cf37af0f09d4dc67c47f2d1dd9b Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 30 Nov 2023 08:37:03 -0500 Subject: [PATCH 3/4] Fix event name --- .../packages/opentelemetry-instrumentation-http/src/http.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index a76f1a918da..81e08ac860b 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -330,7 +330,7 @@ export class HttpInstrumentation extends InstrumentationBase { 'response', (response: http.IncomingMessage & { aborted?: boolean }) => { this._diag.debug('outgoingRequest on response()'); - if (request.listenerCount('request') <= 0) { + if (request.listenerCount('response') <= 1) { response.resume(); } const responseAttributes = From 470a2bf79b010e075ba53812b68d8309cbc9e186 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 30 Nov 2023 08:38:58 -0500 Subject: [PATCH 4/4] test: make rawRequest HTTP-compliant --- .../opentelemetry-instrumentation-http/test/utils/rawRequest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/utils/rawRequest.ts b/experimental/packages/opentelemetry-instrumentation-http/test/utils/rawRequest.ts index c2fefa284a6..dc00fabfad3 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/utils/rawRequest.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/utils/rawRequest.ts @@ -22,7 +22,7 @@ export async function sendRequestTwice( port: number ): Promise { return new Promise((resolve, reject) => { - const request = 'GET /raw HTTP/1.1\n\n'; + const request = `GET /raw HTTP/1.1\r\nHost: ${host}:${port}\r\n\r\n`; const socket = net.createConnection({ host, port }, () => { socket.write(`${request}${request}`, err => { if (err) reject(err);