Skip to content

Commit 44e56bd

Browse files
committed
test: release HTTP keep-alive span retainers, raise baseline to 10
The dominant span retainer on HTTP-server suites is Node's per-connection keep-alive/headers `Timeout`: under AsyncContextFrame its callback closes over the `{ ...store, span }` frame active when the request ran and stays in the active-timer list, reachable from the still-open `Server`, until it fires. That pinned a fixed handful of finished spans at teardown on every express, fastify, koa, connect, and AppSec/IAST suite, so each raised its own baseline for benign non-scaling residue. Release it at the source: at agent teardown, before forcing GC, call `closeIdleConnections()` on every live server (Node >=18.2; drops only sockets with no in-flight request). The default baseline moves 3 -> 10 to absorb the recency-bounded residue that survives. 10 is an absolute cap, not a per-request allowance, so a #9227-style leak still retains one span per request and blows past it by orders of magnitude — the margin costs no detection power. Refs: #9227
1 parent a444d94 commit 44e56bd

2 files changed

Lines changed: 53 additions & 21 deletions

File tree

‎packages/dd-trace/test/plugins/span-leak-detector.js‎

Lines changed: 45 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,24 +44,52 @@ const MAX_GC_CYCLES = 10
4444

4545
/**
4646
* Default upper bound on finished spans that may still be reachable at teardown
47-
* without indicating a leak. Benign retention is the handful of most-recent
48-
* traces whose async-context frames have not been overwritten yet (see above); a
49-
* sweep of the http/http2 suites topped out at two, so this is that measured
50-
* maximum plus one slot of margin. It is an absolute cap, not a per-request
51-
* allowance: a #9227-style leak retains one span per request and blows past it by
52-
* orders of magnitude, so the margin costs no detection power.
47+
* without indicating a leak. Benign retention has two sources: the handful of
48+
* most-recent traces whose async-context frames have not been overwritten yet
49+
* (see above), and the small residue that survives `releaseKeepAliveRetainers`
50+
* on HTTP-server suites (a middleware `next` closure still holding the last
51+
* request's `req`). Both are recency-bounded, not per-request: an http-suite
52+
* sweep tops out at a handful, so 10 covers it with margin. It is an absolute
53+
* cap, not a per-request allowance — a #9227-style leak retains one span per
54+
* request and blows past 10 by orders of magnitude, so the margin costs no
55+
* detection power.
5356
*
5457
* A few suites exercise upstream libraries that pin a *fixed* number of finished
55-
* spans regardless of request count — e.g. `@grpc/grpc-js` keeps a cleared
56-
* `Timeout` on each pooled `Subchannel`, and under AsyncContextFrame that timer
57-
* retains the `{ ...store, span }` frame active when the subchannel was built.
58-
* That retention does not scale, so it is not the leak this detector hunts, but
59-
* it does exceed the default. Those suites raise the bound with
60-
* {@link SpanLeakDetector#setBaseline} in a `before` hook and restore it with
61-
* {@link SpanLeakDetector#resetBaseline} in `after`, so every other suite keeps
62-
* the tight default.
58+
* spans regardless of request count, through a retainer `closeIdleConnections`
59+
* cannot reach — e.g. `@grpc/grpc-js` keeps a cleared `Timeout` on each pooled
60+
* `Subchannel`, and under AsyncContextFrame that timer retains the
61+
* `{ ...store, span }` frame active when the subchannel was built. That retention
62+
* does not scale, but it exceeds the default, so those suites raise the bound
63+
* with {@link SpanLeakDetector#withSpanLeakBaseline}, keeping the tight default
64+
* for every other suite.
6365
*/
64-
const BASELINE_RETAINED = 3
66+
const BASELINE_RETAINED = 10
67+
68+
/**
69+
* Close idle connections on every live HTTP/net server so their per-connection
70+
* keep-alive timers stop pinning finished spans. Node arms a keep-alive/headers
71+
* `Timeout` per connection; under AsyncContextFrame that timer's callback closes
72+
* over the `{ ...store, span }` frame active when the request ran, and it stays in
73+
* the active-timer list — reachable from the still-open `Server` — until it fires.
74+
* That is the dominant retainer for HTTP-server suites (express, fastify, koa,
75+
* connect, the AppSec/IAST specs). Releasing it at the source lets those suites
76+
* keep the tight default instead of each raising a baseline for benign residue.
77+
*
78+
* Best-effort: `_getActiveHandles` is internal and its entries vary by Node
79+
* version, so guard every access. `closeIdleConnections` (Node >=18.2) only drops
80+
* sockets with no in-flight request, so it never interrupts real work; the
81+
* assertion runs at agent teardown, after the suite's requests have completed.
82+
*
83+
* @returns {void}
84+
*/
85+
function releaseKeepAliveRetainers () {
86+
const handles = process._getActiveHandles?.()
87+
if (handles === undefined) return
88+
89+
for (const handle of handles) {
90+
handle?.closeIdleConnections?.()
91+
}
92+
}
6593

6694
class SpanLeakDetector {
6795
#registry
@@ -166,6 +194,8 @@ class SpanLeakDetector {
166194
return
167195
}
168196

197+
releaseKeepAliveRetainers()
198+
169199
// Let any pending flush / microtasks settle, then force GC and give the
170200
// registry callbacks a turn. Repeat until retention is within the baseline or
171201
// the cycle budget is spent (polling because FinalizationRegistry callbacks

‎packages/dd-trace/test/plugins/span-leak-detector.spec.js‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,14 @@ if (typeof global.gc !== 'function') {
1717
// grows with the request count must be reported as a leak, while the small,
1818
// recency-bounded baseline (the few most-recent traces whose async-context frames
1919
// have not been overwritten yet) must pass. The detector tolerates up to
20-
// BASELINE_RETAINED (3) survivors — a measured maximum of two across the http
21-
// suites plus one slot of margin — so the boundary cases below pin 3 (accepted)
22-
// and 4 (the first rejected). Uses fresh detector instances (not the agent
23-
// singleton) so every outcome can be asserted without a real teardown.
24-
25-
const BASELINE_RETAINED = 3
20+
// BASELINE_RETAINED (10) survivors — the residual left after `closeIdleConnections`
21+
// releases the HTTP keep-alive retainer, still small enough that a real
22+
// one-span-per-request leak (dozens–hundreds) trips it — so the boundary cases
23+
// below pin 10 (accepted) and 11 (the first rejected). Uses fresh detector
24+
// instances (not the agent singleton) so every outcome can be asserted without a
25+
// real teardown.
26+
27+
const BASELINE_RETAINED = 10
2628

2729
async function forceGc () {
2830
for (let i = 0; i < 10; i++) {

0 commit comments

Comments
 (0)