Surface missing or late diagnostics-channel injection instead of silently dropping spans #25677

Activity

1. linear-code commented on Oct 23, 2026

   

   linear-codeBot

   on Oct 23, 2026 – with Linear Code

   More actions

   JS-3990
2. linear-code

   added

   Bug

   javascriptPull requests that update javascript codePull requests that update javascript code

   on Oct 23, 2026
3. mydea

   assigned

   isaacs

   on Oct 23, 2026
4. isaacs commented on Oct 23, 2026

   

   isaacs

   on Oct 23, 2026

   Member

   More actions

   One tiny correction: as of #25546, Deno does register on init(), just like Node. But of course, that doesn't change the overall point, just means that Deno needs effectively the same detection as Node. Ie, we can't shortcut Deno by warning whenever --preload is missing, since init() might be doing it.
5. isaacs commented on Oct 23, 2026

   

   isaacs

   on Oct 23, 2026

   Member

   More actions

   Digging a bit deeper into this:

   Avoiding a class of user footguns is appealing. But there's some obstacles to doing all three parts of this as described.

   Warn by default when nothing is injected, on every runtime.

   On Node and Deno, a successful registration in the client constructor sets marker.runtime = [] before detection runs. The "nothing injected" branch is then only reached in three cases:

   1. Registration failed. That already warns without debug.
   2. There is no hook API. That only warns with debug, but also, all supported versions of node and deno have either Module.registerHooks or Module.register, so there's really no gap there to worry about.
   3. The user set enableRuntimeChannelInjection: false and has no bundler. They asked for that, so a default warning would be noise.

   The other case for Node and Deno where nothing gets injected is if there's no preload, and modules get loaded before calling Sentry.init(), discussed below, because that's Item 2.

   That leaves Bun and Cloudflare as the cases this item really fixes, and "cannot be a false positive" is not true:

   • An app with tracing on may use no orchestrion-instrumented libraries at all, for example only Bun.serve or plain Workers fetch handlers. So they're getting spans, but from our pov, nothing was injected and no module was instrumented.
   • Users can bring their own OTel instrumentations.
   • Such apps would get a warning on every start, or on every isolate start on Cloudflare, which spams the logs.
   • A Cloudflare user who builds with wrangler/esbuild instead of Vite has no plugin to install. So, the warning gives them no fix. (Officially we only support Cloudflare with Vite, I believe, at least for tracing, so this might not be too relevant.)

   Cloudflare detection has to look at __SENTRY_ORCHESTRION__.integrations (packages/cloudflare/src/baseSdk.ts line 38), not only at runtime/bundler. The Vite plugin writes there, and detectOrchestrionSetup() does not read it.

   scan require.cache for libraries loaded before init()

   require.cache doesn't work on ESM, and there's no corollary to require.cache for ESM modules. So we'd have to decide that it's worth detecting this sort of shortcoming, but only in CommonJS modules, which are likely to continue to drop in popularity.

   • Matching node_modules/<name>/ is too coarse. The configs target specific files: module.filePath, e.g. mysql → lib/Connection.js. If a package has loaded but its target file has not, that file still gets instrumented when it loads later. Packages that load sub-files lazily (AWS SDK, Prisma, AI SDKs) would warn wrongly. So we'd have to match node_modules/<name>/<filePath> instead, and normalize Windows separators.
   • The scan must run only when registration actually happens inside the constructor. If --import already registered the hooks, modules in the cache were already transformed. The early return in registerDiagnosticsChannelInjection handles this as long as the scan lives after that guard.
   • A loaded version outside versionRange would never be traced anyway, so "use --import" is the wrong fix for it. We'll have to either check the version, or word the message more softly.

   Imo, being unable to detect this failure for ESM pretty dramatically reduces the value of this scan.

   report the injection state in SDK metadata

   This is reasonable, and imo a good idea. We need to decide the field name.

   • The envelope header sdk carries only name and version.
   • The event sdk has features (a string array) and settings, which Relay keeps.
   • sdk.features: ['orchestrion:runtime'] or similar would avoid schema work.
   • Under span streaming, check which payload actually carries the event sdk.
   • The bundler Set is empty at boot and fills as modules load. "bundler" therefore means "the plugin ran", not "the plugin instrumented something". That matches the proposed runtime/bundler/both/none values, but the data has to be read that way.