Skip to content

Commit cd06d5b

Browse files
tlhunterrochdev
tlhunter
authored and
rochdev
committed
Revert "always enable tracing header injection for AWS requests (#4717)" (#4867)
- this reverts commit 1d2543c. - reverts a change that would automatically inject tracing headers into AWS requests - this appears to break S3 requests (and DynamoDB?) when using AWS SDK v2 - we don't have any reports of other services or of AWS SDK v3 breaking - for follow up work we need to make this a configurable environment variable instead of just an init setting - this is because folks using the lambda layer need to configure the tracer via env vars - alternatively we only block s3 and dynamo? however there could be other services that fail... - alternatively we only block aws sdk v2? however it seems that a bunch of the services are fine... - internal stuff: APMS-13694, APMS-13713 - more discussion in #4717
1 parent 2ba82f7 commit cd06d5b

File tree

8 files changed

+454
-24
lines changed

8 files changed

+454
-24
lines changed

‎docs/test.ts

+3
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,9 @@ tracer.use('http', {
324324
tracer.use('http', {
325325
client: httpClientOptions
326326
});
327+
tracer.use('http', {
328+
enablePropagationWithAmazonHeaders: true
329+
});
327330
tracer.use('http2');
328331
tracer.use('http2', {
329332
server: http2ServerOptions

‎index.d.ts

+8
Original file line numberDiff line numberDiff line change
@@ -1043,6 +1043,14 @@ declare namespace tracer {
10431043
* @default code => code < 500
10441044
*/
10451045
validateStatus?: (code: number) => boolean;
1046+
1047+
/**
1048+
* Enable injection of tracing headers into requests signed with AWS IAM headers.
1049+
* Disable this if you get AWS signature errors (HTTP 403).
1050+
*
1051+
* @default false
1052+
*/
1053+
enablePropagationWithAmazonHeaders?: boolean;
10461054
}
10471055

10481056
/** @hidden */

‎packages/datadog-plugin-aws-sdk/test/aws-sdk.spec.js

-22
Original file line numberDiff line numberDiff line change
@@ -114,28 +114,6 @@ describe('Plugin', () => {
114114
s3.listBuckets({}, e => e && done(e))
115115
})
116116

117-
// different versions of aws-sdk use different casings and different AWS headers
118-
it('should include tracing headers and not cause a 403 error', (done) => {
119-
const HttpClientPlugin = require('../../datadog-plugin-http/src/client.js')
120-
const spy = sinon.spy(HttpClientPlugin.prototype, 'bindStart')
121-
agent.use(traces => {
122-
const headers = new Set(
123-
Object.keys(spy.firstCall.firstArg.args.options.headers)
124-
.map(x => x.toLowerCase())
125-
)
126-
spy.restore()
127-
128-
expect(headers).to.include('authorization')
129-
expect(headers).to.include('x-amz-date')
130-
expect(headers).to.include('x-datadog-trace-id')
131-
expect(headers).to.include('x-datadog-parent-id')
132-
expect(headers).to.include('x-datadog-sampling-priority')
133-
expect(headers).to.include('x-datadog-tags')
134-
}).then(done, done)
135-
136-
s3.listBuckets({}, e => e && done(e))
137-
})
138-
139117
it('should mark error responses', (done) => {
140118
let error
141119

‎packages/datadog-plugin-fetch/test/index.spec.js

+96
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,102 @@ describe('Plugin', () => {
215215
})
216216
})
217217

218+
it('should skip injecting if the Authorization header contains an AWS signature', done => {
219+
const app = express()
220+
221+
app.get('/', (req, res) => {
222+
try {
223+
expect(req.get('x-datadog-trace-id')).to.be.undefined
224+
expect(req.get('x-datadog-parent-id')).to.be.undefined
225+
226+
res.status(200).send()
227+
228+
done()
229+
} catch (e) {
230+
done(e)
231+
}
232+
})
233+
234+
appListener = server(app, port => {
235+
fetch(`http://localhost:${port}/`, {
236+
headers: {
237+
Authorization: 'AWS4-HMAC-SHA256 ...'
238+
}
239+
})
240+
})
241+
})
242+
243+
it('should skip injecting if one of the Authorization headers contains an AWS signature', done => {
244+
const app = express()
245+
246+
app.get('/', (req, res) => {
247+
try {
248+
expect(req.get('x-datadog-trace-id')).to.be.undefined
249+
expect(req.get('x-datadog-parent-id')).to.be.undefined
250+
251+
res.status(200).send()
252+
253+
done()
254+
} catch (e) {
255+
done(e)
256+
}
257+
})
258+
259+
appListener = server(app, port => {
260+
fetch(`http://localhost:${port}/`, {
261+
headers: {
262+
Authorization: ['AWS4-HMAC-SHA256 ...']
263+
}
264+
})
265+
})
266+
})
267+
268+
it('should skip injecting if the X-Amz-Signature header is set', done => {
269+
const app = express()
270+
271+
app.get('/', (req, res) => {
272+
try {
273+
expect(req.get('x-datadog-trace-id')).to.be.undefined
274+
expect(req.get('x-datadog-parent-id')).to.be.undefined
275+
276+
res.status(200).send()
277+
278+
done()
279+
} catch (e) {
280+
done(e)
281+
}
282+
})
283+
284+
appListener = server(app, port => {
285+
fetch(`http://localhost:${port}/`, {
286+
headers: {
287+
'X-Amz-Signature': 'abc123'
288+
}
289+
})
290+
})
291+
})
292+
293+
it('should skip injecting if the X-Amz-Signature query param is set', done => {
294+
const app = express()
295+
296+
app.get('/', (req, res) => {
297+
try {
298+
expect(req.get('x-datadog-trace-id')).to.be.undefined
299+
expect(req.get('x-datadog-parent-id')).to.be.undefined
300+
301+
res.status(200).send()
302+
303+
done()
304+
} catch (e) {
305+
done(e)
306+
}
307+
})
308+
309+
appListener = server(app, port => {
310+
fetch(`http://localhost:${port}/?X-Amz-Signature=abc123`)
311+
})
312+
})
313+
218314
it('should handle connection errors', done => {
219315
let error
220316

‎packages/datadog-plugin-http/src/client.js

+42-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ class HttpClientPlugin extends ClientPlugin {
5858
span._spanContext._trace.record = false
5959
}
6060

61-
if (this.config.propagationFilter(uri)) {
61+
if (this.shouldInjectTraceHeaders(options, uri)) {
6262
this.tracer.inject(span, HTTP_HEADERS, options.headers)
6363
}
6464

@@ -71,6 +71,18 @@ class HttpClientPlugin extends ClientPlugin {
7171
return message.currentStore
7272
}
7373

74+
shouldInjectTraceHeaders (options, uri) {
75+
if (hasAmazonSignature(options) && !this.config.enablePropagationWithAmazonHeaders) {
76+
return false
77+
}
78+
79+
if (!this.config.propagationFilter(uri)) {
80+
return false
81+
}
82+
83+
return true
84+
}
85+
7486
bindAsyncStart ({ parentStore }) {
7587
return parentStore
7688
}
@@ -200,6 +212,31 @@ function getHooks (config) {
200212
return { request }
201213
}
202214

215+
function hasAmazonSignature (options) {
216+
if (!options) {
217+
return false
218+
}
219+
220+
if (options.headers) {
221+
const headers = Object.keys(options.headers)
222+
.reduce((prev, next) => Object.assign(prev, {
223+
[next.toLowerCase()]: options.headers[next]
224+
}), {})
225+
226+
if (headers['x-amz-signature']) {
227+
return true
228+
}
229+
230+
if ([].concat(headers.authorization).some(startsWith('AWS4-HMAC-SHA256'))) {
231+
return true
232+
}
233+
}
234+
235+
const search = options.search || options.path
236+
237+
return search && search.toLowerCase().indexOf('x-amz-signature=') !== -1
238+
}
239+
203240
function extractSessionDetails (options) {
204241
if (typeof options === 'string') {
205242
return new URL(options).host
@@ -211,4 +248,8 @@ function extractSessionDetails (options) {
211248
return { host, port }
212249
}
213250

251+
function startsWith (searchString) {
252+
return value => String(value).startsWith(searchString)
253+
}
254+
214255
module.exports = HttpClientPlugin

‎packages/datadog-plugin-http/test/client.spec.js

+154
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,116 @@ describe('Plugin', () => {
446446
})
447447
})
448448

449+
it('should skip injecting if the Authorization header contains an AWS signature', done => {
450+
const app = express()
451+
452+
app.get('/', (req, res) => {
453+
try {
454+
expect(req.get('x-datadog-trace-id')).to.be.undefined
455+
expect(req.get('x-datadog-parent-id')).to.be.undefined
456+
457+
res.status(200).send()
458+
459+
done()
460+
} catch (e) {
461+
done(e)
462+
}
463+
})
464+
465+
appListener = server(app, port => {
466+
const req = http.request({
467+
port,
468+
headers: {
469+
Authorization: 'AWS4-HMAC-SHA256 ...'
470+
}
471+
})
472+
473+
req.end()
474+
})
475+
})
476+
477+
it('should skip injecting if one of the Authorization headers contains an AWS signature', done => {
478+
const app = express()
479+
480+
app.get('/', (req, res) => {
481+
try {
482+
expect(req.get('x-datadog-trace-id')).to.be.undefined
483+
expect(req.get('x-datadog-parent-id')).to.be.undefined
484+
485+
res.status(200).send()
486+
487+
done()
488+
} catch (e) {
489+
done(e)
490+
}
491+
})
492+
493+
appListener = server(app, port => {
494+
const req = http.request({
495+
port,
496+
headers: {
497+
Authorization: ['AWS4-HMAC-SHA256 ...']
498+
}
499+
})
500+
501+
req.end()
502+
})
503+
})
504+
505+
it('should skip injecting if the X-Amz-Signature header is set', done => {
506+
const app = express()
507+
508+
app.get('/', (req, res) => {
509+
try {
510+
expect(req.get('x-datadog-trace-id')).to.be.undefined
511+
expect(req.get('x-datadog-parent-id')).to.be.undefined
512+
513+
res.status(200).send()
514+
515+
done()
516+
} catch (e) {
517+
done(e)
518+
}
519+
})
520+
521+
appListener = server(app, port => {
522+
const req = http.request({
523+
port,
524+
headers: {
525+
'X-Amz-Signature': 'abc123'
526+
}
527+
})
528+
529+
req.end()
530+
})
531+
})
532+
533+
it('should skip injecting if the X-Amz-Signature query param is set', done => {
534+
const app = express()
535+
536+
app.get('/', (req, res) => {
537+
try {
538+
expect(req.get('x-datadog-trace-id')).to.be.undefined
539+
expect(req.get('x-datadog-parent-id')).to.be.undefined
540+
541+
res.status(200).send()
542+
543+
done()
544+
} catch (e) {
545+
done(e)
546+
}
547+
})
548+
549+
appListener = server(app, port => {
550+
const req = http.request({
551+
port,
552+
path: '/?X-Amz-Signature=abc123'
553+
})
554+
555+
req.end()
556+
})
557+
})
558+
449559
it('should run the callback in the parent context', done => {
450560
const app = express()
451561

@@ -983,6 +1093,50 @@ describe('Plugin', () => {
9831093
})
9841094
})
9851095

1096+
describe('with config enablePropagationWithAmazonHeaders enabled', () => {
1097+
let config
1098+
1099+
beforeEach(() => {
1100+
config = {
1101+
enablePropagationWithAmazonHeaders: true
1102+
}
1103+
1104+
return agent.load('http', config)
1105+
.then(() => {
1106+
http = require(pluginToBeLoaded)
1107+
express = require('express')
1108+
})
1109+
})
1110+
1111+
it('should inject tracing header into AWS signed request', done => {
1112+
const app = express()
1113+
1114+
app.get('/', (req, res) => {
1115+
try {
1116+
expect(req.get('x-datadog-trace-id')).to.be.a('string')
1117+
expect(req.get('x-datadog-parent-id')).to.be.a('string')
1118+
1119+
res.status(200).send()
1120+
1121+
done()
1122+
} catch (e) {
1123+
done(e)
1124+
}
1125+
})
1126+
1127+
appListener = server(app, port => {
1128+
const req = http.request({
1129+
port,
1130+
headers: {
1131+
Authorization: 'AWS4-HMAC-SHA256 ...'
1132+
}
1133+
})
1134+
1135+
req.end()
1136+
})
1137+
})
1138+
})
1139+
9861140
describe('with validateStatus configuration', () => {
9871141
let config
9881142

0 commit comments

Comments
 (0)