[timfish]

This adds a diagnostics_channel TracingChannel for each node:fs operation. It is a different approach to #65370, based on the review feedback there.

Each operation publishes to tracing:fs.<operation>:*, for example tracing:fs.stat:start. The sync, callback and promise forms of an operation share a channel, so a subscriber to fs.readFile sees fs.readFile(), fs.readFileSync(), fsPromises.readFile() and filehandle.readFile(). The context has api ('sync', 'callback' or 'promise'), args (a copy of the arguments as passed), and the leading arguments by name, such as path, fd and dest. The docs have the full list.

The events are published from JS with traceSync(), traceCallback() and tracePromise(), so all events of one call share one context object, error is always followed by end or asyncEnd, asyncStart has result, and bindStore() works. Buffers and the other arguments are passed by reference in args. The readFile and writeFile one-round-trip paths, fsPromises.readFile(), readFileSync() with an encoding, existsSync() and the FileHandle methods all publish.

Each traced function starts with one line, for example if (shouldTraceFs('stat')) return traceFsCallback('stat', stat, this, arguments);. When the channel has subscribers, the helper calls the same function again inside the trace, and a flag makes that inner call skip the check. The whole body then runs inside the start scope. This matters for bindStore(): the FSReqCallback or binding promise must be created inside the scope, because a request created before it does not carry the store to the callback. Nested fs calls also run inside the parent's store. Because the check is in the function body, captured references and ESM named imports still publish. When nothing subscribes, the rest of the body is the same as on main.

Calls with invalid arguments publish start, error and end. Calls that a VFS handler serves also publish. fsPromises.glob(), fs.watch(), fs.watchFile() and dir.read() do not publish. An operation that uses other fs operations publishes those too, inside its own events. For example, fs.readFileSync() without an encoding publishes fs.open, fs.read and fs.close inside fs.readFile.

With plain compare.js, the async and stream benchmarks in benchmark/fs do not change, but some short sync benchmarks are slower. With 15 runs and no subscribers, fstatSync was about 5% slower, statSync about 3%, and failing readFileSync calls 5% to 9%. Most of this is the cost of TracingChannel.hasSubscribers before V8 optimizes it. Many of these benchmarks do 10,000 calls in a new process, so most checks run while the getter is still in the interpreter or in Sparkplug. There, the getter calls BoundedChannel.hasSubscribers and Channel.hasSubscribers for all five channels, which costs 60 to 100 ns per check. Once it is optimised, it costs about 3 ns. Next to a 300 to 900 ns sync call, the difference shows.

Once hasSubscribers has been optimized, calling it becomes insignificant apart from sync calls that throw. These are 1-3% slower. When an exception passes through a function that runs as Sparkplug code, V8 finds the bytecode offset with a linear walk from the start of the function (Code::GetBytecodeOffsetForBaselinePC, from Isolate::Throw and LookupExceptionHandlerInTable). So any code before the binding call makes each throw slower. A single if (x.active !== 0) return; costs about 50 ns per failing openSync() there. I did not find a way around this.

I did not change the benchmarks to warm the hasSubscribers getters. A follow-up could make TracingChannel.hasSubscribers cheaper before optimisation.

The new tests cover the event order, the shared context, bindStore() across callbacks, promises and nested calls, and the FileHandle methods. A coverage test fails if a public fs function does not publish, or publishes more than once.

Fixes: #65330
Refs: #65370
CC: @nodejs/diagnostics

---

This PR was partly AI generated but everything was reviewed by a water based human

[codecov]

Codecov Report

❌ Patch coverage is 98.22485% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.52%. Comparing base (df2ea49) to head (87bf499).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/fs.js	98.10%	8 Missing ⚠️
lib/internal/fs/promises.js	97.10%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #66575      +/-   ##
==========================================
+ Coverage   90.51%   90.52%   +0.01%     
==========================================
  Files         797      797              
  Lines      278252   279085     +833     
  Branches    53472    53694     +222     
==========================================
+ Hits       251868   252650     +782     
- Misses      16761    16820      +59     
+ Partials     9623     9615       -8     

Files with missing lines	Coverage Δ
lib/internal/fs/dir.js	94.36% <100.00%> (+0.52%)	⬆️
lib/internal/fs/utils.js	96.67% <100.00%> (+0.37%)	⬆️
lib/internal/fs/promises.js	91.74% <97.10%> (+0.51%)	⬆️
lib/fs.js	97.49% <98.10%> (+0.13%)	⬆️

... and 29 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[hamidrezaghavami]

The implementation in lib/fs.js and lib/internal/fs/promises.js is incredibly clean. I didn't review the C++ bindings, but LGTM on the JS/Promises side!

[jsumners-nr]

Why not?

[Qard]

Rather than having the getFsChannel, shouldTraceFs, and the traceFs* functions, we should be defining the channels at the top level. By using these functions which take a string name for the channels at publish time you're eliminating the performance benefit diagnostic channels have of splitting the lookup cost out of the hot publishing path, leaving it with basically the same performance you'd get with a plain event emitter.

The diagnostics channel design is intended to be used with explicit top-level defined channels which V8 can then optimize the calling of as simply a held pointer from the outer scope which is very fast to reference as V8 will optimize it, rather than doing channel lookups at every publish or check.

[Qard]

Why this FsTracer wrapper type rather than just using the tracing channel instance directly? You're wasting a bunch of computation on every publish of recomputing the message shape for each channel when that would not change between publishes to that channel. Seems to me like it'd be better to just use the tracing channel instances directly, have each publish site craft its own particular shape based on the fields relevant to that channel, and use hasSubscribers in place of the shouldTrace wrapper function. I don't really see a reason for wrapping like this. 🤔

[timfish]

Done in c9571ff.

Each operation now has a TracingChannel constant in internal/fs/utils (fsStat = tracingChannel('fs.stat') and so on), each call site builds its own context literal, and the check is this !== kTraced && fsStat.hasSubscribers. The helpers call the function again with kTraced as this, which replaces the module flag and its try/finally.

Cost of the check at a call site, with the previous two versions for comparison. The main column is the same loop calling a function that only returns false, since main has no check:

Tier	main (no check)	name lookup	module-level tracer	this change
Interpreter	18 ns	146 ns	124 ns	109 ns
Sparkplug	10 ns	80 ns	78 ns	67 ns
Maglev	0.6 ns	7.0 ns	5.1 ns	4.7 ns
TurboFan	~1 ns	2.4 ns	1.2 ns	~1 ns

Against main, the fs benchmarks for the callback and promise forms and for successful sync reads and writes are within noise. Sync calls that throw are 2 to 10% slower, which I think is the cost of the extra frame when Sparkplug walks the bytecode to build the stack trace, not the check itself. Startup is unchanged.