Repository navigation
diagnostics_channel: opt-in subscriber suppression #63623
Description
What is the problem this feature will solve?
APM agents and other tools that subscribe to diagnostics channels often
need to suppress nested instrumentation when their own internal code calls
into an instrumented library. Example: a tracer's HTTP exporter
sends a span over HTTP, the HTTP client is instrumented, the exporter
recurses unless the nested call is suppressed.
Every implementation I have looked at solves it the same way: a custom
AsyncLocalStorage carrying a { noop: true } marker, plus a wrapper
closure around every subscribe and bindStore registration that reads
the marker and short-circuits. Datadog's dd-trace-js does it and I believe the other major APMs do equivalent things. That wrapper runs on every publish, not
only on the suppression path. Thus, we pay one extra JS frame, one extra ALS lookup,
one extra branch per subscribed handler, on the hot path of every channel
in the process.
What is the feature you are proposing to solve the problem?
Move the suppression check inside Channel.prototype.publish and
Channel.prototype.runStores, gated by per-subscription opt-in:
const { channel, suppressed } = require('node:diagnostics_channel')
const kMyTracer = Symbol('my-tracer')
const ch = channel('apm:foo:bar')
ch.subscribe(handler, { suppressedBy: kMyTracer })
ch.bindStore(als, transform, { suppressedBy: kMyTracer })
suppressed(kMyTracer, () => instrumentedCall())suppressedBy names the Symbol that identifies the suppression scope. suppressed(symbol, fn) enters that scope on the active AsyncContextFrame. Subscribers that opted in are skipped while the scope is active. Subscribers that did not opt in keep firing. That gives multi-APM coordination, and it does not change semantics for any existing caller.
Wins: The userland wrapper closure on every subscribed handler goes away. The check folds into the publish loop where diagnostic_channel already iterates subscribers, and V8 inlines it against the (always-empty in production) marker.
Vendors stop re-implementing the same ALS dance, each with their own subtle differences in how the marker propagates across setImmediate, queueMicrotask, and unhandled-rejection paths.
There are probably some details to be fledged out but I wanted to get some feedback about the overall idea first. @bengl @timfish @Qard @trentm and others: opinions?
What alternatives have you considered?
No response
Activity
isaacs commentedon May 28, 2026
This seems pretty useful and straightforward, and adding the feature in the diagnostics_channel implementation is the correct choke-point to add this kind of functionality.
My thoughts:
{ noop: true }check in the subscriber function anyway. But other platforms are likely to follow soon-ish, and at least it's forward-looking, so that's good. We'll also have to do this for legacy node versions, for example, so "not everywhere already" is not really an objection per se, of course, but if Bun, Deno, Cloudflare, et al, are opposed in principle, that'd make this less good, for sure.suppressedByvalue need to be a Symbol per se? Or will any===-identified value do the trick? (I'm assuming it'd be fine to just say "anything that===thesuppressedByvalue will suppress the subscriber", because it's stored in a Map or something.)BridgeAR commentedon May 28, 2026
For bun/deno: we have
dc-polyfillthat could just introduce the functionality.suppressedByaccepting any reference equal data: fine with me. Creating a empty object to use it as reference point should work as well.bengl commentedon May 28, 2026
Overall, the intent makes a lot of sense here. I'm a little confused by the API. Wrapping functions generally requires active instrumentation, so I'd like to avoid that if we can. Is something like an
enterWith()-esque function possible here? Something likech.suppressFor(MyTracer), and then it's simply suppressed for rest of the ALS chain. That should cover nearly all use cases (indeed, any Datadog ones I'm aware of), but if it doesn't, a symmetricch.unsuppressFor(MyTracer)can restore it for the rest of the ALS chain.I'd like to see this (or whatever comes of this) implemented in
dc-polyfill. (Also, whoops, Ruben beat me to it 😄.)bengl commentedon May 28, 2026
Two more suggestions.
s/suppressedBy/subscriberId/g. First of all, it's not who is suppressing it, it's who it's being suppressed for. Second, the ID could have future use cases, and suppression only happens if/whensuppressis called.ignoreinstead ofsuppress. The effect we're going for here is to allow the subscriber to ignore events in particular cases. "Suppress" is a word with a lot of baggage we should maybe try to avoid. I also find it vague enough to find it unclear what the intended effect is without combing through docs.DivyanshuX9 commentedon May 29, 2026
Hey @BridgeAR @bengl @Qard,
Really glad this is being surfaced as a first-class feature , I've run into
exactly this recursion problem and the userland ALS dance is painful to get right.
A few thoughts before I take a stab at implementing this:
On the API shape:
I lean toward @bengl's suggestion of separating the identity (
subscriberId)from the suppression verb. Something like:
This keeps the door open for future uses of the ID (e.g. priority,
filtering, debugging) without baking "suppression" into the registration
permanently.
On the internal implementation:
My plan would be a single internal AsyncLocalStorage carrying a
Setofcurrently active keys, cloned on each
suppressed()entry so nested callscompose correctly without mutating the parent context. The check would fold
into the existing subscriber iteration loop in
publish()andrunStores(),so non-opted subscribers pay zero cost.
On edge cases I want to make sure I cover:
suppressed()calls with same key (Set.add is idempotent, safe)unsubscribe()correctly cleaning up the subscriberId metadatabindStore()getting the same opt-in skip logic assubscribe()One open question:
Should
suppressedBy/subscriberIdbe restricted to Symbols and objects only(reference equality), and throw
ERR_INVALID_ARG_TYPEfor primitives likestrings? I think yes — a string key would be too easy to accidentally collide
across vendors. Symbols feel like the right primitive here.
On dc-polyfill:
Happy to mirror the implementation there too so Bun/Deno users aren't left out
while runtimes catch up.
Would love a thumbs up on the general direction before I open a PR.
Happy to start with just
lib/diagnostics_channel.js+ testsand leave the TracingChannel wiring for a follow-up if that's cleaner.
DivyanshuX9 commentedon May 29, 2026
I did some and would love to if any of member would guide if its in a right direction or not
PR #63651
github-actions commentedon Aug 28, 2026
This issue has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.
github-actions commentedon Sep 28, 2026
This issue has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 120 days).
If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.