Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update ids to correct v2 api format #584

Merged
merged 1 commit into from
Sep 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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')
watson marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -570,6 +570,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