Merge pull request newrelic#1378 from NodeJS-agent/feat/protocol-15

NODE-1646 Backpack

.eslintrc

@@ -9,12 +9,13 @@
   },
   "rules": {
     "brace-style": "error",
+    "comma-dangle": "off",
     "comma-style": ["error", "last"],
     "consistent-return": "off",
     "curly": "off",
     "eol-last": "error",
     "eqeqeq": ["error", "smart"],
-    "camelcase": "off",
+    "camelcase": ["off", {"properties": "never"}],
     "dot-notation": "error",
     "func-names": "error",
     "guard-for-in": "error",
@@ -23,7 +24,6 @@
     "max-nested-callbacks": ["error", 3],
     "max-params": ["error", 5],
     "new-cap": "error",
-    "no-comma-dangle": "off",
     "no-console": "warn",
     "no-else-return": "error",
     "no-floating-decimal": "error",
@@ -35,9 +35,8 @@
     "no-new-func": "warn",
     "no-shadow": ["warn", {"allow": ["shim"]}],
     "no-undef": "error",
-    "no-underscore-dangle": "off",
     "no-unused-vars": "error",
-    "no-use-before-define": "off",
+    "no-use-before-define": ["off", {"functions": false}],
     "one-var": ["off", "never"],
     "padded-blocks": ["error", "never"],
     "radix": "error",

lib/collector/api.js

@@ -60,7 +60,7 @@ function CollectorAPI(agent) {
    * save some GC time?
    */
   this._methods = {
-    redirect: new RemoteMethod('get_redirect_host', agent.config),
+    redirect: new RemoteMethod('preconnect', agent.config),
     handshake: new RemoteMethod('connect', agent.config),
     settings: new RemoteMethod('agent_settings', agent.config),
     errors: new RemoteMethod('error_data', agent.config),
@@ -151,7 +151,7 @@ CollectorAPI.prototype._login = function _login(callback) {
         )
 
         agent.config.host = parts[0]
-        agent.config.port = parts[1] || (agent.config.ssl ? 443 : 80)
+        agent.config.port = parts[1] || 443
       }
     }
 

lib/collector/http-agents.js

@@ -35,7 +35,7 @@ function proxyOptions(config) {
     var parsed_url = parse(config.proxy)
 
     var proxy_url = {
-      protocol: parsed_url.protocol || 'http:',
+      protocol: parsed_url.protocol || 'https:',
       host: parsed_url.hostname,
       port: parsed_url.port || 80,
       auth: parsed_url.auth
@@ -48,7 +48,7 @@ function proxyOptions(config) {
 
     // Unless a proxy config is provided, default to HTTP.
     proxy_url = {
-      protocol: 'http:',
+      protocol: 'https:',
       host: config.proxy_host || 'localhost',
       port: config.proxy_port || 80,
       auth: proxy_auth

lib/collector/key-parser.js

@@ -0,0 +1,6 @@
+'use strict'
+
+module.exports.parseKey = function parseKey(licenseKey) {
+  var regionMatch = /^(.+?)x/.exec(licenseKey)
+  return regionMatch && regionMatch[1]
+}

lib/collector/remote-method.js

@@ -2,7 +2,6 @@
 
 var util = require('util')
 var url = require('url')
-var http = require('http')
 var https = require('https')
 var zlib = require('zlib')
 var logger = require('../logger').child({component: 'remote_method'})
@@ -17,11 +16,11 @@ var certificates = require('./ssl/certificates')
  * CONSTANTS
  *
  */
-var PROTOCOL_VERSION = 14
+var PROTOCOL_VERSION = 15
 var RUN_ID_NAME = 'run_id'
 var RAW_METHOD_PATH = '/agent_listener/invoke_raw_method'
   // see job/collector-master/javadoc/com/nr/servlet/AgentListener.html on NR Jenkins
-var USER_AGENT_FORMAT = "NewRelic-NodeAgent/%s (nodejs %s %s-%s)"
+var USER_AGENT_FORMAT = 'NewRelic-NodeAgent/%s (nodejs %s %s-%s)'
 var ENCODING_HEADER = 'CONTENT-ENCODING'
 var CONTENT_TYPE_HEADER = 'Content-Type'
 var DEFAULT_ENCODING = 'identity'
@@ -31,7 +30,7 @@ var COMPRESSED_CONTENT_TYPE = 'application/octet-stream'
 
 function RemoteMethod(name, config) {
   if (!name) {
-    throw new TypeError("Must include name of method to invoke on collector.")
+    throw new TypeError('Must include name of method to invoke on collector.')
   }
 
   this.name = name
@@ -42,8 +41,8 @@ RemoteMethod.prototype.serialize = function serialize(payload, callback) {
   try {
     var res = stringify(payload)
   } catch (error) {
-    logger.error(error, "Unable to serialize payload for method %s.", this.name)
-    return process.nextTick(function cb_nextTick() {
+    logger.error(error, 'Unable to serialize payload for method %s.', this.name)
+    return process.nextTick(function onNextTick() {
       return callback(error)
     })
   }
@@ -62,7 +61,7 @@ RemoteMethod.prototype.invoke = function call(payload, callback) {
   if (!payload) payload = []
   logger.trace('Invoking remote method %s', this.name)
 
-  this.serialize(payload, function cb_serialize(err, serialized) {
+  this.serialize(payload, function onSerialize(err, serialized) {
     if (err) return callback(err)
     this._post(serialized, callback)
   }.bind(this))
@@ -81,9 +80,9 @@ RemoteMethod.prototype._post = function _post(data, callback) {
 
   // set up standard response handling
   function onResponse(response) {
-    response.on('end', function handle_end() {
+    response.on('end', function onEnd() {
       logger.debug(
-        "Finished receiving data back from the collector for %s.",
+        'Finished receiving data back from the collector for %s.',
         method.name
       )
     })
@@ -102,21 +101,21 @@ RemoteMethod.prototype._post = function _post(data, callback) {
   }
 
   if (options.compressed) {
-    logger.trace({data: data}, "Sending %s on collector API with (COMPRESSED)", this.name)
+    logger.trace({data: data}, 'Sending %s on collector API with (COMPRESSED)', this.name)
 
     var useGzip = this._config.compressed_content_encoding === 'gzip'
     var compressor = useGzip ? zlib.gzip : zlib.deflate
-    compressor(data, function cb_compressor(err, compressed) {
+    compressor(data, function onCompress(err, compressed) {
       if (err) {
-        logger.warn(err, "Error compressing JSON for delivery. Not sending.")
+        logger.warn(err, 'Error compressing JSON for delivery. Not sending.')
         return callback(err)
       }
 
       options.body = compressed
       method._safeRequest(options)
     })
   } else {
-    logger.debug({data: data}, "Calling %s on collector API", this.name)
+    logger.debug({data: data}, 'Calling %s on collector API', this.name)
 
     options.body = data
     this._safeRequest(options)
@@ -137,15 +136,15 @@ RemoteMethod.prototype._post = function _post(data, callback) {
  * @param object options A dictionary of request parameters.
  */
 RemoteMethod.prototype._safeRequest = function _safeRequest(options) {
-  if (!options) throw new Error("Must include options to make request!")
-  if (!options.host) throw new Error("Must include collector hostname!")
-  if (!options.port) throw new Error("Must include collector port!")
-  if (!options.onError) throw new Error("Must include error handler!")
-  if (!options.onResponse) throw new Error("Must include response handler!")
-  if (!options.body) throw new Error("Must include body to send to collector!")
-  if (!options.path) throw new Error("Must include URL to request!")
-
-  var protocol = this._config.ssl ? 'https' : 'http'
+  if (!options) throw new Error('Must include options to make request!')
+  if (!options.host) throw new Error('Must include collector hostname!')
+  if (!options.port) throw new Error('Must include collector port!')
+  if (!options.onError) throw new Error('Must include error handler!')
+  if (!options.onResponse) throw new Error('Must include response handler!')
+  if (!options.body) throw new Error('Must include body to send to collector!')
+  if (!options.path) throw new Error('Must include URL to request!')
+
+  var protocol = 'https'
   var logConfig = this._config.logging
   var auditLog = this._config.audit_log
   var level = 'trace'
@@ -215,21 +214,19 @@ RemoteMethod.prototype._request = function _request(options) {
     //
     // This goes against keep-alive, but for now letting the application die
     // gracefully is more important.
-    request.on('response', function cb_on_response(sock) {
-      sock.on('end', function cb_on_end() {
+    request.on('response', function onResponse(sock) {
+      sock.on('end', function onEnd() {
         sock.destroy()
       })
     })
-  } else if (this._config.ssl) {
+  } else {
     if (this._config.certificates && this._config.certificates.length > 0) {
       logger.debug(
         'Adding custom certificate to the cert bundle.'
       )
       requestOptions.ca = this._config.certificates.concat(certificates)
     }
     request = https.request(requestOptions)
-  } else {
-    request = http.request(requestOptions)
   }
 
   request.on('error', options.onError)

lib/config/default.js

@@ -25,7 +25,7 @@ exports.config = {
    *
    * @env NEW_RELIC_HOST
    */
-  host: 'collector.newrelic.com',
+  host: '',
   /**
    * The port on which the collector proxy will be listening.
    *
@@ -37,6 +37,8 @@ exports.config = {
   /**
    * Whether or not to use SSL to connect to New Relic's servers.
    *
+   * NOTE: This option can no longer be disabled.
+   *
    * @env NEW_RELIC_USE_SSL
    */
   ssl: true,

lib/config/index.js

@@ -12,6 +12,7 @@ var flatten = require('../util/flatten')
 var hashes = require('../util/hashes')
 var exists = fs.existsSync || path.existsSync
 var stringify = require('json-stringify-safe')
+var parseKey = require('../collector/key-parser').parseKey
 var psemver = require('../util/process-version')
 var os = require('os')
 var logger = null // Lazy-loaded in `initialize`.
@@ -321,12 +322,6 @@ function Config(config) {
     logger.warn('Unable to clone the default config, %s: %s', DEFAULT_CONFIG_PATH, err)
   }
 
-  if (config &&
-      (process.env[ENV_MAPPING.ssl] === 'false' || config.ssl === false) &&
-      process.env[ENV_MAPPING.port] === undefined && config.port === undefined ) {
-    config.port = 80
-  }
-
   // 2. initialize undocumented, internal-only default values
 
   // feature flags are mostly private settings for gating unreleased features
@@ -494,8 +489,13 @@ Config.prototype._fromServer = function _fromServer(params, key) {
       this._emitIfSet(params, key)
       break
 
-    // setting these can be disabled by ignore_server_configuration
     case 'ssl':
+      if (!isTruthular(params.ssl)) {
+        logger.warn('SSL config key can no longer be disabled, not updating.')
+      }
+      break
+
+    // setting these can be disabled by ignore_server_configuration
     case 'apdex_t':
     case 'web_transactions_apdex':
     case 'data_report_period':
@@ -979,6 +979,11 @@ Config.prototype._fromPassed = function _fromPassed(external, internal, arbitrar
     // if it's not in the defaults, it doesn't exist
     if (!arbitrary && internal[key] === undefined) return
 
+    if (key === 'ssl' && !isTruthular(external.ssl)) {
+      logger.warn('SSL config key can no longer be disabled, not updating.')
+      return
+    }
+
     if (key === 'ignored_params') {
       warnDeprecated(key, 'attributes.exclude')
     }
@@ -1045,7 +1050,9 @@ Config.prototype._fromEnvironment = function _fromEnvironment(metadata, data) {
 
   Object.keys(metadata).forEach(function applyEnvDefault(value) {
     // if it's not in the config, it doesn't exist
-    if (data[value] === undefined) return
+    if (data[value] === undefined) {
+      return
+    }
 
     var node = metadata[value]
     if (typeof node === 'string') {
@@ -1058,6 +1065,10 @@ Config.prototype._fromEnvironment = function _fromEnvironment(metadata, data) {
         } else if (OBJECT_LIST_VARS.indexOf(node) > -1) {
           data[value] = fromObjectList(setting)
         } else if (BOOLEAN_VARS.indexOf(node) > -1) {
+          if (value === 'ssl' && !isTruthular(setting)) {
+            logger.warn('SSL config key can no longer be disabled, not updating.')
+            return
+          }
           data[value] = isTruthular(setting)
         } else if (FLOAT_VARS.indexOf(node) > -1) {
           data[value] = parseFloat(setting, 10)
@@ -1096,6 +1107,15 @@ Config.prototype._canonicalize = function _canonicalize() {
   var level = this.logging.level
   this.logging.level = logAliases[level] || level
 
+  if (this.host === '') {
+    var region = parseKey(this.license_key)
+    if (region) {
+      this.host = 'collector.' + region + '.nr-data.net'
+    } else {
+      this.host = 'collector.newrelic.com'
+    }
+  }
+
   // If new props are explicitly set (ie, not the default), use those
   this.attributes.exclude = this.attributes.exclude.length
     ? this.attributes.exclude

test/integration/agent/harvest.tap.js

@@ -1,51 +1,50 @@
 'use strict'
 
-var path = require('path')
-var test = require('tap').test
+var tap = require('tap')
 var configurator = require('../../../lib/config')
 var Agent = require('../../../lib/agent')
 
 
-test("Agent should send a whole harvest to New Relic", function (t) {
+tap.test('Agent should send a whole harvest to New Relic', function(t) {
   var config = configurator.initialize({
-        'app_name': 'node.js Tests',
-        'license_key': 'ed2a0ac637297d08c5592c0200050fe234802223',
-        'port': 80,
-        'ssl': false,
-        'utilization': {
-          'detect_aws': false,
-          'detect_pcf': false,
-          'detect_gcp': false,
-          'detect_docker': false
-        },
-        'logging': {
-          'level': 'trace'
-        }
-      })
+    ssl: true,
+    app_name: 'node.js Tests',
+    license_key: 'ed2a0ac637297d08c5592c0200050fe234802223',
+    port: 443,
+    utilization: {
+      detect_aws: false,
+      detect_pcf: false,
+      detect_gcp: false,
+      detect_docker: false
+    },
+    logging: {
+      level: 'trace'
+    }
+  })
   var agent = new Agent(config)
 
 
-  agent.start(function cb_start(error) {
-    t.notOk(error, "connected without error")
+  agent.start(function(error) {
+    t.notOk(error, 'connected without error')
 
     agent.metrics.measureMilliseconds('TEST/discard', null, 101)
 
     var transaction
-    var proxy = agent.tracer.transactionProxy(function cb_transactionProxy() {
+    var proxy = agent.tracer.transactionProxy(function() {
       transaction = agent.getTransaction()
       transaction.finalizeNameFromUri('/nonexistent', 501)
     })
     proxy()
     // ensure it's slow enough to get traced
     transaction.trace.setDurationInMillis(5001)
     transaction.end(function() {
-      t.ok(agent.traces.trace, "have a slow trace to send")
+      t.ok(agent.traces.trace, 'have a slow trace to send')
 
-      agent.harvest(function cb_harvest(error) {
-        t.notOk(error, "harvest ran correctly")
+      agent.harvest(function(error) {
+        t.notOk(error, 'harvest ran correctly')
 
-        agent.stop(function cb_stop(error) {
-          t.notOk(error, "stopped without error")
+        agent.stop(function(error) {
+          t.notOk(error, 'stopped without error')
 
           t.end()
         })