Skip to content

Commit 24c71b6

Browse files
authored
fix(ws): avoid retaining connection span (#7469)
Run ws socket setup in a store scope without the connection span so internal ws handlers don't capture and keep it alive.
1 parent bc78996 commit 24c71b6

3 files changed

Lines changed: 55 additions & 0 deletions

File tree

packages/datadog-instrumentations/src/ws.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const producerCh = tracingChannel('ws:send')
1313
const receiverCh = tracingChannel('ws:receive')
1414
const closeCh = tracingChannel('ws:close')
1515
const emitCh = channel('tracing:ws:server:connect:emit')
16+
const setSocketCh = channel('tracing:ws:server:connect:setSocket')
1617
// TODO: Add a error channel / handle error events properly.
1718

1819
/**
@@ -181,12 +182,33 @@ addHook({
181182
return ws
182183
})
183184

185+
/**
186+
* Prevent internal event handlers (data, close, etc.) registered by the ws library to
187+
* capture the connection span in their async context. Otherwise, the
188+
* finished connection span is retained for the entire lifetime of the WebSocket
189+
* (via ACF -> handle -> WeakMap).
190+
*
191+
* @param {Function} setSocket
192+
* @returns {(...args: unknown[]) => unknown}
193+
*/
194+
function wrapSetSocket (setSocket) {
195+
return function wrappedSetSocket (...args) {
196+
if (!setSocketCh.hasSubscribers) {
197+
return setSocket.apply(this, args)
198+
}
199+
return setSocketCh.runStores({}, () => {
200+
return setSocket.apply(this, args)
201+
})
202+
}
203+
}
204+
184205
addHook({
185206
name: 'ws',
186207
file: 'lib/websocket.js',
187208
versions: ['>=8.0.0'],
188209
}, moduleExports => {
189210
const ws = /** @type {WebSocketClass} */ (moduleExports)
211+
shimmer.wrap(ws.prototype, 'setSocket', wrapSetSocket)
190212
shimmer.wrap(ws.prototype, 'send', wrapSend)
191213
shimmer.wrap(ws.prototype, 'close', wrapClose)
192214

packages/datadog-plugin-ws/src/server.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,14 @@ class WSServerPlugin extends TracingPlugin {
1717
static get type () { return 'websocket' }
1818
static get kind () { return 'request' }
1919

20+
constructor (...args) {
21+
super(...args)
22+
23+
// Bind the setSocket channel so internal ws event handlers (data, close)
24+
// don't capture their async context.
25+
this.addBind('tracing:ws:server:connect:setSocket', () => {})
26+
}
27+
2028
bindStart (ctx) {
2129
const req = ctx.req
2230

packages/datadog-plugin-ws/test/index.spec.js

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
const assert = require('node:assert')
44
const { once } = require('node:events')
55

6+
const dc = require('dc-polyfill')
67
const { after, afterEach, before, beforeEach, describe, it } = require('mocha')
78

89
const agent = require('../../dd-trace/test/plugins/agent')
10+
const { storage } = require('../../datadog-core')
911
const { withVersions } = require('../../dd-trace/test/setup/mocha')
1012
const { assertObjectContains } = require('../../../integration-tests/helpers')
1113

@@ -65,6 +67,29 @@ describe('Plugin', () => {
6567
agent.close({ ritmReset: false, wipe: true })
6668
})
6769

70+
it('should not retain the connection span during socket setup', async () => {
71+
const setSocketCh = dc.channel('tracing:ws:server:connect:setSocket')
72+
let resolve
73+
const promise = new Promise((_resolve) => {
74+
resolve = _resolve
75+
})
76+
77+
const handler = () => {
78+
resolve(storage('legacy').getStore())
79+
setSocketCh.unsubscribe(handler)
80+
}
81+
setSocketCh.subscribe(handler)
82+
83+
// Trigger setSocket
84+
const newClient = new WebSocket(`ws://localhost:${clientPort}/test`)
85+
newClient.on('open', () => newClient.close())
86+
87+
const store = await promise
88+
89+
assert.strictEqual(store?.span, undefined,
90+
'connection span should not be in the store during setSocket')
91+
})
92+
6893
it('should do automatic instrumentation and remove broken handler', () => {
6994
wsServer.on('connection', (ws) => {
7095
connectionReceived = true

0 commit comments

Comments
 (0)