Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update bluebird instrumentation to account for getNewLibraryCopy #813

Merged
merged 18 commits into from
Jan 20, 2020
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c20af16
handle usage of getNewLibraryCopy by sequelize
ericmustin Dec 27, 2019
8165515
[wip] add promise spec for getNewLibraryCopy need to determine min ve…
ericmustin Dec 27, 2019
a5f4159
fix unwrap typo, add temp fix and todos for version requirements for …
ericmustin Dec 28, 2019
ec3a81c
add getNewLibraryCopy patching only for versions where it exists
ericmustin Dec 29, 2019
98320d7
handle unwrapping copies _then by maintaining reference to copies on …
ericmustin Dec 31, 2019
9bd9ea6
make hidden property non enumerable
ericmustin Dec 31, 2019
11d627c
[wip] add specs for patching and unpatching behavior , clean up hidde…
ericmustin Jan 1, 2020
0838c00
add specs for mechanics of tracking wrapped bluebird library copies f…
ericmustin Jan 1, 2020
9297097
remove duplicate patching specs
ericmustin Jan 2, 2020
2d6860d
add version filter for bluebird version specific instrumentation
ericmustin Jan 7, 2020
f2a97d2
generalize unpatching test for all promise libraries
ericmustin Jan 7, 2020
5d26c79
handle edge case when non native promise libraries default to native …
ericmustin Jan 7, 2020
2cc45bb
only instrument promisejs when it doesnt export global promise, move …
ericmustin Jan 7, 2020
361c6d0
only instrument promise-js on versions it isnt used native promises, …
ericmustin Jan 7, 2020
29152ed
simplify version checking and test pre / post patching and unpatching…
ericmustin Jan 10, 2020
afdf878
es6 syntax improvement
ericmustin Jan 10, 2020
7f834c2
fix beforeEach handling of promises
ericmustin Jan 10, 2020
99c0518
add test for copy of copy being unwrapped
ericmustin Jan 16, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions packages/datadog-plugin-bluebird/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,57 @@

const tx = require('../../dd-trace/src/plugins/util/promise')

const DD_LIB_COPIES = '_datadog_library_copies'

function createGetNewLibraryCopyWrap (tracer, config, originalLib, shim) {
return function wrapGetNewLibraryCopy (getNewLibraryCopy) {
return function getNewLibraryCopyWithTrace () {
const libraryCopy = getNewLibraryCopy.apply(this, arguments)
shim.wrap(libraryCopy.prototype, '_then', tx.createWrapThen(tracer, config))
addToLibraryCopies(originalLib, libraryCopy)
return libraryCopy
}
}
}

function addToLibraryCopies (originalLib, libraryCopy) {
let libraryCopies = originalLib[DD_LIB_COPIES]

if (!libraryCopies) {
libraryCopies = new Set()

Object.defineProperty(originalLib, DD_LIB_COPIES, {
writable: true,
configurable: true,
value: libraryCopies
})
}

libraryCopies.add(libraryCopy)
}

function unwrapLibraryCopies (originalLib, shim) {
const libraryCopies = originalLib[DD_LIB_COPIES]

if (libraryCopies) {
libraryCopies.forEach(libraryCopy => shim.unwrap(libraryCopy.prototype, '_then'))
libraryCopies.clear()
delete originalLib[DD_LIB_COPIES]
}
}

module.exports = [
{
name: 'bluebird',
versions: ['^2.11.0', '^3.4.1'],
patch (Promise, tracer, config) {
this.wrap(Promise, 'getNewLibraryCopy', createGetNewLibraryCopyWrap(tracer, config, Promise, this))
},
unpatch (Promise) {
this.unwrap(Promise, 'getNewLibraryCopy')
unwrapLibraryCopies(Promise, this)
}
},
{
name: 'bluebird',
versions: ['>=2.0.2'], // 2.0.0 and 2.0.1 were removed from npm
Expand Down
2 changes: 2 additions & 0 deletions packages/datadog-plugin-bluebird/test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@
const assertPromise = require('../../dd-trace/test/plugins/promise')

assertPromise('bluebird')

assertPromise('bluebird', bluebird => bluebird.getNewLibraryCopy(), '^2.11.0 || ^3.4.1')
rochdev marked this conversation as resolved.
Show resolved Hide resolved
8 changes: 6 additions & 2 deletions packages/datadog-plugin-promise-js/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@ module.exports = [
name: 'promise-js',
versions: ['>=0.0.3'],
patch (Promise, tracer, config) {
this.wrap(Promise.prototype, 'then', tx.createWrapThen(tracer, config))
if (Promise !== global.Promise) {
this.wrap(Promise.prototype, 'then', tx.createWrapThen(tracer, config))
}
},
unpatch (Promise) {
this.unwrap(Promise.prototype, 'then')
if (Promise !== global.Promise) {
this.unwrap(Promise.prototype, 'then')
}
}
}
]
60 changes: 55 additions & 5 deletions packages/dd-trace/test/plugins/promise.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
'use strict'

module.exports = (name, factory) => {
module.exports = (name, factory, versionRange) => {
const agent = require('./agent')
const plugin = require(`../../../datadog-plugin-${name}/src`)
const semver = require('semver')

wrapIt()

Expand All @@ -12,14 +13,12 @@ module.exports = (name, factory) => {

describe(name, () => {
withVersions(plugin, name, version => {
if (versionRange && !semver.intersects(version, versionRange)) return

beforeEach(() => {
tracer = require('../..')
})

afterEach(() => {
return agent.close()
})

describe('without configuration', () => {
beforeEach(() => {
return agent.load(plugin, name)
Expand All @@ -31,6 +30,10 @@ module.exports = (name, factory) => {
Promise = factory ? factory(moduleExports) : moduleExports
})

afterEach(() => {
return agent.close()
})

it('should run the then() callbacks in the context where then() was called', () => {
if (process.env.DD_CONTEXT_PROPAGATION === 'false') return

Expand Down Expand Up @@ -101,6 +104,53 @@ module.exports = (name, factory) => {
})
})
})

describe('unpatching', () => {
beforeEach(() => {
const moduleExports = require(`../../../../versions/${name}@${version}`).get()

Promise = factory ? factory(moduleExports) : moduleExports
})

afterEach(() => {
return agent.close()
})

it('should behave the same before and after unpatching', async () => {
const span = {}

const testPatching = async function (Promise, tracer) {
const promise = new Promise((resolve, reject) => {
setImmediate(() => {
tracer.scope().activate({}, () => {
reject(new Error())
})
})
})

await tracer.scope().activate(span, () => {
return promise
.catch(err => {
throw err
})
.catch(() => {
return tracer.scope().active() !== span
})
})
}

await agent.load()

const beforePatching = await testPatching(Promise, tracer)

tracer.use(name)
tracer._instrumenter.disable()

const afterUnpatching = await testPatching(Promise, tracer)

expect(beforePatching).to.equal(afterUnpatching)
})
})
})
})
})
Expand Down