From 91c43717be2e7136d80dd1fcc60bb0c4b1e3cf97 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Wed, 30 Oct 2024 08:04:45 +0100 Subject: [PATCH] Add support for exit spans in Code Origin for Spans (#4772) --- packages/datadog-code-origin/index.js | 8 +-- .../datadog-core/src/utils/src/parse-tags.js | 33 ++++++++++ .../test/utils/src/parse-tags.spec.js | 23 +++++++ .../datadog-plugin-fastify/src/code_origin.js | 4 +- .../test/code_origin.spec.js | 15 ++--- .../test/code_origin.spec.js | 63 +++++++++++++++++++ packages/dd-trace/src/plugins/outbound.js | 9 +++ packages/dd-trace/test/plugins/helpers.js | 5 ++ .../dd-trace/test/plugins/outbound.spec.js | 48 ++++++++++++++ .../test/plugins/util/stacktrace.spec.js | 5 +- 10 files changed, 194 insertions(+), 19 deletions(-) create mode 100644 packages/datadog-core/src/utils/src/parse-tags.js create mode 100644 packages/datadog-core/test/utils/src/parse-tags.spec.js create mode 100644 packages/datadog-plugin-http/test/code_origin.spec.js diff --git a/packages/datadog-code-origin/index.js b/packages/datadog-code-origin/index.js index 530dd3cc8ae..278aac265ab 100644 --- a/packages/datadog-code-origin/index.js +++ b/packages/datadog-code-origin/index.js @@ -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) } diff --git a/packages/datadog-core/src/utils/src/parse-tags.js b/packages/datadog-core/src/utils/src/parse-tags.js new file mode 100644 index 00000000000..4142e770e4e --- /dev/null +++ b/packages/datadog-core/src/utils/src/parse-tags.js @@ -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 +} diff --git a/packages/datadog-core/test/utils/src/parse-tags.spec.js b/packages/datadog-core/test/utils/src/parse-tags.spec.js new file mode 100644 index 00000000000..ded1bb5974f --- /dev/null +++ b/packages/datadog-core/test/utils/src/parse-tags.spec.js @@ -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({}) + }) +}) diff --git a/packages/datadog-plugin-fastify/src/code_origin.js b/packages/datadog-plugin-fastify/src/code_origin.js index 3e6f58d5624..6c9ddc7b028 100644 --- a/packages/datadog-plugin-fastify/src/code_origin.js +++ b/packages/datadog-plugin-fastify/src/code_origin.js @@ -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') @@ -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) }) } } diff --git a/packages/datadog-plugin-fastify/test/code_origin.spec.js b/packages/datadog-plugin-fastify/test/code_origin.spec.js index 711c2ffff6c..18f591dc6b9 100644 --- a/packages/datadog-plugin-fastify/test/code_origin.spec.js +++ b/packages/datadog-plugin-fastify/test/code_origin.spec.js @@ -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' @@ -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(() => { @@ -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() }) @@ -134,7 +135,7 @@ describe('Plugin', () => { next() }) - const routeRegisterLine = getNextLineNumber() + const routeRegisterLine = String(getNextLineNumber()) app.get('/user', function userHandler (request, reply) { reply.send() }) @@ -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() }) @@ -210,7 +211,3 @@ describe('Plugin', () => { }) }) }) - -function getNextLineNumber () { - return String(Number(new Error().stack.split('\n')[2].match(/:(\d+):/)[1]) + 1) -} diff --git a/packages/datadog-plugin-http/test/code_origin.spec.js b/packages/datadog-plugin-http/test/code_origin.spec.js new file mode 100644 index 00000000000..4bb1a9003e0 --- /dev/null +++ b/packages/datadog-plugin-http/test/code_origin.spec.js @@ -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) + }) +} diff --git a/packages/dd-trace/src/plugins/outbound.js b/packages/dd-trace/src/plugins/outbound.js index f0a9509269e..44dbfa35aaa 100644 --- a/packages/dd-trace/src/plugins/outbound.js +++ b/packages/dd-trace/src/plugins/outbound.js @@ -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', @@ -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 diff --git a/packages/dd-trace/test/plugins/helpers.js b/packages/dd-trace/test/plugins/helpers.js index b35793b6664..add1361e167 100644 --- a/packages/dd-trace/test/plugins/helpers.js +++ b/packages/dd-trace/test/plugins/helpers.js @@ -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 diff --git a/packages/dd-trace/test/plugins/outbound.spec.js b/packages/dd-trace/test/plugins/outbound.spec.js index 5709c789575..2d801cd1f4c 100644 --- a/packages/dd-trace/test/plugins/outbound.spec.js +++ b/packages/dd-trace/test/plugins/outbound.spec.js @@ -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', () => { @@ -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) + }) + }) }) diff --git a/packages/dd-trace/test/plugins/util/stacktrace.spec.js b/packages/dd-trace/test/plugins/util/stacktrace.spec.js index 3fefc2b29ef..a96ed87f965 100644 --- a/packages/dd-trace/test/plugins/util/stacktrace.spec.js +++ b/packages/dd-trace/test/plugins/util/stacktrace.spec.js @@ -1,6 +1,7 @@ 'use strict' const { isAbsolute } = require('path') +const { getNextLineNumber } = require('../helpers') require('../../setup/tap') @@ -62,7 +63,3 @@ describe('stacktrace utils', () => { }) }) }) - -function getNextLineNumber () { - return Number(new Error().stack.split('\n')[2].match(/:(\d+):/)[1]) + 1 -}