Skip to content

Commit

Permalink
fix: update ids to correct v2 api format (#584)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
watson committed Nov 6, 2018
1 parent b0a9f64 commit 0d73c26
Show file tree
Hide file tree
Showing 14 changed files with 76 additions and 40 deletions.
4 changes: 2 additions & 2 deletions lib/agent.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict'

var crypto = require('crypto')
var http = require('http')
var parseUrl = require('url').parse
var path = require('path')
Expand All @@ -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')
Expand Down Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion lib/instrumentation/span.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict'

var crypto = require('crypto')

var afterAll = require('after-all-results')
var Value = require('async-value-promise')

Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 5 additions & 2 deletions lib/instrumentation/transaction.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(),
Expand Down
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
12 changes: 12 additions & 0 deletions test/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' })
Expand Down
22 changes: 12 additions & 10 deletions test/instrumentation/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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')
Expand Down Expand Up @@ -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()
})
Expand All @@ -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()
})
Expand All @@ -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()
})
Expand All @@ -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()
})
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
})
Expand Down Expand Up @@ -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()))
Expand Down
4 changes: 2 additions & 2 deletions test/instrumentation/modules/express-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
2 changes: 1 addition & 1 deletion test/instrumentation/modules/http/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions test/instrumentation/modules/http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion test/instrumentation/modules/mysql/mysql.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
2 changes: 1 addition & 1 deletion test/instrumentation/modules/mysql2/mysql.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
2 changes: 1 addition & 1 deletion test/instrumentation/modules/pg/pg.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
22 changes: 16 additions & 6 deletions test/instrumentation/span.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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')
Expand All @@ -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')
Expand All @@ -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')
Expand Down
23 changes: 14 additions & 9 deletions test/instrumentation/transaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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')
Expand All @@ -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')
})
Expand All @@ -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')
Expand All @@ -276,16 +280,17 @@ 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')
})
var trans = new Transaction(ins._agent)
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')
Expand Down

0 comments on commit 0d73c26

Please sign in to comment.