From 0d73c26d268ea66efe40b427018265b0642bab5f Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Tue, 18 Sep 2018 18:25:19 +0200 Subject: [PATCH] fix: update ids to correct v2 api format (#584) All the new id's are hex encoded random numbers. Their sizes are: - error id: 128 bit - transaction id: 64 bit - span id: 64 bit - trace id: 128 bit The implemented id generator is temporary (except error id's) and will be replaced by the OpenTracing id engine once that lands. --- lib/agent.js | 4 ++-- lib/instrumentation/span.js | 7 +++++- lib/instrumentation/transaction.js | 7 ++++-- package.json | 3 +-- test/agent.js | 12 ++++++++++ test/instrumentation/index.js | 22 ++++++++++-------- test/instrumentation/modules/express-queue.js | 4 ++-- test/instrumentation/modules/http/request.js | 2 +- test/instrumentation/modules/http2.js | 4 ++-- test/instrumentation/modules/mysql/mysql.js | 2 +- test/instrumentation/modules/mysql2/mysql.js | 2 +- test/instrumentation/modules/pg/pg.js | 2 +- test/instrumentation/span.js | 22 +++++++++++++----- test/instrumentation/transaction.js | 23 +++++++++++-------- 14 files changed, 76 insertions(+), 40 deletions(-) diff --git a/lib/agent.js b/lib/agent.js index b743dd8ffd..c8ed144023 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -1,5 +1,6 @@ 'use strict' +var crypto = require('crypto') var http = require('http') var parseUrl = require('url').parse var path = require('path') @@ -8,7 +9,6 @@ var afterAll = require('after-all-results') var ElasticAPMHttpClient = require('elastic-apm-http-client') var isError = require('core-util-is').isError var ancestors = require('require-ancestors') -var uuid = require('uuid') var config = require('./config') var connect = require('./middleware/connect') @@ -243,7 +243,7 @@ Agent.prototype.captureError = function (err, opts, cb) { var agent = this var trans = this.currentTransaction var timestamp = new Date().toISOString() - var id = uuid.v4() + var id = crypto.randomBytes(128 / 8).toString('hex') var req = opts && opts.request instanceof IncomingMessage ? opts.request : trans && trans.req diff --git a/lib/instrumentation/span.js b/lib/instrumentation/span.js index 71148abc87..204f4c8e42 100644 --- a/lib/instrumentation/span.js +++ b/lib/instrumentation/span.js @@ -1,5 +1,7 @@ 'use strict' +var crypto = require('crypto') + var afterAll = require('after-all-results') var Value = require('async-value-promise') @@ -12,6 +14,7 @@ const TEST = process.env.ELASTIC_APM_TEST module.exports = Span function Span (transaction) { + this.id = crypto.randomBytes(64 / 8).toString('hex') // TODO: Replace with correct id once OT is ready this.transaction = transaction this.started = false this.ended = false @@ -144,7 +147,9 @@ Span.prototype._encode = function (cb) { } var payload = { - transactionId: self.transaction.id, + id: self.id, + transaction_id: self.transaction.id, + trace_id: self.transaction.traceId, timestamp: self.transaction.timestamp, name: self.name, type: self.type, diff --git a/lib/instrumentation/transaction.js b/lib/instrumentation/transaction.js index f94cd5a4f7..251676fa19 100644 --- a/lib/instrumentation/transaction.js +++ b/lib/instrumentation/transaction.js @@ -1,7 +1,8 @@ 'use strict' +var crypto = require('crypto') + var truncate = require('unicode-byte-truncate') -var uuid = require('uuid') var config = require('../config') var getPathFromRequest = require('./express-utils').getPathFromRequest @@ -58,7 +59,8 @@ function Transaction (agent, name, type) { } }) - this.id = uuid.v4() + this.id = crypto.randomBytes(64 / 8).toString('hex') // TODO: Replace with correct id once OT is ready + this.traceId = crypto.randomBytes(128 / 8).toString('hex') // TODO: Replace with correct id once OT is ready this._defaultName = name || '' this._customName = '' this._user = null @@ -136,6 +138,7 @@ Transaction.prototype.buildSpan = function () { Transaction.prototype.toJSON = function () { var payload = { id: this.id, + trace_id: this.traceId, name: this.name, type: this.type, duration: this.duration(), diff --git a/package.json b/package.json index e9209a595b..ac6577c54f 100644 --- a/package.json +++ b/package.json @@ -88,8 +88,7 @@ "set-cookie-serde": "^1.0.0", "sql-summary": "^1.0.1", "stackman": "^3.0.2", - "unicode-byte-truncate": "^1.0.0", - "uuid": "^3.2.1" + "unicode-byte-truncate": "^1.0.0" }, "devDependencies": { "@commitlint/cli": "^7.0.0", diff --git a/test/agent.js b/test/agent.js index e3d6f4dd69..ce934fc1bd 100644 --- a/test/agent.js +++ b/test/agent.js @@ -572,6 +572,18 @@ test('#captureError()', function (t) { }) }) + t.test('generate error id', function (t) { + t.plan(1 + APMServerWithDefaultAsserts.asserts) + APMServerWithDefaultAsserts(t, {}, { expect: 'error' }) + .on('listening', function () { + this.agent.captureError(new Error('foo')) + }) + .on('data-error', function (data) { + t.ok(/^[\da-f]{32}$/.test(data.id), `should have valid id (was: ${data.id})`) + t.end() + }) + }) + t.test('should send a plain text message to the server', function (t) { t.plan(1 + APMServerWithDefaultAsserts.asserts) APMServerWithDefaultAsserts(t, {}, { expect: 'error' }) diff --git a/test/instrumentation/index.js b/test/instrumentation/index.js index 0e530b5a16..1c63ea5b18 100644 --- a/test/instrumentation/index.js +++ b/test/instrumentation/index.js @@ -24,7 +24,8 @@ test('basic', function (t) { t.equal(data.spans.length, 4) data.transactions.forEach(function (trans, index) { - t.ok(/[\da-f]{8}-([\da-f]{4}-){3}[\da-f]{12}/.test(trans.id)) + t.ok(/^[\da-f]{16}$/.test(trans.id)) + t.ok(/^[\da-f]{32}$/.test(trans.trace_id)) t.equal(trans.name, 'foo' + index) t.equal(trans.type, 'bar' + index) t.ok(trans.duration > 0, 'duration should be >0ms') @@ -36,7 +37,7 @@ test('basic', function (t) { const name = 't' + index + i const span = findObjInArray(data.spans, 'name', name) t.ok(span, 'should have span named ' + name) - t.equal(span.transactionId, trans.id, 'should belong to correct transaction') + t.equal(span.transaction_id, trans.id, 'should belong to correct transaction') t.equal(span.type, 'type') t.ok(span.start > 0, 'span start should be >0ms') t.ok(span.start < 100, 'span start should be <100ms') @@ -88,7 +89,7 @@ test('same tick', function (t) { const name = 't' + i const span = findObjInArray(data.spans, 'name', name) t.ok(span, 'should have span named ' + name) - t.equal(span.transactionId, trans.id, 'should belong to correct transaction') + t.equal(span.transaction_id, trans.id, 'should belong to correct transaction') } t.end() }) @@ -111,7 +112,7 @@ test('serial - no parents', function (t) { const name = 't' + i const span = findObjInArray(data.spans, 'name', name) t.ok(span, 'should have span named ' + name) - t.equal(span.transactionId, trans.id, 'should belong to correct transaction') + t.equal(span.transaction_id, trans.id, 'should belong to correct transaction') } t.end() }) @@ -138,7 +139,7 @@ test('serial - with parents', function (t) { const name = 't' + i const span = findObjInArray(data.spans, 'name', name) t.ok(span, 'should have span named ' + name) - t.equal(span.transactionId, trans.id, 'should belong to correct transaction') + t.equal(span.transaction_id, trans.id, 'should belong to correct transaction') } t.end() }) @@ -165,7 +166,7 @@ test('stack branching - no parents', function (t) { const name = 't' + i const span = findObjInArray(data.spans, 'name', name) t.ok(span, 'should have span named ' + name) - t.equal(span.transactionId, trans.id, 'should belong to correct transaction') + t.equal(span.transaction_id, trans.id, 'should belong to correct transaction') } t.end() }) @@ -191,7 +192,7 @@ test('currentTransaction missing - recoverable', function (t) { const name = 't0' const span = findObjInArray(data.spans, 'name', name) t.ok(span, 'should have span named ' + name) - t.equal(span.transactionId, trans.id, 'should belong to correct transaction') + t.equal(span.transaction_id, trans.id, 'should belong to correct transaction') t.end() }) var ins = agent._instrumentation @@ -219,7 +220,7 @@ test('currentTransaction missing - not recoverable - last span failed', function const name = 't0' const span = findObjInArray(data.spans, 'name', name) t.ok(span, 'should have span named ' + name) - t.equal(span.transactionId, trans.id, 'should belong to correct transaction') + t.equal(span.transaction_id, trans.id, 'should belong to correct transaction') t.end() }) var ins = agent._instrumentation @@ -250,7 +251,7 @@ test('currentTransaction missing - not recoverable - middle span failed', functi for (const name of names) { const span = findObjInArray(data.spans, 'name', name) t.ok(span, 'should have span named ' + name) - t.equal(span.transactionId, trans.id, 'should belong to correct transaction') + t.equal(span.transaction_id, trans.id, 'should belong to correct transaction') } t.end() }) @@ -374,7 +375,8 @@ test('unsampled transactions do not include spans', function (t) { t.equal(data.transactions.length, 1) data.transactions.forEach(function (trans) { - t.ok(/[\da-f]{8}-([\da-f]{4}-){3}[\da-f]{12}/.test(trans.id)) + t.ok(/^[\da-f]{16}$/.test(trans.id)) + t.ok(/^[\da-f]{32}$/.test(trans.trace_id)) t.ok(trans.duration > 0, 'duration should be >0ms') t.ok(trans.duration < 100, 'duration should be <100ms') t.notOk(Number.isNaN((new Date(trans.timestamp)).getTime())) diff --git a/test/instrumentation/modules/express-queue.js b/test/instrumentation/modules/express-queue.js index fd42d96cd3..b68497ed25 100644 --- a/test/instrumentation/modules/express-queue.js +++ b/test/instrumentation/modules/express-queue.js @@ -85,8 +85,8 @@ function done (t, query) { t.comment('request ' + (i + 1)) t.equal(trans.name, 'GET /', 'name should be GET /') t.equal(trans.type, 'request', 'type should be request') - t.equal(data.spans.filter(span => span.transactionId === trans.id).length, 1, 'transaction should have 1 span') - const span = findObjInArray(data.spans, 'transactionId', trans.id) + t.equal(data.spans.filter(span => span.transaction_id === trans.id).length, 1, 'transaction should have 1 span') + const span = findObjInArray(data.spans, 'transaction_id', trans.id) t.equal(span.name, 'foo', 'span name should be foo') t.equal(span.type, 'bar', 'span name should be bar') t.ok(span.start + span.duration < trans.duration, 'span should have valid timings') diff --git a/test/instrumentation/modules/http/request.js b/test/instrumentation/modules/http/request.js index dfcac4c493..a0ad339c65 100644 --- a/test/instrumentation/modules/http/request.js +++ b/test/instrumentation/modules/http/request.js @@ -21,7 +21,7 @@ test('request', function (t) { var root = data.transactions[1] t.equal(root.name, 'GET /') - const span = findObjInArray(data.spans, 'transactionId', root.id) + const span = findObjInArray(data.spans, 'transaction_id', root.id) t.equal(span.name, 'GET localhost:' + server.address().port + '/test') server.close() diff --git a/test/instrumentation/modules/http2.js b/test/instrumentation/modules/http2.js index 11c803cbea..bcd687009b 100644 --- a/test/instrumentation/modules/http2.js +++ b/test/instrumentation/modules/http2.js @@ -280,7 +280,7 @@ isSecure.forEach(secure => { var root = data.transactions[1] assertPath(t, root, secure, port, '/') - var span = findObjInArray(data.spans, 'transactionId', root.id) + var span = findObjInArray(data.spans, 'transaction_id', root.id) t.ok(span, 'root transaction should have span') t.equal(span.type, 'ext.http2') t.equal(span.name, `undefined http${secure ? 's' : ''}://localhost:${port}/sub`) @@ -340,7 +340,7 @@ isSecure.forEach(secure => { }) }) -var matchId = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/ +var matchId = /^[\da-f]{16}$/ var matchTimestamp = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d{3}Z$/ function assertPath (t, trans, secure, port, path) { diff --git a/test/instrumentation/modules/mysql/mysql.js b/test/instrumentation/modules/mysql/mysql.js index 336f890519..44fba3c73f 100644 --- a/test/instrumentation/modules/mysql/mysql.js +++ b/test/instrumentation/modules/mysql/mysql.js @@ -328,7 +328,7 @@ factories.forEach(function (f) { t.deepEqual(names, ['bar', 'baz', 'foo']) data.transactions.forEach(function (trans) { - const span = findObjInArray(data.spans, 'transactionId', trans.id) + const span = findObjInArray(data.spans, 'transaction_id', trans.id) t.ok(span, 'transaction should have span') t.equal(span.name, 'SELECT') t.equal(span.type, 'db.mysql.query') diff --git a/test/instrumentation/modules/mysql2/mysql.js b/test/instrumentation/modules/mysql2/mysql.js index 25ca21fc21..3c17d898fe 100644 --- a/test/instrumentation/modules/mysql2/mysql.js +++ b/test/instrumentation/modules/mysql2/mysql.js @@ -265,7 +265,7 @@ factories.forEach(function (f) { t.deepEqual(names, ['bar', 'baz', 'foo']) data.transactions.forEach(function (trans) { - const span = findObjInArray(data.spans, 'transactionId', trans.id) + const span = findObjInArray(data.spans, 'transaction_id', trans.id) t.ok(span, 'transaction should have span') t.equal(span.name, 'SELECT') t.equal(span.type, 'db.mysql.query') diff --git a/test/instrumentation/modules/pg/pg.js b/test/instrumentation/modules/pg/pg.js index 9fc21bba98..85fffb4183 100644 --- a/test/instrumentation/modules/pg/pg.js +++ b/test/instrumentation/modules/pg/pg.js @@ -318,7 +318,7 @@ factories.forEach(function (f) { t.deepEqual(names, ['bar', 'baz', 'foo']) data.transactions.forEach(function (trans) { - const span = findObjInArray(data.spans, 'transactionId', trans.id) + const span = findObjInArray(data.spans, 'transaction_id', trans.id) t.ok(span, 'transaction should have span') t.equal(span.name, 'SELECT') t.equal(span.type, 'db.postgresql.query') diff --git a/test/instrumentation/span.js b/test/instrumentation/span.js index 33dc5f1255..ab7fb8c743 100644 --- a/test/instrumentation/span.js +++ b/test/instrumentation/span.js @@ -16,6 +16,7 @@ test('properties', function (t) { var trans = new Transaction(agent) var span = new Span(trans) span.start('sig', 'type') + t.ok(/^[\da-f]{16}$/.test(span.id)) t.equal(span.transaction, trans) t.equal(span.name, 'sig') t.equal(span.type, 'type') @@ -108,8 +109,11 @@ test('#_encode() - ended unnamed', function myTest1 (t) { span.end() span._encode(function (err, payload) { t.error(err) - t.deepEqual(Object.keys(payload), ['transactionId', 'timestamp', 'name', 'type', 'start', 'duration', 'stacktrace']) - t.equal(payload.transactionId, trans.id) + t.deepEqual(Object.keys(payload), ['id', 'transaction_id', 'trace_id', 'timestamp', 'name', 'type', 'start', 'duration', 'stacktrace']) + t.ok(/^[\da-f]{16}$/.test(payload.id)) + t.ok(/^[\da-f]{16}$/.test(payload.transaction_id)) + t.ok(/^[\da-f]{32}$/.test(payload.trace_id)) + t.equal(payload.transaction_id, trans.id) t.equal(payload.timestamp, trans.timestamp) t.notOk(Number.isNaN(Date.parse(payload.timestamp))) t.equal(payload.name, 'unnamed') @@ -128,8 +132,11 @@ test('#_encode() - ended named', function myTest2 (t) { span.end() span._encode(function (err, payload) { t.error(err) - t.deepEqual(Object.keys(payload), ['transactionId', 'timestamp', 'name', 'type', 'start', 'duration', 'stacktrace']) - t.equal(payload.transactionId, trans.id) + t.deepEqual(Object.keys(payload), ['id', 'transaction_id', 'trace_id', 'timestamp', 'name', 'type', 'start', 'duration', 'stacktrace']) + t.ok(/^[\da-f]{16}$/.test(payload.id)) + t.ok(/^[\da-f]{16}$/.test(payload.transaction_id)) + t.ok(/^[\da-f]{32}$/.test(payload.trace_id)) + t.equal(payload.transaction_id, trans.id) t.equal(payload.timestamp, trans.timestamp) t.notOk(Number.isNaN(Date.parse(payload.timestamp))) t.equal(payload.name, 'foo') @@ -150,8 +157,11 @@ test('#_encode() - disabled stack traces', function (t) { span.end() span._encode(function (err, payload) { t.error(err) - t.deepEqual(Object.keys(payload), ['transactionId', 'timestamp', 'name', 'type', 'start', 'duration']) - t.equal(payload.transactionId, trans.id) + t.deepEqual(Object.keys(payload), ['id', 'transaction_id', 'trace_id', 'timestamp', 'name', 'type', 'start', 'duration']) + t.ok(/^[\da-f]{16}$/.test(payload.id)) + t.ok(/^[\da-f]{16}$/.test(payload.transaction_id)) + t.ok(/^[\da-f]{32}$/.test(payload.trace_id)) + t.equal(payload.transaction_id, trans.id) t.equal(payload.timestamp, trans.timestamp) t.notOk(Number.isNaN(Date.parse(payload.timestamp))) t.equal(payload.name, 'unnamed') diff --git a/test/instrumentation/transaction.js b/test/instrumentation/transaction.js index 2c34396300..7b69cec90e 100644 --- a/test/instrumentation/transaction.js +++ b/test/instrumentation/transaction.js @@ -12,6 +12,8 @@ test('init', function (t) { t.ok(false) }) var trans = new Transaction(ins._agent, 'name', 'type') + t.ok(/^[\da-f]{16}$/.test(trans.id)) + t.ok(/^[\da-f]{32}$/.test(trans.traceId)) t.equal(trans.name, 'name') t.equal(trans.type, 'type') t.equal(trans.result, 'success') @@ -232,15 +234,16 @@ test('#_encode() - un-ended', function (t) { }) test('#_encode() - ended', function (t) { - t.plan(10) + t.plan(11) var ins = mockInstrumentation(function () { t.pass('should end the transaction') }) var trans = new Transaction(ins._agent) trans.end() const payload = trans._encode() - t.deepEqual(Object.keys(payload), ['id', 'name', 'type', 'duration', 'timestamp', 'result', 'sampled', 'context', 'span_count']) - t.equal(typeof payload.id, 'string') + t.deepEqual(Object.keys(payload), ['id', 'trace_id', 'name', 'type', 'duration', 'timestamp', 'result', 'sampled', 'context', 'span_count']) + t.ok(/^[\da-f]{16}$/.test(payload.id)) + t.ok(/^[\da-f]{32}$/.test(payload.trace_id)) t.equal(payload.id, trans.id) t.equal(payload.name, 'unnamed') t.equal(payload.type, 'custom') @@ -252,7 +255,7 @@ test('#_encode() - ended', function (t) { }) test('#_encode() - with meta data', function (t) { - t.plan(10) + t.plan(11) var ins = mockInstrumentation(function () { t.pass('should end the transaction') }) @@ -263,8 +266,9 @@ test('#_encode() - with meta data', function (t) { trans.setCustomContext({ baz: 1 }) trans.end() const payload = trans._encode() - t.deepEqual(Object.keys(payload), ['id', 'name', 'type', 'duration', 'timestamp', 'result', 'sampled', 'context', 'span_count']) - t.equal(typeof payload.id, 'string') + t.deepEqual(Object.keys(payload), ['id', 'trace_id', 'name', 'type', 'duration', 'timestamp', 'result', 'sampled', 'context', 'span_count']) + t.ok(/^[\da-f]{16}$/.test(payload.id)) + t.ok(/^[\da-f]{32}$/.test(payload.trace_id)) t.equal(payload.id, trans.id) t.equal(payload.name, 'foo') t.equal(payload.type, 'bar') @@ -276,7 +280,7 @@ test('#_encode() - with meta data', function (t) { }) test('#_encode() - http request meta data', function (t) { - t.plan(10) + t.plan(11) var ins = mockInstrumentation(function () { t.pass('should end the transaction') }) @@ -284,8 +288,9 @@ test('#_encode() - http request meta data', function (t) { trans.req = mockRequest() trans.end() const payload = trans._encode() - t.deepEqual(Object.keys(payload), ['id', 'name', 'type', 'duration', 'timestamp', 'result', 'sampled', 'context', 'span_count']) - t.equal(typeof payload.id, 'string') + t.deepEqual(Object.keys(payload), ['id', 'trace_id', 'name', 'type', 'duration', 'timestamp', 'result', 'sampled', 'context', 'span_count']) + t.ok(/^[\da-f]{16}$/.test(payload.id)) + t.ok(/^[\da-f]{32}$/.test(payload.trace_id)) t.equal(payload.id, trans.id) t.equal(payload.name, 'POST unknown route') t.equal(payload.type, 'custom')