Skip to content

Commit

Permalink
Call options fn on each invocation in tracer.wrap (#1191)
Browse files Browse the repository at this point in the history
Because the options variable in tracer.wrap was being re-assigned,
options functions were only being executing a single time, and then the
same result would be cached. This is fixed by using a variable local to
the function invocation.

Fixes #1181
  • Loading branch information
bengl committed Jan 8, 2021
1 parent 8e8c438 commit c565a85
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 8 deletions.
11 changes: 6 additions & 5 deletions packages/dd-trace/src/tracer.js
Expand Up @@ -69,11 +69,12 @@ class DatadogTracer extends Tracer {
const tracer = this

return function () {
if (typeof options === 'function' && typeof fn === 'function') {
options = options.apply(this, arguments)
let optionsObj = options
if (typeof optionsObj === 'function' && typeof fn === 'function') {
optionsObj = optionsObj.apply(this, arguments)
}

if (options.orphanable === false && !tracer.scope().active()) {
if (optionsObj.orphanable === false && !tracer.scope().active()) {
return fn.apply(this, arguments)
}

Expand All @@ -82,7 +83,7 @@ class DatadogTracer extends Tracer {

if (typeof cb === 'function') {
const scopeBoundCb = tracer.scope().bind(cb)
return tracer.trace(name, options, (span, done) => {
return tracer.trace(name, optionsObj, (span, done) => {
arguments[lastArgId] = function (err) {
done(err)
return scopeBoundCb.apply(this, arguments)
Expand All @@ -91,7 +92,7 @@ class DatadogTracer extends Tracer {
return fn.apply(this, arguments)
})
} else {
return tracer.trace(name, options, () => fn.apply(this, arguments))
return tracer.trace(name, optionsObj, () => fn.apply(this, arguments))
}
}
}
Expand Down
15 changes: 12 additions & 3 deletions packages/dd-trace/test/tracer.spec.js
Expand Up @@ -383,14 +383,17 @@ describe('Tracer', () => {
})
})

it('should accept an options function', () => {
it('should accept an options function, invoked on every invocation of the wrapped function', () => {
const it = {}

let invocations = 0

function options (foo, bar) {
invocations++
expect(this).to.equal(it)
expect(foo).to.equal('hello')
expect(bar).to.equal('goodbye')
return { tags: { sometag: 'somevalue' } }
return { tags: { sometag: 'somevalue', invocations } }
}

const fn = tracer.wrap('name', options, function () {})
Expand All @@ -400,7 +403,13 @@ describe('Tracer', () => {
fn.call(it, 'hello', 'goodbye')

expect(tracer.trace).to.have.been.calledWith('name', {
tags: { sometag: 'somevalue' }
tags: { sometag: 'somevalue', invocations: 1 }
})

fn.call(it, 'hello', 'goodbye')

expect(tracer.trace).to.have.been.calledWith('name', {
tags: { sometag: 'somevalue', invocations: 2 }
})
})

Expand Down

0 comments on commit c565a85

Please sign in to comment.