DataDog / dd-trace-js

JavaScript APM Tracer
https://docs.datadoghq.com/tracing/
Other
650 stars 306 forks source link

Error with ESM support when using the `openai` package #4277

Open dyarleniber opened 6 months ago

dyarleniber commented 6 months ago

When running a Node.js (v20) server with ESM modules using dd-trace and openai packages, the process crashes with the following error:

➜  dd-trace-openai-nodejs-example git:(main) node --env-file=.env --import dd-trace/register.js index.js
file:///Users/admin/workspace/personal/dd-trace-openai-nodejs-example/node_modules/openai/resources/index.mjs?iitm=true:72
    let $Completions = namespace.Completions
        ^

SyntaxError: Identifier '$Completions' has already been declared
    at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:167:18)
    at callTranslator (node:internal/modules/esm/loader:285:14)
    at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:291:30)

Node.js v20.11.0

Expected Behavior

Running node --import dd-trace/register.js index.js should start the server without crashing.

Steps to Reproduce

The following repository contains the code to reproduce the issue: dd-trace-openai-nodejs-example.

Using Node.js v20.11.0:

  1. Run npm init -y
  2. Run npm i dd-trace@5.12.0 openai@4.40.0
  3. Add to the package.json file:
{
  "type": "module"
}
  1. Import dd-trace and openai in the index.js file:
import dataDogTracer from "dd-trace";
import OpenAI from "openai";

dataDogTracer.init({
  logInjection: true,
  runtimeMetrics: true,
});

const openAIClient = new OpenAI({
  apiKey: process.env.OPENAI_API_KEY,
});

...
  1. Run node --import dd-trace/register.js index.js

The following repository contains the code to reproduce the issue: dd-trace-openai-nodejs-example.

Your Environment

rattrayalex commented 2 months ago

This may be similar to https://github.com/getsentry/sentry-javascript/issues/12414 and thus https://github.com/openai/openai-node/issues/1010 and https://github.com/openai/openai-node/issues/903.

If that is the case, this problem would likely now surface like this:

TypeError: getDefaultAgent is not a function
    at OpenAI.buildRequest (file:///my-project/node_modules/openai/core.mjs:208:66)
    at OpenAI.makeRequest (file:///my-project/node_modules/openai/core.mjs:279:44)

and the workaround is to do this:

// loader.mjs
import { register } from "node:module";

register("import-in-the-middle/hook.mjs", import.meta.url, {
  parentURL: import.meta.url,
  data: { include: ["openai"]}, // this is the important bit here
});

which I believe may need to be done on the Datadog side.

rattrayalex commented 2 months ago

According to @guidev, this bug (or perhaps another one?) makes Datadog unusable in any node projects that use the openai node library, which gets almost as many weekly downloads as dd-trace.

@sabrenner, worth a look?

adam-on-tailwind commented 2 months ago

FYI the loader.mjs workaround prevented the error in my project, but also stopped traces from being sent. The way I was able to fix this was by importing the openai web shims using the --import flag, enabling experimental fetch --experimental-fetch and supplying an http Agent to the openai client at initialization.

Start node like this:

node --import openai/shims/web --import dd-trace/register.js --require dd-trace/init --experimental-fetch src/main/index.js

When initializing the openai client:

import 'http'
import OpenAI from 'openai'

const openai = new OpenAI({
  httpAgent: new http.Agent()
})

And it all works! Same steps apply if you're using Anthropic btw.

sabrenner commented 2 months ago

Thanks for the ping @rattrayalex! I'm wondering if folks here have tried @adam-on-tailwind's solution, and how it worked for them.

In any case, I try out including the { data: { include: ['openai'] } } workaround in dd-trace's register.js, and verify that using OpenAI doesn't break when using dd-trace. From what I've seen, just doing this won't fix tracing of OpenAI when using ESM, just resolve the TypeError: getDefaultAgent is not a function error. But as noted here and on other threads, I think that is an underlying issue in import-in-the-middle itself, and I'll work with some other folks to see what we can do about that.

rattrayalex commented 2 months ago

Awesome, thanks Sam! Let me know if there's anything I can do to help.