fix(otel-thread-ctx): detect AsyncContextFrame by reading CPED natively (#398)

* fix(otel-thread-ctx): detect AsyncContextFrame by reading CPED natively

#397 replaced the execArgv inference with a feature detection, but the probe
was indirect: it overrode `enterWith` on a throwaway AsyncLocalStorage and
checked whether `run()` dispatched through it. That `run()` goes through the
instance property is unspecified, and anything patching AsyncLocalStorage can
break it — including dd-trace-js, which patches async-context machinery. The
resulting false negative is the failure #397 set out to fix: `enter()` throwing
inside a diagnostic-channel subscriber, in application code.

Ask the question directly instead. `cpedMapContains(key, value)` reports
whether the isolate's ContinuationPreservedEmbedderData binds a key to a value,
so calling it from inside a `run()` with the probe storage and its own store
observes the property the addon actually depends on. It is the same slot, and
the same "is it a Map" question, that WallProfiler::SetContext asks before
storing a context; the key is the one whose identity hash is published as
otel_thread_ctx_nodejs_v1.als_identity_hash for the out-of-process reader to
look up. Verified empirically that the frame is keyed by the storage instance
with the store as value.

Checking the key and value rather than just "CPED holds a Map" matters: CPED is
a general embedder slot, so a Map another addon left there must not answer for
us — that would resurrect the silent false positive, where the writer looks
healthy from JS while readers see records nothing updates.

* test: use the real 22.7.0 AsyncContextFrame cutoff

Author: szegedi

bindings/binding.cc

@@ -29,6 +29,40 @@
 #include <unistd.h>
 #endif
 
+// Whether the isolate's ContinuationPreservedEmbedderData is a JS Map that
+// currently binds `key` to `value`.
+//
+// This exists for AsyncContextFrame feature detection. With ACF active, Node
+// implements AsyncLocalStorage#run by installing an AsyncContextFrame — a JS
+// Map keyed by the AsyncLocalStorage instance — as the CPED of the running
+// continuation. Calling this from inside a run() with the storage and its
+// store therefore observes the property this addon actually depends on,
+// instead of inferring it from the Node version, process.execArgv, or whether
+// run() happens to dispatch through the instance's enterWith.
+static NAN_METHOD(CpedMapContains) {
+#if NODE_MAJOR_VERSION >= 22
+  // A malformed call must not accidentally answer true by comparing an absent
+  // key's undefined against an undefined expected value.
+  if (info.Length() >= 2) {
+    auto isolate = info.GetIsolate();
+    auto cped = isolate->GetContinuationPreservedEmbedderData();
+    if (!cped.IsEmpty() && cped->IsMap()) {
+      auto context = isolate->GetCurrentContext();
+      if (!context.IsEmpty()) {
+        v8::Local<v8::Value> found;
+        if (cped.As<v8::Map>()->Get(context, info[0]).ToLocal(&found)) {
+          info.GetReturnValue().Set(found->StrictEquals(info[1]));
+          return;
+        }
+      }
+    }
+  }
+#endif
+  // Either code above didn't reach the innermost if statement, or
+  // we're compiling for Node.js < 22.
+  info.GetReturnValue().Set(false);
+}
+
 static NAN_METHOD(GetNativeThreadId) {
 #ifdef __APPLE__
   uint64_t native_id;
@@ -56,4 +90,5 @@ NODE_MODULE_INIT(/* exports, module, context */) {
   dd::WallProfiler::Init(exports);
   dd::OtelThreadCtx::Init(exports);
   Nan::SetMethod(exports, "getNativeThreadId", GetNativeThreadId);
+  Nan::SetMethod(exports, "cpedMapContains", CpedMapContains);
 }

ts/src/async-context-frame.ts

@@ -15,6 +15,23 @@
  */
 
 import {AsyncLocalStorage} from 'node:async_hooks';
+import {join} from 'path';
+
+interface Addon {
+  cpedMapContains(key: unknown, value: unknown): boolean;
+}
+
+let addon: Addon | undefined;
+
+// Required lazily so importing this module doesn't force the addon to load;
+// memoized by isAsyncContextFrameActive, so this runs at most once per thread.
+function bindings(): Addon {
+  if (!addon) {
+    const findBinding = require('node-gyp-build');
+    addon = findBinding(join(__dirname, '..', '..')) as Addon;
+  }
+  return addon;
+}
 
 let active: boolean | undefined;
 
@@ -27,9 +44,10 @@ let active: boolean | undefined;
  * `process.execArgv`, because the two disagree in both directions and each
  * combination is reachable today:
  *
- * - `NODE_OPTIONS=--experimental-async-context-frame` is accepted on Node 22
- *   and 23 and turns ACF on without appearing in `execArgv`. Inferring "off"
- *   there makes callers refuse to run in a process that would have worked.
+ * - `NODE_OPTIONS=--experimental-async-context-frame` is accepted from Node
+ *   22.7.0 through 23 and turns ACF on without appearing in `execArgv`.
+ *   Inferring "off" there makes callers refuse to run in a process that would
+ *   have worked.
  * - `NODE_OPTIONS=--no-async-context-frame` is accepted on Node 24 and turns
  *   ACF off without appearing in `execArgv`. Inferring "on" there is the worse
  *   error: the CPED slot is never written, so a writer that starts anyway keeps
@@ -39,19 +57,34 @@ let active: boolean | undefined;
  *   main thread's command line either, and tooling sometimes rewrites
  *   `process.execArgv` outright.
  *
- * With ACF, `run()` is implemented in terms of `enterWith()`; without it, it
- * isn't. Memoized: the answer is fixed for the life of the thread.
+ * Detected by asking the addon what is in the CPED slot during a `run()`. With
+ * ACF, Node installs an AsyncContextFrame — a JS Map keyed by the
+ * `AsyncLocalStorage` instance, valued by its store — as the running
+ * continuation's CPED; without it, nothing writes the slot. So a probe storage
+ * whose own store is visible there is direct evidence, and it is evidence about
+ * the exact slot both consumers read: `WallProfiler::SetContext` requires that
+ * Map, and the thread-ctx reader looks this very key up by the identity hash
+ * published as `als_identity_hash`.
+ *
+ * Observing whether `run()` delegates to `enterWith()` would be an indirect
+ * proxy for the same thing: it holds today, but it depends on `run()`
+ * dispatching through the instance property, which is unspecified and which
+ * anything patching `AsyncLocalStorage` can break — and the failure would be
+ * silent and in the dangerous direction.
+ *
+ * Memoized: the answer is fixed for the life of the thread.
  */
 export function isAsyncContextFrameActive(): boolean {
   if (active === undefined) {
-    const probe = new AsyncLocalStorage<number>();
-    let delegated = false;
-    probe.enterWith = () => {
-      delegated = true;
-    };
-    probe.run(0, () => {});
+    const probe = new AsyncLocalStorage<object>();
+    // Object identity, so a stray equal-valued binding can't answer for us.
+    const sentinel = {};
+    let bound = false;
+    probe.run(sentinel, () => {
+      bound = bindings().cpedMapContains(probe, sentinel);
+    });
     probe.disable();
-    active = delegated;
+    active = bound;
   }
   return active;
 }
@@ -65,8 +98,10 @@ export function isAsyncContextFrameActive(): boolean {
  */
 export function asyncContextFrameHint(): string {
   const version = process.versions.node;
-  const major = Number(version.split('.')[0]);
-  if (major < 22) {
+  const [major, minor] = version.split('.').map(Number);
+  // Hand-rolled rather than semver.satisfies: semver is a devDependency, and
+  // this module ships.
+  if (major < 22 || (major === 22 && minor < 7)) {
     return `Node ${version} does not support it at all; Node 24 and later enable it by default`;
   }
   if (major < 24) {

ts/test/test-async-context-frame.ts

@@ -15,14 +15,23 @@
  */
 
 import {strict as assert} from 'assert';
+import {AsyncLocalStorage} from 'node:async_hooks';
 import {fork} from 'node:child_process';
 import {join} from 'node:path';
 
+import {satisfies} from 'semver';
+
 import {isAsyncContextFrameActive} from '../src/async-context-frame';
 
+const addon = require('node-gyp-build')(join(__dirname, '..', '..')) as {
+  cpedMapContains(key?: unknown, value?: unknown): boolean;
+};
+
 const CHILD = join(__dirname, 'async-context-frame-child.js');
 
 const major = Number(process.versions.node.split('.')[0]);
+// ACF landed in 22.7.0, so the opt-in routes are gated on that, not on major 22.
+const hasAcfSupport = satisfies(process.versions.node, '>=22.7.0');
 
 interface ChildReport {
   active: boolean;
@@ -79,7 +88,7 @@ describe('isAsyncContextFrameActive', () => {
   });
 
   it('reports it inactive when Node has no support for it', async function () {
-    if (major >= 22) return this.skip();
+    if (hasAcfSupport) return this.skip();
     const {active} = await probeChild();
     assert.equal(active, false);
   });
@@ -107,15 +116,83 @@ describe('isAsyncContextFrameActive', () => {
   });
 
   it('reports it active when NODE_OPTIONS turns it on', async function () {
-    // The mirror image, on the other Node line: 22 and 23 accept the flag in
+    // The mirror image, on the other Node line: 22.7.0 through 23 accept the flag in
     // NODE_OPTIONS (24 rejects it outright), again without it reaching execArgv,
     // so inferring from execArgv concludes ACF is off when it is on — and the
     // caller refuses to run in a process that would have worked.
-    if (major < 22 || major >= 24) return this.skip();
+    if (!hasAcfSupport || major >= 24) return this.skip();
     const {active, execArgv} = await probeChild({
       nodeOptions: '--experimental-async-context-frame',
     });
     assert.deepEqual(execArgv, []);
     assert.equal(active, true);
   });
 });
+
+// The detection asks whether the running storage is bound to its own store,
+// not merely whether the CPED slot holds a Map. These pin that difference:
+// without them, weakening the helper to a bare IsMap check would still pass
+// every test above.
+describe('cpedMapContains', () => {
+  beforeEach(function () {
+    // With ACF off nothing writes the slot, so every answer here is false for
+    // an uninteresting reason. The routes that discriminate on/off are covered
+    // by the child-process cases above.
+    if (!isAsyncContextFrameActive()) this.skip();
+  });
+
+  it('finds the running storage bound to its store', () => {
+    const als = new AsyncLocalStorage<object>();
+    const store = {};
+    let found = false;
+    als.run(store, () => {
+      found = addon.cpedMapContains(als, store);
+    });
+    als.disable();
+    assert.equal(found, true);
+  });
+
+  it('does not match a foreign key', () => {
+    // CPED is a general embedder slot. Another native addon storing a Map there
+    // must not be able to answer for us, which is the false positive an IsMap
+    // check would admit.
+    const als = new AsyncLocalStorage<object>();
+    const store = {};
+    let found = true;
+    als.run(store, () => {
+      found = addon.cpedMapContains(new AsyncLocalStorage<object>(), store);
+    });
+    als.disable();
+    assert.equal(found, false);
+  });
+
+  it('does not match a different value for the right key', () => {
+    const als = new AsyncLocalStorage<object>();
+    let found = true;
+    als.run({}, () => {
+      found = addon.cpedMapContains(als, {});
+    });
+    als.disable();
+    assert.equal(found, false);
+  });
+
+  it('is false outside any run', () => {
+    const als = new AsyncLocalStorage<object>();
+    const store = {};
+    als.run(store, () => {});
+    als.disable();
+    assert.equal(addon.cpedMapContains(als, store), false);
+  });
+
+  it('is false when called without a key and value', () => {
+    // An absent key reads as undefined; so would a missing expected value, so
+    // a malformed call must not compare the two and report success.
+    const als = new AsyncLocalStorage<object>();
+    let found = true;
+    als.run({}, () => {
+      found = addon.cpedMapContains();
+    });
+    als.disable();
+    assert.equal(found, false);
+  });
+});

ts/test/test-get-value-from-map-profiler.ts

@@ -29,15 +29,13 @@
 import assert from 'assert';
 import {join} from 'path';
 import {AsyncLocalStorage} from 'async_hooks';
-import {satisfies} from 'semver';
 
 import {isAsyncContextFrameActive} from '../src/async-context-frame';
 
 const findBinding = require('node-gyp-build');
 const profiler = findBinding(join(__dirname, '..', '..'));
 
-const useCPED =
-  isAsyncContextFrameActive() && satisfies(process.versions.node, '>=22.7.0');
+const useCPED = isAsyncContextFrameActive();
 
 const supportedPlatform =
   process.platform === 'darwin' || process.platform === 'linux';

ts/test/test-otel-thread-ctx.ts

@@ -62,8 +62,9 @@ function tcIsTruncated(): boolean {
 }
 
 const isLinux = process.platform === 'linux';
-// AsyncContextFrame is the writer's discovery substrate: opt-in on Node 22/23
-// (via --experimental-async-context-frame) and on by default from Node 24
+// AsyncContextFrame is the writer's discovery substrate: opt-in from Node
+// 22.7.0 through 23 (via --experimental-async-context-frame) and on by
+// default from Node 24
 // (disable-able via --no-async-context-frame). The TS layer refuses to install
 // the hook when it isn't active, so the entire describe block is skipped then.
 // Asks the same question the source side asks, the same way.

ts/test/test-time-profiler.ts

@@ -32,8 +32,7 @@ import {fork} from 'child_process';
 
 import assert from 'assert';
 
-const useCPED =
-  isAsyncContextFrameActive() && satisfies(process.versions.node, '>=22.7.0');
+const useCPED = isAsyncContextFrameActive();
 
 const collectAsyncId = satisfies(process.versions.node, '>=24.0.0');
 

ts/test/worker.ts

@@ -12,10 +12,7 @@ const DURATION_MILLIS = 1000;
 const intervalMicros = 10000;
 const withContexts =
   process.platform === 'darwin' || process.platform === 'linux';
-const useCPED =
-  withContexts &&
-  isAsyncContextFrameActive() &&
-  satisfies(process.versions.node, '>=22.7.0');
+const useCPED = withContexts && isAsyncContextFrameActive();
 const collectAsyncId =
   withContexts && satisfies(process.versions.node, '>=24.0.0');
 

ts/test/worker2.ts

@@ -10,10 +10,7 @@ const INTERVAL_MICROS = 10000;
 const withContexts =
   process.platform === 'darwin' || process.platform === 'linux';
 
-const useCPED =
-  withContexts &&
-  isAsyncContextFrameActive() &&
-  satisfies(process.versions.node, '>=22.7.0');
+const useCPED = withContexts && isAsyncContextFrameActive();
 
 const collectAsyncId =
   withContexts && satisfies(process.versions.node, '>=24.0.0');