cloudflare / cloudflare/vinext

Node build bundles serverExternalPackages, breaking OpenTelemetry / APM auto-instrumentation

Open
#2,515 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
8.8k
Forks
406
Avg merge
2d 6h
Merged PRs (30d)
120

Description

## Summary

On a Node server build, packages listed in `serverExternalPackages` still get bundled into per package files under `.output/server/_libs/*.mjs` and imported by a relative path. Runtime APM agents (Sentry, Datadog, New Relic, and anything built on OpenTelemetry) patch modules by their bare package name using `import-in-the-middle` and `require-in-the-middle`. A relative import like `../_libs/pg.mjs` never matches the `pg` hook, so the instrumentation silently never applies and you get zero DB / cache spans in production.

This used to work on Next.js because `serverExternalPackages` kept those packages as real bare imports resolved from `node_modules`, which the hooks could match. On vinext it looks like a regression for anyone relying on APM auto-instrumentation.

## What I see

For a serverExternalPackage that has an OTel instrumentation (say `pg`, which is in the default list), the built server does:

```js
import { ... } from "../_libs/pg.mjs"; // bundled, relative, not hookable
```

instead of:

```js
import { ... } from "pg"; // bare, hookable
```

Only packages with native bindings (bcrypt, @napi-rs/canvas, and similar) stay bare in `.output/server/node_modules`. Everything else, including the DB and cache drivers people most want traced (`pg`, `ioredis`, `tedious`, `mysql2`, `mongodb`), gets rolled into `_libs` and becomes invisible to the agents.

My read is that `serverExternalPackages` only affects the Vite SSR/RSC stage (it keeps them out of that bundle), but the Nitro trace/bundle stage that produces `_libs` re-bundles them anyway.

## Why it matters

Auto-instrumentation is the reason people drop in Sentry or an OTel SDK in the first place. With the current behavior it silently does nothing for server side DB and cache calls. There is no error, just missing spans, so it is very easy to ship without noticing, and it is a real gap versus Next.js for anyone migrating.

## Secondary point: init ordering

Even once a driver is bare and hookable, there is a second problem. `instrumentation.ts` `register()` runs lazily on the first request (via `ensureInstrumentation()` at the top of the app-rsc handler), after the server entry has already imported the driver at module eval time. ESM static imports are evaluated before the importing module's body runs, so the hook is installed too late to catch the driver. Next.js handles this for ESM by preloading the instrumentation with `node --import` before the app graph loads. vinext does not wire that up, so the agent init has to run in an earlier module graph to catch the load. This is separate from the bundling issue, but you need both solved to actually get spans.

## Reproduction (generic)

1. App with a serverExternalPackage that has an OTel instrumentation, for example `pg` with `@opentelemetry/instrumentation-pg` (or `@sentry/node` with `postgresIntegration()`).
2. Init the agent at startup.
3. Build for the node server target, run it, hit a route that queries the DB.
4. No `db.*` span is produced. Inspect `.output/server` and you find `_libs/pg.mjs` imported by a relative path, and `pg` is not present under `.output/server/node_modules`.

## Possible solutions

1. Route `serverExternalPackages` into the Nitro trace step so those packages stay bare externals in `.output/server/node_modules` (copied, not bundled) on a node target. Right now the manual workaround is to pass them to Nitro's `traceDeps` yourself, but ideally `serverExternalPackages` already implies that.
2. Provide a first class way to preload an instrumentation entry before the app module graph on the node target, the equivalent of what Next.js does with `--import`, so the agent init runs before any traced module loads. Today you have to add `--import ./instrument.mjs` to the start command by hand.
3. At minimum, document that `serverExternalPackages` does not keep packages hookable for APM on a node target, and document the `traceDeps` plus `--import` workaround.

## Workaround for anyone hitting this now

1. Add the drivers you want traced to Nitro's `traceDeps` so they land bare in `node_modules`. Also add the agent packages your preload imports (for example `@sentry/node` and its OTel deps), otherwise the preload fails at runtime with `ERR_MODULE_NOT_FOUND` because those are bundled into `_libs` too and are not resolvable from the runner's `node_modules`.
2. Start the server with `node --import ./instrument.mjs .output/server/index.mjs` so the agent inits before the app graph.
3. For CJS packages without an `exports` field (for example `ioredis` and `tedious`), `import-in-the-middle` does not intercept them, only `require-in-the-middle` does. Forcing a `require()` of them right after init (via `createRequire`) triggers the require hook and patches the prototype, which later ESM imports inherit.

Verified end to end on a Node server build in a real container: with the packages in `traceDeps` and the `--import` preload, `pg`, `ioredis`, and `tedious` all get patched and emit real `db.*` spans. Without `traceDeps` they stay in `_libs` and nothing is instrumented.

## Environment

- vinext `0.2.1`
- vite `8.1.3`
- nitro `nitro-nightly@3.0.1-20260702-173353-03122331` (Nitro v3, rolldown)
- Node `25` (also relevant on the vinext minimum of `>=22`)
- Server target: node
- Agent: `@sentry/node` `10.58.0` (`@sentry/core` `10.58.0`), which uses `@opentelemetry/instrumentation` `0.215.0`, `import-in-the-middle` `3.1.0`, and `require-in-the-middle` `8.0.1`. The same class of problem applies to a plain `@opentelemetry/sdk-node` setup or any other APM agent built on those loader hooks.

Contributor guide

Open the contributing guide

Research direction

Start by tracing how serverExternalPackages flows through the Vite SSR/RSC stage into Nitro's trace/bundle stage, then inspect .output/server/_libs and .output/server/node_modules in the reproduction. Review instrumentation.ts, ensureInstrumentation(), and the app-rsc handler for the initialization ordering. Done means externalized packages remain bare and preload initialization runs before the app module graph, with the supplied APM reproduction producing database spans.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript, vite
Domain
backend, build-system
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.