Repository: newrelic/newrelic-node-apollo-server-plugin
Pull request #69 (2021-02-23 20:55:33)

Fixes issues setting transaction name when state loss occurs.
<!--
Thank you for submitting a Pull Request.

This code is leveraged to monitor critical services. Please consider the following:
* Tests are required.
* Performance matters.
* Features that are specific to just your app are unlikely to make it in.

Please fill out the relevant sections as follows:
* Proposed Release Notes: Bulleted list of recommended release notes for the change(s).
* Links: Any relevant links for the change.
* Details: In-depth description of changes, other technical notes, etc.
-->

## Proposed Release Notes

* Fixed issue where state loss introduced through another module could result in the plugin causing a query to error while trying to set the transaction name.

## Links

* Fixes: https://github.com/newrelic/newrelic-node-apollo-server-plugin/issues/67

## Details

This does not solve the underlying issue of state loss, which is potentially from the usage of AWS XRay in the linked issue. That is a problem likely outside this plugin.


Patch:
diff --git a/lib/create-plugin.js b/lib/create-plugin.js
index 00f7861..ac89eb0 100644
--- a/lib/create-plugin.js
+++ b/lib/create-plugin.js
@@ -221,7 +221,7 @@ function createPlugin(instrumentationApi, config = {}) {
             transactionName = formattedOperation
           }
 
-          setTransactionName(instrumentationApi.agent, transactionName)
+          setTransactionName(operationSegment.transaction, transactionName)
 
           operationSegment.name = `${OPERATION_PREFIX}/${segmentName}`
           operationSegment.end()
@@ -308,9 +308,7 @@ function flattenToArray(fieldPath) {
   return pathArray
 }
 
-function setTransactionName(agent, transactionName) {
-  const transaction = agent.tracer.getTransaction()
-
+function setTransactionName(transaction, transactionName) {
   const nameState = transaction.nameState
   if (!nameState.graphql) {
     // Override previously set path stack set thus far by web framework.


Test patch:
diff --git a/tests/integration/plugin-state-loss-tester.js b/tests/integration/plugin-state-loss-tester.js
new file mode 100644
index 0000000..cb060ef
--- /dev/null
+++ b/tests/integration/plugin-state-loss-tester.js
@@ -0,0 +1,51 @@
+'use strict'
+
+function createStateLossPlugin(stateLossConfig, instrumentationApi) {
+  return {
+    requestDidStart() {
+      triggerStateLoss(stateLossConfig.onRequestDidStart)
+
+      return {
+        willSendResponse() {
+          triggerStateLoss(stateLossConfig.onWillSendResponse)
+        }
+      }
+    }
+  }
+
+  function triggerStateLoss(shouldTrigger) {
+    if (shouldTrigger) {
+      // Setting active segment to null to mimic state loss
+      instrumentationApi.setActiveSegment(null)
+    }
+  }
+}
+
+class PluginStateLossTester {
+  constructor() {
+    this.stateLossConfig = {
+      onRequestDidStart: false,
+      onWillSendResponse: false
+    }
+  }
+
+  getCreatePlugin() {
+    return createStateLossPlugin.bind(null, this.stateLossConfig)
+  }
+
+  clearStateLoss() {
+    Object.keys(this.stateLossConfig).forEach((key) => {
+      this.stateLossConfig[key] = false
+    })
+  }
+
+  triggerOnRequestDidStart() {
+    this.stateLossConfig.onRequestDidStart = true
+  }
+
+  tiggerOnWillSendResponse() {
+    this.stateLossConfig.onWillSendResponse = true
+  }
+}
+
+module.exports = PluginStateLossTester
diff --git a/tests/integration/state-loss.test.js b/tests/integration/state-loss.test.js
index b3a7dc4..7fe562d 100644
--- a/tests/integration/state-loss.test.js
+++ b/tests/integration/state-loss.test.js
@@ -10,16 +10,26 @@ const { setupEnvConfig } = require('../agent-testing')
 
 const { setupApolloServerTests } = require('../apollo-server-setup')
 
+const PluginStateLossTester = require('./plugin-state-loss-tester')
+const stateLossTester = new PluginStateLossTester()
+
 setupApolloServerTests({
   suiteName: 'State Loss',
   createTests: createStateLossTests,
-  startingPlugins: [createStateLossPlugin]
+  startingPlugins: [stateLossTester.getCreatePlugin()]
 })
 
 function createStateLossTests(t) {
   setupEnvConfig(t)
 
-  t.test('should should not error when state loss prior to query', (t) => {
+  t.afterEach((done) => {
+    stateLossTester.clearStateLoss()
+    done()
+  })
+
+  t.test('should not error when state loss prior to query', (t) => {
+    stateLossTester.triggerOnRequestDidStart()
+
     const { serverUrl } = t.context
 
     const expectedName = 'GetAllForLibrary'
@@ -53,13 +63,41 @@ function createStateLossTests(t) {
       t.end()
     })
   })
-}
 
-function createStateLossPlugin(instrumentationApi) {
-  return {
-    requestDidStart() {
-      // Setting active segment to null to mimic state loss
-      instrumentationApi.setActiveSegment(null)
-    }
-  }
+  t.test('should not error when state loss just before sending response', (t) => {
+    stateLossTester.tiggerOnWillSendResponse()
+
+    const { serverUrl } = t.context
+
+    const expectedName = 'GetAllForLibrary'
+    const query = `query ${expectedName} {
+      library(branch: "downtown") {
+        books {
+          title
+          author {
+            name
+          }
+        }
+        magazines {
+          title
+          issue
+        }
+      }
+    }`
+
+    executeQuery(serverUrl, query, (err, result) => {
+      t.error(err)
+
+      if (result.errors) {
+        result.errors.forEach((error) => {
+          t.error(error)
+        })
+      }
+
+      t.ok(result.data)
+      t.ok(result.data.library)
+
+      t.end()
+    })
+  })
 }