v8 Node OpenTelemetry Instrumentation Blockers

[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.

import-in-the-middle

These are import-in-the-middle bugs, which are encountered by Sentry users who want to use Sentry in their ESM app.

import-in-the-middle PRs to be merged

Beta Give feedback

1. Fix resolution of lib/register.js when using multiple loaders DataDog/import-in-the-middle#76
   +0
2. fix: Resolve re-exports of external modules DataDog/import-in-the-middle#78
   +0
3. fix: Duplicate declarations DataDog/import-in-the-middle#79
   +0
4. test: skip static-import on > v21 DataDog/import-in-the-middle#81
   +0
5. fix: Handle cyclical reference to current file DataDog/import-in-the-middle#83
   +0
6. fix: Handle CallExpression in ExportDefaultDeclaration node DataDog/import-in-the-middle#86
   +0
7. fix: handling of default and star exports DataDog/import-in-the-middle#85
   +0
8. feat: Add Hook named export DataDog/import-in-the-middle#88
   +0
9. fix: Update import-in-the-middle open-telemetry/opentelemetry-js#4745
   +0
10. getFullCjsExports does not resolve re-exports from external dependencies DataDog/import-in-the-middle#95
    +0

1. import-in-the-middle does not work with multiple loaders.

Reported issue: #12011
Node.js issue: nodejs/node#52987

Fix: nodejs/import-in-the-middle#76

2. relative resolution fails for some modules

import-in-the-middle expects modules to be relative to the current file, but this doesn't always apply (re-exporting for example). Instead we should just use node's resolve

Reported issue: #12059
IITM issues: nodejs/import-in-the-middle#59, nodejs/import-in-the-middle#62, nodejs/import-in-the-middle#63

Fix: nodejs/import-in-the-middle#78

3. import-in-the-middle breaks with duplicate wildcard exports

Reported issue: #12154
IITM issue: nodejs/import-in-the-middle#60

Fix: nodejs/import-in-the-middle#79

4. Does not handle import * of current file

Reported issue: #12237
IITM issue: nodejs/import-in-the-middle#82

Fix: nodejs/import-in-the-middle#83

5. Does not handle CallExpression in ExportDefaultDeclaration node

IITM issue: nodejs/import-in-the-middle#77

Fix: nodejs/import-in-the-middle#85

6. tsx and --import seems to not play nicely

Reported issue: #12357

Instrumentation and Span Data Quality

This is being tracked by #12109

Performance Degradation with v8 PR.

Tasks

Beta Give feedback

1. Investigate PostgresIntegration for Memory issues #12243
   Feature: Performance Package-Meta: OpenTelemetry Package: node +3
   

PostgresIntegration problems

We've gotten two different reports (#11897, https://discord.com/channels/621778831602221064/1242436171737333810) that using the postgres integration with Sentry is causing memory issues.

[timfish]

Here is a patch for import-in-the-middle that can be used with patch-package which includes all the pending PR's.
import-in-the-middle+1.7.4.patch

If you encounter issues with this patch applied, please open a new issue!

[timfish]

import-in-the-middle@1.8.0 has been released and I opened an otel PR to update it!

[jd-carroll]

FYI - This is fixed, but for the dependencies in @prisma

▲ [WARNING] Constructing "ImportInTheMiddle" will crash at run-time because it's an import namespace object, not a constructor [call-import-namespace]

    ../../node_modules/@prisma/instrumentation/node_modules/@opentelemetry/instrumentation/build/esm/platform/node/instrumentation.js:286:30:
      286 │ ...     var esmHook = new ImportInTheMiddle([module_2.name], { in...
          ╵                           ~~~~~~~~~~~~~~~~~

  Consider changing "ImportInTheMiddle" to a default import instead:

    ../../node_modules/@prisma/instrumentation/node_modules/@opentelemetry/instrumentation/build/esm/platform/node/instrumentation.js:48:7:
      48 │ import * as ImportInTheMiddle from 'import-in-the-middle';
         │        ~~~~~~~~~~~~~~~~~~~~~~
         ╵        ImportInTheMiddle

                            ~~~~~~~

Not sure where / how / when this would be encountered, if at all, but just thought I would share.

Thank you for all the effort in getting the ESM stuff resolved! ❤️

[jd-carroll]

Ohh, I should note:

Adding a resolution takes care of this:

# package.json
  ...
  "resolutions": {
    "@opentelemetry/instrumentation": "0.52.0"
  },
}

I should also note that we do not use any of the @prisma integrations.

[mydea]

Yeah, I guess this is because @prisma/instrumentation has not yet bumped to the latest version of @opentelemetry/instrumentation - I opened a PR for this here: prisma/prisma#24480

A resolution is the best short-term fix for this IMHO! 👍