From 4acdc8c7f0682ac128878e1bfee266124d93831f Mon Sep 17 00:00:00 2001 From: rochdev Date: Tue, 1 Nov 2022 16:51:26 -0400 Subject: [PATCH] fix configuring an ipv6 agent hostname --- .../dd-trace/src/exporters/agent/index.js | 8 +- .../src/exporters/span-stats/index.js | 8 +- packages/dd-trace/src/metrics.js | 7 +- packages/dd-trace/src/profiling/config.js | 13 +- .../test/exporters/agent/exporter.spec.js | 8 + packages/dd-trace/test/metrics.spec.js | 245 ++++++++++-------- .../dd-trace/test/profiling/config.spec.js | 12 + 7 files changed, 180 insertions(+), 121 deletions(-) diff --git a/packages/dd-trace/src/exporters/agent/index.js b/packages/dd-trace/src/exporters/agent/index.js index 46319214d5f..9b6dda6f30b 100644 --- a/packages/dd-trace/src/exporters/agent/index.js +++ b/packages/dd-trace/src/exporters/agent/index.js @@ -1,6 +1,6 @@ 'use strict' -const URL = require('url').URL +const { URL, format } = require('url') const log = require('../../log') const Writer = require('./writer') @@ -8,7 +8,11 @@ class AgentExporter { constructor (config, prioritySampler) { this._config = config const { url, hostname, port, lookup, protocolVersion, stats = {} } = config - this._url = url || new URL(`http://${hostname || 'localhost'}:${port}`) + this._url = url || new URL(format({ + protocol: 'http:', + hostname: hostname || 'localhost', + port + })) const headers = {} if (stats.enabled) { diff --git a/packages/dd-trace/src/exporters/span-stats/index.js b/packages/dd-trace/src/exporters/span-stats/index.js index d53d573b4de..7feec80f578 100644 --- a/packages/dd-trace/src/exporters/span-stats/index.js +++ b/packages/dd-trace/src/exporters/span-stats/index.js @@ -1,11 +1,15 @@ -const { URL } = require('url') +const { URL, format } = require('url') const { Writer } = require('./writer') class SpanStatsExporter { constructor (config) { const { hostname = '127.0.0.1', port = 8126, tags, url } = config - this._url = url || new URL(`http://${hostname || 'localhost'}:${port}`) + this._url = url || new URL(format({ + protocol: 'http:', + hostname: hostname || 'localhost', + port + })) this._writer = new Writer({ url: this._url, tags }) } diff --git a/packages/dd-trace/src/metrics.js b/packages/dd-trace/src/metrics.js index 5644a548669..b3f9c4f2dab 100644 --- a/packages/dd-trace/src/metrics.js +++ b/packages/dd-trace/src/metrics.js @@ -2,6 +2,7 @@ // TODO: capture every second and flush every 10 seconds +const { URL, format } = require('url') const v8 = require('v8') const os = require('os') const Client = require('./dogstatsd') @@ -57,7 +58,11 @@ module.exports = { if (config.url) { clientConfig.metricsProxyUrl = config.url } else if (config.port) { - clientConfig.metricsProxyUrl = new URL(`http://${config.hostname || 'localhost'}:${config.port}`) + clientConfig.metricsProxyUrl = new URL(format({ + protocol: 'http:', + hostname: config.hostname || 'localhost', + port: config.port + })) } client = new Client(clientConfig) diff --git a/packages/dd-trace/src/profiling/config.js b/packages/dd-trace/src/profiling/config.js index bcc0e4010aa..cd4a6b04c67 100644 --- a/packages/dd-trace/src/profiling/config.js +++ b/packages/dd-trace/src/profiling/config.js @@ -2,7 +2,7 @@ const coalesce = require('koalas') const os = require('os') -const { URL } = require('url') +const { URL, format } = require('url') const { AgentExporter } = require('./exporters/agent') const { FileExporter } = require('./exporters/file') const { ConsoleLogger } = require('./loggers/console') @@ -59,10 +59,13 @@ class Config { this.sourceMap = sourceMap this.endpointCollection = endpointCollection - const hostname = coalesce(options.hostname, DD_AGENT_HOST, 'localhost') - const port = coalesce(options.port, DD_TRACE_AGENT_PORT, 8126) - this.url = new URL(coalesce(options.url, DD_TRACE_AGENT_URL, - `http://${hostname || 'localhost'}:${port || 8126}`)) + const hostname = coalesce(options.hostname, DD_AGENT_HOST) || 'localhost' + const port = coalesce(options.port, DD_TRACE_AGENT_PORT) || 8126 + this.url = new URL(coalesce(options.url, DD_TRACE_AGENT_URL, format({ + protocol: 'http:', + hostname, + port + }))) this.exporters = ensureExporters(options.exporters || [ new AgentExporter(this) diff --git a/packages/dd-trace/test/exporters/agent/exporter.spec.js b/packages/dd-trace/test/exporters/agent/exporter.spec.js index 08f86687077..a73f8b4bc22 100644 --- a/packages/dd-trace/test/exporters/agent/exporter.spec.js +++ b/packages/dd-trace/test/exporters/agent/exporter.spec.js @@ -41,6 +41,14 @@ describe('Exporter', () => { }) }) + it('should support IPv6', () => { + const stats = { enabled: true } + exporter = new Exporter({ hostname: '::1', flushInterval, stats }, prioritySampler) + expect(Writer).to.have.been.calledWithMatch({ + url: new URL('http://[::1]') + }) + }) + describe('when interval is set to a positive number', () => { beforeEach(() => { exporter = new Exporter({ url, flushInterval }, prioritySampler) diff --git a/packages/dd-trace/test/metrics.spec.js b/packages/dd-trace/test/metrics.spec.js index 59b9e7c138e..16201da0c45 100644 --- a/packages/dd-trace/test/metrics.spec.js +++ b/packages/dd-trace/test/metrics.spec.js @@ -49,6 +49,8 @@ describe('metrics', () => { describe('start', () => { it('it should initialize the Dogstatsd client with the correct options', function () { + metrics.start(config) + this.timeout(10000) expect(Client).to.have.been.calledWithMatch({ @@ -61,7 +63,26 @@ describe('metrics', () => { }) }) + it('it should initialize the Dogstatsd client with an IPv6 URL', function () { + config.hostname = '::1' + + metrics.start(config) + + this.timeout(10000) + + expect(Client).to.have.been.calledWithMatch({ + metricsProxyUrl: new URL('http://[::1]:8126'), + host: 'localhost', + tags: [ + 'str:bar', + 'invalid:t_e_s_t5-:./' + ] + }) + }) + it('should start collecting metrics every 10 seconds', () => { + metrics.start(config) + global.gc() clock.tick(10000) @@ -124,172 +145,174 @@ describe('metrics', () => { }) }) - describe('stop', () => { - it('should stop collecting metrics every 10 seconds', () => { - metrics.stop() + describe('when started', () => { + describe('stop', () => { + it('should stop collecting metrics every 10 seconds', () => { + metrics.stop() - clock.tick(10000) + clock.tick(10000) - expect(client.gauge).to.not.have.been.called + expect(client.gauge).to.not.have.been.called + }) }) - }) - - describe('histogram', () => { - it('should add a record to a histogram', () => { - metrics.histogram('test', 1) - metrics.histogram('test', 2) - metrics.histogram('test', 3) - - clock.tick(10000) - expect(client.gauge).to.have.been.calledWith('test.max', 3) - expect(client.gauge).to.have.been.calledWith('test.min', 1) - expect(client.increment).to.have.been.calledWith('test.sum', 6) - expect(client.increment).to.have.been.calledWith('test.total', 6) - expect(client.gauge).to.have.been.calledWith('test.avg', 2) - expect(client.gauge).to.have.been.calledWith('test.median', sinon.match.number) - expect(client.gauge).to.have.been.calledWith('test.95percentile', sinon.match.number) - expect(client.increment).to.have.been.calledWith('test.count', 3) + describe('histogram', () => { + it('should add a record to a histogram', () => { + metrics.histogram('test', 1) + metrics.histogram('test', 2) + metrics.histogram('test', 3) + + clock.tick(10000) + + expect(client.gauge).to.have.been.calledWith('test.max', 3) + expect(client.gauge).to.have.been.calledWith('test.min', 1) + expect(client.increment).to.have.been.calledWith('test.sum', 6) + expect(client.increment).to.have.been.calledWith('test.total', 6) + expect(client.gauge).to.have.been.calledWith('test.avg', 2) + expect(client.gauge).to.have.been.calledWith('test.median', sinon.match.number) + expect(client.gauge).to.have.been.calledWith('test.95percentile', sinon.match.number) + expect(client.increment).to.have.been.calledWith('test.count', 3) + }) }) - }) - describe('increment', () => { - it('should increment a gauge', () => { - metrics.increment('test') + describe('increment', () => { + it('should increment a gauge', () => { + metrics.increment('test') - clock.tick(10000) + clock.tick(10000) - expect(client.gauge).to.have.been.calledWith('test', 1) - }) + expect(client.gauge).to.have.been.calledWith('test', 1) + }) - it('should increment a gauge with a tag', () => { - metrics.increment('test', 'foo:bar') + it('should increment a gauge with a tag', () => { + metrics.increment('test', 'foo:bar') - clock.tick(10000) + clock.tick(10000) - expect(client.gauge).to.have.been.calledWith('test', 1, ['foo:bar']) - }) + expect(client.gauge).to.have.been.calledWith('test', 1, ['foo:bar']) + }) - it('should increment a monotonic counter', () => { - metrics.increment('test', true) + it('should increment a monotonic counter', () => { + metrics.increment('test', true) - clock.tick(10000) + clock.tick(10000) - expect(client.increment).to.have.been.calledWith('test', 1) + expect(client.increment).to.have.been.calledWith('test', 1) - client.increment.resetHistory() + client.increment.resetHistory() - clock.tick(10000) + clock.tick(10000) - expect(client.increment).to.not.have.been.calledWith('test') - }) + expect(client.increment).to.not.have.been.calledWith('test') + }) - it('should increment a monotonic counter with a tag', () => { - metrics.increment('test', 'foo:bar', true) + it('should increment a monotonic counter with a tag', () => { + metrics.increment('test', 'foo:bar', true) - clock.tick(10000) + clock.tick(10000) - expect(client.increment).to.have.been.calledWith('test', 1, ['foo:bar']) + expect(client.increment).to.have.been.calledWith('test', 1, ['foo:bar']) - client.increment.resetHistory() + client.increment.resetHistory() - clock.tick(10000) + clock.tick(10000) - expect(client.increment).to.not.have.been.calledWith('test') + expect(client.increment).to.not.have.been.calledWith('test') + }) }) - }) - describe('decrement', () => { - it('should increment a gauge', () => { - metrics.decrement('test') + describe('decrement', () => { + it('should increment a gauge', () => { + metrics.decrement('test') - clock.tick(10000) + clock.tick(10000) - expect(client.gauge).to.have.been.calledWith('test', -1) - }) + expect(client.gauge).to.have.been.calledWith('test', -1) + }) - it('should decrement a gauge with a tag', () => { - metrics.decrement('test', 'foo:bar') + it('should decrement a gauge with a tag', () => { + metrics.decrement('test', 'foo:bar') - clock.tick(10000) + clock.tick(10000) - expect(client.gauge).to.have.been.calledWith('test', -1, ['foo:bar']) + expect(client.gauge).to.have.been.calledWith('test', -1, ['foo:bar']) + }) }) - }) - describe('gauge', () => { - it('should set a gauge', () => { - metrics.gauge('test', 10) + describe('gauge', () => { + it('should set a gauge', () => { + metrics.gauge('test', 10) - clock.tick(10000) + clock.tick(10000) - expect(client.gauge).to.have.been.calledWith('test', 10) - }) + expect(client.gauge).to.have.been.calledWith('test', 10) + }) - it('should set a gauge with a tag', () => { - metrics.gauge('test', 10, 'foo:bar') + it('should set a gauge with a tag', () => { + metrics.gauge('test', 10, 'foo:bar') - clock.tick(10000) + clock.tick(10000) - expect(client.gauge).to.have.been.calledWith('test', 10, ['foo:bar']) + expect(client.gauge).to.have.been.calledWith('test', 10, ['foo:bar']) + }) }) - }) - describe('boolean', () => { - it('should set a gauge', () => { - metrics.boolean('test', true) + describe('boolean', () => { + it('should set a gauge', () => { + metrics.boolean('test', true) - clock.tick(10000) + clock.tick(10000) - expect(client.gauge).to.have.been.calledWith('test', 1) - }) + expect(client.gauge).to.have.been.calledWith('test', 1) + }) - it('should set a gauge with a tag', () => { - metrics.boolean('test', true, 'foo:bar') + it('should set a gauge with a tag', () => { + metrics.boolean('test', true, 'foo:bar') - clock.tick(10000) + clock.tick(10000) - expect(client.gauge).to.have.been.calledWith('test', 1, ['foo:bar']) + expect(client.gauge).to.have.been.calledWith('test', 1, ['foo:bar']) + }) }) - }) - describe('without native metrics', () => { - beforeEach(() => { - metrics = proxyquire('../src/metrics', { - './dogstatsd': Client, - 'node-gyp-build': sinon.stub().returns(null) + describe('without native metrics', () => { + beforeEach(() => { + metrics = proxyquire('../src/metrics', { + './dogstatsd': Client, + 'node-gyp-build': sinon.stub().returns(null) + }) }) - }) - it('should fallback to only metrics available to JavaScript code', () => { - clock.tick(10000) + it('should fallback to only metrics available to JavaScript code', () => { + clock.tick(10000) - expect(client.gauge).to.have.been.calledWith('runtime.node.cpu.user') - expect(client.gauge).to.have.been.calledWith('runtime.node.cpu.system') - expect(client.gauge).to.have.been.calledWith('runtime.node.cpu.total') + expect(client.gauge).to.have.been.calledWith('runtime.node.cpu.user') + expect(client.gauge).to.have.been.calledWith('runtime.node.cpu.system') + expect(client.gauge).to.have.been.calledWith('runtime.node.cpu.total') - expect(client.gauge).to.have.been.calledWith('runtime.node.mem.rss') - expect(client.gauge).to.have.been.calledWith('runtime.node.mem.heap_total') - expect(client.gauge).to.have.been.calledWith('runtime.node.mem.heap_used') + expect(client.gauge).to.have.been.calledWith('runtime.node.mem.rss') + expect(client.gauge).to.have.been.calledWith('runtime.node.mem.heap_total') + expect(client.gauge).to.have.been.calledWith('runtime.node.mem.heap_used') - expect(client.gauge).to.have.been.calledWith('runtime.node.process.uptime') + expect(client.gauge).to.have.been.calledWith('runtime.node.process.uptime') - expect(client.gauge).to.have.been.calledWith('runtime.node.heap.total_heap_size') - expect(client.gauge).to.have.been.calledWith('runtime.node.heap.total_heap_size_executable') - expect(client.gauge).to.have.been.calledWith('runtime.node.heap.total_physical_size') - expect(client.gauge).to.have.been.calledWith('runtime.node.heap.total_available_size') - expect(client.gauge).to.have.been.calledWith('runtime.node.heap.total_heap_size') - expect(client.gauge).to.have.been.calledWith('runtime.node.heap.heap_size_limit') + expect(client.gauge).to.have.been.calledWith('runtime.node.heap.total_heap_size') + expect(client.gauge).to.have.been.calledWith('runtime.node.heap.total_heap_size_executable') + expect(client.gauge).to.have.been.calledWith('runtime.node.heap.total_physical_size') + expect(client.gauge).to.have.been.calledWith('runtime.node.heap.total_available_size') + expect(client.gauge).to.have.been.calledWith('runtime.node.heap.total_heap_size') + expect(client.gauge).to.have.been.calledWith('runtime.node.heap.heap_size_limit') - expect(client.gauge).to.have.been.calledWith('runtime.node.heap.malloced_memory') - expect(client.gauge).to.have.been.calledWith('runtime.node.heap.peak_malloced_memory') + expect(client.gauge).to.have.been.calledWith('runtime.node.heap.malloced_memory') + expect(client.gauge).to.have.been.calledWith('runtime.node.heap.peak_malloced_memory') - expect(client.gauge).to.have.been.calledWith('runtime.node.heap.size.by.space') - expect(client.gauge).to.have.been.calledWith('runtime.node.heap.used_size.by.space') - expect(client.gauge).to.have.been.calledWith('runtime.node.heap.available_size.by.space') - expect(client.gauge).to.have.been.calledWith('runtime.node.heap.physical_size.by.space') + expect(client.gauge).to.have.been.calledWith('runtime.node.heap.size.by.space') + expect(client.gauge).to.have.been.calledWith('runtime.node.heap.used_size.by.space') + expect(client.gauge).to.have.been.calledWith('runtime.node.heap.available_size.by.space') + expect(client.gauge).to.have.been.calledWith('runtime.node.heap.physical_size.by.space') - expect(client.flush).to.have.been.called + expect(client.flush).to.have.been.called + }) }) }) }) diff --git a/packages/dd-trace/test/profiling/config.spec.js b/packages/dd-trace/test/profiling/config.spec.js index 9813480f9e7..c2676487d68 100644 --- a/packages/dd-trace/test/profiling/config.spec.js +++ b/packages/dd-trace/test/profiling/config.spec.js @@ -122,4 +122,16 @@ describe('config', () => { expect(config.tags).to.include({ env, service, version }) }) + + it('should support IPv6 hostname', () => { + const options = { + hostname: '::1' + } + + const config = new Config(options) + const exporterUrl = config.exporters[0]._url.toString() + const expectedUrl = new URL('http://[::1]:8126').toString() + + expect(exporterUrl).to.equal(expectedUrl) + }) })