fs: add per-operation fs diagnostics channels by mcollina · Pull Request #65370 · nodejs/node

mcollina · 2026-08-18T04:20:03Z

Adds built-in node:diagnostics_channel channels for file system operations performed through node:fs and node:fs/promises, addressing #65330.

Each operation gets its own TracingChannel family named fs.<operation> (e.g. fs.open, fs.read, fs.stat), with channels tracing:fs.<operation>:start, :end, :asyncStart, :asyncEnd, and :error. The event payload carries the API (sync/callback/promise), path/dest/fd fields when applicable, plus result/error following TracingChannel conventions. Subscribers can use diagnostics_channel.tracingChannel('fs.open') to subscribe to all events of one operation at once, or subscribe to individual channels by name.

Events are published from the internal shared file system layer rather than the JS wrappers, so captured function references still emit events. Adds documentation and a new test/parallel/test-diagnostics-channel-fs.js.

AI generated, humanly reviewed.

mcollina · 2026-08-18T18:55:33Z

@nodejs/diagnostics wdyt? Should we do this or should we have one event for each operation?

jasnell · 2026-08-18T22:55:46Z

+  if (!path.empty()) {
+    obj->Set(context,
+             env->path_string(),
+             String::NewFromUtf8(isolate,


ToV8Value?

jasnell · 2026-08-18T22:56:35Z

SGTM

timfish · 2026-08-19T12:57:48Z

Does this impact perf. much when the channels are not subscribed to?
What's the impact like when there is a subscriber that does nothing?

Depending on the impact with a subscriber it might be worth having a channel per operation so that we can be selective over which ops we want to impact?

logaretm · 2026-08-19T15:11:02Z

I think having granularity via multiple channels is better because it allows APMs to choose what to subscribe to without adding overhead to NO-OP events.

Add built-in node:diagnostics_channel channels for file system operations performed through node:fs and node:fs/promises. Each operation gets its own TracingChannel family named fs.<operation>, with channels tracing:fs.<operation>:start, :end, :asyncStart, :asyncEnd, and :error. The event payload carries the API (sync/callback/promise), path/dest/fd fields when applicable, plus result/error following TracingChannel conventions. Events are published from the internal shared file system layer rather than the JS wrappers, so captured function references still emit events. Signed-off-by: Matteo Collina <hello@matteocollina.com>

Define the inline channel helpers in node_file-inl.h so that including node_file.h alone builds under -Werror=undefined-inline. Hold the cached channels weakly, as the permission code does, and skip the instrumentation while building a snapshot: linking a native channel creates JS channel objects that cannot be serialized. Signed-off-by: Matteo Collina <hello@matteocollina.com>

The libuv request's file field is private and a union on Windows, so reading it does not compile there. Every descriptor-based libuv fs operation takes the descriptor as its first argument, so pick it up from the call arguments instead. Signed-off-by: Matteo Collina <hello@matteocollina.com>

codecov · 2026-09-17T11:53:57Z

Codecov Report

❌ Patch coverage is 82.38636% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.26%. Comparing base (2d22505) to head (7954bfc).
⚠️ Report is 239 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_file-inl.h	70.58%	13 Missing and 12 partials ⚠️
src/node_file.cc	92.59%	1 Missing and 5 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #65370      +/-   ##
==========================================
+ Coverage   90.19%   90.26%   +0.06%     
==========================================
  Files         770      789      +19     
  Lines      264410   271799    +7389     
  Branches    50243    51879    +1636     
==========================================
+ Hits       238479   245333    +6854     
+ Misses      16926    16919       -7     
- Partials     9005     9547     +542

Files with missing lines	Coverage Δ
src/node_file.h	`81.96% <100.00%> (+3.53%)`	⬆️
src/node_file.cc	`75.64% <92.59%> (+0.76%)`	⬆️
src/node_file-inl.h	`81.59% <70.58%> (-3.47%)`	⬇️

... and 202 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.

bengl · 2026-10-01T09:39:42Z

+* `fd` {number|undefined} The file descriptor for operations that operate on
+  an existing file descriptor, such as `read`, `write`, `fsync`, or `close`.
+
+Large read/write buffers are not copied into the event payload. The `start`


Why not include them? Not copy, but pass the reference.

bengl · 2026-10-01T09:41:25Z

+Large read/write buffers are not copied into the event payload. The `start`
+and `asyncStart` events carry no `result` or `error`; the `end` and `asyncEnd`
+events carry the `result` of the operation, and the `error` event carries the
+`error`, following the [TracingChannel Channels][] conventions.


The convention includes result fields in asyncStart https://nodejs.org/docs/latest/api/diagnostics_channel.html#asyncstartevent

In fact this is necessary for many kinds of instrumentation.

This one is the only blocker, the rest can be handled in future PRs if desired.

bengl · 2026-10-01T09:43:22Z

+`error`, following the [TracingChannel Channels][] conventions.
+
+Operations performed through streams (`fs.createReadStream` and
+`fs.createWriteStream`), most `FileHandle` methods, and the `fs.readFile`


Why not FileHandle?

The streams should be doable with diagnostics channels, but maybe just not TracingChannels.

bengl · 2026-10-01T09:47:27Z

+function reference was captured before subscribing or whether the operation
+uses the callback, promise, or synchronous API.
+
+Each event carries an object with the following common fields:


Other args may be necessary for o11y, for example the args to chmod/chown other than the path.

logaretm

A few comments on top of what @bengl brought up, so doing the channels from C++ as it is now does a few things:

Skips the single context object correlation, I think this is a major blocker. Otherwise APMs cannot use this to create spans and will mostly rely on external maps to guess the correlation.
It also means the context propagation isn't there, because it publishes without runStores, so bindStore() on these channels does nothing. Technically there isn't any nested operations here so it may not matter. So this is more of a nit.
The error event issue that I brought below, the error event is not a terminal event, there must be always an end or asyncEnd after it.

I think switching to the JS API here would solve all of these issues and make sure the publishing semantics are consistent/correct.

logaretm · 2026-10-05T19:07:52Z

+  Isolate* isolate = env->isolate();
+  HandleScope scope(isolate);
+  Local<Context> context = env->context();
+  Local<Object> obj = Object::New(isolate);


Each event gets its own payload object here. Subscribers usually store per-operation state (like a span) on the context object in start and read it back in asyncEnd or error.

With separate objects there's no way to corrolate those events, especially for concurrent calls on the same path.

Could one object be created per operation and reused for every event? If not then it might be better to use the JS API here for those channels.

logaretm · 2026-10-05T19:07:52Z

+Operations performed through streams (`fs.createReadStream` and
+`fs.createWriteStream`), most `FileHandle` methods, and the `fs.readFile`
+fast path (which batches open/stat/read/close into a single background job)
+are not covered by these channel families, and may not emit the full set of
+events.


This says the readFile fast path isn't covered, but the paragraph above says every public fs operation is.

Also, writeFile and fs.promises.readFile skip the channels, and so do readFileSync with an encoding and existsSync. Those are the fs calls we instrument the most.

I think the PR should cover them too, so all the public fs APIs emit.

logaretm · 2026-10-05T19:23:08Z

+                                                 req_wrap->dest_p);
+        PublishFSOperationEvent(env,
+                                *channels,
+                                FSOperationChannel::kError,


On failure this emits error and then stops, with no end after it. traceSync always emits end after error, for APMs end is important because that's when they end the span or flush any telemetry.

logaretm · 2026-10-05T19:23:09Z

 }

 void FSReqCallback::Reject(Local<Value> reject) {
+  PublishFSOpCompletionEvent(this, FSOperationChannel::kError, "error", reject);


Same for callback/promise calls: error is emitted but asyncEnd never follows it, while traceCallback/tracePromise always emit asyncEnd after error.

Qard

As has already been stated, this needs a shared context object between tracing channel events, and we'd also need a proper runStores for this to be complete TracingChannel behaviour.

However, it's also worth pointing out it'd likely actually be faster to just do all this in JS as also already suggested as the entire data and subscription model is already in JS. Putting it in native code adds an extra native barrier cross for not really any reason. We should generally avoid native-side publishing except for cases where there is no viable hook point from JS.

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Aug 18, 2026

mcollina requested review from Qard, RafaelGSS, bengl and jasnell August 18, 2026 18:54

jasnell reviewed Aug 18, 2026

View reviewed changes

mcollina force-pushed the fs-diagnostics-channel branch from 657db86 to 508ffc0 Compare August 28, 2026 18:17

mcollina changed the title ~~fs: add built-in fs.operation diagnostics channels~~ fs: add per-operation fs diagnostics channels Aug 28, 2026

mcollina force-pushed the fs-diagnostics-channel branch from 508ffc0 to 6e1aa61 Compare September 5, 2026 14:17

mcollina marked this pull request as ready for review September 17, 2026 08:09

mcollina added 2 commits September 17, 2026 11:18

bengl requested changes Oct 1, 2026

View reviewed changes

bengl reviewed Oct 1, 2026

View reviewed changes

logaretm reviewed Oct 5, 2026

View reviewed changes

Qard reviewed Oct 6, 2026

View reviewed changes

timfish mentioned this pull request Oct 7, 2026

fs: add diagnostics_channel tracing #66575

Open

Conversation

mcollina commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcollina commented Aug 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasnell commented Aug 18, 2026

Uh oh!

timfish commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

logaretm commented Aug 19, 2026

Uh oh!

codecov Bot commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

logaretm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Qard left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mcollina commented Aug 18, 2026 •

edited

Loading

timfish commented Aug 19, 2026 •

edited

Loading

codecov Bot commented Sep 17, 2026 •

edited

Loading