insights: added request_uri attribute to transaction and error events

lib/errors/aggregator.js

@@ -1,5 +1,6 @@
 'use strict'
 
+var copy = require('../util/copy')
 var urltils = require('../util/urltils')
 var logger = require('../logger').child({component: 'error_tracer'})
 var NAMES = require('../metrics/names')
@@ -198,7 +199,27 @@ function _collect(transaction, exception, customParameters, timestamp) {
 
   if (this.errors.length < MAX_ERRORS) {
     logger.debug({error: error}, "Error to be sent to collector:")
-    this.errors.push(error)
+
+    // XXX: 2016-05-24 Remove this when APM UI is updated to use correct request_uri
+    //
+    // For right now, when this flag is enabled, the request_uri will be added
+    // to the error data. This will result in duplicated data being displayed on
+    // APM which is a no-go, so we need to remove it here. However, we want the
+    // data to still be there for error events metrics, so we need to perform a
+    // deep copy and only remove it from this data.
+    //
+    // In order to save cycles, we perform a smart deep copy in the form of a
+    // series of shallow copies down just the path that needs to change.
+    if (this.config.feature_flag.send_request_uri_attribute) {
+      var err = []
+      err.push.apply(err, error)
+      err[4] = copy.shallow(err[4])
+      err[4].agentAttributes = copy.shallow(err[4].agentAttributes)
+      delete err[4].agentAttributes.request_uri
+      this.errors.push(err)
+    } else {
+      this.errors.push(error)
+    }
   } else {
     logger.debug("Already have %d errors to send to collector, not keeping.",
                  MAX_ERRORS)

lib/errors/index.js

@@ -62,7 +62,7 @@ function createError(transaction, exception, customParameters, config) {
       params.request_uri = transaction.getScrubbedUrl()
     }
 
-    // Copy all the parameters off of the transaction.
+    // Copy all of the parameters off of the transaction.
     params.agentAttributes = transaction.trace.parameters
     params.intrinsics = transaction.getIntrinsicAttributes()
 
@@ -167,4 +167,3 @@ function _getErrorEventIntrinsicAttrs(transaction, errorClass, message, timestam
 
   return attributes
 }
-

lib/feature_flags.js

@@ -8,7 +8,8 @@ exports.prerelease = {
   express5: false,
   synthetics: true,
   express_segments: true,
-  promise_segments: false
+  promise_segments: false,
+  send_request_uri_attribute: false
 }
 
 // flags that are no longer used for released features

lib/instrumentation/core/http.js

@@ -82,6 +82,11 @@ function wrapListener(agent, listener) {
     transaction.url = transaction.parsedUrl.pathname
     transaction.verb = request.method
 
+    // URL is sent as an agent attribute with transaction events
+    if (agent.config.feature_flag.send_request_uri_attribute) {
+      transaction.addAgentAttribute('request_uri', transaction.url)
+    }
+
     // store the port on which this transaction runs
     if (typeof this.address === 'function') {
       var address = this.address()

lib/transaction/trace/index.js

@@ -1,6 +1,7 @@
 'use strict'
 
 var codec = require('../../util/codec')
+var copy = require('../../util/copy')
 var Segment = require('./segment')
 var NAMES = require('../../metrics/names.js')
 
@@ -165,6 +166,13 @@ Trace.prototype.getTotalTimeDurationInMillis = function getTotalTimeDurationInMi
  *                            the serialized transaction trace.
  */
 Trace.prototype.generateJSON = function generateJSON(callback) {
+  var attributes = {
+    agentAttributes: copy.shallow(this.parameters),
+    userAttributes: this.custom,
+    intrinsics: this.intrinsics
+  }
+  delete attributes.agentAttributes.request_uri
+
   var rootNode = [
     this.root.timer.start * FROM_MILLIS,
     {}, // moved to agentAttributes
@@ -173,11 +181,7 @@ Trace.prototype.generateJSON = function generateJSON(callback) {
       nr_flatten_leading: false
     }, // moved to userAttributes
     this.root.toJSON(),
-    {
-      agentAttributes: this.parameters,
-      userAttributes: this.custom,
-      intrinsics: this.intrinsics
-    },
+    attributes,
     []  // FIXME: parameter groups
   ]
 

lib/util/copy.js

@@ -0,0 +1,21 @@
+'use strict'
+
+exports.shallow = shallowCopy
+
+/**
+ * Performs a shallow copy of all properties on the source object.
+ *
+ * @param {object} source     - The object to copy the properties from.
+ * @param {object} [dest={}]  - The object to copy the properties to.
+ *
+ * @return {object} The destination object.
+ */
+function shallowCopy(source, dest) {
+  dest = dest || {}
+  for (var k in source) {
+    if (source.hasOwnProperty(k)) {
+      dest[k] = source[k]
+    }
+  }
+  return dest
+}

test/integration/agent/send-errors.tap.js

@@ -1,47 +1,97 @@
 'use strict'
 
+var helper = require('../../lib/agent_helper')
 var path = require('path')
 var test = require('tap').test
-var configurator = require('../../../lib/config')
 var Agent = require('../../../lib/agent')
 
-test("Agent should send errors to staging-collector.newrelic.com", function (t) {
-  var config = configurator.initialize({
-        'app_name': 'node.js Tests',
-        'license_key': 'd67afc830dab717fd163bfcb0b8b88423e9a1a3b',
-        'host': 'staging-collector.newrelic.com',
-        'port': 80,
-        'ssl': false,
-        'utilization': {
-          'detect_aws': false,
-          'detect_docker': false
-        },
-        'logging': {
-          'level': 'trace'
+test('Agent#_sendErrors', function(t) {
+  var config = {
+    'app_name': 'node.js Tests',
+    'license_key': 'd67afc830dab717fd163bfcb0b8b88423e9a1a3b',
+    'host': 'staging-collector.newrelic.com',
+    'port': 80,
+    'ssl': false,
+    'utilization': {
+      'detect_aws': false,
+      'detect_docker': false
+    },
+    'logging': {
+      'level': 'trace'
+    }
+  }
+
+  t.test('without ssl', function(t) {
+    config.port = 80
+    config.ssl = false
+    var agent = setupAgent(t, config)
+    _testSendErrors(t, agent)
+  })
+
+  t.test('with ssl', function(t) {
+    config.port = 443
+    config.ssl = true
+    var agent = setupAgent(t, config)
+    _testSendErrors(t, agent)
+  })
+
+  function _testSendErrors(t, agent) {
+    t.plan(6)
+
+    agent.start(function(err) {
+      if (!t.notOk(err, 'should connect without error')) {
+        console.log('Connection error:', err)
+        return t.end()
+      }
+
+      agent.collector.errorData = function(payload, cb) {
+        // console.log('errorData', payload)
+        if (!t.ok(payload, 'should get the payload')) {
+          return cb()
         }
-      })
-  var agent = new Agent(config)
 
-  agent.start(function cb_start(error) {
-    t.notOk(error, "connected without error")
+        var errData = payload[1][0][4]
+        if (!t.ok(errData, 'should contain error information')) {
+          return cb()
+        }
 
-    var transaction
-    var proxy = agent.tracer.transactionProxy(function cb_transactionProxy() {
-      transaction = agent.getTransaction()
-      transaction.setName('/nonexistent', 501)
-    })
-    proxy()
-    t.ok(transaction, "got a transaction")
-    agent.errors.add(transaction, new Error('test error'))
+        t.equal(errData.request_uri, '/nonexistent', 'should have request_uri')
+
+        var attrs = errData.agentAttributes
+        t.deepEqual(attrs, {foo: 'bar'}, 'should have the correct attributes')
+
+        cb()
+      }
+
+      agent.on('transactionFinished', function(transaction) {
+        agent._sendErrors(function(error) {
+          if (!t.notOk(error, "sent errors without error")) {
+            console.log('Send error:', error)
+          }
 
-    agent._sendErrors(function cb__sendErrors(error) {
-      t.notOk(error, "sent errors without error")
+          agent.stop(function(error) {
+            t.notOk(error, "stopped without error")
 
-      agent.stop(function cb_stop(error) {
-        t.notOk(error, "stopped without error")
+            t.end()
+          })
+        })
+      })
 
-        t.end()
+      helper.runInTransaction(agent, function(tx) {
+        tx.setName('/nonexistent', 501)
+        tx.addAgentAttribute('foo', 'bar')
+        tx.addAgentAttribute('request_uri', '/nonexistent')
+        agent.errors.add(tx, new Error('test error'))
+        tx.end()
       })
     })
-  })
+  }
+
+  function setupAgent(t, config) {
+    var agent = helper.loadMockedAgent({'send_request_uri_attribute': true}, config)
+    t.tearDown(function() {
+      helper.unloadAgent(agent)
+    })
+    return agent
+  }
 })

test/integration/agent/send-metrics.tap.js

@@ -31,11 +31,12 @@ test("Agent should send metrics to staging-collector.newrelic.com", function (t)
     t.ok(findMetric(metrics, 'TEST/discard'), 'the test metric should be present')
 
     agent._sendMetrics(function cb__sendMetrics(error) {
-      t.notOk(error, "sent metrics without error")
+      if (!t.notOk(error, "sent metrics without error")) {
+        console.error(error)
+      }
 
       agent.stop(function cb_stop(error) {
         t.notOk(error, "stopped without error")
-
         t.end()
       })
     })
@@ -47,4 +48,4 @@ function findMetric(metrics, name) {
     var metric = metrics[i]
     if (metric[0].name === name) return metric
   }
-}
+}