diff --git a/.github/workflows/plugins.yml b/.github/workflows/plugins.yml index ae8b6233d43..60028b6cc8e 100644 --- a/.github/workflows/plugins.yml +++ b/.github/workflows/plugins.yml @@ -1242,6 +1242,23 @@ jobs: uses: ./.github/actions/testagent/logs - uses: codecov/codecov-action@v3 + undici: + runs-on: ubuntu-latest + env: + PLUGINS: undici + steps: + - uses: actions/checkout@v4 + - uses: ./.github/actions/testagent/start + - uses: ./.github/actions/node/setup + - run: yarn install + - uses: ./.github/actions/node/oldest + - run: yarn test:plugins:ci + - uses: ./.github/actions/node/latest + - run: yarn test:plugins:ci + - if: always() + uses: ./.github/actions/testagent/logs + - uses: codecov/codecov-action@v3 + when: runs-on: ubuntu-latest env: diff --git a/docs/API.md b/docs/API.md index a43507f9437..68cdc3747cb 100644 --- a/docs/API.md +++ b/docs/API.md @@ -94,6 +94,7 @@ tracer.use('pg', {
+

Available Plugins

@@ -146,6 +147,7 @@ tracer.use('pg', { * [restify](./interfaces/export_.plugins.restify.html) * [router](./interfaces/export_.plugins.router.html) * [tedious](./interfaces/export_.plugins.tedious.html) +* [undici](./interfaces/export_.plugins.undici.html) * [when](./interfaces/export_.plugins.when.html) * [winston](./interfaces/export_.plugins.winston.html) diff --git a/docs/add-redirects.sh b/docs/add-redirects.sh index b738562979c..fd0590a934a 100755 --- a/docs/add-redirects.sh +++ b/docs/add-redirects.sh @@ -60,6 +60,7 @@ declare -a plugins=( "restify" "router" "tedious" + "undici" "when" "winston" ) diff --git a/docs/test.ts b/docs/test.ts index 91fafd48734..7734dad4098 100644 --- a/docs/test.ts +++ b/docs/test.ts @@ -352,6 +352,7 @@ tracer.use('selenium'); tracer.use('sharedb'); tracer.use('sharedb', sharedbOptions); tracer.use('tedious'); +tracer.use('undici'); tracer.use('winston'); tracer.use('express', false) diff --git a/index.d.ts b/index.d.ts index 003ecd9be9c..d6fc2733ef9 100644 --- a/index.d.ts +++ b/index.d.ts @@ -197,6 +197,7 @@ interface Plugins { "selenium": tracer.plugins.selenium; "sharedb": tracer.plugins.sharedb; "tedious": tracer.plugins.tedious; + "undici": tracer.plugins.undici; "winston": tracer.plugins.winston; } @@ -1800,6 +1801,12 @@ declare namespace tracer { */ interface tedious extends Instrumentation {} + /** + * This plugin automatically instruments the + * [undici](https://github.com/nodejs/undici) module. + */ + interface undici extends HttpClient {} + /** * This plugin patches the [winston](https://github.com/winstonjs/winston) * to automatically inject trace identifiers in log records when the diff --git a/packages/datadog-instrumentations/src/helpers/hooks.js b/packages/datadog-instrumentations/src/helpers/hooks.js index 34654182ddd..0723ceabd84 100644 --- a/packages/datadog-instrumentations/src/helpers/hooks.js +++ b/packages/datadog-instrumentations/src/helpers/hooks.js @@ -109,6 +109,7 @@ module.exports = { sequelize: () => require('../sequelize'), sharedb: () => require('../sharedb'), tedious: () => require('../tedious'), + undici: () => require('../undici'), when: () => require('../when'), winston: () => require('../winston') } diff --git a/packages/datadog-instrumentations/src/undici.js b/packages/datadog-instrumentations/src/undici.js new file mode 100644 index 00000000000..cd3207ea9c3 --- /dev/null +++ b/packages/datadog-instrumentations/src/undici.js @@ -0,0 +1,18 @@ +'use strict' + +const { + addHook +} = require('./helpers/instrument') +const shimmer = require('../../datadog-shimmer') + +const tracingChannel = require('dc-polyfill').tracingChannel +const ch = tracingChannel('apm:undici:fetch') + +const { createWrapFetch } = require('./helpers/fetch') + +addHook({ + name: 'undici', + versions: ['^4.4.1', '5', '>=6.0.0'] +}, undici => { + return shimmer.wrap(undici, 'fetch', createWrapFetch(undici.Request, ch)) +}) diff --git a/packages/datadog-plugin-undici/src/index.js b/packages/datadog-plugin-undici/src/index.js new file mode 100644 index 00000000000..c436aceb882 --- /dev/null +++ b/packages/datadog-plugin-undici/src/index.js @@ -0,0 +1,12 @@ +'use strict' + +const FetchPlugin = require('../../datadog-plugin-fetch/src/index.js') + +class UndiciPlugin extends FetchPlugin { + static get id () { return 'undici' } + static get prefix () { + return 'tracing:apm:undici:fetch' + } +} + +module.exports = UndiciPlugin diff --git a/packages/datadog-plugin-undici/test/index.spec.js b/packages/datadog-plugin-undici/test/index.spec.js new file mode 100644 index 00000000000..734e8f6c9a9 --- /dev/null +++ b/packages/datadog-plugin-undici/test/index.spec.js @@ -0,0 +1,525 @@ +'use strict' + +const getPort = require('get-port') +const agent = require('../../dd-trace/test/plugins/agent') +const tags = require('../../../ext/tags') +const { expect } = require('chai') +const { rawExpectedSchema } = require('./naming') +const { DD_MAJOR } = require('../../../version') +const { ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/constants') + +const HTTP_REQUEST_HEADERS = tags.HTTP_REQUEST_HEADERS +const HTTP_RESPONSE_HEADERS = tags.HTTP_RESPONSE_HEADERS + +const SERVICE_NAME = DD_MAJOR < 3 ? 'test-http-client' : 'test' + +describe('Plugin', () => { + let express + let fetch + let appListener + + describe('undici-fetch', () => { + withVersions('undici', 'undici', version => { + function server (app, port, listener) { + const server = require('http').createServer(app) + server.listen(port, 'localhost', listener) + return server + } + + beforeEach(() => { + appListener = null + }) + + afterEach(() => { + if (appListener) { + appListener.close() + } + return agent.close({ ritmReset: false }) + }) + + describe('without configuration', () => { + beforeEach(() => { + return agent.load('undici', { + service: 'test' + }) + .then(() => { + express = require('express') + fetch = require(`../../../versions/undici@${version}`, {}).get() + }) + }) + + afterEach(() => { + express = null + }) + + withNamingSchema( + () => { + const app = express() + app.get('/user', (req, res) => { + res.status(200).send() + }) + + getPort().then(port => { + appListener = server(app, port, () => { + fetch.fetch(`http://localhost:${port}/user`, { method: 'GET' }) + }) + }) + }, + rawExpectedSchema.client + ) + + it('should do automatic instrumentation', function (done) { + const app = express() + app.get('/user', (req, res) => { + res.status(200).send() + }) + getPort().then(port => { + agent + .use(traces => { + expect(traces[0][0]).to.have.property('service', 'test') + expect(traces[0][0]).to.have.property('type', 'http') + expect(traces[0][0]).to.have.property('resource', 'GET') + expect(traces[0][0].meta).to.have.property('span.kind', 'client') + expect(traces[0][0].meta).to.have.property('http.url', `http://localhost:${port}/user`) + expect(traces[0][0].meta).to.have.property('http.method', 'GET') + expect(traces[0][0].meta).to.have.property('http.status_code', '200') + expect(traces[0][0].meta).to.have.property('component', 'undici') + expect(traces[0][0].meta).to.have.property('out.host', 'localhost') + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch.fetch(`http://localhost:${port}/user`, { method: 'GET' }) + }) + }) + }) + + it('should support URL input', done => { + const app = express() + app.post('/user', (req, res) => { + res.status(200).send() + }) + getPort().then(port => { + agent + .use(traces => { + expect(traces[0][0]).to.have.property('service', SERVICE_NAME) + expect(traces[0][0]).to.have.property('type', 'http') + expect(traces[0][0]).to.have.property('resource', 'POST') + expect(traces[0][0].meta).to.have.property('span.kind', 'client') + expect(traces[0][0].meta).to.have.property('http.url', `http://localhost:${port}/user`) + expect(traces[0][0].meta).to.have.property('http.method', 'POST') + expect(traces[0][0].meta).to.have.property('http.status_code', '200') + expect(traces[0][0].meta).to.have.property('component', 'undici') + expect(traces[0][0].meta).to.have.property('out.host', 'localhost') + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch.fetch(new URL(`http://localhost:${port}/user`), { method: 'POST' }) + }) + }) + }) + + it('should return the response', done => { + const app = express() + app.get('/user', (req, res) => { + res.status(200).send() + }) + getPort().then(port => { + appListener = server(app, port, () => { + fetch.fetch((`http://localhost:${port}/user`)) + .then(res => { + expect(res).to.have.property('status', 200) + done() + }) + .catch(done) + }) + }) + }) + + it('should remove the query string from the URL', done => { + const app = express() + + app.get('/user', (req, res) => { + res.status(200).send() + }) + + getPort().then(port => { + agent + .use(traces => { + expect(traces[0][0].meta).to.have.property('http.status_code', '200') + expect(traces[0][0].meta).to.have.property('http.url', `http://localhost:${port}/user`) + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch.fetch(`http://localhost:${port}/user?foo=bar`) + }) + }) + }) + + it('should inject its parent span in the headers', done => { + const app = express() + + app.get('/user', (req, res) => { + expect(req.get('x-datadog-trace-id')).to.be.a('string') + expect(req.get('x-datadog-parent-id')).to.be.a('string') + + res.status(200).send() + }) + + getPort().then(port => { + agent + .use(traces => { + expect(traces[0][0].meta).to.have.property('http.status_code', '200') + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch.fetch(`http://localhost:${port}/user?foo=bar`) + }) + }) + }) + + it('should inject its parent span in the existing headers', done => { + const app = express() + + app.get('/user', (req, res) => { + expect(req.get('foo')).to.be.a('string') + expect(req.get('x-datadog-trace-id')).to.be.a('string') + expect(req.get('x-datadog-parent-id')).to.be.a('string') + + res.status(200).send() + }) + + getPort().then(port => { + agent + .use(traces => { + expect(traces[0][0].meta).to.have.property('http.status_code', '200') + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch.fetch(`http://localhost:${port}/user?foo=bar`, { headers: { foo: 'bar' } }) + }) + }) + }) + it('should handle connection errors', done => { + getPort().then(port => { + let error + + agent + .use(traces => { + expect(traces[0][0].meta).to.have.property(ERROR_TYPE, error.name) + expect(traces[0][0].meta).to.have.property(ERROR_MESSAGE, error.message || error.code) + expect(traces[0][0].meta).to.have.property(ERROR_STACK, error.stack) + expect(traces[0][0].meta).to.have.property('component', 'undici') + }) + .then(done) + .catch(done) + + fetch.fetch(`http://localhost:${port}/user`).catch(err => { + error = err + }) + }) + }) + it('should not record HTTP 5XX responses as errors by default', done => { + const app = express() + + app.get('/user', (req, res) => { + res.status(500).send() + }) + + getPort().then(port => { + agent + .use(traces => { + expect(traces[0][0]).to.have.property('error', 0) + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch.fetch(`http://localhost:${port}/user`) + }) + }) + }) + + it('should record HTTP 4XX responses as errors by default', done => { + const app = express() + + app.get('/user', (req, res) => { + res.status(400).send() + }) + + getPort().then(port => { + agent + .use(traces => { + expect(traces[0][0]).to.have.property('error', 1) + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch.fetch(`http://localhost:${port}/user`) + }) + }) + }) + + it('should not record aborted requests as errors', done => { + const app = express() + + app.get('/user', (req, res) => {}) + + getPort().then(port => { + agent + .use(traces => { + expect(traces[0][0]).to.have.property('error', 0) + expect(traces[0][0].meta).to.not.have.property('http.status_code') + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + const controller = new AbortController() + + fetch.fetch(`http://localhost:${port}/user`, { + signal: controller.signal + }).catch(() => {}) + + controller.abort() + }) + }) + }) + + it('should record when the request was aborted', done => { + const app = express() + + app.get('/abort', (req, res) => { + res.status(200).send() + }) + + getPort().then(port => { + agent + .use(traces => { + expect(traces[0][0]).to.have.property('service', SERVICE_NAME) + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + const controller = new AbortController() + + fetch.fetch(`http://localhost:${port}/user`, { + signal: controller.signal + }).catch(() => {}) + + controller.abort() + }) + }) + }) + }) + describe('with service configuration', () => { + let config + + beforeEach(() => { + config = { + service: 'custom' + } + + return agent.load('undici', config) + .then(() => { + express = require('express') + fetch = require(`../../../versions/undici@${version}`, {}).get() + }) + }) + + it('should be configured with the correct values', done => { + const app = express() + + app.get('/user', (req, res) => { + res.status(200).send() + }) + + getPort().then(port => { + agent + .use(traces => { + expect(traces[0][0]).to.have.property('service', 'custom') + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch.fetch(`http://localhost:${port}/user`).catch(() => {}) + }) + }) + }) + }) + describe('with headers configuration', () => { + let config + + beforeEach(() => { + config = { + headers: ['x-baz', 'x-foo'] + } + + return agent.load('undici', config) + .then(() => { + express = require('express') + fetch = require(`../../../versions/undici@${version}`, {}).get() + }) + }) + + it('should add tags for the configured headers', done => { + const app = express() + + app.get('/user', (req, res) => { + res.setHeader('x-foo', 'bar') + res.status(200).send() + }) + + getPort().then(port => { + agent + .use(traces => { + const meta = traces[0][0].meta + expect(meta).to.have.property(`${HTTP_REQUEST_HEADERS}.x-baz`, 'qux') + expect(meta).to.have.property(`${HTTP_RESPONSE_HEADERS}.x-foo`, 'bar') + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch.fetch(`http://localhost:${port}/user`, { + headers: { + 'x-baz': 'qux' + } + }).catch(() => {}) + }) + }) + }) + }) + describe('with hooks configuration', () => { + let config + + beforeEach(() => { + config = { + hooks: { + request: (span, req, res) => { + span.setTag('foo', '/foo') + } + } + } + + return agent.load('undici', config) + .then(() => { + express = require('express') + fetch = require(`../../../versions/undici@${version}`, {}).get() + }) + }) + + it('should run the request hook before the span is finished', done => { + const app = express() + + app.get('/user', (req, res) => { + res.status(200).send() + }) + + getPort().then(port => { + agent + .use(traces => { + expect(traces[0][0].meta).to.have.property('foo', '/foo') + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch.fetch(`http://localhost:${port}/user`).catch(() => {}) + }) + }) + }) + }) + + describe('with propagationBlocklist configuration', () => { + let config + + beforeEach(() => { + config = { + propagationBlocklist: [/\/users/] + } + + return agent.load('undici', config) + .then(() => { + express = require('express') + fetch = require(`../../../versions/undici@${version}`, {}).get() + }) + }) + + it('should skip injecting if the url matches an item in the propagationBlacklist', done => { + const app = express() + + app.get('/users', (req, res) => { + try { + expect(req.get('x-datadog-trace-id')).to.be.undefined + expect(req.get('x-datadog-parent-id')).to.be.undefined + + res.status(200).send() + + done() + } catch (e) { + done(e) + } + }) + + getPort().then(port => { + appListener = server(app, port, () => { + fetch.fetch(`http://localhost:${port}/users`).catch(() => {}) + }) + }) + }) + }) + + describe('with blocklist configuration', () => { + let config + + beforeEach(() => { + config = { + blocklist: [/\/user/] + } + + return agent.load('undici', config) + .then(() => { + express = require('express') + fetch = require(`../../../versions/undici@${version}`, {}).get() + }) + }) + + it('should skip recording if the url matches an item in the blocklist', done => { + const app = express() + + app.get('/user', (req, res) => { + res.status(200).send() + }) + + getPort().then(port => { + const timer = setTimeout(done, 100) + + agent + .use(() => { + clearTimeout(timer) + done(new Error('Blocklisted requests should not be recorded.')) + }) + .catch(done) + + appListener = server(app, port, () => { + fetch.fetch(`http://localhost:${port}/users`).catch(() => {}) + }) + }) + }) + }) + }) + }) +}) diff --git a/packages/datadog-plugin-undici/test/naming.js b/packages/datadog-plugin-undici/test/naming.js new file mode 100644 index 00000000000..5bf2be387c3 --- /dev/null +++ b/packages/datadog-plugin-undici/test/naming.js @@ -0,0 +1,19 @@ +const { resolveNaming } = require('../../dd-trace/test/plugins/helpers') + +const rawExpectedSchema = { + client: { + v0: { + serviceName: 'test', + opName: 'undici.request' + }, + v1: { + serviceName: 'test', + opName: 'undici.request' + } + } +} + +module.exports = { + rawExpectedSchema, + expectedSchema: resolveNaming(rawExpectedSchema) +} diff --git a/packages/dd-trace/src/plugins/index.js b/packages/dd-trace/src/plugins/index.js index d7193917b05..0b98cd9c076 100644 --- a/packages/dd-trace/src/plugins/index.js +++ b/packages/dd-trace/src/plugins/index.js @@ -81,5 +81,6 @@ module.exports = { get 'selenium-webdriver' () { return require('../../../datadog-plugin-selenium/src') }, get sharedb () { return require('../../../datadog-plugin-sharedb/src') }, get tedious () { return require('../../../datadog-plugin-tedious/src') }, + get undici () { return require('../../../datadog-plugin-undici/src') }, get winston () { return require('../../../datadog-plugin-winston/src') } } diff --git a/packages/dd-trace/src/service-naming/schemas/v0/web.js b/packages/dd-trace/src/service-naming/schemas/v0/web.js index c63f83fac52..0c2228a563b 100644 --- a/packages/dd-trace/src/service-naming/schemas/v0/web.js +++ b/packages/dd-trace/src/service-naming/schemas/v0/web.js @@ -30,6 +30,10 @@ const web = { lambda: { opName: () => 'aws.request', serviceName: awsServiceV0 + }, + undici: { + opName: () => 'undici.request', + serviceName: httpPluginClientService } }, server: { diff --git a/packages/dd-trace/src/service-naming/schemas/v1/web.js b/packages/dd-trace/src/service-naming/schemas/v1/web.js index dfe3e6594e9..333ccae51c3 100644 --- a/packages/dd-trace/src/service-naming/schemas/v1/web.js +++ b/packages/dd-trace/src/service-naming/schemas/v1/web.js @@ -29,6 +29,10 @@ const web = { lambda: { opName: () => 'aws.lambda.invoke', serviceName: identityService + }, + undici: { + opName: () => 'undici.request', + serviceName: httpPluginClientService } }, server: {