Skip to content
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
48 changes: 26 additions & 22 deletions packages/datadog-instrumentations/src/dns.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict'

const { channel, addHook, AsyncResource } = require('./helpers/instrument')
const { channel, addHook } = require('./helpers/instrument')
const shimmer = require('../../datadog-shimmer')

const rrtypes = {
Expand Down Expand Up @@ -48,11 +48,12 @@ function patchResolveShorthands (prototype) {

function wrap (prefix, fn, expectedArgs, rrtype) {
const startCh = channel(prefix + ':start')
const finishCh = channel(prefix + ':finish')
const asyncEndCh = channel(prefix + ':async_end')
const endCh = channel(prefix + ':end')
const errorCh = channel(prefix + ':error')

const wrapped = function () {
const cb = AsyncResource.bind(arguments[arguments.length - 1])
const cb = arguments[arguments.length - 1]
if (
!startCh.hasSubscribers ||
arguments.length < expectedArgs ||
Expand All @@ -67,28 +68,31 @@ function wrap (prefix, fn, expectedArgs, rrtype) {
startArgs.push(rrtype)
}

const asyncResource = new AsyncResource('bound-anonymous-fn')
return asyncResource.runInAsyncScope(() => {
startCh.publish(startArgs)
const context = { args: startArgs }
startCh.publish(context)

arguments[arguments.length - 1] = asyncResource.bind(function (error, result) {
if (error) {
errorCh.publish(error)
}
finishCh.publish(result)
cb.apply(this, arguments)
})
arguments[arguments.length - 1] = function (error, result) {
if (error) {
context.error = error
errorCh.publish(context)
}
context.result = result
asyncEndCh.publish(context)
cb.apply(this, arguments)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline, let's make sure this is wrapped between 2 events so that the subscriber can restore the right context only in the callback, similar to promises and to what is described in nodejs/node#44943 (comment).

}

try {
return fn.apply(this, arguments)
try {
return fn.apply(this, arguments)
// TODO deal with promise versions when we support `dns/promises`
} catch (error) {
error.stack // trigger getting the stack at the original throwing point
errorCh.publish(error)

throw error
}
})
} catch (error) {
error.stack // trigger getting the stack at the original throwing point
context.error = error
errorCh.publish(context)

throw error
} finally {
endCh.publish(context)
}
}

return shimmer.wrap(fn, wrapped)
Expand Down
4 changes: 2 additions & 2 deletions packages/datadog-plugin-dns/src/lookup.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class DNSLookupPlugin extends ClientPlugin {
static get name () { return 'dns' }
static get operation () { return 'lookup' }

start ([hostname]) {
start ({ args: [hostname] }) {
this.startSpan('dns.lookup', {
service: this.config.service,
resource: hostname,
Expand All @@ -19,7 +19,7 @@ class DNSLookupPlugin extends ClientPlugin {
})
}

finish (result) {
finish ({ result }) {
const span = this.activeSpan

if (Array.isArray(result)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/datadog-plugin-dns/src/lookup_service.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class DNSLookupServicePlugin extends ClientPlugin {
static get name () { return 'dns' }
static get operation () { return 'lookup_service' }

start ([address, port]) {
start ({ args: [address, port] }) {
this.startSpan('dns.lookup_service', {
service: this.config.service,
resource: `${address}:${port}`,
Expand Down
2 changes: 1 addition & 1 deletion packages/datadog-plugin-dns/src/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class DNSResolvePlugin extends ClientPlugin {
static get name () { return 'dns' }
static get operation () { return 'resolve' }

start ([hostname, maybeType]) {
start ({ args: [hostname, maybeType] }) {
const rrtype = typeof maybeType === 'string' ? maybeType : 'A'

this.startSpan('dns.resolve', {
Expand Down
2 changes: 1 addition & 1 deletion packages/datadog-plugin-dns/src/reverse.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class DNSReversePlugin extends ClientPlugin {
static get name () { return 'dns' }
static get operation () { return 'reverse' }

start ([ip]) {
start ({ args: [ip] }) {
this.startSpan('dns.reverse', {
service: this.config.service,
resource: ip,
Expand Down
5 changes: 5 additions & 0 deletions packages/dd-trace/src/plugins/outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ class OutgoingPlugin extends TracingPlugin {
this.addHost(url.hostname, url.port)
}

asyncEnd (...args) {
super.asyncEnd(...args)
this.exit(...args)
}

addHost (hostname, port) {
const span = this.activeSpan

Expand Down
13 changes: 13 additions & 0 deletions packages/dd-trace/src/plugins/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ module.exports = class Plugin {
this._subscriptions = []
this._enabled = false
this._tracer = tracer
this._storesByContext = new WeakMap()
}

get tracer () {
Expand All @@ -41,6 +42,10 @@ module.exports = class Plugin {
storage.enterWith({ ...store, span })
}

exit (ctx) {
storage.enterWith(this.getStoreByContext(ctx))
}

// TODO: Implement filters on resource name for all plugins.
/** Prevents creation of spans here and for all async descendants. */
skip () {
Expand All @@ -61,6 +66,14 @@ module.exports = class Plugin {
}
}

setStoreByContext (context, store) {
this._storesByContext.set(context, store)
}

getStoreByContext (context) {
return this._storesByContext.get(context)
}

configure (config) {
if (typeof config === 'boolean') {
config = { enabled: config }
Expand Down
26 changes: 25 additions & 1 deletion packages/dd-trace/src/plugins/tracing.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,28 @@ class TracingPlugin extends Plugin {
this.operation = this.constructor.operation

this.addTraceSub('start', message => {
if (message && typeof message === 'object') {
this.setStoreByContext(message, storage.getStore())
}
this.start(message)
})

this.addTraceSub('error', err => {
this.error(err)
})

// TODO remove the finish event everywhere
this.addTraceSub('finish', message => {
this.finish(message)
})

this.addTraceSub('async_end', message => {
this.asyncEnd(message)
})

this.addTraceSub('end', message => {
this.end(message)
})
}

get activeSpan () {
Expand All @@ -46,7 +58,19 @@ class TracingPlugin extends Plugin {
this.activeSpan.finish()
}

asyncEnd (...args) {
this.finish(...args)
// any non-IncomingPlugin plugins need to call exit here.
}

end (...args) {
this.exit(...args)
}

error (error) {
if (error && typeof error === 'object' && error.error) {
error = error.error
}
this.addError(error)
}

Expand Down Expand Up @@ -83,7 +107,7 @@ class TracingPlugin extends Plugin {

analyticsSampler.sample(span, this.config.measured)

storage.enterWith({ ...store, span })
this.enter(span, store)

return span
}
Expand Down