Skip to content

Commit

Permalink
update bluebird instrumentation to account for getNewLibraryCopy (#813)
Browse files Browse the repository at this point in the history
* handle usage of getNewLibraryCopy by sequelize

* [wip] add promise spec for getNewLibraryCopy need to determine min version still

* fix unwrap typo, add temp fix and todos for version requirements for getNetLibraryCopy for 2.11.0 and 3.4.1

* add getNewLibraryCopy patching only for versions where it exists

* handle unwrapping copies _then by maintaining reference to copies on original promise library

* make hidden property non enumerable

* [wip] add specs for patching and unpatching behavior , clean up hiddenproperty settings, use promise constructor instead of prototype

* add specs for mechanics of tracking wrapped bluebird library copies for unpatching

* remove duplicate patching specs

* add version filter for bluebird version specific instrumentation

* generalize unpatching test for all promise libraries

* handle edge case when non native promise libraries default to native promises

* only instrument promisejs when it doesnt export global promise, move semver check to promise util specs, clean up bluebird spec syntax

* only instrument promise-js on versions it isnt used native promises, add promise-js version range filtering in specs

* simplify version checking and test pre / post patching and unpatching behavior

* es6 syntax improvement

* fix beforeEach handling of promises

* add test for copy of copy being unwrapped
  • Loading branch information
ericmustin authored and rochdev committed Jan 20, 2020
1 parent 1f1356e commit 271c740
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 7 deletions.
53 changes: 53 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,60 @@

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))
shim.wrap(libraryCopy, 'getNewLibraryCopy', createGetNewLibraryCopyWrap(tracer, config, originalLib, shim))
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')
shim.unwrap(libraryCopy, 'getNewLibraryCopy')
})
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
4 changes: 4 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,7 @@
const assertPromise = require('../../dd-trace/test/plugins/promise')

assertPromise('bluebird')

assertPromise('bluebird', bluebird => bluebird.getNewLibraryCopy(), '^2.11.0 || ^3.4.1')

assertPromise('bluebird', bluebird => bluebird.getNewLibraryCopy().getNewLibraryCopy(), '^2.11.0 || ^3.4.1')
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

0 comments on commit 271c740

Please sign in to comment.