dd-trace-js
dd-trace-js copied to clipboard
Add support for ES modules
Right now we rely on require to patch modules automatically. We should also support ES modules as well as the esm module.
Any news on this? 🙂
We're experiencing some issues instrumenting redis using a node app with only native ES modules (package.json has type "module")
app.js:
import 'dotenv/config.js';
import './ddTracer.js'; // This line must come before importing any instrumented module.
import redis from 'redis';
const app = express()
// redis stuff
....
ddTracer.js:
import trace from 'dd-trace';
const tracer = trace.init({ plugins: true });
export default tracer;
Manually enabling redis does not work either:
tracer.use('redis', { service: 'redis' });
The plugins http-client, dns, tcp and express seemingly works.
Thanks 🙏
This is still on our roadmap but we're not actively working on it. What is your use case for using ES modules?
Thank you @rochdev. I would not say that there is one particular use case but the app is only running modern native Node.js and JS functionality and not any transpiled code.
We're having a similar issue when using koa and redis.
It seems I can either require and init dd-trace in the main server.js file and get redis tracing but lose endpoints being tagged automatically,
OR
I can import './datadog'; a file with import datadogTrace from 'dd-trace'; and get automatic endpoint tagged as expected, but redis will be disabled with the trace error TypeError: Cannot read property 'prototype' of undefined at Instrumenter.patch (/app/app/node_modules/dd-trace/packages/datadog-plugin-redis/src/index.js:57:35)
For now we're erring on the side of actually getting redis tracing, but I'd really like to not have to manually tag my endpoints, too.
@provstevi Can you clarify what the issue is when not using import? When using require everything should work out of the box. What is not tagged automatically? (assuming here that tagged means traced)
OK, to restate, for our services that use koa, we also use esm.
If I have the following, I get everything except redis out of the box, including desired resource names from the route, and other expected plugins, e.g. servicename-http-client, servicename-tcp, servicename-dns.
// startServer.js
require = require('esm')(module);
module.exports = require('./server.js');
// server.js
import './datadog';
// datadog.js
const datadogTrace = require('dd-trace');
const tracer = datadogTrace.init();
const opentracing = require('opentracing');
opentracing.initGlobalTracer(tracer);
Debug reports the following error:
TypeError: Cannot read property 'prototype' of undefined
at Instrumenter.patch (/app/app/node_modules/dd-trace/packages/datadog-plugin-redis/src/index.js:57:35)
at Instrumenter.patch (/app/app/node_modules/dd-trace/packages/dd-trace/src/instrumenter.js:219:36)
at /app/app/node_modules/dd-trace/packages/dd-trace/src/platform/node/loader.js:101:52
at Array.forEach (<anonymous>)
at /app/app/node_modules/dd-trace/packages/dd-trace/src/platform/node/loader.js:97:14
at Map.forEach (<anonymous>)
at Loader._hookModule (/app/app/node_modules/dd-trace/packages/dd-trace/src/platform/node/loader.js:92:8)
at Object.Module.require (/app/app/node_modules/require-in-the-middle/index.js:83:30)
at Object.t (/app/app/node_modules/esm/esm.js:1:279775)
at n (/app/app/node_modules/esm/esm.js:1:279589)
at Object.<anonymous> (/app/app/node_modules/redis/lib/debug.js:3:13)
at Object.<anonymous> (/app/app/node_modules/esm/esm.js:1:251206)
at Object.<anonymous> (/app/app/node_modules/esm/esm.js:1:287446)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
at Object.Ph (/app/app/node_modules/esm/esm.js:1:286187)
at p (/app/app/node_modules/esm/esm.js:1:287490)
TypeError: Cannot read property 'prototype' of undefined
at Instrumenter.unpatch (/app/app/node_modules/dd-trace/packages/datadog-plugin-redis/src/index.js:60:37)
at /app/app/node_modules/dd-trace/packages/dd-trace/src/instrumenter.js:229:35
at Set.forEach (<anonymous>)
at Instrumenter.unpatch (/app/app/node_modules/dd-trace/packages/dd-trace/src/instrumenter.js:227:20)
at /app/app/node_modules/dd-trace/packages/dd-trace/src/instrumenter.js:197:14
at Array.forEach (<anonymous>)
at Instrumenter.unload (/app/app/node_modules/dd-trace/packages/dd-trace/src/instrumenter.js:196:8)
at /app/app/node_modules/dd-trace/packages/dd-trace/src/platform/node/loader.js:106:30
at Map.forEach (<anonymous>)
at Loader._hookModule (/app/app/node_modules/dd-trace/packages/dd-trace/src/platform/node/loader.js:92:8)
at Object.Module.require (/app/app/node_modules/require-in-the-middle/index.js:83:30)
at Object.t (/app/app/node_modules/esm/esm.js:1:279775)
at n (/app/app/node_modules/esm/esm.js:1:279589)
at Object.<anonymous> (/app/app/node_modules/redis/lib/debug.js:3:13)
at Object.<anonymous> (/app/app/node_modules/esm/esm.js:1:251206)
at Object.<anonymous> (/app/app/node_modules/esm/esm.js:1:287446)
Error while trying to patch redis. The plugin has been disabled.
and I get the following integrations loaded
"integrations_loaded":["http","https","fs","net","dns","[email protected]","http2","[email protected]","[email protected]","[email protected]","[email protected]"]
If I instead do the following, I get only redis out of the box. I added logic to manually tag the endpoints so I got that to mostly work, but no tracing of any other upstream calls, which really clouds my observations:
// server.js
const datadogTrace = require('dd-trace');
const tracer = datadogTrace.init();
const opentracing = require('opentracing');
opentracing.initGlobalTracer(tracer);
and I get the following integrations:
"integrations_loaded":["[email protected]","[email protected]","[email protected]","[email protected]","[email protected]","[email protected]"]
With this config, I had to manually setting the http.route tag to get the resources to show up in APM, and I really don't want to have to try to manually tag the other plugins, but at the moment I have half the picture of what's going on in my service.
In our services that use express, which do not use esm, we are getting redis, express, and the other plugins we expect with no issue.
I also have a support ticket open for this - 426982
@provstevi Gotcha, thanks for the clarification! I expected an all or nothing situation, not individual plugins working and other not working. Definitely something we'll be looking into at some point.
One thing we've been trying to understand is the use case for using ESM over CommonJS. Can you share why you're using ESM?
@rochdev it was a technical decision made by one our teams, and is now baked pretty deeply into several supporting packages. Same reason we have some services using koa and some using express.
We also just ran into this issue as well. A potential workaround would be to migrate to Typescript (there's some tooling for this) but support for ESM would be fantastic until we can do that.
I've also hit this issue. We have some isomorphic code that uses ESM both in Node and in modern browsers (without transpilation), so we just use ESM throughout the project. It would be too much work to revisit that decision now just to get Datadog tracing to work.
We're working on a plan right now to support ESM soon. Since ESM in Node only supports a single loader hook, it's a bit complicated because it means we need to figure out a way to basically hook into any existing hook. While there is work to address that in Node itself, we still need a workaround that can land beforehand, especially since we support older versions of Node. Supporting this also means a rewrite of some core components of the tracer which will be non-trivial. However, we'll be prioritizing this soon as usage of ESM has been increasing. I will update this issue as progress is made.
Experimental support for ESM is in dd-trace@>=1.3.0. Simply add the loader hook hook using the following CLI argument to Node.js.
--experimental-loader dd-trace/loader-hook.mjs
Please contact us directly at [email protected] if any issues arise.
Experimental support for ESM is in
dd-trace@>=1.3.0. Simply add the loader hook hook using the following CLI argument to Node.js.
--experimental-loader dd-trace/loader-hook.mjsPlease contact us directly at
[email protected]if any issues arise.
I think this information should includes somewhere in the doc.
I think this information should includes somewhere in the doc.
We're still working on the functionality which is still in experimental stage, but we'll definitely add it to the doc once it's stable. There are a few known issues right now that can cause problems which we are actively working on. We definitely recommend testing this thoroughly before going to production with the loader, and to contact us if there are any issues.
Great; thanks folks.
From: Roch Devost @.> Sent: 20 December 2021 14:58 To: DataDog/dd-trace-js @.> Cc: Richard Turner @.>; Comment @.> Subject: Re: [DataDog/dd-trace-js] Add support for ES modules (#349)
I think this information should includes somewhere in the doc.
We're still working on the functionality which is still in experimental stage, but we'll definitely add it to the doc once it's stable. There are a few known issues right now that can cause problems which we are actively working on. We definitely recommend testing this thoroughly before going to production with the loader, and to contact us if there are any issues.
— Reply to this email directly, view it on GitHubhttps://github.com/DataDog/dd-trace-js/issues/349#issuecomment-997997730, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AF3XTKHQRGXW72DW6UI7L6DUR4775ANCNFSM4GB2V2HQ. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you commented.Message ID: @.***>
[https://media.riverford.co.uk/email/email-footer-christmas-2021.jpg] Riverford Organic Farmers Ltd, Buckfastleigh, Devon, TQ11 0JU, UK Registered in England No. 3731570
When can we expect support for Node >= 16.12.1 where the loader API changed? Trying to add the hook with dd-trace 1.7.0 on Node 16.13.1 outputs
(node:391569) DeprecationWarning: Obsolete loader hook(s) supplied and will be ignored: getFormat, getSource
...and the process exits immediately.
@robaca I believe that was fixed a little while ago, maybe it was just not released. We're releasing 2.0 soon which will include everything on master, so I'd say to try that version when it lands (should be later today).
Hi @rochdev it seems that the immediate exit of our service has nothing to do with the deprecation warning. It's also happening with the older Node 16.11.0 as soon as we add the loader argument - difficult to troubleshoot.
We found the root cause of the immediate exit: we had a circular dependency between module files. After fixing that, the server started as expected. It would be great if the loader could at least render some descriptive error message in such a case.
@rochdev
For example I cannot even start my project with --experimental-loader hook. It just gets stuck. Other loaders work ok.
Though this may be due to TypeScript 4.7.4, top level await and usage of reflect-metadata and libraries dependent on reflect-metadata.
Most NodeJS ESM projects have packages which are compiled as CommonJS modules.
I have pinpointed issues in profiler. It gets stuck on ritm.js: 51:56 when trying to process @aws-sdk v3.
Everything works ok on old CommonJS Node 12 project with aws sdk v2 written in pure JS.
It seems like aws-sdk v3 is the culprit when loader loads and tries to instrument code from libraries written and published in CommonJS format.
@rochdev
There are no dd-trace startup logs when using standard Node ESM loader.
When using experimental, there are logs, but service won't start.
dd-trace: 3.3.1, Node: 16.17.0
@duki994 We're working on refactoring our tests so that eventually we can test each integration with ESM, but this is a several months long project. In the meantime, can you provide a reproduction so we can at least address the immediate issue?
@rochdev
I can give you V8 profiling logs during startup of project.
Raw log v8-10-4-22_2-44-50-PM-.log
Parsed txt statistics v8_statistics.txt
Generally, sample project I made to try and demonstrate issue cannot demonstrate it, because it doesn't occur in sample project.
@duki994 is this still happening with the latest v4 dd-trace release?
Closing due to inactivity, if you are still having this problem please feel free to reopen