Skip to content

Commit 483096d

Browse files
divybotlittledivy
andauthored
fix(ext/node): flesh out node:trace_events polyfill (#34216)
## Summary The `node:trace_events` polyfill was a stub — `createTracing` returned an object with no `.enable`/`.disable` methods and `getEnabledCategories` always returned `""`. This fleshes out the polyfill enough that the four ignored compat tests now run end-to-end. - `createTracing` validates input, returns a `Tracing` with `enabled` / `categories` properties and `enable()`/`disable()` methods, and emits the `>10` instances memory-leak warning. - `getEnabledCategories()` returns `undefined` when no instance is enabled, otherwise the comma-joined union of every enabled instance's categories. - `internalBinding('trace_events')` now exposes `trace(phase, cat, name, id, scope)` and `getCategoryEnabledBuffer(cat)` (with refcount semantics) so fixtures going through `internal/test/binding` observe the same state as the public API. - When `node.async_hooks` is enabled, `setTimeout` / `setImmediate` are wrapped to emit `b`/`e` events with `cat="node,node.async_hooks"`. On process exit each isolate writes its recorded events; workers write per-thread slice files in `cwd` and the main isolate aggregates them into a single `node_trace.${rotation}.log` so consumers see one combined log (matching Node's process-wide `TracingController` behavior). - `--trace-event-categories` is now plumbed through `node_shim` → `child_process` as `DENO_NODE_TRACE_EVENT_CATEGORIES`. The node bootstrap (`01_require.js`) reads it in every isolate (main + workers), pre-enables the listed categories, and pushes the flag back onto `process.execArgv` so test fixtures that probe `execArgv` see the expected shape. Enables in `tests/node_compat/config.jsonc`: - `parallel/test-trace-events-api.js` - `parallel/test-trace-events-async-hooks-dynamic.js` - `parallel/test-trace-events-async-hooks-worker.js` - `parallel/test-trace-events-get-category-enabled-buffer.js` Closes denoland/orchid#136 ## Test plan - [x] `cargo test --test node_compat -- test-trace-events-api.js` - [x] `cargo test --test node_compat -- test-trace-events-async-hooks-dynamic.js` - [x] `cargo test --test node_compat -- test-trace-events-async-hooks-worker.js` - [x] `cargo test --test node_compat -- test-trace-events-get-category-enabled-buffer.js` - [x] `cargo test --test node_compat -- test-trace-events-dynamic-enable-workers-disabled.js` (already-passing test still passes) - [x] `cargo test -p node_shim` (127 tests pass) - [x] `cargo clippy --bin deno` --------- Co-authored-by: divybot <divybot@users.noreply.github.com> Co-authored-by: Divy Srivastava <me@littledivy.com>
1 parent 53de64d commit 483096d

7 files changed

Lines changed: 430 additions & 25 deletions

File tree

ext/node/ops/node_cli_parser.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ pub struct TranslatedArgs {
4848
pub use_openssl_ca: bool,
4949
/// Whether the child process needs npm process state
5050
pub needs_npm_process_state: bool,
51+
/// Comma-separated trace event categories from --trace-event-categories,
52+
/// to be propagated via DENO_NODE_TRACE_EVENT_CATEGORIES.
53+
pub trace_event_categories: Option<String>,
5154
}
5255

5356
/// Translate parsed Node.js CLI arguments to Deno CLI arguments.
@@ -76,12 +79,19 @@ fn translate_to_deno_args(
7679
}
7780
};
7881

82+
let trace_event_categories = if result.trace_event_categories.is_empty() {
83+
None
84+
} else {
85+
Some(result.trace_event_categories)
86+
};
87+
7988
TranslatedArgs {
8089
deno_args: result.deno_args,
8190
node_options: result.node_options,
8291
ca_stores,
8392
use_openssl_ca,
8493
needs_npm_process_state: script_in_npm_package,
94+
trace_event_categories,
8595
}
8696
}
8797

@@ -109,6 +119,7 @@ pub fn op_node_translate_cli_args(
109119
ca_stores: None,
110120
use_openssl_ca: false,
111121
needs_npm_process_state: script_in_npm_package,
122+
trace_event_categories: None,
112123
});
113124
}
114125

ext/node/polyfills/01_require.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import { core, internals, primordials } from "ext:core/mod.js";
66
import {
77
op_fs_cwd,
8+
op_get_env_no_permission_check,
89
op_import_sync,
910
op_import_sync_with_source,
1011
op_module_default_resolve,
@@ -3157,6 +3158,33 @@ function initialize(args) {
31573158
nativeModuleExports["console"],
31583159
nativeModuleExports["process"],
31593160
);
3161+
// Pre-enable any trace event categories requested via the spawning
3162+
// process's --trace-event-categories flag (propagated as an env var by
3163+
// child_process). This must run in every isolate, including workers,
3164+
// so that node.async_hooks tracing covers worker threads too.
3165+
const traceCategoriesEnv = op_get_env_no_permission_check(
3166+
"DENO_NODE_TRACE_EVENT_CATEGORIES",
3167+
);
3168+
if (traceCategoriesEnv) {
3169+
const traceEvents = core.loadExtScript("ext:deno_node/trace_events.ts");
3170+
const categories = traceCategoriesEnv.split(",").filter((c) =>
3171+
c.length > 0
3172+
);
3173+
if (categories.length > 0) {
3174+
// Surface the flag through process.execArgv so test fixtures that
3175+
// probe it (e.g. node's test-trace-events-api) see the same shape
3176+
// they would in Node.
3177+
const proc = nativeModuleExports["process"];
3178+
if (proc && Array.isArray(proc.execArgv)) {
3179+
proc.execArgv.push("--trace-event-categories", traceCategoriesEnv);
3180+
}
3181+
try {
3182+
traceEvents.createTracing({ categories }).enable();
3183+
} catch {
3184+
// Invalid categories must not block startup.
3185+
}
3186+
}
3187+
}
31603188
} else {
31613189
internals.__bootstrapNodeProcess(
31623190
undefined,

ext/node/polyfills/child_process.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,12 @@ function fork(
205205
} else if (options.env?.DENO_NODE_USE_OPENSSL_CA) {
206206
delete options.env.DENO_NODE_USE_OPENSSL_CA;
207207
}
208+
if (result.traceEventCategories) {
209+
options.env = {
210+
...(options.env ?? process.env),
211+
DENO_NODE_TRACE_EVENT_CATEGORIES: result.traceEventCategories,
212+
};
213+
}
208214
}
209215

210216
if (typeof options.stdio === "string") {

ext/node/polyfills/internal/child_process.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1496,6 +1496,9 @@ function buildCommand(
14961496
} else {
14971497
delete env.DENO_NODE_USE_OPENSSL_CA;
14981498
}
1499+
if (result.traceEventCategories) {
1500+
env.DENO_NODE_TRACE_EVENT_CATEGORIES = result.traceEventCategories;
1501+
}
14991502

15001503
// Update NODE_OPTIONS if needed
15011504
if (result.nodeOptions.length > 0) {

0 commit comments

Comments
 (0)