From d52c53a1ddd1afdf1e17f30a872ccf2fa141abf1 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Thu, 10 Feb 2022 12:19:06 -0800 Subject: [PATCH 1/2] fix: memory leak in @elastic/elasticsearch instrumentation This leak was introduced in #2484. --- CHANGELOG.asciidoc | 18 ++++++++++++++++++ .../modules/@elastic/elasticsearch.js | 4 ++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index f8c2864e54..02665bb1db 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -28,10 +28,28 @@ Notes: [[release-notes-3.x]] === Node.js Agent version 3.x +==== Unreleased + +[float] +===== Breaking changes + +[float] +===== Features + +[float] +===== Bug fixes + +* Fix a bug in instrumentation of `@elastic/elasticsearch` that caused a + memory leak. ({issues}2569[#2569]) + [[release-notes-3.28.0]] ==== 3.28.0 2022/02/08 +Known issue: This release includes a memory leak in instrumentation of the +`@elastic/elasticsearch` package. If you use that package, you should not +use v3.28.0 of this APM agent. ({issues}2569[#2569]) + [float] ===== Breaking changes diff --git a/lib/instrumentation/modules/@elastic/elasticsearch.js b/lib/instrumentation/modules/@elastic/elasticsearch.js index 04fd3aabce..087e03b345 100644 --- a/lib/instrumentation/modules/@elastic/elasticsearch.js +++ b/lib/instrumentation/modules/@elastic/elasticsearch.js @@ -70,7 +70,7 @@ module.exports = function (elasticsearch, agent, { version, enabled }) { if (result) { const currSpan = ins.currSpan() if (currSpan) { - diagResultFromSpan[currSpan] = result + diagResultFromSpan.set(currSpan, result) } } }) @@ -91,7 +91,7 @@ module.exports = function (elasticsearch, agent, { version, enabled }) { const conn = origGetConnection.apply(this, arguments) const currSpan = ins.currSpan() if (conn && currSpan) { - connFromSpan[currSpan] = conn + connFromSpan.set(currSpan, conn) } return conn } From cc6891affdebad42336f8da782a9b01a0543e7ea Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Thu, 10 Feb 2022 13:39:45 -0800 Subject: [PATCH 2/2] correctly .get from WeakMaps, duh --- lib/instrumentation/modules/@elastic/elasticsearch.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/instrumentation/modules/@elastic/elasticsearch.js b/lib/instrumentation/modules/@elastic/elasticsearch.js index 087e03b345..be03a2345d 100644 --- a/lib/instrumentation/modules/@elastic/elasticsearch.js +++ b/lib/instrumentation/modules/@elastic/elasticsearch.js @@ -136,7 +136,7 @@ module.exports = function (elasticsearch, agent, { version, enabled }) { // Otherwise, fallback to using the first connection on // `Transport#connectionPool`, if any. (This is the best parsed // representation of connection options passed to the Client ctor.) - let conn = connFromSpan[span] + let conn = connFromSpan.get(span) if (conn) { connFromSpan.delete(span) } else if (this.connectionPool && this.connectionPool.connections) { @@ -154,7 +154,7 @@ module.exports = function (elasticsearch, agent, { version, enabled }) { // content-length: ... // // Getting the ES client request "DiagnosticResult" object has some edge cases: - // - In v7 using a callback, we always get `result`. + // - In v7 using a callback, we always get it as `result`. // - In v7 using a Promise, if the promise is rejected, then `result` is // not passed. // - In v8, `result` only includes HTTP response info if `options.meta` @@ -166,7 +166,7 @@ module.exports = function (elasticsearch, agent, { version, enabled }) { // `diagResult`. let diagResult = isGteV8 ? null : result if (!diagResult) { - diagResult = diagResultFromSpan[span] + diagResult = diagResultFromSpan.get(span) if (diagResult) { diagResultFromSpan.delete(span) }