fix: instrument ESM package export submodule

[isaacs]

Add support for patching an ESM internal module that is exported with a submodule path name in the package.json exports object.

Which problem is this PR solving?

It is not currently possible to patch an ESM submodule export.

Fixes # (issue)

Short description of the changes

When determining whether to apply a patch, the file.name is compared against the normalizedPath. However, this makes it impossible to get import-in-the-middle to consider the module a valid hook target.

Allow passing in the submodule specifier as the file name, treating it as the effective "path" to be hooked.

This is the continuation (one level higher in the stack) of nodejs/import-in-the-middle#215

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update (maybe?)

I believe it could be argued that this is a new feature, and perhaps even not the most elegant approach. It does feel a bit clunky, tbh, since the "filename" being passed in is actually a virtual path defined in package.json, not an actual path on disk, since those are fully abstracted away by the ESM module loading semantics. But, I can't see how it would break any existing use cases.

How Has This Been Tested?

Test included in the patch, at experimental/packages/opentelemetry-instrumentation/test/node/EsmSubmoduleInstrumentation.test.mjs

Checklist:

• Followed the style guidelines of this project
• Unit tests have been added
• Documentation has been updated

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: isaacs / name: isaacs (2d95e95)

[isaacs]

ping @open-telemetry/javascript-approvers

Any guidance would be much appreciated, if this isn't the best way to get this done, but it's blocking the instrumenting of certain package submodule exports in ESM mode.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.39%. Comparing base (118f5b5) to head (2d95e95).
⚠️ Report is 33 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6246   +/-   ##
=======================================
  Coverage   95.39%   95.39%           
=======================================
  Files         316      316           
  Lines        9574     9574           
  Branches     2216     2216           
=======================================
  Hits         9133     9133           
  Misses        441      441           

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[JamieDanielson]

Sorry for the radio silence on this, we are still reviewing and will post an update soon.

[trentm]

Assigning myself for now. Thar be dragons... or at least I'm getting stuck on somethings.

Note to self: see getsentry/sentry-javascript#18403 for motivating example.

[trentm]

Sorry for taking so long here. I would like to move the IITM hook in @opentelemetry/instrumentation to using internals: true, then hooking internal files in ESM modules (like @langchain/langgraph that is motivating here) would be straightforward, e.g.:

      new InstrumentationNodeModuleDefinition(
        // This works to hook `@langchain/langgraph` for both CJS and ESM.
        '@langchain/langgraph',
        ['*'],
        function (mod, modVer) {
          console.log('HIT: %s@%s', this.name, modVer);
          return mod;
        },
        null,
        [
          new InstrumentationNodeModuleFile(
            '@langchain/langgraph/dist/prebuilt/react_agent_executor.cjs',
            ['*'],
            function (mod, modVer) {
              console.log('HIT: %s@%s', this.name, modVer);
              return mod;
            },
            mod => mod,
          ),
          new InstrumentationNodeModuleFile(
            '@langchain/langgraph/dist/prebuilt/react_agent_executor.js',
            ['*'],
            function (mod, modVer) {
              console.log('HIT: %s@%s', this.name, modVer);
              return mod;
            },
            mod => mod,
          ),
        ]
      ),

However, there is a hiccup: IITM with internals: true behaves slightly different to RITM, of course in a way that breaks the _onRequire callback in @opentelemetry/instrumentation. See nodejs/import-in-the-middle#238 for that.

I could probably also change the Hook handling in @opentelemetry/instrumentation to work without IITM having to change -- and still might.

[trentm]

The reason I don't want start using sub-module paths / entry-points in Hooks for OTel instrumentation, e.g.:

      new InstrumentationNodeModuleDefinition(
        '@langchain/langgraph/prebuilt',
        supportedVersions,
        this._patch.bind(this),
        exports => exports,
        [
          new InstrumentationNodeModuleFile(
            /**
             * ESM builds use dist/prebuilt/index.js (without .cjs extension)
             * This catches ESM imports that resolve through the main package,
             * using the package.json submodule export
             */
            '@langchain/langgraph/prebuilt',
            supportedVersions,
            this._patch.bind(this),
            exports => exports,
          ),

is because it doesn't always work to hook the target implementation file. It doesn't work if the target file (e.g. .../node_modules/@langchain/langgraph/dist/prebuilt/react_agent_executor.js in this case) is indirectly loaded via some other entry-point to the module.

That can't happen in this particular case -- because AFAICT nothing else in @langchain/langgraph imports that file or the dist/prebuilt/index.js file other than the @langchain/langgraph/prebuilt entry point. However, I had an example back from when a similar feature was added to RITM in nodejs/require-in-the-middle#82

In that case one of the motivating examples was @langchain/core/vectorstores. With both IITM and RITM if you hook that entry-point:

Hook(['@langchain/core/vectorstores'], ...)

it will be matched when a user does:

import ... from '@langchain/core/vectorstores';
// OR
require('@langchain/core/vectorstores')

However, the underlying functionality (in node_modules/@langchain/core/dist/vectorstores.{cjs,js}) would not be matched when indirectly imported via import/require of some other entrypoints: @langchain/core/load, @langchain/core/structured_query.

[trentm]

@isaacs #6344 is finally the alternative that I propose for this.
Closing in favour of that. I assume it'll work for your @langchain/langgraph case, but it would be great if you could confirm that.

[isaacs]

I will check it out, thanks!