From c338efc6551c70e2344441611fdcd38581ae2297 Mon Sep 17 00:00:00 2001 From: rochdev Date: Mon, 19 Jun 2023 20:03:39 -0400 Subject: [PATCH 1/4] add support for global fetch --- .github/workflows/plugins.yml | 19 +- docs/API.md | 2 + docs/test.ts | 2 + index.d.ts | 7 + .../datadog-instrumentations/src/fetch.js | 46 ++ .../src/helpers/register.js | 3 + packages/datadog-plugin-fetch/src/index.js | 36 + .../datadog-plugin-fetch/test/index.spec.js | 697 ++++++++++++++++++ packages/datadog-plugin-http/src/client.js | 26 +- packages/dd-trace/src/plugin_manager.js | 9 +- 10 files changed, 836 insertions(+), 11 deletions(-) create mode 100644 packages/datadog-instrumentations/src/fetch.js create mode 100644 packages/datadog-plugin-fetch/src/index.js create mode 100644 packages/datadog-plugin-fetch/test/index.spec.js diff --git a/.github/workflows/plugins.yml b/.github/workflows/plugins.yml index dcf3c8f46db..48e0b78cdd4 100644 --- a/.github/workflows/plugins.yml +++ b/.github/workflows/plugins.yml @@ -330,6 +330,23 @@ jobs: uses: ./.github/actions/testagent/logs - uses: codecov/codecov-action@v2 + fetch: + runs-on: ubuntu-latest + env: + PLUGINS: fetch + steps: + - uses: actions/checkout@v2 + - 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@v2 + generic-pool: runs-on: ubuntu-latest env: @@ -802,7 +819,7 @@ jobs: - 5500:5500 testagent: image: ghcr.io/datadog/dd-apm-test-agent/ddapm-test-agent:latest - env: + env: LOG_LEVEL: DEBUG TRACE_LANGUAGE: javascript DISABLED_CHECKS: trace_content_length diff --git a/docs/API.md b/docs/API.md index dc6aa9d300c..c80af395fd4 100644 --- a/docs/API.md +++ b/docs/API.md @@ -40,6 +40,7 @@ tracer.use('pg', {
+
@@ -110,6 +111,7 @@ tracer.use('pg', { * [elasticsearch](./interfaces/plugins.elasticsearch.html) * [express](./interfaces/plugins.express.html) * [fastify](./interfaces/plugins.fastify.html) +* [fetch](./interfaces/plugins.fetch.html) * [generic-pool](./interfaces/plugins.generic_pool.html) * [google-cloud-pubsub](./interfaces/plugins.google_cloud_pubsub.html) * [graphql](./interfaces/plugins.graphql.html) diff --git a/docs/test.ts b/docs/test.ts index 583b65c76a0..ec12ed40ed5 100644 --- a/docs/test.ts +++ b/docs/test.ts @@ -248,6 +248,8 @@ tracer.use('express'); tracer.use('express', httpServerOptions); tracer.use('fastify'); tracer.use('fastify', httpServerOptions); +tracer.use('fetch'); +tracer.use('fetch', httpClientOptions); tracer.use('generic-pool'); tracer.use('google-cloud-pubsub'); tracer.use('graphql'); diff --git a/index.d.ts b/index.d.ts index 1c1cc5b021b..a0f65bf6366 100644 --- a/index.d.ts +++ b/index.d.ts @@ -749,6 +749,7 @@ interface Plugins { "elasticsearch": plugins.elasticsearch; "express": plugins.express; "fastify": plugins.fastify; + "fetch": plugins.fetch; "generic-pool": plugins.generic_pool; "google-cloud-pubsub": plugins.google_cloud_pubsub; "graphql": plugins.graphql; @@ -1092,6 +1093,12 @@ declare namespace plugins { */ interface fastify extends HttpServer {} + /** + * This plugin automatically instruments the + * [fetch](https://nodejs.org/api/globals.html#fetch) global. + */ + interface fetch extends HttpClient {} + /** * This plugin patches the [generic-pool](https://github.com/coopernurse/node-pool) * module to bind the callbacks the the caller context. diff --git a/packages/datadog-instrumentations/src/fetch.js b/packages/datadog-instrumentations/src/fetch.js new file mode 100644 index 00000000000..6b8ff8a6735 --- /dev/null +++ b/packages/datadog-instrumentations/src/fetch.js @@ -0,0 +1,46 @@ +'use strict' + +const shimmer = require('../../datadog-shimmer') +const { channel } = require('./helpers/instrument') + +const startChannel = channel('apm:fetch:request:start') +const finishChannel = channel('apm:fetch:request:finish') +const errorChannel = channel('apm:fetch:request:error') + +function wrapFetch (fetch, Request) { + if (typeof fetch !== 'function') return fetch + + return function (input, init) { + if (!startChannel.hasSubscribers) return fetch.apply(this, arguments) + + const req = new Request(input, init) + const headers = req.headers + const message = { req, headers } + + startChannel.publish(message) + + // Request object is read-only so we need new objects to change headers. + arguments[0] = message.req + arguments[1] = { headers: message.headers } + + return fetch.apply(this, arguments) + .then( + res => { + finishChannel.publish({ req, res }) + }, + err => { + if (err.name !== 'AbortError') { + errorChannel.publish(err) + } + + finishChannel.publish({ req }) + + throw err + } + ) + } +} + +if (globalThis.fetch) { + globalThis.fetch = shimmer.wrap(fetch, wrapFetch(fetch, globalThis.Request)) +} diff --git a/packages/datadog-instrumentations/src/helpers/register.js b/packages/datadog-instrumentations/src/helpers/register.js index 188a6145ae8..e819fbd1915 100644 --- a/packages/datadog-instrumentations/src/helpers/register.js +++ b/packages/datadog-instrumentations/src/helpers/register.js @@ -14,6 +14,9 @@ const pathSepExpr = new RegExp(`\\${path.sep}`, 'g') const loadChannel = channel('dd-trace:instrumentation:load') +// Globals +require('../fetch') + // TODO: make this more efficient for (const packageName of names) { diff --git a/packages/datadog-plugin-fetch/src/index.js b/packages/datadog-plugin-fetch/src/index.js new file mode 100644 index 00000000000..c3063dad51b --- /dev/null +++ b/packages/datadog-plugin-fetch/src/index.js @@ -0,0 +1,36 @@ +'use strict' + +const HttpClientPlugin = require('../../datadog-plugin-http/src/client') +const { HTTP_HEADERS } = require('../../../ext/formats') + +class FetchPlugin extends HttpClientPlugin { + static get id () { return 'fetch' } + + addTraceSub (eventName, handler) { + this.addSub(`apm:${this.constructor.id}:${this.operation}:${eventName}`, handler) + } + + start (message) { + const req = message.req + const options = new URL(req.url) + const headers = options.headers = Object.fromEntries(req.headers.entries()) + + const args = { options } + + super.start({ args }) + + message.req = new globalThis.Request(req, { headers }) + } + + _inject (span, headers) { + const carrier = {} + + this.tracer.inject(span, HTTP_HEADERS, carrier) + + for (const name in carrier) { + headers.append(name, carrier[name]) + } + } +} + +module.exports = FetchPlugin diff --git a/packages/datadog-plugin-fetch/test/index.spec.js b/packages/datadog-plugin-fetch/test/index.spec.js new file mode 100644 index 00000000000..0521fd2f661 --- /dev/null +++ b/packages/datadog-plugin-fetch/test/index.spec.js @@ -0,0 +1,697 @@ +'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 { storage } = require('../../datadog-core') +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 describe = globalThis.fetch ? globalThis.describe : globalThis.describe.skip + +describe('Plugin', () => { + let express + let fetch + let appListener + + describe('fetch', () => { + 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('fetch') + .then(() => { + express = require('express') + fetch = globalThis.fetch + }) + }) + + it('should do automatic instrumentation', 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', 'fetch') + expect(traces[0][0].meta).to.have.property('out.host', 'localhost') + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch(`http://localhost:${port}/user`) + }) + }) + }) + + it('should support URL input', 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', 'fetch') + expect(traces[0][0].meta).to.have.property('out.host', 'localhost') + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch(new URL(`http://localhost:${port}/user`)) + }) + }) + }) + + it('should support Request input', 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', 'fetch') + expect(traces[0][0].meta).to.have.property('out.host', 'localhost') + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch(new globalThis.Request(`http://localhost:${port}/user`)) + }) + }) + }) + + 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(`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(`http://localhost:${port}/user?foo=bar`) + }) + }) + }) + + it('should skip injecting if the Authorization header contains an AWS signature', done => { + const app = express() + + app.get('/', (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(`http://localhost:${port}/`, { + headers: { + Authorization: 'AWS4-HMAC-SHA256 ...' + } + }) + }) + }) + }) + + it('should skip injecting if one of the Authorization headers contains an AWS signature', done => { + const app = express() + + app.get('/', (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(`http://localhost:${port}/`, { + headers: { + Authorization: ['AWS4-HMAC-SHA256 ...'] + } + }) + }) + }) + }) + + it('should skip injecting if the X-Amz-Signature header is set', done => { + const app = express() + + app.get('/', (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(`http://localhost:${port}/`, { + headers: { + 'X-Amz-Signature': 'abc123' + } + }) + }) + }) + }) + + it('should skip injecting if the X-Amz-Signature query param is set', done => { + const app = express() + + app.get('/', (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(`http://localhost:${port}/?X-Amz-Signature=abc123`) + }) + }) + }) + + 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', 'fetch') + }) + .then(done) + .catch(done) + + 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(`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(`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(`http://localhost:${port}/user`, { + signal: controller.signal + }).catch(e => {}) + + 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', 'test') + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + const controller = new AbortController() + + fetch(`http://localhost:${port}/user`, { + signal: controller.signal + }).catch(e => {}) + + controller.abort() + }) + }) + }) + + it('should skip requests marked as noop', done => { + const app = express() + + app.get('/user', (req, res) => { + res.status(200).send() + }) + + getPort().then(port => { + const timer = setTimeout(done, 100) + + agent + .use(() => { + done(new Error('Noop request was traced.')) + clearTimeout(timer) + }) + + appListener = server(app, port, () => { + const store = storage.getStore() + + storage.enterWith({ noop: true }) + + fetch(`http://localhost:${port}/user`).catch(() => {}) + + storage.enterWith(store) + }) + }) + }) + }) + + describe('with service configuration', () => { + let config + + beforeEach(() => { + config = { + service: 'custom' + } + + return agent.load('fetch', config) + .then(() => { + express = require('express') + fetch = globalThis.fetch + }) + }) + + 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(`http://localhost:${port}/user`).catch(() => {}) + }) + }) + }) + }) + + describe('with validateStatus configuration', () => { + let config + + beforeEach(() => { + config = { + validateStatus: status => status < 500 + } + + return agent.load('fetch', config) + .then(() => { + express = require('express') + fetch = globalThis.fetch + }) + }) + + it('should use the supplied function to decide if a response is an error', 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', 1) + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch(`http://localhost:${port}/user`).catch(() => {}) + }) + }) + }) + }) + + describe('with splitByDomain configuration', () => { + let config + + beforeEach(() => { + config = { + splitByDomain: true + } + + return agent.load('fetch', config) + .then(() => { + express = require('express') + fetch = globalThis.fetch + }) + }) + + it('should use the remote endpoint as the service name', 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', `localhost:${port}`) + }) + .then(done) + .catch(done) + + appListener = server(app, port, () => { + fetch(`http://localhost:${port}/user`).catch(() => {}) + }) + }) + }) + }) + + describe('with headers configuration', () => { + let config + + beforeEach(() => { + config = { + headers: ['x-baz', 'x-foo'] + } + + return agent.load('fetch', config) + .then(() => { + express = require('express') + fetch = globalThis.fetch + }) + }) + + 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(`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('fetch', config) + .then(() => { + express = require('express') + fetch = globalThis.fetch + }) + }) + + 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(`http://localhost:${port}/user`).catch(() => {}) + }) + }) + }) + }) + + describe('with propagationBlocklist configuration', () => { + let config + + beforeEach(() => { + config = { + propagationBlocklist: [/\/users/] + } + + return agent.load('fetch', config) + .then(() => { + express = require('express') + fetch = globalThis.fetch + }) + }) + + 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(`http://localhost:${port}/users`).catch(() => {}) + }) + }) + }) + }) + + describe('with blocklist configuration', () => { + let config + + beforeEach(() => { + config = { + blocklist: [/\/user/] + } + + return agent.load('fetch', config) + .then(() => { + express = require('express') + fetch = globalThis.fetch + }) + }) + + 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(`http://localhost:${port}/users`).catch(() => {}) + }) + }) + }) + }) + }) +}) diff --git a/packages/datadog-plugin-http/src/client.js b/packages/datadog-plugin-http/src/client.js index e1fbf18a09b..1845d3043c1 100644 --- a/packages/datadog-plugin-http/src/client.js +++ b/packages/datadog-plugin-http/src/client.js @@ -24,15 +24,17 @@ class HttpClientPlugin extends ClientPlugin { this.addSub(`apm:${this.constructor.id}:client:${this.operation}:${eventName}`, handler) } - start ({ args, http }) { + start ({ args, http = {} }) { const store = storage.getStore() const options = args.options - const agent = options.agent || options._defaultAgent || http.globalAgent + const agent = options.agent || options._defaultAgent || http.globalAgent || {} const protocol = options.protocol || agent.protocol || 'http:' const hostname = options.hostname || options.host || 'localhost' const host = options.port ? `${hostname}:${options.port}` : hostname - const path = options.path ? options.path.split(/[?#]/)[0] : '/' + const pathname = options.pathname || options.path + const path = pathname ? pathname.split(/[?#]/)[0] : '/' const uri = `${protocol}//${host}${path}` + const allowed = this.config.filter(uri) const method = (options.method || 'GET').toUpperCase() @@ -71,9 +73,11 @@ class HttpClientPlugin extends ClientPlugin { finish ({ req, res }) { const span = storage.getStore().span if (res) { - span.setTag(HTTP_STATUS_CODE, res.statusCode) + const status = res.status || res.statusCode + + span.setTag(HTTP_STATUS_CODE, status) - if (!this.config.validateStatus(res.statusCode)) { + if (!this.config.validateStatus(status)) { span.setTag('error', 1) } @@ -106,8 +110,10 @@ class HttpClientPlugin extends ClientPlugin { } function addResponseHeaders (res, span, config) { + const headers = new Map(res.headers) + config.headers.forEach(key => { - const value = res.headers[key] + const value = headers.get(key) if (value) { span.setTag(`${HTTP_RESPONSE_HEADERS}.${key}`, value) @@ -116,8 +122,10 @@ function addResponseHeaders (res, span, config) { } function addRequestHeaders (req, span, config) { + const headers = new Map(req.headers || req.getHeaders()) + config.headers.forEach(key => { - const value = req.getHeader(key) + const value = headers.get(key) if (value) { span.setTag(`${HTTP_REQUEST_HEADERS}.${key}`, Array.isArray(value) ? value.toString() : value) @@ -193,7 +201,9 @@ function hasAmazonSignature (options) { } } - return options.path && options.path.toLowerCase().indexOf('x-amz-signature=') !== -1 + const search = options.search || options.path + + return search && search.toLowerCase().indexOf('x-amz-signature=') !== -1 } function getServiceName (tracer, config, options) { diff --git a/packages/dd-trace/src/plugin_manager.js b/packages/dd-trace/src/plugin_manager.js index 7057045872b..4f5209e35d5 100644 --- a/packages/dd-trace/src/plugin_manager.js +++ b/packages/dd-trace/src/plugin_manager.js @@ -26,8 +26,13 @@ const disabledPlugins = new Set( const pluginClasses = {} loadChannel.subscribe(({ name }) => { - const Plugin = plugins[name] + maybeEnable(name, plugins[name]) +}) + +// Globals +maybeEnable(require('../../datadog-plugin-fetch/src')) +function maybeEnable (Plugin) { if (!Plugin || typeof Plugin !== 'function') return if (!pluginClasses[Plugin.id]) { const envName = `DD_TRACE_${Plugin.id.toUpperCase()}_ENABLED` @@ -42,7 +47,7 @@ loadChannel.subscribe(({ name }) => { pluginClasses[Plugin.id] = Plugin } } -}) +} // TODO this must always be a singleton. module.exports = class PluginManager { From 5121516500c6ac6482834008d2be87cb0dd0dbec Mon Sep 17 00:00:00 2001 From: rochdev Date: Mon, 19 Jun 2023 20:19:43 -0400 Subject: [PATCH 2/4] fix plugin manager --- packages/dd-trace/src/plugin_manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/dd-trace/src/plugin_manager.js b/packages/dd-trace/src/plugin_manager.js index 4f5209e35d5..fe500edde82 100644 --- a/packages/dd-trace/src/plugin_manager.js +++ b/packages/dd-trace/src/plugin_manager.js @@ -26,7 +26,7 @@ const disabledPlugins = new Set( const pluginClasses = {} loadChannel.subscribe(({ name }) => { - maybeEnable(name, plugins[name]) + maybeEnable(plugins[name]) }) // Globals From c78c354048f0318f2372938bfdbe15ea3956d3ae Mon Sep 17 00:00:00 2001 From: rochdev Date: Mon, 19 Jun 2023 21:23:04 -0400 Subject: [PATCH 3/4] fix headers --- packages/datadog-plugin-http/src/client.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/datadog-plugin-http/src/client.js b/packages/datadog-plugin-http/src/client.js index 1845d3043c1..9cc3164ab85 100644 --- a/packages/datadog-plugin-http/src/client.js +++ b/packages/datadog-plugin-http/src/client.js @@ -31,7 +31,7 @@ class HttpClientPlugin extends ClientPlugin { const protocol = options.protocol || agent.protocol || 'http:' const hostname = options.hostname || options.host || 'localhost' const host = options.port ? `${hostname}:${options.port}` : hostname - const pathname = options.pathname || options.path + const pathname = options.path || options.pathname const path = pathname ? pathname.split(/[?#]/)[0] : '/' const uri = `${protocol}//${host}${path}` @@ -110,10 +110,14 @@ class HttpClientPlugin extends ClientPlugin { } function addResponseHeaders (res, span, config) { - const headers = new Map(res.headers) + if (!res.headers) return + + const headers = typeof res.headers.entries === 'function' + ? Object.fromEntries(res.headers.entries()) + : res.headers config.headers.forEach(key => { - const value = headers.get(key) + const value = headers[key] if (value) { span.setTag(`${HTTP_RESPONSE_HEADERS}.${key}`, value) @@ -122,10 +126,12 @@ function addResponseHeaders (res, span, config) { } function addRequestHeaders (req, span, config) { - const headers = new Map(req.headers || req.getHeaders()) + const headers = req.headers && typeof req.headers.entries === 'function' + ? Object.fromEntries(req.headers.entries()) + : req.headers || req.getHeaders() config.headers.forEach(key => { - const value = headers.get(key) + const value = headers[key] if (value) { span.setTag(`${HTTP_REQUEST_HEADERS}.${key}`, Array.isArray(value) ? value.toString() : value) From e547537ab494c830c489b40adc7aef2b6c76a82f Mon Sep 17 00:00:00 2001 From: rochdev Date: Tue, 20 Jun 2023 14:02:36 -0400 Subject: [PATCH 4/4] fix missing return value --- packages/datadog-instrumentations/src/fetch.js | 2 ++ .../datadog-plugin-fetch/test/index.spec.js | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/packages/datadog-instrumentations/src/fetch.js b/packages/datadog-instrumentations/src/fetch.js index 6b8ff8a6735..58a700e49cf 100644 --- a/packages/datadog-instrumentations/src/fetch.js +++ b/packages/datadog-instrumentations/src/fetch.js @@ -27,6 +27,8 @@ function wrapFetch (fetch, Request) { .then( res => { finishChannel.publish({ req, res }) + + return res }, err => { if (err.name !== 'AbortError') { diff --git a/packages/datadog-plugin-fetch/test/index.spec.js b/packages/datadog-plugin-fetch/test/index.spec.js index 0521fd2f661..0d25e3a78d4 100644 --- a/packages/datadog-plugin-fetch/test/index.spec.js +++ b/packages/datadog-plugin-fetch/test/index.spec.js @@ -125,6 +125,23 @@ describe('Plugin', () => { }) }) + 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(new globalThis.Request(`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()