refactor: Refactored bluebird instrumentation to subscribe to events emitted (#3858)

Author: bizob2828

lib/instrumentation/bluebird.js

@@ -16,7 +16,6 @@ module.exports = function instrumentations() {
     '@hapi/vision': { type: InstrumentationDescriptor.TYPE_WEB_FRAMEWORK },
     '@prisma/client': { type: InstrumentationDescriptor.TYPE_DATASTORE },
     'aws-sdk': { module: './instrumentation/aws-sdk' },
-    bluebird: { type: InstrumentationDescriptor.TYPE_PROMISE },
     connect: { type: InstrumentationDescriptor.TYPE_WEB_FRAMEWORK },
     fastify: { type: InstrumentationDescriptor.TYPE_WEB_FRAMEWORK },
     kafkajs: { type: InstrumentationDescriptor.TYPE_MESSAGE },

lib/instrumentations.js

@@ -9,6 +9,7 @@
 // 'package-name': [ { path: 'subscriberPath', instrumentations: [] }, ... ]
 const subscribers = {
   ...require('./subscribers/amqplib/config'),
+  ...require('./subscribers/bluebird/config'),
   ...require('./subscribers/bunyan/config'),
   ...require('./subscribers/cassandra-driver/config'),
   ...require('./subscribers/elasticsearch/config'),

lib/subscriber-configs.js

@@ -0,0 +1,21 @@
+/*
+ * Copyright 2026 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+'use strict'
+
+module.exports = {
+  bluebird: [
+    {
+      path: './bluebird/instrumentation',
+      instrumentations: [{
+        channelName: 'nr_then',
+        module: { name: 'bluebird', versionRange: '>=2.02', filePath: 'js/release/promise.js' },
+        functionQuery: {
+          functionName: 'Promise',
+          kind: 'Sync'
+        }
+      }]
+    },
+  ]
+}

lib/subscribers/bluebird/config.js

@@ -0,0 +1,59 @@
+/*
+ * Copyright 2026 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+'use strict'
+
+const BaseSubscriber = require('../base')
+
+module.exports = class BluebirdPromiseSubscriber extends BaseSubscriber {
+  constructor({ agent, logger }) {
+    super({ agent, logger, channelName: 'nr_then', packageName: 'bluebird' })
+  }
+
+  /**
+   * We have to wrap the Promise prototype, not just the _then method.
+   * This is because we need to get the current context from when the promise was constructed.
+   * If we only wrapped `_then`, the context is not the same, you can see from the versioned tests
+   * Where it will defer promise resolution
+   *
+   * @param {object} data event data
+   * @param {Context} ctx the current context
+   * @returns {Context} in this case it is the same context, just with both `_then` and its respective callbacks bound to the current context
+   */
+  handler(data, ctx) {
+    const { self: BluebirdPromise } = data
+    const origThen = BluebirdPromise._then
+    BluebirdPromise._then = this.#wrapThenHandler({ ctx, origThen })
+    return ctx
+  }
+
+  /**
+   * wraps the current `_then` method on the instance.
+   * It binds the context from when promise was constructed to the callbacks(didFulfill, didReject, didProgress)
+   *
+   * @param {object} params to function
+   * @param {Context} params.ctx active context
+   * @param {Function} params.origThen the original `_then` method on promise instance
+   * @returns {Function} wrapped `_then` function
+   */
+  #wrapThenHandler({ ctx, origThen }) {
+    const self = this
+    return function wrappedThenHandler(...args) {
+      const [didFulfill, didReject, didProgress] = args
+      // only bind the callback if the transaction from context is still active
+      if (ctx.transaction.isActive() === false) {
+        return origThen.apply(this, args)
+      }
+      args[0] = self.agent.tracer.bindFunction(didFulfill, ctx)
+      args[1] = self.agent.tracer.bindFunction(didReject, ctx)
+
+      // didProgress is only applicable to v2.x
+      if (didProgress) {
+        args[2] = self.agent.tracer.bindFunction(didProgress, ctx)
+      }
+      return self.agent.tracer.bindFunction(origThen, ctx).apply(this, args)
+    }
+  }
+}

lib/subscribers/bluebird/instrumentation.js

@@ -6,6 +6,7 @@
 'use strict'
 
 const test = require('node:test')
+const assert = require('node:assert')
 
 const helper = require('../../lib/agent_helper')
 const { version } = require('bluebird/package.json')
@@ -21,9 +22,6 @@ const {
   testThrowBehavior,
   testTryBehavior,
 } = require('./common-tests')
-const {
-  areMethodsWrapped,
-} = require('./helpers')
 
 testTryBehavior('try')
 testTryBehavior('attempt')
@@ -43,11 +41,18 @@ testAsCallbackBehavior('nodeify')
 testCatchBehavior('catch')
 testCatchBehavior('caught')
 
-test('bluebird static and instance methods check', function (t) {
+test('tracking metrics', async function (t) {
   const agent = helper.loadTestAgent(t)
+  t.after(() => {
+    helper.unloadAgent(agent)
+  })
   const Promise = require('bluebird')
+  await helper.runInTransaction(agent, async (tx) => {
+    assert.ok(tx)
+    await Promise.resolve()
+    const ctx = agent.tracer.getContext()
+    assert.ok(ctx.transaction)
+  })
 
-  areMethodsWrapped(Promise)
-  areMethodsWrapped(Promise.prototype)
-  assertPackageMetrics({ agent, pkg: 'bluebird', version })
+  assertPackageMetrics({ agent, pkg: 'bluebird', version, subscriberType: true })
 })

test/versioned/bluebird/baseline.test.js

@@ -19,13 +19,12 @@ const {
 async function testPromiseContext({ t, factory }) {
   await t.test('context switch', async function (t) {
     const { agent, Promise } = t.nr
-    factory = factory.bind(null, Promise)
     const plan = tspl(t, { plan: 2 })
 
     const ctxA = helper.runInTransaction(agent, function (tx) {
       return {
         transaction: tx,
-        promise: factory('[tx a] ')
+        promise: factory(Promise, '[tx a] ')
       }
     })
 
@@ -50,7 +49,6 @@ async function testPromiseContext({ t, factory }) {
   await t.test('context loss', async function (t) {
     const plan = tspl(t, { plan: 2 })
     const { agent, Promise } = t.nr
-    factory = factory.bind(null, Promise)
 
     const ctxA = helper.runInTransaction(agent, function (tx) {
       t.after(function () {
@@ -59,7 +57,7 @@ async function testPromiseContext({ t, factory }) {
 
       return {
         transaction: tx,
-        promise: factory('[tx a] ')
+        promise: factory(Promise, '[tx a] ')
       }
     })
 
@@ -77,9 +75,8 @@ async function testPromiseContext({ t, factory }) {
   await t.test('context gain', async function (t) {
     const plan = tspl(t, { plan: 2 })
     const { agent, Promise } = t.nr
-    factory = factory.bind(null, Promise)
 
-    const promise = factory('[no tx] ')
+    const promise = factory(Promise, '[no tx] ')
 
     plan.ok(!agent.tracer.getTransaction(), 'should not be in transaction')
     helper.runInTransaction(agent, function (tx) {
@@ -97,12 +94,11 @@ async function testPromiseContext({ t, factory }) {
   await t.test('context expiration', async function (t) {
     const plan = tspl(t, { plan: 2 })
     const { agent, Promise } = t.nr
-    factory = factory.bind(null, Promise)
 
     const ctxA = helper.runInTransaction(agent, function (tx) {
       return {
         transaction: tx,
-        promise: factory('[tx a] ')
+        promise: factory(Promise, '[tx a] ')
       }
     })
 

test/versioned/bluebird/common-tests.js

@@ -4,12 +4,11 @@
  */
 
 'use strict'
-const assert = require('node:assert')
 const { runMultiple } = require('../../lib/promises/helpers')
 const { tspl } = require('@matteo.collina/tspl')
-const symbols = require('../../../lib/symbols')
 const helper = require('../../lib/agent_helper')
 const { setImmediate } = require('timers/promises')
+const { removeModules } = require('../../lib/cache-buster')
 
 async function beforeEach(ctx) {
   ctx.nr = {}
@@ -28,6 +27,7 @@ async function beforeEach(ctx) {
 async function afterEach(ctx) {
   helper.unloadAgent(ctx.nr.agent)
   clearInterval(ctx.nr.interval)
+  removeModules(['bluebird'])
 
   await setImmediate()
 }
@@ -123,27 +123,9 @@ function testPromiseMethod({ t, count, factory, end }) {
   }
 }
 
-function areMethodsWrapped(source) {
-  const methods = Object.keys(source).sort()
-  methods.forEach((method) => {
-    const wrapped = source[method]
-    const original = wrapped[symbols.original]
-
-    // Skip this property if it is internal (starts or ends with underscore), is
-    // a class (starts with a capital letter), or is not a function.
-    if (/(?:^[_A-Z]|_$)/.test(method) || typeof original !== 'function') {
-      return
-    }
-
-    assert.ok(original, `${method} original exists`)
-    assert.notEqual(wrapped, original, `${method} wrapped is not diff from original`)
-  })
-}
-
 module.exports = {
   addTask,
   afterEach,
-  areMethodsWrapped,
   beforeEach,
   id,
   testPromiseClassMethod,

test/versioned/bluebird/helpers.js

@@ -6,20 +6,24 @@
 'use strict'
 
 // Some tests in this file need to assert that we handle non-error rejections:
-const assert = require('node:assert')
 const test = require('node:test')
 const semver = require('semver')
-
-const symbols = require('../../../lib/symbols')
-const helper = require('../../lib/agent_helper')
-
+const { testPromiseContext } = require('./common-tests')
+const { beforeEach, afterEach } = require('./helpers')
 const { version: pkgVersion } = require('bluebird/package')
 
-test('Promise.getNewLibraryCopy', { skip: semver.lt(pkgVersion, '3.4.1') }, function (t) {
-  helper.loadTestAgent(t)
-  const Promise = require('bluebird')
-  const Promise2 = Promise.getNewLibraryCopy()
+test('Promise.getNewLibraryCopy', { skip: semver.lt(pkgVersion, '3.4.1') }, async function (t) {
+  t.beforeEach((ctx) => {
+    beforeEach(ctx)
+    ctx.nr.Promise = ctx.nr.Promise.getNewLibraryCopy()
+  })
+
+  t.afterEach(afterEach)
 
-  assert.ok(Promise2.resolve[symbols.original], 'should have wrapped class methods')
-  assert.ok(Promise2.prototype.then[symbols.original], 'should have wrapped instance methods')
+  await testPromiseContext({
+    t,
+    factory: function (Promise, name) {
+      return Promise.resolve(name)
+    }
+  })
 })