Merge pull request newrelic#1349 from NodeJS-agent/nwolfe/start-segment

NODE-1012 Added `startSegment` to replace `createTracer`

api.js

@@ -629,13 +629,23 @@ API.prototype.getBrowserTimingHeader = function getBrowserTimingHeader() {
   return out
 }
 
+API.prototype.createTracer = util.deprecate(
+  createTracer, [
+    'API#createTracer is being deprecated!',
+    'Please use API#startSegment for segment creation.'
+  ].join(' ')
+)
+
 /**
  * This creates a new tracer with the passed in name. It then wraps the
  * callback and binds it to the current transaction and segment so any further
  * custom instrumentation as well as auto instrumentation will also be able to
  * find the current transaction and segment.
+ *
+ * @memberof API#
+ * @deprecated use {@link API#startSegment} instead
  */
-API.prototype.createTracer = function createTracer(name, callback) {
+function createTracer(name, callback) {
   var metric = this.agent.metrics.getOrCreateMetric(
     NAMES.SUPPORTABILITY.API + '/createTracer'
   )
@@ -687,6 +697,70 @@ API.prototype.createTracer = function createTracer(name, callback) {
   return arity.fixArity(callback, tracer.bindFunction(callback, segment, true))
 }
 
+/**
+ * Wraps the given handler in a segment which may optionally be turned into a
+ * metric.
+ *
+ * @example
+ *  newrelic.startSegment('mySegment', false, function handler() {
+ *    // The returned promise here will signify the end of the segment.
+ *    return myAsyncTask().then(myNextTask)
+ *  })
+ *
+ * @param {string} name
+ *  The name to give the new segment. This will also be the name of the metric.
+ *
+ * @param {bool} record
+ *  Indicates if the segment should be recorded as a metric. Metrics will show
+ *  up on the transaction breakdown table and server breakdown graph. Segments
+ *  just show up in transaction traces.
+ *
+ * @param {function(cb) -> ?Promise} handler
+ *  The function to track as a segment.
+ *
+ * @param {function} [callback]
+ *  An optional callback for the handler. This will indicate the end of the
+ *  timing if provided.
+ *
+ * @return {*} Returns the result of calling `handler`.
+ */
+API.prototype.startSegment = function startSegment(name, record, handler, callback) {
+  this.agent.metrics.getOrCreateMetric(
+    NAMES.SUPPORTABILITY.API + '/startSegment'
+  ).incrementCallCount()
+
+  // Check that we have usable arguments.
+  if (!name || typeof handler !== 'function') {
+    logger.warn('Name and handler function are both required for startSegment')
+    if (typeof handler === 'function') {
+      return handler(callback)
+    }
+    return
+  }
+  if (callback && typeof callback !== 'function') {
+    logger.warn('If using callback, it must be a function')
+    return handler(callback)
+  }
+
+  // Are we inside a transaction?
+  if (!this.shim.getActiveSegment()) {
+    logger.debug('startSegment(%j) called outside of a transaction, not recording.', name)
+    return handler(callback)
+  }
+
+  // Create the segment and call the handler.
+  var wrappedHandler = this.shim.record(handler, function handlerNamer(shim) {
+    return {
+      name: name,
+      recorder: record ? customRecorder : null,
+      callback: callback ? shim.FIRST : null,
+      promise: !callback
+    }
+  })
+
+  return wrappedHandler(callback)
+}
+
 API.prototype.createWebTransaction = util.deprecate(
   createWebTransaction, [
     'API#createWebTransaction is being deprecated!',
@@ -713,7 +787,7 @@ API.prototype.createWebTransaction = util.deprecate(
                                 name and not iclude any variable parameters.
  * @param {Function}  handle    Function that represents the transaction work.
  *
- * @memberOf API#
+ * @memberof API#
  *
  * @deprecated since version 2.0
  */

examples/api/background-transactions/example1-basic.js

@@ -4,7 +4,7 @@ var newrelic = require('newrelic')
 
 var transactionName = 'myCustomTransaction'
 
-// startBackgroundTransaction() takes a name, group, and a handler function to
+// `startBackgroundTransaction()` takes a name, group, and a handler function to
 // execute. The group is optional. The last parameter is the function performing
 // the work inside the transaction. Once the transaction starts, there are
 // three ways to end it:

examples/api/segments/example1-callbacks.js

@@ -0,0 +1,33 @@
+'use strict'
+
+var newrelic = require('newrelic')
+
+// Segments can only be created inside of transactions. They could be automatically
+// generated HTTP transactions or custom transactions.
+newrelic.startBackgroundTransaction('bg-tx', function transHandler() {
+  var tx = newrelic.getTransaction()
+
+  // `startSegment()` takes a segment name, a boolean if a metric should be
+  // created for this segment, the handler function, and an optional callback.
+  // The handler is the function that will be wrapped with the new segment. When
+  // a callback is provided, the segment timing will end when the callback is
+  // called.
+
+  newrelic.startSegment('myCustomSegment', false, someTask, function cb(err, output) {
+    // Handle the error and output as appropriate.
+    console.log(output)
+    tx.end()
+  })
+})
+
+function someTask(cb) {
+  myAsyncTask(function firstCb(err, result) {
+    if (err) {
+      return cb(err)
+    }
+
+    myNextTask(result, function secondCb(err, output) {
+      cb(err, output)
+    })
+  })
+}

examples/api/segments/example2-promises.js

@@ -0,0 +1,24 @@
+'use strict'
+
+var newrelic = require('newrelic')
+
+// Segments can only be created inside of transactions. They could be automatically
+// generated HTTP transactions or custom transactions.
+newrelic.startBackgroundTransaction('bg-tx', function transHandler() {
+  // `startSegment()` takes a segment name, a boolean if a metric should be
+  // created for this segment, the handler function, and an optional callback.
+  // The handler is the function that will be wrapped with the new segment. If
+  // a promise is returned from the handler, the segment's ending will be tied
+  // to that promise resolving or rejecting.
+
+  return newrelic.startSegment('myCustomSegment', false, someTask)
+    .then(function thenAfter(output) {
+      console.log(output)
+    })
+})
+
+function someTask() {
+  return myAsyncTask().then(function thenNext(result) {
+    return myNextTask(result)
+  })
+}

examples/api/segments/example3-async.js

@@ -0,0 +1,22 @@
+'use strict'
+
+var newrelic = require('newrelic')
+
+// Segments can only be created inside of transactions. They could be automatically
+// generated HTTP transactions or custom transactions.
+newrelic.startBackgroundTransaction('bg-tx', async function transHandler() {
+  // `startSegment()` takes a segment name, a boolean if a metric should be
+  // created for this segment, the handler function, and an optional callback.
+  // The handler is the function that will be wrapped with the new segment.
+  // Since `async` functions just return a promise, they are covered just the
+  // same as the promise example.
+
+  var output = await newrelic.startSegment('myCustomSegment', false, someTask)
+  console.log(output)
+})
+
+async function someTask() {
+  var result = await myAsyncTask()
+  var output = await myNextTask(result)
+  return output
+}

examples/api/segments/example4-sync-assign.js

@@ -0,0 +1,22 @@
+'use strict'
+
+var newrelic = require('newrelic')
+
+// Segments can only be created inside of transactions. They could be automatically
+// generated HTTP transactions or custom transactions.
+newrelic.startBackgroundTransaction('bg-tx', function transHandler() {
+  // `startSegment()` takes a segment name, a boolean if a metric should be
+  // created for this segment, the handler function, and an optional callback.
+  // The handler is the function that will be wrapped with the new segment.
+
+  var output = newrelic.startSegment('myCustomSegment', false, function timedFunction() {
+    return someSyncTask()
+  })
+  console.log(output)
+})
+
+function someSyncTask() {
+  var result = mySyncTask()
+  var output = myNextTask(result)
+  return output
+}

stub_api.js

@@ -30,7 +30,12 @@ for (var i = 0; i < length; i++) {
   Stub.prototype[functionName] = stubFunction(functionName)
 }
 
-Stub.prototype.createTracer = createTracer
+Stub.prototype.createTracer = util.deprecate(
+  createTracer, [
+    'API#createTracer is being deprecated!',
+    'Please use API#startSegment for segment creation.'
+  ].join(' ')
+)
 Stub.prototype.createWebTransaction = util.deprecate(
   createWebTransaction, [
     'API#createWebTransaction is being deprecated!',
@@ -47,6 +52,7 @@ Stub.prototype.createBackgroundTransaction = util.deprecate(
     'ending transactions.'
   ].join(' ')
 )
+Stub.prototype.startSegment = startSegment
 Stub.prototype.startWebTransaction = startWebTransaction
 Stub.prototype.startBackgroundTransaction = startBackgroundTransaction
 Stub.prototype.getTransaction = getTransaction
@@ -81,6 +87,14 @@ function createBackgroundTransaction(name, group, callback) {
   return (callback === undefined) ? group : callback
 }
 
+function startSegment(name, record, handler, callback) {
+  logger.debug('Not calling `startSegment` becuase New Relic is disabled.')
+  if (typeof handler === 'function') {
+    return handler(callback)
+  }
+  return null
+}
+
 function startWebTransaction(url, callback) {
   logger.debug('Not calling startWebTransaction because New Relic is disabled.')
   if (typeof callback === 'function') {

test/unit/api/api.test.js

@@ -262,6 +262,91 @@ describe('the New Relic agent API', function() {
     })
   })
 
+  describe('when creating a segment with `startSegment`', function() {
+    it('should name the segment as provided', function() {
+      helper.runInTransaction(agent, function() {
+        api.startSegment('foobar', false, function() {
+          var segment = api.shim.getSegment()
+          expect(segment).to.exist().and.have.property('name', 'foobar')
+        })
+      })
+    })
+
+    it('should return the return value of the handler', function() {
+      helper.runInTransaction(agent, function() {
+        var obj = {}
+        var ret = api.startSegment('foobar', false, function() {
+          return obj
+        })
+        expect(ret).to.equal(obj)
+      })
+    })
+
+    it('should not record a metric when `record` is `false`', function(done) {
+      helper.runInTransaction(agent, function(tx) {
+        tx.name = 'test'
+        api.startSegment('foobar', false, function() {
+          var segment = api.shim.getSegment()
+          expect(segment).to.exist().and.have.property('name', 'foobar')
+        })
+        tx.end(function() {
+          expect(tx.metrics.scoped).to.not.have.property(tx.name)
+          expect(tx.metrics.unscoped).to.not.have.property('Custom/foobar')
+          done()
+        })
+      })
+    })
+
+    it('should record a metric when `record` is `true`', function(done) {
+      helper.runInTransaction(agent, function(tx) {
+        tx.name = 'test'
+        api.startSegment('foobar', true, function() {
+          var segment = api.shim.getSegment()
+          expect(segment).to.exist().and.have.property('name', 'foobar')
+        })
+        tx.end(function() {
+          expect(tx.metrics.scoped).property(tx.name)
+            .to.have.property('Custom/foobar')
+          expect(tx.metrics.unscoped).to.have.property('Custom/foobar')
+          done()
+        })
+      })
+    })
+
+    it('should time the segment from the callback if provided', function(done) {
+      helper.runInTransaction(agent, function() {
+        api.startSegment('foobar', false, function(cb) {
+          var segment = api.shim.getSegment()
+          setTimeout(cb, 150, null, segment)
+        }, function(err, segment) {
+          expect(err).to.be.null()
+          expect(segment).to.exist()
+          expect(segment.getDurationInMillis()).to.be.within(100, 200)
+          done()
+        })
+      })
+    })
+
+    it('should time the segment from a returned promise', function() {
+      // TODO: Once Node <0.12 is deprecated, remove this check for Promise.
+      if (!global.Promise) {
+        return
+      }
+
+      return helper.runInTransaction(agent, function() {
+        return api.startSegment('foobar', false, function() {
+          var segment = api.shim.getSegment()
+          return new Promise(function(resolve) {
+            setTimeout(resolve, 150, segment)
+          })
+        }).then(function(segment) {
+          expect(segment).to.exist()
+          expect(segment.getDurationInMillis()).to.be.within(100, 200)
+        })
+      })
+    })
+  })
+
   describe("when starting a web transaction using startWebTransaction", function() {
     var thenCalled = false
     var FakePromise = {