Skip to content

Commit

Permalink
Add support for exit spans in Code Origin for Spans (#4772)
Browse files Browse the repository at this point in the history
  • Loading branch information
watson authored Oct 30, 2024
1 parent 111c14a commit 91c4371
Show file tree
Hide file tree
Showing 10 changed files with 194 additions and 19 deletions.
8 changes: 4 additions & 4 deletions packages/datadog-code-origin/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ const { getUserLandFrames } = require('../dd-trace/src/plugins/util/stacktrace')
const limit = Number(process.env._DD_CODE_ORIGIN_MAX_USER_FRAMES) || 8

module.exports = {
entryTag,
exitTag
entryTags,
exitTags
}

function entryTag (topOfStackFunc) {
function entryTags (topOfStackFunc) {
return tag('entry', topOfStackFunc)
}

function exitTag (topOfStackFunc) {
function exitTags (topOfStackFunc) {
return tag('exit', topOfStackFunc)
}

Expand Down
33 changes: 33 additions & 0 deletions packages/datadog-core/src/utils/src/parse-tags.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
'use strict'

const digitRegex = /^\d+$/

/**
* Converts a flat object of tags into a nested object. For example:
* { 'a.b.c': 'value' } -> { a: { b: { c: 'value' } } }
* Also supports array-keys. For example:
* { 'a.0.b': 'value' } -> { a: [{ b: 'value' }] }
*
* @param {Object} tags - Key/value pairs of tags
* @returns Object - Parsed tags
*/
module.exports = tags => {
const parsedTags = {}
for (const [tag, value] of Object.entries(tags)) {
const keys = tag.split('.')
let current = parsedTags
let depth = 0
for (const key of keys) {
if (!current[key]) {
if (depth === keys.length - 1) {
current[key] = value
break
}
current[key] = keys[depth + 1]?.match(digitRegex) ? [] : {}
}
current = current[key]
depth++
}
}
return parsedTags
}
23 changes: 23 additions & 0 deletions packages/datadog-core/test/utils/src/parse-tags.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict'

require('../../../../dd-trace/test/setup/tap')

const parseTags = require('../../../src/utils/src/parse-tags')

describe('parseTags', () => {
it('should parse tags to object', () => {
const obj = {
'a.0.a': 'foo',
'a.0.b': 'bar',
'a.1.a': 'baz'
}

expect(parseTags(obj)).to.deep.equal({
a: [{ a: 'foo', b: 'bar' }, { a: 'baz' }]
})
})

it('should work with empty object', () => {
expect(parseTags({})).to.deep.equal({})
})
})
4 changes: 2 additions & 2 deletions packages/datadog-plugin-fastify/src/code_origin.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict'

const { entryTag } = require('../../datadog-code-origin')
const { entryTags } = require('../../datadog-code-origin')
const Plugin = require('../../dd-trace/src/plugins/plugin')
const web = require('../../dd-trace/src/plugins/util/web')

Expand All @@ -23,7 +23,7 @@ class FastifyCodeOriginForSpansPlugin extends Plugin {

this.addSub('apm:fastify:route:added', ({ routeOptions, onRoute }) => {
if (!routeOptions.config) routeOptions.config = {}
routeOptions.config[kCodeOriginForSpansTagsSym] = entryTag(onRoute)
routeOptions.config[kCodeOriginForSpansTagsSym] = entryTags(onRoute)
})
}
}
Expand Down
15 changes: 6 additions & 9 deletions packages/datadog-plugin-fastify/test/code_origin.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const axios = require('axios')
const semver = require('semver')
const agent = require('../../dd-trace/test/plugins/agent')
const { getNextLineNumber } = require('../../dd-trace/test/plugins/helpers')
const { NODE_MAJOR } = require('../../../version')

const host = 'localhost'
Expand Down Expand Up @@ -49,13 +50,13 @@ describe('Plugin', () => {

// Wrap in a named function to have at least one frame with a function name
function wrapperFunction () {
routeRegisterLine = getNextLineNumber()
routeRegisterLine = String(getNextLineNumber())
app.get('/user', function userHandler (request, reply) {
reply.send()
})
}

const callWrapperLine = getNextLineNumber()
const callWrapperLine = String(getNextLineNumber())
wrapperFunction()

app.listen(() => {
Expand Down Expand Up @@ -95,7 +96,7 @@ describe('Plugin', () => {
let routeRegisterLine

app.register(function v1Handler (app, opts, done) {
routeRegisterLine = getNextLineNumber()
routeRegisterLine = String(getNextLineNumber())
app.get('/user', function userHandler (request, reply) {
reply.send()
})
Expand Down Expand Up @@ -134,7 +135,7 @@ describe('Plugin', () => {
next()
})

const routeRegisterLine = getNextLineNumber()
const routeRegisterLine = String(getNextLineNumber())
app.get('/user', function userHandler (request, reply) {
reply.send()
})
Expand Down Expand Up @@ -170,7 +171,7 @@ describe('Plugin', () => {
// number of where the route handler is defined. However, this might not be the right choice and it might be
// better to point to the middleware.
it.skip('should point to middleware if middleware responds early', function testCase (done) {
const middlewareRegisterLine = getNextLineNumber()
const middlewareRegisterLine = String(getNextLineNumber())
app.use(function middleware (req, res, next) {
res.end()
})
Expand Down Expand Up @@ -210,7 +211,3 @@ describe('Plugin', () => {
})
})
})

function getNextLineNumber () {
return String(Number(new Error().stack.split('\n')[2].match(/:(\d+):/)[1]) + 1)
}
63 changes: 63 additions & 0 deletions packages/datadog-plugin-http/test/code_origin.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
'use strict'

const agent = require('../../dd-trace/test/plugins/agent')

describe('Plugin', () => {
describe('http', () => {
describe('Code Origin for Spans', () => {
before(() => {
// Needed when this spec file run together with other spec files, in which case the agent config is not
// re-loaded unless the existing agent is wiped first. And we need the agent config to be re-loaded in order to
// enable Code Origin for Spans.
agent.wipe()
})

beforeEach(async () => {
return agent.load('http', { server: false }, { codeOriginForSpans: { enabled: true } })
})

afterEach(() => {
return agent.close({ ritmReset: false })
})

it('should add code_origin tags for outbound requests', done => {
server((port) => {
const http = require('http')

agent
.use(traces => {
const span = traces[0][0]
expect(span.meta).to.have.property('_dd.code_origin.type', 'exit')

// Just validate that frame 0 tags are present. The detailed validation is performed in a different test.
expect(span.meta).to.have.property('_dd.code_origin.frames.0.file')
expect(span.meta).to.have.property('_dd.code_origin.frames.0.line')
expect(span.meta).to.have.property('_dd.code_origin.frames.0.column')
expect(span.meta).to.have.property('_dd.code_origin.frames.0.method')
expect(span.meta).to.have.property('_dd.code_origin.frames.0.type')
})
.then(done)
.catch(done)

const req = http.request(`http://localhost:${port}/`, res => {
res.resume()
})

req.end()
})
})
})
})
})

function server (callback) {
const http = require('http')

const server = http.createServer((req, res) => {
res.end()
})

server.listen(() => {
callback(server.address().port)
})
}
9 changes: 9 additions & 0 deletions packages/dd-trace/src/plugins/outbound.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const {
PEER_SERVICE_REMAP_KEY
} = require('../constants')
const TracingPlugin = require('./tracing')
const { exitTags } = require('../../../datadog-code-origin')

const COMMON_PEER_SVC_SOURCE_TAGS = [
'net.peer.name',
Expand All @@ -25,6 +26,14 @@ class OutboundPlugin extends TracingPlugin {
})
}

startSpan (...args) {
const span = super.startSpan(...args)
if (this._tracerConfig.codeOriginForSpans.enabled) {
span.addTags(exitTags(this.startSpan))
}
return span
}

getPeerService (tags) {
/**
* Compute `peer.service` and associated metadata from available tags, based
Expand Down
5 changes: 5 additions & 0 deletions packages/dd-trace/test/plugins/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,16 @@ function unbreakThen (promise) {
}
}

function getNextLineNumber () {
return Number(new Error().stack.split('\n')[2].match(/:(\d+):/)[1]) + 1
}

module.exports = {
breakThen,
compare,
deepInclude,
expectSomeSpan,
getNextLineNumber,
resolveNaming,
unbreakThen,
withDefaults
Expand Down
48 changes: 48 additions & 0 deletions packages/dd-trace/test/plugins/outbound.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
require('../setup/tap')

const { expect } = require('chai')
const { getNextLineNumber } = require('./helpers')
const OutboundPlugin = require('../../src/plugins/outbound')
const parseTags = require('../../../datadog-core/src/utils/src/parse-tags')

describe('OuboundPlugin', () => {
describe('peer service decision', () => {
Expand Down Expand Up @@ -157,4 +159,50 @@ describe('OuboundPlugin', () => {
})
})
})

describe('code origin tags', () => {
let instance = null

beforeEach(() => {
const tracerStub = {
_tracer: {
startSpan: sinon.stub().returns({
addTags: sinon.spy()
})
}
}
instance = new OutboundPlugin(tracerStub)
})

it('should not add exit tags to span if codeOriginForSpans.enabled is false', () => {
sinon.stub(instance, '_tracerConfig').value({ codeOriginForSpans: { enabled: false } })
const span = instance.startSpan('test')
expect(span.addTags).to.not.have.been.called
})

it('should add exit tags to span if codeOriginForSpans.enabled is true', () => {
sinon.stub(instance, '_tracerConfig').value({ codeOriginForSpans: { enabled: true } })

const lineNumber = String(getNextLineNumber())
const span = instance.startSpan('test')

expect(span.addTags).to.have.been.calledOnce
const args = span.addTags.args[0]
expect(args).to.have.property('length', 1)
const tags = parseTags(args[0])

expect(tags).to.nested.include({ '_dd.code_origin.type': 'exit' })
expect(tags._dd.code_origin).to.have.property('frames').to.be.an('array').with.length.above(0)

for (const frame of tags._dd.code_origin.frames) {
expect(frame).to.have.property('file', __filename)
expect(frame).to.have.property('line').to.match(/^\d+$/)
expect(frame).to.have.property('column').to.match(/^\d+$/)
expect(frame).to.have.property('type').to.a('string')
}

const topFrame = tags._dd.code_origin.frames[0]
expect(topFrame).to.have.property('line', lineNumber)
})
})
})
5 changes: 1 addition & 4 deletions packages/dd-trace/test/plugins/util/stacktrace.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict'

const { isAbsolute } = require('path')
const { getNextLineNumber } = require('../helpers')

require('../../setup/tap')

Expand Down Expand Up @@ -62,7 +63,3 @@ describe('stacktrace utils', () => {
})
})
})

function getNextLineNumber () {
return Number(new Error().stack.split('\n')[2].match(/:(\d+):/)[1]) + 1
}

0 comments on commit 91c4371

Please sign in to comment.