Skip to content

Commit aaeb9a8

Browse files
committed
fix(core): release finished spans from captured async-context frames
`Plugin#enter` writes a fresh `{ ...store, span }` object into the current async-context frame. Any async resource created while that frame is active snapshots it and keeps the object — and its `span` — reachable for the resource's whole lifetime. When the resource is never freed (a pooled socket, a long-lived emitter), the finished span and its entire parent chain stay pinned, so retention grows one span per request until the process runs out of memory (#9227). `enter` now returns the store it wrote into storage; a caller whose span outlives the request keeps that object and nulls its `span` once the span is finished and no in-scope continuation can still read it as active. Applied at the three sites that enter a store for such a span: 1. router — null every entered store at `request:finish`, not at `middleware:finish` (which fires before `next.apply(...)`, while work the middleware schedules after `next()` must still see the correct active span). 2. google-cloud-pubsub push — null the push-receive store at request finish. 3. test-api-manual — null the entered store once the manual test is finished. Nulling the store in place, rather than routing through a span-keyed WeakMap on the base class, keeps the release off the hot path (no per-span map insert plus lookup and delete) and off the npm-exported `Plugin` surface. Refs: #9228
1 parent 0175f0c commit aaeb9a8

4 files changed

Lines changed: 50 additions & 12 deletions

File tree

‎packages/datadog-plugin-google-cloud-pubsub/src/pubsub-push-subscription.js‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ const SpanContext = require('../../dd-trace/src/opentracing/span_context')
55
const id = require('../../dd-trace/src/id')
66
const log = require('../../dd-trace/src/log')
77

8-
// WeakMap to track push receive spans by request
8+
// Track each request's push-receive span and the store it was entered into, so
9+
// the store's `span` can be nulled at request finish (a captured async-context
10+
// frame would otherwise pin the finished span).
911
const pushReceiveSpans = new WeakMap()
1012

1113
class GoogleCloudPubsubPushSubscriptionPlugin extends TracingPlugin {
@@ -35,10 +37,13 @@ class GoogleCloudPubsubPushSubscriptionPlugin extends TracingPlugin {
3537
}
3638

3739
#finishPushReceiveSpan (req) {
38-
const pushReceiveSpan = pushReceiveSpans.get(req)
39-
if (pushReceiveSpan && !pushReceiveSpan._duration) {
40-
pushReceiveSpan.finish()
40+
const entry = pushReceiveSpans.get(req)
41+
if (entry && !entry.span._duration) {
42+
entry.span.finish()
4143
pushReceiveSpans.delete(req)
44+
// The request is done; null the span on the store entered for it so a
45+
// captured async-context frame stops retaining the finished span.
46+
entry.store.span = null
4247
}
4348
}
4449

@@ -90,8 +95,8 @@ class GoogleCloudPubsubPushSubscriptionPlugin extends TracingPlugin {
9095
return
9196
}
9297

93-
this.enter(pushReceiveSpan, { req, res })
94-
pushReceiveSpans.set(req, pushReceiveSpan)
98+
const store = this.enter(pushReceiveSpan, { req, res })
99+
pushReceiveSpans.set(req, { span: pushReceiveSpan, store })
95100
}
96101

97102
#parseMessage (req) {

‎packages/datadog-plugin-router/src/index.js‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,12 @@ class RouterPlugin extends WebPlugin {
3838
}
3939

4040
context.storeStack.push(store)
41-
this.enter(span, store)
41+
// Keep the entered store for the whole request so its `span` can be nulled
42+
// at request finish. `context.middleware` is drained early at
43+
// `middleware:finish`, so it cannot serve as the release list; a middleware
44+
// span finished mid-request would otherwise stay pinned by any async
45+
// resource that captured its frame.
46+
context.enteredStores.push(this.enter(span, store))
4247

4348
web.patch(req)
4449
web.setRoute(req, context.route)
@@ -91,6 +96,17 @@ class RouterPlugin extends WebPlugin {
9196
while ((span = context.middleware.pop())) {
9297
span.finish()
9398
}
99+
100+
// The request/response is fully done: no async continuation can still need
101+
// a middleware span as the active span. Null the `span` on every store we
102+
// entered so an async resource that captured that frame no longer retains
103+
// the finished span or its parent chain. This happens only here — never at
104+
// `middleware:finish`, which fires before `next.apply(...)`, so work a
105+
// middleware schedules after `next()` still sees the correct active span.
106+
let store
107+
while ((store = context.enteredStores.pop())) {
108+
store.span = null
109+
}
94110
})
95111
}
96112

@@ -130,14 +146,17 @@ class RouterPlugin extends WebPlugin {
130146
return context
131147
}
132148

133-
// Five-property shape pinned at allocation so every request shares the
134-
// same hidden class — no per-field transitions after construction.
149+
// Fixed shape pinned at allocation so every request shares the same hidden
150+
// class — no per-field transitions after construction.
135151
context = {
136152
span,
137153
stack: [route],
138154
route,
139155
middleware: [],
140156
storeStack: [],
157+
// Every store entered for this request, retained until request finish so
158+
// its `span` can be nulled there (see the `request:finish` handler).
159+
enteredStores: [],
141160
}
142161

143162
this.#contexts.set(req, context)

‎packages/dd-trace/src/ci-visibility/test-api-manual/test-api-manual-plugin.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ class TestApiManualPlugin extends CiPlugin {
3434
}
3535
testSpan.finish()
3636
finishAllTraceSpans(testSpan)
37+
// Null the span on the entered store so a captured async-context frame no
38+
// longer retains the finished test span. `store` is the frame entered for
39+
// this test at `:start`; the same read already drives `finish` above.
40+
store.span = null
3741
}
3842
})
3943
this.unconfiguredAddSub('dd-trace:ci:manual:test:addTags', (tags) => {

‎packages/dd-trace/src/plugins/plugin.js‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,23 @@ module.exports = class Plugin {
9898
/**
9999
* Enter a context with the provided span bound in storage.
100100
*
101+
* A fresh `{ ...store, span }` object is written into the current
102+
* async-context frame, so any async resource created while it is active
103+
* snapshots the frame and keeps that object — and its `span` — reachable for
104+
* the resource's whole lifetime, even after the span finishes. A caller whose
105+
* span outlives the request should keep the returned store and null its `span`
106+
* once the span is finished and no in-scope continuation can still read it as
107+
* active; otherwise a never-freed resource pins the finished span (and its
108+
* parent chain) forever.
109+
*
101110
* @param {object} span The span to bind as current.
102111
* @param {object=} store Optional existing store to extend; if omitted, uses current store.
103-
* @returns {void}
112+
* @returns {{ span: object | null }} The store object written into storage.
104113
*/
105114
enter (span, store) {
106-
store = store || legacyStorage.getStore()
107-
legacyStorage.enterWith({ ...store, span })
115+
const activeStore = { ...(store || legacyStorage.getStore()), span }
116+
legacyStorage.enterWith(activeStore)
117+
return activeStore
108118
}
109119

110120
/**

0 commit comments

Comments
 (0)