Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(opentelemetry-node): improved ESM instrumentation support #499

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

trentm
Copy link
Member

@trentm trentm commented Dec 21, 2024

This updates to usage of IITM's support for only hooking modules intended
to be hooked (added in IITM 1.11.0, see nodejs/import-in-the-middle#146).
This helps workaround cases where IITM hooking breaks some modules.
The openai-chat.mjs is one such example.

current status

This PR is incomplete (see XXXs and TODOs in the diff).

  • We need to think about and sort out the explicit import-in-the-middle dep, and try to see if there are bad side-effects from this if there are multiple versions of import-in-the-middle in play. Upstream @opentelemetry/instrumentation has this dep: "import-in-the-middle": "^1.8.1",, so it is possible to have too old of an IITM installed. There are also potential issues if multiple separate IITM installations are used. I think we need the explicit diff, but there can still be issues if a user brings their own instrumentation along with its own IITM install.

(/cc @codefromthecrypt This will allow instrumenting openai usage used from ESM code.)

This updates to usage of IITM's support for only hooking modules intended
to be hooked (added in IITM 1.11.0, see nodejs/import-in-the-middle#146).
This helps workaround cases where IITM hooking breaks some modules.
The openai-chat.mjs is one such example.
@trentm trentm requested a review from david-luna December 21, 2024 00:46
@trentm trentm self-assigned this Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant