Sentry Node.js ESM + OTEL instrumentation blockers - Part 3

[AbhiPrasad]

With the release of v8 of the Sentry SDK, the Node SDK now relies on OpenTelemetry. OpenTelemetry instrumentation does have some problems though (particularly with ESM because of import-in-the-middle), so this issue aims at documented these gaps.

Part 1: https://github.com/getsentry/sentry-javascript/issues/12242 Part 2: https://github.com/getsentry/sentry-javascript/issues/12485

With 8.15.0 + import-in-the-middle@1.9.0 we released most of the fixes for Part 2, but there are a new set of bugs for us to release fixes for. This is tracked in this issue.

### import-in-the-middle PRs to be merged
- [ ] https://github.com/nodejs/import-in-the-middle/pull/115
- [ ] https://github.com/nodejs/import-in-the-middle/pull/142
- [ ] https://github.com/nodejs/import-in-the-middle/pull/140
- [ ] https://github.com/nodejs/import-in-the-middle/pull/145
- [ ] https://github.com/nodejs/import-in-the-middle/pull/146
- [ ] https://github.com/nodejs/import-in-the-middle/pull/148

### Sentry issues
- [ ] https://github.com/getsentry/sentry-javascript/issues/12414
- [ ] https://github.com/getsentry/sentry-javascript/issues/12622
- [ ] https://github.com/getsentry/sentry-javascript/issues/12807
- [ ] https://github.com/getsentry/sentry-javascript/issues/12912

1. openai shims causes issues

Issue: https://github.com/getsentry/sentry-javascript/issues/12414

IITM issue: https://github.com/nodejs/import-in-the-middle/issues/38 Asking about this to Node.js loaders team: https://github.com/nodejs/loaders/pull/198#discussion_r1664546188

2. CJS identifiers that don't work in ESM break

Issue: https://github.com/getsentry/sentry-javascript/issues/12622

IITM issue: https://github.com/nodejs/import-in-the-middle/issues/94 Fix: https://github.com/nodejs/import-in-the-middle/pull/115

3. Unexpected closing brace error from import-in-the-middle (affects discordjs)

Issue: https://github.com/getsentry/sentry-javascript/issues/12807

Repro: https://github.com/bubooon/sentry-nuxt-nitro-example

4. No spans are created for Node http.get calls in ESM mode

Issue: https://github.com/open-telemetry/opentelemetry-js/issues/4857

Repro: https://github.com/Lms24/otel-node-http-get-esm-reproduction

[RIP21]

Unsure if it's related to the import-in-the-middle but I have problems with drizzle-orm imports.

Like that

import { eq, ilike, or } from "drizzle-orm";
         ^^
SyntaxError: The requested module 'drizzle-orm' does not provide an export named 'eq'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:134:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:217:5)
    at async ModuleLoader.import (node:internal/modules/esm/loader:316:24)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:123:5)

Node.js v20.13.1

This is how I'm running my node:

 node --import @sentry/node/preload --import @swc-node/register/esm-register ./src/main.ts

[timfish]

@RIP21 this issue tracked here:

• https://github.com/getsentry/sentry-javascript/issues/12912

[timfish]

I'm hoping that this import-in-the-middle PR should finally close a lot of these issues:

• https://github.com/nodejs/import-in-the-middle/pull/146

For a bit of context... import-in-the-middle and instrumentation of ESM in Node is fundamentally broken in a couple of ways. Since ES modules are immutable, import-in-the-middle creates wrapper modules for every module which allows for changes to be made after they are loaded. Unfortunately, wrapper modules do not allow import-in-the-middle to fully represent what a regular module looks like to those consuming it.

The above PR should allow us to signal to import-in-the-middle to only wrap the libraries that we intend to later instrument. While this doesn't fix the fundamental issues surrounding instrumenting ESM, it significantly reduces the number of libraries that import-in-the-middle needs to be compatible with.

[AbhiPrasad]

We've released https://github.com/getsentry/sentry-javascript/releases/tag/8.29.0 which includes https://github.com/getsentry/sentry-javascript/pull/13139 as part of this PR.

As such I think we can close this issue.

@timfish mind helping follow up on the linked issues?

[timfish]

Yep and I think many of the issues are assigned to me which should help!