From f214673c6e8974af7c483c8bc520ea0e99fd0e5f Mon Sep 17 00:00:00 2001 From: Sam Brenner <106700075+sabrenner@users.noreply.github.com> Date: Fri, 17 Jan 2025 14:54:05 -0500 Subject: [PATCH 1/2] use url if provided from DD_TRACE_AGENT_URL (#5128) --- packages/dd-trace/src/llmobs/writers/spans/agentProxy.js | 6 +++--- .../dd-trace/test/llmobs/writers/spans/agentProxy.spec.js | 8 ++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/dd-trace/src/llmobs/writers/spans/agentProxy.js b/packages/dd-trace/src/llmobs/writers/spans/agentProxy.js index 6274f6117e0..62e497f487c 100644 --- a/packages/dd-trace/src/llmobs/writers/spans/agentProxy.js +++ b/packages/dd-trace/src/llmobs/writers/spans/agentProxy.js @@ -10,10 +10,10 @@ const LLMObsBaseSpanWriter = require('./base') class LLMObsAgentProxySpanWriter extends LLMObsBaseSpanWriter { constructor (config) { super({ - intake: config.hostname || 'localhost', - protocol: 'http:', + intake: config.url?.hostname || config.hostname || 'localhost', + protocol: config.url?.protocol || 'http:', endpoint: EVP_PROXY_AGENT_ENDPOINT, - port: config.port + port: config.url?.port || config.port }) this._headers[EVP_SUBDOMAIN_HEADER_NAME] = EVP_SUBDOMAIN_HEADER_VALUE diff --git a/packages/dd-trace/test/llmobs/writers/spans/agentProxy.spec.js b/packages/dd-trace/test/llmobs/writers/spans/agentProxy.spec.js index 6ed0f150885..412b43133a4 100644 --- a/packages/dd-trace/test/llmobs/writers/spans/agentProxy.spec.js +++ b/packages/dd-trace/test/llmobs/writers/spans/agentProxy.spec.js @@ -25,4 +25,12 @@ describe('LLMObsAgentProxySpanWriter', () => { expect(writer._url.href).to.equal('http://localhost:8126/evp_proxy/v2/api/v2/llmobs') }) + + it('uses the url property if provided on the config', () => { + writer = new LLMObsAgentProxySpanWriter({ + url: new URL('http://test-agent:12345') + }) + + expect(writer._url.href).to.equal('http://test-agent:12345/evp_proxy/v2/api/v2/llmobs') + }) }) From 4ef12fc3235dc7355c17b0920632d50eafa50f27 Mon Sep 17 00:00:00 2001 From: William Conti <58711692+wconti27@users.noreply.github.com> Date: Fri, 17 Jan 2025 16:44:07 -0500 Subject: [PATCH 2/2] fix aws-sdk invalid signature exception (#5127) Enables tracing header injection on AWS signed requests and fixes problem causing InvalidSignatureException. --- packages/datadog-plugin-http/src/client.js | 38 +---- .../datadog-plugin-http/test/client.spec.js | 141 ++---------------- 2 files changed, 17 insertions(+), 162 deletions(-) diff --git a/packages/datadog-plugin-http/src/client.js b/packages/datadog-plugin-http/src/client.js index d4c105d2508..2bc408e648b 100644 --- a/packages/datadog-plugin-http/src/client.js +++ b/packages/datadog-plugin-http/src/client.js @@ -59,6 +59,11 @@ class HttpClientPlugin extends ClientPlugin { } if (this.shouldInjectTraceHeaders(options, uri)) { + // Clone the headers object in case an upstream lib has a reference to the original headers + // Implemented due to aws-sdk issue where request signing is broken if we mutate the headers + // Explained further in: + // https://github.com/open-telemetry/opentelemetry-js-contrib/issues/1609#issuecomment-1826167348 + options.headers = Object.assign({}, options.headers) this.tracer.inject(span, HTTP_HEADERS, options.headers) } @@ -72,10 +77,6 @@ class HttpClientPlugin extends ClientPlugin { } shouldInjectTraceHeaders (options, uri) { - if (hasAmazonSignature(options) && !this.config.enablePropagationWithAmazonHeaders) { - return false - } - if (!this.config.propagationFilter(uri)) { return false } @@ -212,31 +213,6 @@ function getHooks (config) { return { request } } -function hasAmazonSignature (options) { - if (!options) { - return false - } - - if (options.headers) { - const headers = Object.keys(options.headers) - .reduce((prev, next) => Object.assign(prev, { - [next.toLowerCase()]: options.headers[next] - }), {}) - - if (headers['x-amz-signature']) { - return true - } - - if ([].concat(headers.authorization).some(startsWith('AWS4-HMAC-SHA256'))) { - return true - } - } - - const search = options.search || options.path - - return search && search.toLowerCase().indexOf('x-amz-signature=') !== -1 -} - function extractSessionDetails (options) { if (typeof options === 'string') { return new URL(options).host @@ -248,8 +224,4 @@ function extractSessionDetails (options) { return { host, port } } -function startsWith (searchString) { - return value => String(value).startsWith(searchString) -} - module.exports = HttpClientPlugin diff --git a/packages/datadog-plugin-http/test/client.spec.js b/packages/datadog-plugin-http/test/client.spec.js index 42f4c8436f8..ff2d220d0cd 100644 --- a/packages/datadog-plugin-http/test/client.spec.js +++ b/packages/datadog-plugin-http/test/client.spec.js @@ -446,97 +446,24 @@ describe('Plugin', () => { }) }) - 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) - } - }) - - appListener = server(app, port => { - const req = http.request({ - port, - headers: { - Authorization: 'AWS4-HMAC-SHA256 ...' - } - }) + it('should inject tracing header into request without mutating the header', done => { + // ensures that the tracer clones request headers instead of mutating. + // Fixes aws-sdk InvalidSignatureException, more info: + // https://github.com/open-telemetry/opentelemetry-js-contrib/issues/1609#issuecomment-1826167348 - req.end() - }) - }) - - 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) - } - }) - - appListener = server(app, port => { - const req = http.request({ - port, - headers: { - Authorization: ['AWS4-HMAC-SHA256 ...'] - } - }) - - req.end() - }) - }) - - it('should skip injecting if the X-Amz-Signature header is set', done => { - const app = express() + const originalHeaders = { + Authorization: 'AWS4-HMAC-SHA256 ...' + } 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) - } - }) - - appListener = server(app, port => { - const req = http.request({ - port, - headers: { - 'X-Amz-Signature': 'abc123' - } - }) - - req.end() - }) - }) - - it('should skip injecting if the X-Amz-Signature query param is set', done => { - const app = express() + expect(req.get('x-datadog-trace-id')).to.be.a('string') + expect(req.get('x-datadog-parent-id')).to.be.a('string') - 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 + expect(originalHeaders['x-datadog-trace-id']).to.be.undefined + expect(originalHeaders['x-datadog-parent-id']).to.be.undefined res.status(200).send() @@ -549,7 +476,7 @@ describe('Plugin', () => { appListener = server(app, port => { const req = http.request({ port, - path: '/?X-Amz-Signature=abc123' + headers: originalHeaders }) req.end() @@ -1093,50 +1020,6 @@ describe('Plugin', () => { }) }) - describe('with config enablePropagationWithAmazonHeaders enabled', () => { - let config - - beforeEach(() => { - config = { - enablePropagationWithAmazonHeaders: true - } - - return agent.load('http', config) - .then(() => { - http = require(pluginToBeLoaded) - express = require('express') - }) - }) - - it('should inject tracing header into AWS signed request', done => { - const app = express() - - app.get('/', (req, res) => { - try { - 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() - - done() - } catch (e) { - done(e) - } - }) - - appListener = server(app, port => { - const req = http.request({ - port, - headers: { - Authorization: 'AWS4-HMAC-SHA256 ...' - } - }) - - req.end() - }) - }) - }) - describe('with validateStatus configuration', () => { let config