DataDog / dd-trace-js

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

Cannot plug dd-trace to NextJS instrumentation module #3457

Open thollander opened 11 months ago

thollander commented 11 months ago

Hello,

I'd like to use the predefined configuration of NextJS instrumentation (experimental feature) to plug dd-trace.

This way of working permits to have the dependency really used inside the application and to benefit from NextJS Standalone feature (because if we pass through the NODE_OPTIONS="-r dd-trace/init", dd-trace gets removed on production Docker build as NextJS considers it's not "unused").

Not really sure on if this issue should be open on dd-trace side or NextJS, but as the error seems to be coming from dd-trace file, I begin to open it here.

Expected behaviour I would like to be able to plug instrumentation without error.

Actual behaviour We currently have issues on requiring some "nodejs" module inside dd-trace.

Steps to reproduce Repro : https://github.com/thollander/repro-datadog-nextjs

yarn install 
yarn build

Here is the stack trace :

❯ yarn build

> @ build /Users/TERENCE/Dev/workspace-perso/repro-datadog-nextjs
> next build

- warn You have enabled experimental feature (instrumentationHook) in next.config.js.
- warn Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use at your own risk.

Failed to compile.

./node_modules/@datadog/native-iast-rewriter/js/source-map/index.js:2:0
Module not found: Can't resolve 'path'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module:
./node_modules/@datadog/native-iast-rewriter/main.js
./node_modules/dd-trace/packages/dd-trace/src/appsec/iast/taint-tracking/rewriter.js
./node_modules/dd-trace/packages/dd-trace/src/appsec/iast/taint-tracking/index.js
./node_modules/dd-trace/packages/dd-trace/src/appsec/iast/index.js
./node_modules/dd-trace/packages/dd-trace/src/proxy.js
./node_modules/dd-trace/packages/dd-trace/src/index.js
./node_modules/dd-trace/packages/dd-trace/index.js
./node_modules/dd-trace/index.js
./instrumentation.ts

./node_modules/@datadog/native-iast-rewriter/js/source-map/index.js:3:0
Module not found: Can't resolve 'fs'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module:
./node_modules/@datadog/native-iast-rewriter/main.js
./node_modules/dd-trace/packages/dd-trace/src/appsec/iast/taint-tracking/rewriter.js
./node_modules/dd-trace/packages/dd-trace/src/appsec/iast/taint-tracking/index.js
./node_modules/dd-trace/packages/dd-trace/src/appsec/iast/index.js
./node_modules/dd-trace/packages/dd-trace/src/proxy.js
./node_modules/dd-trace/packages/dd-trace/src/index.js
./node_modules/dd-trace/packages/dd-trace/index.js
./node_modules/dd-trace/index.js
./instrumentation.ts

./node_modules/@datadog/native-iast-rewriter/wasm/wasm_iast_rewriter.js:506:0
Module not found: Can't resolve 'path'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module:
./node_modules/@datadog/native-iast-rewriter/main.js
./node_modules/dd-trace/packages/dd-trace/src/appsec/iast/taint-tracking/rewriter.js
./node_modules/dd-trace/packages/dd-trace/src/appsec/iast/taint-tracking/index.js
./node_modules/dd-trace/packages/dd-trace/src/appsec/iast/index.js
./node_modules/dd-trace/packages/dd-trace/src/proxy.js
./node_modules/dd-trace/packages/dd-trace/src/index.js
./node_modules/dd-trace/packages/dd-trace/index.js
./node_modules/dd-trace/index.js
./instrumentation.ts

./node_modules/@datadog/native-iast-rewriter/wasm/wasm_iast_rewriter.js:507:0
Module not found: Can't resolve 'fs'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module:
./node_modules/@datadog/native-iast-rewriter/main.js
./node_modules/dd-trace/packages/dd-trace/src/appsec/iast/taint-tracking/rewriter.js
./node_modules/dd-trace/packages/dd-trace/src/appsec/iast/taint-tracking/index.js
./node_modules/dd-trace/packages/dd-trace/src/appsec/iast/index.js
./node_modules/dd-trace/packages/dd-trace/src/proxy.js
./node_modules/dd-trace/packages/dd-trace/src/index.js
./node_modules/dd-trace/packages/dd-trace/index.js
./node_modules/dd-trace/index.js
./instrumentation.ts

./node_modules/@datadog/pprof/out/src/heap-profiler-bindings.js:42:0
Module not found: Can't resolve 'path'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module:
./node_modules/@datadog/pprof/out/src/heap-profiler.js
./node_modules/@datadog/pprof/out/src/index.js
./node_modules/dd-trace/packages/dd-trace/src/profiling/profiler.js
./node_modules/dd-trace/packages/dd-trace/src/profiling/index.js
./node_modules/dd-trace/packages/dd-trace/src/profiler.js
./node_modules/dd-trace/packages/dd-trace/src/proxy.js
./node_modules/dd-trace/packages/dd-trace/src/index.js
./node_modules/dd-trace/packages/dd-trace/index.js
./node_modules/dd-trace/index.js
./instrumentation.ts

> Build failed because of webpack errors
- info Creating an optimized production build . ELIFECYCLE  Command failed with exit code 1.

Environment

Meemaw commented 11 months ago

You should init dd-trace conditionally like this:

if (process.env.NEXT_RUNTIME === "nodejs") {
  // init dd-trace
}

That being said, I've noticed that when you instrument dd-trace like this, next.request spans are lost. I believe this is because instrumentation hook is called too late, after the server has been started.

thollander commented 11 months ago

Thanks for advising, but sadly it still doesn't work (updated the repro repo). I have the same error as previously.

zomgbre commented 11 months ago

I was able to have success with this: File is in src/instrumentation.ts

export async function register() {
  if (process.env.NEXT_RUNTIME === "nodejs") {
    const tracerLib = await import("dd-trace");
    const tracer = tracerLib.default;

    tracer.init({ logInjection: true });
    tracer.use("next");
  }
}

There are still weird things about it which I will open a separate ticket for. (Log injection for pino not quite working right. Not sure if I need the tracer.use piece. Also resource name in APM showing as GET/POST and not the path.)

It is also important to have ENV variables set appropriately for your configuration.

thollander commented 11 months ago

Thanks @zomgbre, it works indeed. 👍🏻 And I have the same issue as you do with resource being the HTTP method instead of the path. image

However, the spans traces are fine with correct URL.

Quramy commented 9 months ago

That being said, I've noticed that when you instrument dd-trace like this, next.request spans are lost.

I have the same issue.

Are there some workarounds for this?

psimk commented 9 months ago

hey, we're also having the same issue after upgrading to Next 13.5, are there any workarounds for this?

dariuskul commented 5 months ago

Any updates on this?

Kevin-McGonigle commented 5 months ago

As suggested by @Qard here, one possible solution might be using the loader hook provided by dd-trace to support ESM: node --loader dd-trace/loader-hook.mjs your-app.js.

I have, however, found that the loader-hook.mjs file is being tree-shaken during the standalone build (or something similar), and have yet to find a way to get it to stick around 😅 If anyone does get this working, please let us know!

Kevin-McGonigle commented 5 months ago

A workaround I've just found to have the resource names set correctly is to set it via the http plugin:

export async function register(): Promise<void> {
  if (process.env.NEXT_RUNTIME === 'nodejs') {
    const tracerLib = await import('dd-trace');
    const tracer = tracerLib.default;

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

    tracer.use('http', {
      hooks: {
        request(span, req) {
          if (span && req) {
            const url = 'path' in req ? req.path : req.url;
            if (url) {
              const method = req.method;
              span.setTag('resource.name', method ? `${method} ${url}` : url);
            }
          }
        },
      },
    });
  }
}
jamesbrooks94 commented 4 months ago

After MONTHS of not being able to get log injection with winston working, adding it to this next.config.js configuration worked! https://nextjs.org/docs/app/api-reference/next-config-js/serverComponentsExternalPackages

seanyboy49 commented 4 months ago

hey @jamesbrooks94 I saw yesterday you had a public repo with your next/datadog/winston implementation that is now private. Do you mind sharing how you got it working? I also am using a standalone build and saw that you were doing a couple things I think.

Was this right?

mogulist commented 3 months ago

After MONTHS of not being able to get log injection with winston working, adding it to this next.config.js configuration worked! https://nextjs.org/docs/app/api-reference/next-config-js/serverComponentsExternalPackages Hi, @jamesbrooks94, did you do something like that? I did as below, but Error: Cannot find module 'dd-trace/init' error happens.

const nextConfig = {
experimental: {
serverComponentsExternalPackages: ['dd-trace/init'],
},
}
radum commented 2 months ago

Hello everyone, I managed to hit the same dead end like most of you here. I am running Next.js 14 with app router.

The only way I managed to get it working (although not sure if it is fully working yet) is to create a JS file server-preload.js

const packageJSON = require('../package.json');

function setUpDatadogTracing() {
    const tracer = require('dd-trace');

    tracer.init({
        runtimeMetrics: true,
        logInjection: true,
        env: 'dev',
        service: `myapp`,
        version: packageJSON?.version ?? 'unknown'
    });
}

setUpDatadogTracing();

And load it within package.json node -r server-preload.js ./node_modules/.bin/next start. Doing this I don't get only GET and POST in Resources and I have GET /_not-found for 404 pages and GET /about etc etc based on the pages I have.

I am also getting the versioning coming through for each new release I make and also the dev envs are set properly.

Logs are ingested also but only the ones that I am logging via an internal logger I made via Pino. The other ones are not coming in as they are not in JSON format.

There is a way in the file above to patch the console log and make it spit out JSON but that is a can of worms because there is lots of cleaning up that needs to be done to make it work and also it could break at any Next update.

Using the instrumentation hook I never managed to get it working, and using the telemetry from Vercel plus DD I always got undefined errors looking for the _traceID in an object.

Even with this setup I am not sure if I can see any spans and I need to check more.

For sourcemaps I am thinking to generate them and load them via the CI before I remove them from the deployed app.

Has anyone found a better way that works with most DD features and can share their setup?

aralroca commented 2 months ago

Very curios that:

if (process.env.NEXT_RUNTIME === 'nodejs') { /* WORKS */ }

And:

if (process.env.NEXT_RUNTIME !== 'nodejs') return
// NOT WORK

This happens with Next.js 13.5.4. How is it possible that with early return it doesn't work? Are you parsing the AST instead of letting JS do its job? I don't understand the error, something is missing.