Skip to content

Commit 8852764

Browse files
authored
fix: memory leak in @elastic/elasticsearch instrumentation (#2571)
tl;dr: Use a WeakMap as a WeakMap, not as an Object. This leak was introduced in #2484. Fixes: #2569
1 parent e29269b commit 8852764

File tree

2 files changed

+23
-5
lines changed

2 files changed

+23
-5
lines changed

CHANGELOG.asciidoc

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,28 @@ Notes:
2828
[[release-notes-3.x]]
2929
=== Node.js Agent version 3.x
3030
31+
==== Unreleased
32+
33+
[float]
34+
===== Breaking changes
35+
36+
[float]
37+
===== Features
38+
39+
[float]
40+
===== Bug fixes
41+
42+
* Fix a bug in instrumentation of `@elastic/elasticsearch` that caused a
43+
memory leak. ({issues}2569[#2569])
44+
3145
3246
[[release-notes-3.28.0]]
3347
==== 3.28.0 2022/02/08
3448
49+
Known issue: This release includes a memory leak in instrumentation of the
50+
`@elastic/elasticsearch` package. If you use that package, you should not
51+
use v3.28.0 of this APM agent. ({issues}2569[#2569])
52+
3553
[float]
3654
===== Breaking changes
3755

lib/instrumentation/modules/@elastic/elasticsearch.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ module.exports = function (elasticsearch, agent, { version, enabled }) {
7070
if (result) {
7171
const currSpan = ins.currSpan()
7272
if (currSpan) {
73-
diagResultFromSpan[currSpan] = result
73+
diagResultFromSpan.set(currSpan, result)
7474
}
7575
}
7676
})
@@ -91,7 +91,7 @@ module.exports = function (elasticsearch, agent, { version, enabled }) {
9191
const conn = origGetConnection.apply(this, arguments)
9292
const currSpan = ins.currSpan()
9393
if (conn && currSpan) {
94-
connFromSpan[currSpan] = conn
94+
connFromSpan.set(currSpan, conn)
9595
}
9696
return conn
9797
}
@@ -136,7 +136,7 @@ module.exports = function (elasticsearch, agent, { version, enabled }) {
136136
// Otherwise, fallback to using the first connection on
137137
// `Transport#connectionPool`, if any. (This is the best parsed
138138
// representation of connection options passed to the Client ctor.)
139-
let conn = connFromSpan[span]
139+
let conn = connFromSpan.get(span)
140140
if (conn) {
141141
connFromSpan.delete(span)
142142
} else if (this.connectionPool && this.connectionPool.connections) {
@@ -154,7 +154,7 @@ module.exports = function (elasticsearch, agent, { version, enabled }) {
154154
// content-length: ...
155155
//
156156
// Getting the ES client request "DiagnosticResult" object has some edge cases:
157-
// - In v7 using a callback, we always get `result`.
157+
// - In v7 using a callback, we always get it as `result`.
158158
// - In v7 using a Promise, if the promise is rejected, then `result` is
159159
// not passed.
160160
// - In v8, `result` only includes HTTP response info if `options.meta`
@@ -166,7 +166,7 @@ module.exports = function (elasticsearch, agent, { version, enabled }) {
166166
// `diagResult`.
167167
let diagResult = isGteV8 ? null : result
168168
if (!diagResult) {
169-
diagResult = diagResultFromSpan[span]
169+
diagResult = diagResultFromSpan.get(span)
170170
if (diagResult) {
171171
diagResultFromSpan.delete(span)
172172
}

0 commit comments

Comments
 (0)