Add protocol as a configuration option for the dd-trace-agent URL (#416)

* Add protocol as a configuration option for the dd-trace agent URL. This is useful for scenarios where a SSL-terminating load balancer sits in front of the datadog-agent * Fix lint errors * Switching from protocol override to url override per CR * Fixing missing comma from previous commit * Fix tests * overrideUrl -> url
DataDog · Jan 24, 2019 · 3b6f332 · 3b6f332
1 parent c4b8f40
commit 3b6f332
Show file tree

Hide file tree

Showing 5 changed files with 96 additions and 4 deletions.
diff --git a/docs/API.md b/docs/API.md
@@ -387,6 +387,7 @@ Options can be configured as a parameter to the [init()](https://datadog.github.
 | enabled       | DD_TRACE_ENABLED             | true      | Whether to enable the tracer. |
 | debug         | DD_TRACE_DEBUG               | false     | Enable debug logging in the tracer. |
 | service       | DD_SERVICE_NAME              |           | The service name to be used for this program. |
+| url           | DD_TRACE_AGENT_URL           |           | The url of the trace agent that the tracer will submit to. Takes priority over hostname and port, if set. |
 | hostname      | DD_TRACE_AGENT_HOSTNAME      | localhost | The address of the trace agent that the tracer will submit to. |
 | port          | DD_TRACE_AGENT_PORT          | 8126      | The port of the trace agent that the tracer will submit to. |
 | env           | DD_ENV                       |           | Set an application’s environment e.g. `prod`, `pre-prod`, `stage`. |

diff --git a/src/config.js b/src/config.js
@@ -11,6 +11,7 @@ class Config {
     const enabled = coalesce(options.enabled, platform.env('DD_TRACE_ENABLED'), true)
     const debug = coalesce(options.debug, platform.env('DD_TRACE_DEBUG'), false)
     const env = coalesce(options.env, platform.env('DD_ENV'))
+    const url = coalesce(options.url, platform.env('DD_TRACE_AGENT_URL'), null)
     const protocol = 'http'
     const hostname = coalesce(
       options.hostname,
@@ -26,7 +27,7 @@ class Config {
     this.enabled = String(enabled) === 'true'
     this.debug = String(debug) === 'true'
     this.env = env
-    this.url = new URL(`${protocol}://${hostname}:${port}`)
+    this.url = url ? new URL(url) : new URL(`${protocol}://${hostname}:${port}`)
     this.tags = Object.assign({}, options.tags)
     this.flushInterval = flushInterval
     this.bufferSize = 100000

diff --git a/src/platform/node/request.js b/src/platform/node/request.js
@@ -1,6 +1,7 @@
 'use strict'
 
 const http = require('http')
+const https = require('https')
 
 function request (options, callback) {
   options = Object.assign({
@@ -14,7 +15,8 @@ function request (options, callback) {
   options.headers['Content-Length'] = byteLength(data)
 
   return new Promise((resolve, reject) => {
-    const req = http.request(options, res => {
+    const client = options.protocol === 'https:' ? https : http
+    const req = client.request(options, res => {
       let data = ''
 
       res.on('data', chunk => { data += chunk })

diff --git a/src/proxy.js b/src/proxy.js
@@ -36,6 +36,8 @@ class Tracer extends BaseTracer {
    * @param {boolean} [options.enabled=true] Whether to enable the tracer.
    * @param {boolean} [options.debug=false] Enable debug logging in the tracer.
    * @param {string} [options.service] The service name to be used for this program.
+   * @param {string} [options.url=null] The url to the trace agent that the tracer will submit to. Takes
+   * precedence over hostname and port, if set.
    * @param {string} [options.hostname=localhost] The address of the trace agent that the tracer will submit to.
    * @param {number|string} [options.port=8126] The port of the trace agent that the tracer will submit to.
    * @param {number} [options.sampleRate=1] Percentage of spans to sample as a float between 0 and 1.

diff --git a/test/config.spec.js b/test/config.spec.js
@@ -49,12 +49,33 @@ describe('Config', () => {
 
     expect(config).to.have.property('enabled', false)
     expect(config).to.have.property('debug', true)
+    expect(config).to.have.nested.property('url.protocol', 'http:')
     expect(config).to.have.nested.property('url.hostname', 'agent')
     expect(config).to.have.nested.property('url.port', '6218')
     expect(config).to.have.property('service', 'service')
     expect(config).to.have.property('env', 'test')
   })
 
+  it('should initialize from environment variables with url taking precedence', () => {
+    platform.env.withArgs('DD_TRACE_AGENT_URL').returns('https://agent2:7777')
+    platform.env.withArgs('DD_TRACE_AGENT_HOSTNAME').returns('agent')
+    platform.env.withArgs('DD_TRACE_AGENT_PORT').returns('6218')
+    platform.env.withArgs('DD_TRACE_ENABLED').returns('false')
+    platform.env.withArgs('DD_TRACE_DEBUG').returns('true')
+    platform.env.withArgs('DD_SERVICE_NAME').returns('service')
+    platform.env.withArgs('DD_ENV').returns('test')
+
+    const config = new Config()
+
+    expect(config).to.have.property('enabled', false)
+    expect(config).to.have.property('debug', true)
+    expect(config).to.have.nested.property('url.protocol', 'https:')
+    expect(config).to.have.nested.property('url.hostname', 'agent2')
+    expect(config).to.have.nested.property('url.port', '7777')
+    expect(config).to.have.property('service', 'service')
+    expect(config).to.have.property('env', 'test')
+  })
+
   it('should initialize from the options', () => {
     const logger = {}
     const tags = { foo: 'bar' }
@@ -74,6 +95,7 @@ describe('Config', () => {
 
     expect(config).to.have.property('enabled', false)
     expect(config).to.have.property('debug', true)
+    expect(config).to.have.nested.property('url.protocol', 'http:')
     expect(config).to.have.nested.property('url.hostname', 'agent')
     expect(config).to.have.nested.property('url.port', '6218')
     expect(config).to.have.property('service', 'service')
@@ -85,6 +107,38 @@ describe('Config', () => {
     expect(config).to.have.property('plugins', false)
   })
 
+  it('should initialize from the options with url taking precedence', () => {
+    const logger = {}
+    const tags = { foo: 'bar' }
+    const config = new Config({
+      enabled: false,
+      debug: true,
+      hostname: 'agent',
+      url: 'https://agent2:7777',
+      port: 6218,
+      service: 'service',
+      env: 'test',
+      sampleRate: 0.5,
+      logger,
+      tags,
+      flushInterval: 5000,
+      plugins: false
+    })
+
+    expect(config).to.have.property('enabled', false)
+    expect(config).to.have.property('debug', true)
+    expect(config).to.have.nested.property('url.protocol', 'https:')
+    expect(config).to.have.nested.property('url.hostname', 'agent2')
+    expect(config).to.have.nested.property('url.port', '7777')
+    expect(config).to.have.property('service', 'service')
+    expect(config).to.have.property('env', 'test')
+    expect(config).to.have.property('sampleRate', 0.5)
+    expect(config).to.have.property('logger', logger)
+    expect(config).to.have.deep.property('tags', tags)
+    expect(config).to.have.property('flushInterval', 5000)
+    expect(config).to.have.property('plugins', false)
+  })
+
   it('should give priority to the common agent environment variable', () => {
     platform.env.withArgs('DD_TRACE_AGENT_HOSTNAME').returns('trace-agent')
     platform.env.withArgs('DD_AGENT_HOST').returns('agent')
@@ -95,6 +149,7 @@ describe('Config', () => {
   })
 
   it('should give priority to the options', () => {
+    platform.env.withArgs('DD_TRACE_AGENT_URL').returns('https://agent2:6218')
     platform.env.withArgs('DD_TRACE_AGENT_HOSTNAME').returns('agent')
     platform.env.withArgs('DD_TRACE_AGENT_PORT').returns('6218')
     platform.env.withArgs('DD_TRACE_ENABLED').returns('false')
@@ -105,6 +160,7 @@ describe('Config', () => {
     const config = new Config({
       enabled: true,
       debug: false,
+      protocol: 'https',
       hostname: 'server',
       port: 7777,
       service: 'test',
@@ -113,8 +169,38 @@ describe('Config', () => {
 
     expect(config).to.have.property('enabled', true)
     expect(config).to.have.property('debug', false)
-    expect(config).to.have.nested.property('url.hostname', 'server')
-    expect(config).to.have.nested.property('url.port', '7777')
+    expect(config).to.have.nested.property('url.protocol', 'https:')
+    expect(config).to.have.nested.property('url.hostname', 'agent2')
+    expect(config).to.have.nested.property('url.port', '6218')
+    expect(config).to.have.property('service', 'test')
+    expect(config).to.have.property('env', 'development')
+  })
+
+  it('should give priority to the options especially url', () => {
+    platform.env.withArgs('DD_TRACE_AGENT_URL').returns('http://agent2:6218')
+    platform.env.withArgs('DD_TRACE_AGENT_HOSTNAME').returns('agent')
+    platform.env.withArgs('DD_TRACE_AGENT_PORT').returns('6218')
+    platform.env.withArgs('DD_TRACE_ENABLED').returns('false')
+    platform.env.withArgs('DD_TRACE_DEBUG').returns('true')
+    platform.env.withArgs('DD_SERVICE_NAME').returns('service')
+    platform.env.withArgs('DD_ENV').returns('test')
+
+    const config = new Config({
+      enabled: true,
+      debug: false,
+      url: 'https://agent3:7778',
+      protocol: 'http',
+      hostname: 'server',
+      port: 7777,
+      service: 'test',
+      env: 'development'
+    })
+
+    expect(config).to.have.property('enabled', true)
+    expect(config).to.have.property('debug', false)
+    expect(config).to.have.nested.property('url.protocol', 'https:')
+    expect(config).to.have.nested.property('url.hostname', 'agent3')
+    expect(config).to.have.nested.property('url.port', '7778')
     expect(config).to.have.property('service', 'test')
     expect(config).to.have.property('env', 'development')
   })