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

Add support for exit spans in Code Origin for Spans #4772

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
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)
}
56 changes: 56 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,56 @@
'use strict'

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

describe('Plugin', () => {
describe('http', () => {
describe('Code Origin for Spans', () => {
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 { exitTag } = 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(exitTag(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
68 changes: 68 additions & 0 deletions packages/dd-trace/test/plugins/outbound.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require('../setup/tap')

const { expect } = require('chai')
const { getNextLineNumber } = require('./helpers')
const OutboundPlugin = require('../../src/plugins/outbound')

describe('OuboundPlugin', () => {
Expand Down Expand Up @@ -157,4 +158,71 @@ 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)
})
})
})

function parseTags (tags) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we already have a utility function that does this that I can re-use? Seem very much overkill to write this code for this test, but it does make the actual test much simpler to write once the tags are converted to a proper object structure 🤔

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(/^\d+$/) ? [] : {}
}
current = current[key]
depth++
}
}
return parsedTags
}
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
}
Loading