Skip to content

Commit

Permalink
enable tracing header injection for AWS requests (DataDog#3796)
Browse files Browse the repository at this point in the history
- removes the code that disabled tracing headers when amazon signature headers were present
- essentially reverts DataDog#205
- was originally done to prevent breaking AWS signatures
- apparently the presence of our headers does not break amazon signatures after all
- fixes DataDog#3719
  • Loading branch information
tlhunter committed Nov 15, 2023
1 parent 9b96ef1 commit 14c1eb0
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 411 deletions.
22 changes: 22 additions & 0 deletions packages/datadog-plugin-aws-sdk/test/aws-sdk.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,28 @@ describe('Plugin', () => {
s3.listBuckets({}, e => e && done(e))
})

// different versions of aws-sdk use different casings and different AWS headers
it('should include tracing headers and not cause a 403 error', (done) => {
const HttpClientPlugin = require('../../datadog-plugin-http/src/client.js')
const spy = sinon.spy(HttpClientPlugin.prototype, 'bindStart')
agent.use(traces => {
const headers = new Set(
Object.keys(spy.firstCall.firstArg.args.options.headers)
.map(x => x.toLowerCase())
)
spy.restore()

expect(headers).to.include('authorization')
expect(headers).to.include('x-amz-date')
expect(headers).to.include('x-datadog-trace-id')
expect(headers).to.include('x-datadog-parent-id')
expect(headers).to.include('x-datadog-sampling-priority')
expect(headers).to.include('x-datadog-tags')
}).then(done, done)

s3.listBuckets({}, e => e && done(e))
})

it('should mark error responses', (done) => {
let error

Expand Down
104 changes: 0 additions & 104 deletions packages/datadog-plugin-fetch/test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,110 +232,6 @@ 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)
}
})

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
Expand Down
31 changes: 1 addition & 30 deletions packages/datadog-plugin-http/src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class HttpClientPlugin extends ClientPlugin {
span._spanContext._trace.record = false
}

if (!(hasAmazonSignature(options) || !this.config.propagationFilter(uri))) {
if (this.config.propagationFilter(uri)) {
this.tracer.inject(span, HTTP_HEADERS, options.headers)
}

Expand Down Expand Up @@ -195,31 +195,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
Expand All @@ -231,8 +206,4 @@ function extractSessionDetails (options) {
return { host, port }
}

function startsWith (searchString) {
return value => String(value).startsWith(searchString)
}

module.exports = HttpClientPlugin
118 changes: 0 additions & 118 deletions packages/datadog-plugin-http/test/client.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -462,124 +462,6 @@ 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)
}
})

getPort().then(port => {
appListener = server(app, port, () => {
const req = http.request({
port,
headers: {
Authorization: 'AWS4-HMAC-SHA256 ...'
}
})

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)
}
})

getPort().then(port => {
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()

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, () => {
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()

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, () => {
const req = http.request({
port,
path: '/?X-Amz-Signature=abc123'
})

req.end()
})
})
})

it('should run the callback in the parent context', done => {
const app = express()

Expand Down
27 changes: 1 addition & 26 deletions packages/datadog-plugin-http2/src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@ class Http2ClientPlugin extends ClientPlugin {

addHeaderTags(span, headers, HTTP_REQUEST_HEADERS, this.config)

if (!hasAmazonSignature(headers, path)) {
this.tracer.inject(span, HTTP_HEADERS, headers)
}
this.tracer.inject(span, HTTP_HEADERS, headers)

message.parentStore = store
message.currentStore = { ...store, span }
Expand Down Expand Up @@ -133,29 +131,6 @@ function extractSessionDetails (authority, options) {
return { protocol, port, host }
}

function hasAmazonSignature (headers, path) {
if (headers) {
headers = Object.keys(headers)
.reduce((prev, next) => Object.assign(prev, {
[next.toLowerCase()]: headers[next]
}), {})

if (headers['x-amz-signature']) {
return true
}

if ([].concat(headers['authorization']).some(startsWith('AWS4-HMAC-SHA256'))) {
return true
}
}

return path && path.toLowerCase().indexOf('x-amz-signature=') !== -1
}

function startsWith (searchString) {
return value => String(value).startsWith(searchString)
}

function getStatusValidator (config) {
if (typeof config.validateStatus === 'function') {
return config.validateStatus
Expand Down

0 comments on commit 14c1eb0

Please sign in to comment.