fix: Fixed transaction-shim to properly create new transactions when the existing transaction is not active (#2912)

Author: bizob2828

lib/shim/transaction-shim.js

@@ -372,12 +372,13 @@ function _makeNestedTransWrapper(shim, func, name, spec) {
     }
 
     let context = shim.tracer.getContext()
+    let transaction = shim.tracer.getTransaction()
 
     // Only create a new transaction if we either do not have a current
     // transaction _or_ the current transaction is not of the type we want.
-    if (!context?.transaction || spec.type !== context?.transaction?.type) {
+    if (!transaction || spec.type !== transaction?.type) {
       shim.logger.trace('Creating new nested %s transaction for %s', spec.type, name)
-      const transaction = new Transaction(shim.agent)
+      transaction = new Transaction(shim.agent)
       transaction.type = spec.type
       context = context.enterTransaction(transaction)
     }
@@ -405,10 +406,11 @@ function _makeNestedTransWrapper(shim, func, name, spec) {
  */
 function _makeTransWrapper(shim, func, name, spec) {
   return function transactionWrapper() {
-    // Don't nest transactions, reuse existing ones!
     let context = shim.tracer.getContext()
-    const existingTransaction = context.transaction
+    // Don't nest transactions, reuse existing ones!
+    const existingTransaction = shim.tracer.getTransaction()
     if (!shim.agent.canCollectData() || existingTransaction) {
+      shim.logger.trace('Transaction %s exists, not creating new transaction %s for %s', existingTransaction?.id, spec.type, name)
       return func.apply(this, arguments)
     }
 

test/unit/shim/transaction-shim.test.js

@@ -12,6 +12,7 @@ const helper = require('../../lib/agent_helper')
 const { TransactionSpec } = require('../../../lib/shim/specs')
 const TransactionShim = require('../../../lib/shim/transaction-shim')
 const notRunningStates = ['stopped', 'stopping', 'errored']
+const sinon = require('sinon')
 
 /**
  * Creates CAT headers to be used in handleMqTracingHeaders
@@ -220,15 +221,16 @@ test('TransactionShim', async function (t) {
 
     await t.test('should not create a nested transaction when `spec.nest` is false', function (t) {
       const { shim } = t.nr
+      sinon.stub(shim.logger, 'trace')
       let webTx = null
       let bgTx = null
       let webCalled = false
       let bgCalled = false
-      const bg = shim.bindCreateTransaction(function () {
+      const bg = shim.bindCreateTransaction(function bgTxFn() {
         bgCalled = true
         bgTx = shim.tracer.getTransaction()
       }, new TransactionSpec({ type: shim.BG }))
-      const web = shim.bindCreateTransaction(function () {
+      const web = shim.bindCreateTransaction(function webTxFn() {
         webCalled = true
         webTx = shim.tracer.getTransaction()
         bg()
@@ -237,7 +239,51 @@ test('TransactionShim', async function (t) {
       web()
       assert.equal(webCalled, true)
       assert.equal(bgCalled, true)
-      assert.equal(webTx, bgTx)
+      assert.equal(webTx.id, bgTx.id)
+      assert.deepEqual(shim.logger.trace.args, [
+        ['Wrapping nodule itself (%s).', 'bgTxFn'],
+        ['Wrapping nodule itself (%s).', 'webTxFn'],
+        ['Creating new %s transaction for %s', 'web', 'webTxFn'],
+        ['Applying segment %s', 'ROOT'],
+        [
+          'Transaction %s exists, not creating new transaction %s for %s',
+          bgTx.id,
+          'bg',
+          'bgTxFn'
+        ]
+      ])
+    })
+
+    await t.test('should create a new transaction when `spec.nest` is false and current transaction is not active', function (t) {
+      const { shim } = t.nr
+      sinon.stub(shim.logger, 'trace')
+      let webTx = null
+      let bgTx = null
+      let webCalled = false
+      let bgCalled = false
+      const bg = shim.bindCreateTransaction(function bgTxFn() {
+        bgCalled = true
+        bgTx = shim.tracer.getTransaction()
+      }, new TransactionSpec({ type: shim.BG }))
+      const web = shim.bindCreateTransaction(function webTxFn() {
+        webCalled = true
+        webTx = shim.tracer.getTransaction()
+        webTx.end()
+        bg()
+      }, new TransactionSpec({ type: shim.WEB }))
+
+      web()
+      assert.equal(webCalled, true)
+      assert.equal(bgCalled, true)
+      assert.notEqual(webTx.id, bgTx.id)
+      assert.deepEqual(shim.logger.trace.args, [
+        ['Wrapping nodule itself (%s).', 'bgTxFn'],
+        ['Wrapping nodule itself (%s).', 'webTxFn'],
+        ['Creating new %s transaction for %s', 'web', 'webTxFn'],
+        ['Applying segment %s', 'ROOT'],
+        ['Creating new %s transaction for %s', 'bg', 'bgTxFn'],
+        ['Applying segment %s', 'ROOT']
+      ])
     })
 
     for (const agentState of notRunningStates) {
@@ -319,6 +365,25 @@ test('TransactionShim', async function (t) {
       }
     })
 
+    await t.test('should not nest transaction if first transaction is inactive and same type', function (t) {
+      const { shim } = t.nr
+      const transactions = []
+      const web = shim.bindCreateTransaction(function (cb) {
+        const tx = shim.tracer.getTransaction()
+        transactions.push(tx)
+        tx.end()
+        if (cb) {
+          cb()
+        }
+      }, new TransactionSpec({ type: shim.WEB, nest: true }))
+
+      const web2 = shim.bindCreateTransaction(function () {
+        transactions.push(shim.tracer.getTransaction())
+      }, new TransactionSpec({ type: shim.WEB, nest: true }))
+      web(web2)
+      assert.notEqual(transactions[0].id, transactions[1].id)
+    })
+
     for (const agentState of notRunningStates) {
       await t.test(`should not create transaction when agent state is ${agentState}`, (t) => {
         const { agent, shim } = t.nr