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

Traverse context to find active span/transaction #1964

Closed
wants to merge 4 commits into from
Closed
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
78 changes: 61 additions & 17 deletions lib/instrumentation/async-hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,28 @@ module.exports = function (ins) {
const asyncHook = asyncHooks.createHook({ init, before, destroy })
const contexts = new WeakMap()

const resources = new Map()

if (resettable) {
if (_asyncHook) _asyncHook.disable()
_asyncHook = asyncHook
}

function traverseContext (asyncId, map) {
const event = map.get(asyncId)
if (!event) {
const resource = resources.get(asyncId)
if (resource) {
return traverseContext(resource.triggerAsyncId, map)
}
}
return event
}

const activeTransactions = new Map()
Object.defineProperty(ins, 'currentTransaction', {
get () {
const asyncId = asyncHooks.executionAsyncId()
return activeTransactions.get(asyncId) || null
return traverseContext(asyncHooks.executionAsyncId(), activeTransactions)
},
set (trans) {
const asyncId = asyncHooks.executionAsyncId()
Expand All @@ -35,8 +47,7 @@ module.exports = function (ins) {
const activeSpans = new Map()
Object.defineProperty(ins, 'activeSpan', {
get () {
const asyncId = asyncHooks.executionAsyncId()
return activeSpans.get(asyncId) || null
return traverseContext(asyncHooks.executionAsyncId(), activeSpans)
},
set (span) {
const asyncId = asyncHooks.executionAsyncId()
Expand All @@ -63,8 +74,40 @@ module.exports = function (ins) {
}
})

shimmer.wrap(ins, 'addEndedSpan', function (addEndedSpan) {
return function wrapEndedSpan (span) {
const asyncIds = contexts.get(span)
if (asyncIds) {
for (const asyncId of asyncIds) {
activeSpans.delete(asyncId)
}
contexts.delete(span)
}

return addEndedSpan.call(this, span)
}
})

asyncHook.enable()

function trackContext (asyncId, transactionOrSpan) {
let asyncIds = contexts.get(transactionOrSpan)
if (!asyncIds) {
asyncIds = []
contexts.set(transactionOrSpan, asyncIds)
}

asyncIds.push(asyncId)
}

function untrackContext (asyncId, transactionOrSpan) {
const asyncIds = contexts.get(transactionOrSpan)
if (asyncIds) {
const index = asyncIds.indexOf(asyncId)
asyncIds.splice(index, 1)
}
}

function init (asyncId, type, triggerAsyncId, resource) {
// We don't care about the TIMERWRAP, as it will only init once for each
// timer that shares the timeout value. Instead we rely on the Timeout
Expand All @@ -74,18 +117,17 @@ module.exports = function (ins) {
const transaction = ins.currentTransaction
if (!transaction) return

resources.set(asyncId, { triggerAsyncId })

activeTransactions.set(asyncId, transaction)

// Track the context by the transaction
let asyncIds = contexts.get(transaction)
if (!asyncIds) {
asyncIds = []
contexts.set(transaction, asyncIds)
}
asyncIds.push(asyncId)
trackContext(asyncId, transaction)

const span = ins.bindingSpan || ins.activeSpan
if (span) activeSpans.set(asyncId, span)
if (span) {
trackContext(asyncId, span)
activeSpans.set(asyncId, span)
}
}

function before (asyncId) {
Expand All @@ -105,14 +147,16 @@ module.exports = function (ins) {
const transaction = span ? span.transaction : activeTransactions.get(asyncId)

if (transaction) {
const asyncIds = contexts.get(transaction)
if (asyncIds) {
const index = asyncIds.indexOf(asyncId)
asyncIds.splice(index, 1)
}
untrackContext(asyncId, transaction)
}

if (span) {
untrackContext(asyncId, span)
}

activeTransactions.delete(asyncId)
activeSpans.delete(asyncId)

resources.delete(asyncId)
}
}
30 changes: 30 additions & 0 deletions test/instrumentation/async-hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,36 @@ test('sync/async tracking', function (t) {
})
})

test('span.end()', function (t) {
var transaction = agent.startTransaction()

var firstSpan = agent.startSpan('first-span')
t.strictEqual(firstSpan.parentId, transaction.id, 'first span is a child of the active transaction')

process.nextTick(function () {
var childSpan = agent.startSpan('child-span')

t.equal(childSpan.parentId, firstSpan.id, 'child-span is a direct child of first-span')

process.nextTick(function () {
childSpan.end()

var siblingSpan = agent.startSpan('sibling-span')

t.notEqual(siblingSpan.parentId, transaction.id, 'sibling-span is not a direct child of the active transaction')
t.equal(siblingSpan.parentId, firstSpan.id, 'sibling-span is a direct child of first-span')

siblingSpan.end()

firstSpan.end()

transaction.end()

t.end()
})
})
})

function twice (fn) {
setImmediate(fn)
setImmediate(fn)
Expand Down
4 changes: 3 additions & 1 deletion test/instrumentation/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ test('nested spans', function (t) {
t.strictEqual(s1.transaction_id, trans.id, 's1 transaction_id matches transaction id')

const s01 = findObjInArray(data.spans, 'name', 's01')
t.strictEqual(s01.parent_id, s0.id, 's01 should descend from s0')
t.strictEqual(s01.parent_id, trans.id, 's01 should descend from the transaction')
t.strictEqual(s01.trace_id, trans.trace_id, 's01 has same trace_id as transaction')
t.strictEqual(s01.transaction_id, trans.id, 's01 transaction_id matches transaction id')

Expand Down Expand Up @@ -650,6 +650,8 @@ test('nested spans', function (t) {
var s0 = ins.startSpan('s0')
process.nextTick(function () {
process.nextTick(function () {
// will adopt the transaction as its parent,
// because s0 ended before the new span started
var s01 = ins.startSpan('s01')
process.nextTick(function () {
s01.end()
Expand Down