From 86735525c823bec2fadc1fc16019f5f16396fa96 Mon Sep 17 00:00:00 2001 From: rochdev Date: Thu, 19 Jul 2018 14:11:41 -0400 Subject: [PATCH 1/2] update service names and span types to match the specification --- src/config.js | 2 +- src/plugins/amqplib.js | 8 ++--- src/plugins/elasticsearch.js | 4 +-- src/plugins/express.js | 2 +- src/plugins/http.js | 49 ++++++++++++++++++++++++------ src/plugins/https.js | 9 ------ src/plugins/mongodb-core.js | 10 +++--- src/plugins/mysql.js | 2 +- src/plugins/mysql2.js | 2 +- src/plugins/pg.js | 2 +- src/plugins/redis.js | 4 +-- test/config.spec.js | 11 +++++-- test/plugins/amqplib.spec.js | 8 ++--- test/plugins/elasticsearch.spec.js | 8 ++--- test/plugins/express.spec.js | 4 +-- test/plugins/http.spec.js | 45 +++++++++++++++++++++++++-- test/plugins/mongodb-core.spec.js | 8 ++--- test/plugins/mysql.spec.js | 4 +-- test/plugins/mysql2.spec.js | 4 +-- test/plugins/pg.spec.js | 2 +- test/plugins/redis.spec.js | 4 +-- 21 files changed, 130 insertions(+), 62 deletions(-) delete mode 100644 src/plugins/https.js diff --git a/src/config.js b/src/config.js index 78af507853e..bf105fc8ee0 100644 --- a/src/config.js +++ b/src/config.js @@ -10,7 +10,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 service = coalesce(options.service, platform.env('DD_SERVICE_NAME'), platform.service()) + const service = coalesce(options.service, platform.env('DD_SERVICE_NAME'), platform.service(), 'node') const env = coalesce(options.env, platform.env('DD_ENV')) const protocol = 'http' const hostname = coalesce(options.hostname, platform.env('DD_TRACE_AGENT_HOSTNAME'), 'localhost') diff --git a/src/plugins/amqplib.js b/src/plugins/amqplib.js index 50695cab425..bd760ed4343 100644 --- a/src/plugins/amqplib.js +++ b/src/plugins/amqplib.js @@ -25,7 +25,7 @@ function createWrapDispatchMessage (tracer, config) { return function dispatchMessageWithTrace (fields, message) { const span = tracer.startSpan('amqp.command') - addTags(this, config, span, 'basic.deliver', fields) + addTags(this, tracer, config, span, 'basic.deliver', fields) setImmediate(() => { tracer.scopeManager().activate(span, true) @@ -46,7 +46,7 @@ function sendWithTrace (send, channel, args, tracer, config, method, fields) { childOf: parentScope && parentScope.span() }) - addTags(channel, config, span, method, fields) + addTags(channel, tracer, config, span, method, fields) try { return send.apply(channel, args) @@ -82,7 +82,7 @@ function addError (span, error) { return error } -function addTags (channel, config, span, method, fields) { +function addTags (channel, tracer, config, span, method, fields) { const fieldNames = [ 'queue', 'exchange', @@ -93,7 +93,7 @@ function addTags (channel, config, span, method, fields) { ] span.addTags({ - 'service.name': config.service || 'amqp', + 'service.name': config.service || `${tracer._service}-amqp`, 'resource.name': getResourceName(method, fields), 'span.type': 'worker' }) diff --git a/src/plugins/elasticsearch.js b/src/plugins/elasticsearch.js index 0798ca1326d..04e877030d6 100644 --- a/src/plugins/elasticsearch.js +++ b/src/plugins/elasticsearch.js @@ -11,9 +11,9 @@ function createWrapRequest (tracer, config) { tags: { [Tags.SPAN_KIND]: Tags.SPAN_KIND_RPC_CLIENT, [Tags.DB_TYPE]: 'elasticsearch', - 'service.name': config.service || 'elasticsearch', + 'service.name': config.service || `${tracer._service}-elasticsearch`, 'resource.name': `${params.method} ${quantizePath(params.path)}`, - 'span.type': 'db', + 'span.type': 'elasticsearch', 'elasticsearch.url': params.path, 'elasticsearch.method': params.method, 'elasticsearch.params': JSON.stringify(params.query) diff --git a/src/plugins/express.js b/src/plugins/express.js index d68e8af5958..dff09a67db6 100644 --- a/src/plugins/express.js +++ b/src/plugins/express.js @@ -41,7 +41,7 @@ function createWrapMethod (tracer, config) { } span.setTag('service.name', config.service || tracer._service) - span.setTag('span.type', 'web') + span.setTag('span.type', 'http') span.setTag(Tags.HTTP_STATUS_CODE, res.statusCode) if (!validateStatus(res.statusCode)) { diff --git a/src/plugins/http.js b/src/plugins/http.js index 72f6db849f2..0b24ddcb468 100644 --- a/src/plugins/http.js +++ b/src/plugins/http.js @@ -21,12 +21,16 @@ function patch (http, tracer, config) { let isFinish = false + options = typeof options === 'string' ? url.parse(uri) : Object.assign({}, options) + options.headers = options.headers || {} + const parentScope = tracer.scopeManager().active() + const parent = parentScope && parentScope.span() const span = tracer.startSpan('http.request', { - childOf: parentScope && parentScope.span(), + childOf: parent, tags: { [Tags.SPAN_KIND]: Tags.SPAN_KIND_RPC_CLIENT, - 'service.name': config.service || 'http-client', + 'service.name': getServiceName(tracer, config, options), 'resource.name': method, 'span.type': 'web', 'http.method': method, @@ -34,9 +38,6 @@ function patch (http, tracer, config) { } }) - options = typeof options === 'string' ? url.parse(uri) : Object.assign({}, options) - options.headers = options.headers || {} - tracer.inject(span, FORMAT_HTTP_HEADERS, options.headers) const req = request.call(this, options, res => { @@ -83,13 +84,41 @@ function patch (http, tracer, config) { } } +function getHost (options) { + if (typeof options === 'string') { + return url.parse(options).host + } + + const hostname = options.hostname || options.host || 'localhost' + const port = options.port + + return [hostname, port].filter(val => val).join(':') +} + +function getServiceName (tracer, config, options) { + if (config.splitByDomain) { + return getHost(options) + } else if (config.service) { + return config.service + } + + return `${tracer._service}-http-client` +} + function unpatch (http) { this.unwrap(http, 'request') this.unwrap(http, 'get') } -module.exports = { - name: 'http', - patch, - unpatch -} +module.exports = [ + { + name: 'http', + patch, + unpatch + }, + { + name: 'https', + patch, + unpatch + } +] diff --git a/src/plugins/https.js b/src/plugins/https.js deleted file mode 100644 index 9f0fed4df8b..00000000000 --- a/src/plugins/https.js +++ /dev/null @@ -1,9 +0,0 @@ -'use strict' - -const http = require('./http') - -module.exports = { - name: 'https', - patch: http.patch, - unpatch: http.unpatch -} diff --git a/src/plugins/mongodb-core.js b/src/plugins/mongodb-core.js index c1fd1941863..8ab879461e3 100644 --- a/src/plugins/mongodb-core.js +++ b/src/plugins/mongodb-core.js @@ -193,7 +193,7 @@ function createWrapOperation (tracer, config, operationName) { childOf: parentScope && parentScope.span() }) - addTags(span, config, resource, ns, this) + addTags(span, tracer, config, resource, ns, this) if (typeof options === 'function') { return operation.call(this, ns, ops, wrapCallback(tracer, span, options)) @@ -214,7 +214,7 @@ function createWrapNext (tracer, config) { childOf: parentScope && parentScope.span() }) - addTags(span, config, resource, this.ns, this.topology) + addTags(span, tracer, config, resource, this.ns, this.topology) if (this.cursorState) { span.addTags({ @@ -227,11 +227,11 @@ function createWrapNext (tracer, config) { } } -function addTags (span, config, resource, ns, topology) { +function addTags (span, tracer, config, resource, ns, topology) { span.addTags({ - 'service.name': config.service || 'mongodb', + 'service.name': config.service || `${tracer._service}-mongodb`, 'resource.name': resource, - 'span.type': 'db', + 'span.type': 'mongodb', 'db.name': ns }) diff --git a/src/plugins/mysql.js b/src/plugins/mysql.js index d3b462b3cc4..f6d0b69d9e7 100644 --- a/src/plugins/mysql.js +++ b/src/plugins/mysql.js @@ -11,7 +11,7 @@ function createWrapQuery (tracer, config) { childOf: parent, tags: { [Tags.SPAN_KIND]: Tags.SPAN_KIND_RPC_CLIENT, - 'service.name': config.service || 'mysql', + 'service.name': config.service || `${tracer._service}-mysql`, 'span.type': 'sql', 'db.type': 'mysql', 'db.user': this.config.user, diff --git a/src/plugins/mysql2.js b/src/plugins/mysql2.js index 26d8ff6b086..faf917569cc 100644 --- a/src/plugins/mysql2.js +++ b/src/plugins/mysql2.js @@ -11,7 +11,7 @@ function createWrapQuery (tracer, config) { childOf: parent, tags: { [Tags.SPAN_KIND]: Tags.SPAN_KIND_RPC_CLIENT, - 'service.name': config.service || 'mysql', + 'service.name': config.service || `${tracer._service}-mysql`, 'span.type': 'sql', 'db.type': 'mysql', 'db.user': this.config.user, diff --git a/src/plugins/pg.js b/src/plugins/pg.js index 67da85a0bec..568dc7e16f7 100644 --- a/src/plugins/pg.js +++ b/src/plugins/pg.js @@ -18,7 +18,7 @@ function patch (pg, tracer, config) { childOf: parent, tags: { [Tags.SPAN_KIND]: Tags.SPAN_KIND_RPC_CLIENT, - 'service.name': config.service || 'postgres', + 'service.name': config.service || `${tracer._service}-postgres`, 'resource.name': statement, 'span.type': 'sql', 'db.type': 'postgres' diff --git a/src/plugins/redis.js b/src/plugins/redis.js index e14c0620103..9c839ff84fc 100644 --- a/src/plugins/redis.js +++ b/src/plugins/redis.js @@ -11,9 +11,9 @@ function createWrapInternalSendCommand (tracer, config) { tags: { [Tags.SPAN_KIND]: Tags.SPAN_KIND_RPC_CLIENT, [Tags.DB_TYPE]: 'redis', - 'service.name': config.service || 'redis', + 'service.name': config.service || `${tracer._service}-redis`, 'resource.name': options.command, - 'span.type': 'db', + 'span.type': 'redis', 'db.name': this.selected_db || '0' } }) diff --git a/test/config.spec.js b/test/config.spec.js index f90cea04f62..a666275ecd9 100644 --- a/test/config.spec.js +++ b/test/config.spec.js @@ -9,7 +9,6 @@ describe('Config', () => { env: sinon.stub(), service: sinon.stub() } - platform.service.returns('test') Config = proxyquire('../src/config', { './platform': platform @@ -19,7 +18,7 @@ describe('Config', () => { it('should initialize with the correct defaults', () => { const config = new Config() - expect(config).to.have.property('service', 'test') + expect(config).to.have.property('service', 'node') expect(config).to.have.property('enabled', true) expect(config).to.have.property('debug', false) expect(config).to.have.nested.property('url.protocol', 'http:') @@ -33,6 +32,14 @@ describe('Config', () => { expect(config).to.have.property('env', undefined) }) + it('should initialize from the platform', () => { + platform.service.returns('test') + + const config = new Config() + + expect(config).to.have.property('service', 'test') + }) + it('should initialize from environment variables', () => { platform.env.withArgs('DD_TRACE_AGENT_HOSTNAME').returns('agent') platform.env.withArgs('DD_TRACE_AGENT_PORT').returns('6218') diff --git a/test/plugins/amqplib.spec.js b/test/plugins/amqplib.spec.js index bbe9bd6c4ed..4fa9a89258d 100644 --- a/test/plugins/amqplib.spec.js +++ b/test/plugins/amqplib.spec.js @@ -50,7 +50,7 @@ describe('Plugin', () => { const span = traces[0][0] expect(span).to.have.property('name', 'amqp.command') - expect(span).to.have.property('service', 'amqp') + expect(span).to.have.property('service', 'test-amqp') expect(span).to.have.property('resource', 'queue.declare test') expect(span).to.have.property('type', 'worker') expect(span.meta).to.have.property('out.host', 'localhost') @@ -68,7 +68,7 @@ describe('Plugin', () => { const span = traces[0][0] expect(span).to.have.property('name', 'amqp.command') - expect(span).to.have.property('service', 'amqp') + expect(span).to.have.property('service', 'test-amqp') expect(span).to.have.property('resource', 'queue.delete test') expect(span).to.have.property('type', 'worker') expect(span.meta).to.have.property('out.host', 'localhost') @@ -111,7 +111,7 @@ describe('Plugin', () => { const span = traces[0][0] expect(span).to.have.property('name', 'amqp.command') - expect(span).to.have.property('service', 'amqp') + expect(span).to.have.property('service', 'test-amqp') expect(span).to.have.property('resource', 'basic.publish exchange routingKey') expect(span).to.have.property('type', 'worker') expect(span.meta).to.have.property('out.host', 'localhost') @@ -159,7 +159,7 @@ describe('Plugin', () => { const span = traces[0][0] expect(span).to.have.property('name', 'amqp.command') - expect(span).to.have.property('service', 'amqp') + expect(span).to.have.property('service', 'test-amqp') expect(span).to.have.property('resource', `basic.deliver ${queue}`) expect(span).to.have.property('type', 'worker') expect(span.meta).to.have.property('out.host', 'localhost') diff --git a/test/plugins/elasticsearch.spec.js b/test/plugins/elasticsearch.spec.js index 1d7eee6b5ef..93efccf78ef 100644 --- a/test/plugins/elasticsearch.spec.js +++ b/test/plugins/elasticsearch.spec.js @@ -83,9 +83,9 @@ describe('Plugin', () => { it('should do automatic instrumentation', done => { agent .use(traces => { - expect(traces[0][0]).to.have.property('service', 'elasticsearch') + expect(traces[0][0]).to.have.property('service', 'test-elasticsearch') expect(traces[0][0]).to.have.property('resource', 'HEAD /') - expect(traces[0][0]).to.have.property('type', 'db') + expect(traces[0][0]).to.have.property('type', 'elasticsearch') }) .then(done) .catch(done) @@ -142,9 +142,9 @@ describe('Plugin', () => { it('should do automatic instrumentation', done => { agent .use(traces => { - expect(traces[0][0]).to.have.property('service', 'elasticsearch') + expect(traces[0][0]).to.have.property('service', 'test-elasticsearch') expect(traces[0][0]).to.have.property('resource', 'HEAD /') - expect(traces[0][0]).to.have.property('type', 'db') + expect(traces[0][0]).to.have.property('type', 'elasticsearch') }) .then(done) .catch(done) diff --git a/test/plugins/express.spec.js b/test/plugins/express.spec.js index 5f78759f6ba..93e909cfc0d 100644 --- a/test/plugins/express.spec.js +++ b/test/plugins/express.spec.js @@ -42,7 +42,7 @@ describe('Plugin', () => { agent .use(traces => { expect(traces[0][0]).to.have.property('service', 'test') - expect(traces[0][0]).to.have.property('type', 'web') + expect(traces[0][0]).to.have.property('type', 'http') expect(traces[0][0]).to.have.property('resource', 'GET /user') expect(traces[0][0].meta).to.have.property('span.kind', 'server') expect(traces[0][0].meta).to.have.property('http.url', `http://localhost:${port}/user`) @@ -74,7 +74,7 @@ describe('Plugin', () => { agent .use(traces => { expect(traces[0][0]).to.have.property('service', 'test') - expect(traces[0][0]).to.have.property('type', 'web') + expect(traces[0][0]).to.have.property('type', 'http') expect(traces[0][0]).to.have.property('resource', 'GET /app/user/:id') expect(traces[0][0].meta).to.have.property('span.kind', 'server') expect(traces[0][0].meta).to.have.property('http.url', `http://localhost:${port}/app/user/1`) diff --git a/test/plugins/http.spec.js b/test/plugins/http.spec.js index d67f4874727..7c15d4ebe10 100644 --- a/test/plugins/http.spec.js +++ b/test/plugins/http.spec.js @@ -42,7 +42,7 @@ describe('Plugin', () => { getPort().then(port => { agent .use(traces => { - expect(traces[0][0]).to.have.property('service', 'http-client') + expect(traces[0][0]).to.have.property('service', 'test-http-client') expect(traces[0][0]).to.have.property('type', 'web') expect(traces[0][0]).to.have.property('resource', 'GET') expect(traces[0][0].meta).to.have.property('span.kind', 'client') @@ -234,7 +234,7 @@ describe('Plugin', () => { }) }) - describe('with configuration', () => { + describe('with service configuration', () => { let config beforeEach(() => { @@ -274,5 +274,46 @@ describe('Plugin', () => { }) }) }) + + describe('with splitByDomain configuration', () => { + let config + + beforeEach(() => { + config = { + splitByDomain: true + } + + return agent.load(plugin, 'http', config) + .then(() => { + http = require('http') + express = require('express') + }) + }) + + it('should use the remote endpoint as the service name', done => { + const app = express() + + app.get('/user', (req, res) => { + res.status(200).send() + }) + + getPort().then(port => { + agent + .use(traces => { + expect(traces[0][0]).to.have.property('service', `localhost:${port}`) + }) + .then(done) + .catch(done) + + appListener = app.listen(port, 'localhost', () => { + const req = http.request(`http://localhost:${port}/user`, res => { + res.on('data', () => {}) + }) + + req.end() + }) + }) + }) + }) }) }) diff --git a/test/plugins/mongodb-core.spec.js b/test/plugins/mongodb-core.spec.js index 8f7627b3151..20778f063f8 100644 --- a/test/plugins/mongodb-core.spec.js +++ b/test/plugins/mongodb-core.spec.js @@ -54,9 +54,9 @@ describe('Plugin', () => { const resource = `insert test.${collection}` expect(span).to.have.property('name', 'mongodb.query') - expect(span).to.have.property('service', 'mongodb') + expect(span).to.have.property('service', 'test-mongodb') expect(span).to.have.property('resource', resource) - expect(span).to.have.property('type', 'db') + expect(span).to.have.property('type', 'mongodb') expect(span.meta).to.have.property('db.name', `test.${collection}`) expect(span.meta).to.have.property('out.host', 'localhost') }) @@ -154,8 +154,8 @@ describe('Plugin', () => { const span = traces[0][0] expect(span).to.have.property('name', 'mongodb.query') - expect(span).to.have.property('service', 'mongodb') - expect(span).to.have.property('type', 'db') + expect(span).to.have.property('service', 'test-mongodb') + expect(span).to.have.property('type', 'mongodb') expect(span.meta).to.have.property('db.name', `test.${collection}`) expect(span.meta).to.have.property('out.host', 'localhost') expect(span.meta).to.have.property('out.port', '27017') diff --git a/test/plugins/mysql.spec.js b/test/plugins/mysql.spec.js index 8f4c13a0d35..cfdcae579bd 100644 --- a/test/plugins/mysql.spec.js +++ b/test/plugins/mysql.spec.js @@ -73,7 +73,7 @@ describe('Plugin', () => { it('should do automatic instrumentation', done => { agent.use(traces => { - expect(traces[0][0]).to.have.property('service', 'mysql') + expect(traces[0][0]).to.have.property('service', 'test-mysql') expect(traces[0][0]).to.have.property('resource', 'SELECT 1 + 1 AS solution') expect(traces[0][0]).to.have.property('type', 'sql') expect(traces[0][0].meta).to.have.property('db.name', 'db') @@ -175,7 +175,7 @@ describe('Plugin', () => { it('should do automatic instrumentation', done => { agent.use(traces => { - expect(traces[0][0]).to.have.property('service', 'mysql') + expect(traces[0][0]).to.have.property('service', 'test-mysql') expect(traces[0][0]).to.have.property('resource', 'SELECT 1 + 1 AS solution') expect(traces[0][0]).to.have.property('type', 'sql') expect(traces[0][0].meta).to.have.property('db.user', 'user') diff --git a/test/plugins/mysql2.spec.js b/test/plugins/mysql2.spec.js index d46921bca17..292b85fd20b 100644 --- a/test/plugins/mysql2.spec.js +++ b/test/plugins/mysql2.spec.js @@ -74,7 +74,7 @@ describe('Plugin', () => { it('should do automatic instrumentation', done => { agent .use(traces => { - expect(traces[0][0]).to.have.property('service', 'mysql') + expect(traces[0][0]).to.have.property('service', 'test-mysql') expect(traces[0][0]).to.have.property('resource', 'SELECT 1 + 1 AS solution') expect(traces[0][0]).to.have.property('type', 'sql') expect(traces[0][0].meta).to.have.property('db.name', 'db') @@ -181,7 +181,7 @@ describe('Plugin', () => { it('should do automatic instrumentation', done => { agent .use(traces => { - expect(traces[0][0]).to.have.property('service', 'mysql') + expect(traces[0][0]).to.have.property('service', 'test-mysql') expect(traces[0][0]).to.have.property('resource', 'SELECT 1 + 1 AS solution') expect(traces[0][0]).to.have.property('type', 'sql') expect(traces[0][0].meta).to.have.property('db.user', 'user') diff --git a/test/plugins/pg.spec.js b/test/plugins/pg.spec.js index 1a0718926d7..69621f1ea16 100644 --- a/test/plugins/pg.spec.js +++ b/test/plugins/pg.spec.js @@ -40,7 +40,7 @@ describe('Plugin', () => { it('should do automatic instrumentation when using callbacks', done => { agent.use(traces => { - expect(traces[0][0]).to.have.property('service', 'postgres') + expect(traces[0][0]).to.have.property('service', 'test-postgres') expect(traces[0][0]).to.have.property('resource', 'SELECT $1::text as message') expect(traces[0][0]).to.have.property('type', 'sql') expect(traces[0][0].meta).to.have.property('db.name', 'postgres') diff --git a/test/plugins/redis.spec.js b/test/plugins/redis.spec.js index 7176c5cea6c..fe5dc4a11d8 100644 --- a/test/plugins/redis.spec.js +++ b/test/plugins/redis.spec.js @@ -37,9 +37,9 @@ describe('Plugin', () => { agent .use(traces => { expect(traces[0][0]).to.have.property('name', 'redis.command') - expect(traces[0][0]).to.have.property('service', 'redis') + expect(traces[0][0]).to.have.property('service', 'test-redis') expect(traces[0][0]).to.have.property('resource', 'get') - expect(traces[0][0]).to.have.property('type', 'db') + expect(traces[0][0]).to.have.property('type', 'redis') expect(traces[0][0].meta).to.have.property('db.name', '0') expect(traces[0][0].meta).to.have.property('db.type', 'redis') expect(traces[0][0].meta).to.have.property('span.kind', 'client') From de3880c1cd0f7732d65ce6809996a2fa02080396 Mon Sep 17 00:00:00 2001 From: rochdev Date: Fri, 20 Jul 2018 10:40:49 -0400 Subject: [PATCH 2/2] add documentation for splitByDomain --- docs/API.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/API.md b/docs/API.md index 34eedebbfed..b8c99f5c3ba 100644 --- a/docs/API.md +++ b/docs/API.md @@ -191,6 +191,7 @@ query HelloWorld { | Option | Default | Description | |------------------|------------------|----------------------------------------| | service | http-client | The service name for this integration. | +| splitByDomain | false | Use the remote endpoint host as the service name instead of the default. |

mongodb-core