Skip to content

Commit 6bfd2dd

Browse files
watsonwatson
authored
[Code Origin for Spans] Add support for Express entry spans (#5423)
To enable, set `DD_CODE_ORIGIN_FOR_SPANS_ENABLED=true`. This is a follow up to PR #4449, which added support for Fastify entry spans.
1 parent d863bc4 commit 6bfd2dd

File tree

5 files changed

+266
-13
lines changed

5 files changed

+266
-13
lines changed

‎packages/datadog-instrumentations/src/router.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ function createWrapRouterMethod (name) {
1212
const finishChannel = channel(`apm:${name}:middleware:finish`)
1313
const errorChannel = channel(`apm:${name}:middleware:error`)
1414
const nextChannel = channel(`apm:${name}:middleware:next`)
15+
const routeAddedChannel = channel(`apm:${name}:route:added`)
1516

1617
const layerMatchers = new WeakMap()
1718
const regexpCache = Object.create(null)
@@ -45,7 +46,7 @@ function createWrapRouterMethod (name) {
4546
}
4647
}
4748

48-
enterChannel.publish({ name, req, route })
49+
enterChannel.publish({ name, req, route, layer })
4950

5051
try {
5152
return original.apply(this, arguments)
@@ -153,6 +154,10 @@ function createWrapRouterMethod (name) {
153154
this.stack = [{ handle: this.stack }]
154155
}
155156

157+
if (routeAddedChannel.hasSubscribers) {
158+
routeAddedChannel.publish({ topOfStackFunc: methodWithTrace, layer: this.stack[0] })
159+
}
160+
156161
wrapStack(this.stack, offset, extractMatchers(fn))
157162

158163
return router

‎packages/datadog-plugin-express/src/code_origin.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
'use strict'
2+
3+
const { entryTags } = require('../../datadog-code-origin')
4+
const Plugin = require('../../dd-trace/src/plugins/plugin')
5+
const web = require('../../dd-trace/src/plugins/util/web')
6+
7+
class ExpressCodeOriginForSpansPlugin extends Plugin {
8+
static get id () {
9+
return 'express'
10+
}
11+
12+
constructor (...args) {
13+
super(...args)
14+
15+
const layerTags = new WeakMap()
16+
17+
this.addSub('apm:express:middleware:enter', ({ req, layer }) => {
18+
const tags = layerTags.get(layer)
19+
if (!tags) return
20+
web.getContext(req).span?.addTags(tags)
21+
})
22+
23+
this.addSub('apm:express:route:added', ({ topOfStackFunc, layer }) => {
24+
if (layerTags.has(layer)) return
25+
layerTags.set(layer, entryTags(topOfStackFunc))
26+
})
27+
}
28+
}
29+
30+
module.exports = ExpressCodeOriginForSpansPlugin

‎packages/datadog-plugin-express/src/index.js

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,16 @@
11
'use strict'
22

3-
const RouterPlugin = require('../../datadog-plugin-router/src')
3+
const ExpressTracingPlugin = require('./tracing')
4+
const ExpressCodeOriginForSpansPlugin = require('./code_origin')
5+
const CompositePlugin = require('../../dd-trace/src/plugins/composite')
46

5-
class ExpressPlugin extends RouterPlugin {
6-
static get id () {
7-
return 'express'
8-
}
9-
10-
constructor (...args) {
11-
super(...args)
12-
13-
this.addSub('apm:express:request:handle', ({ req }) => {
14-
this.setFramework(req, 'express', this.config)
15-
})
7+
class ExpressPlugin extends CompositePlugin {
8+
static get id () { return 'express' }
9+
static get plugins () {
10+
return {
11+
tracing: ExpressTracingPlugin,
12+
codeOriginForSpans: ExpressCodeOriginForSpansPlugin
13+
}
1614
}
1715
}
1816

‎packages/datadog-plugin-express/src/tracing.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
'use strict'
2+
3+
const RouterPlugin = require('../../datadog-plugin-router/src')
4+
5+
class ExpressTracingPlugin extends RouterPlugin {
6+
static get id () {
7+
return 'express'
8+
}
9+
10+
constructor (...args) {
11+
super(...args)
12+
13+
this.addSub('apm:express:request:handle', ({ req }) => {
14+
this.setFramework(req, 'express', this.config)
15+
})
16+
}
17+
}
18+
19+
module.exports = ExpressTracingPlugin

‎packages/datadog-plugin-express/test/code_origin.spec.js

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
'use strict'
2+
3+
const axios = require('axios')
4+
const agent = require('../../dd-trace/test/plugins/agent')
5+
const { getNextLineNumber } = require('../../dd-trace/test/plugins/helpers')
6+
7+
const host = 'localhost'
8+
9+
describe('Plugin', () => {
10+
let express, app, listener
11+
12+
describe('express', () => {
13+
withVersions('express', 'express', (version) => {
14+
beforeEach(() => {
15+
express = require(`../../../versions/express@${version}`).get()
16+
app = express()
17+
})
18+
19+
afterEach(() => listener?.close())
20+
21+
describe('code origin for spans disabled', () => {
22+
const config = { codeOriginForSpans: { enabled: false } }
23+
24+
describe(`with tracer config ${JSON.stringify(config)}`, () => {
25+
before(() => agent.load(['express', 'http'], [{}, {}, { client: false }], config))
26+
27+
after(() => agent.close({ ritmReset: false, wipe: true }))
28+
29+
it('should not add code_origin tag on entry spans', (done) => {
30+
app.get('/user', (req, res) => {
31+
res.end()
32+
})
33+
34+
listener = app.listen(0, host, () => {
35+
Promise.all([
36+
agent.assertSomeTraces(traces => {
37+
const spans = traces[0]
38+
const tagNames = Object.keys(spans[0].meta)
39+
expect(tagNames).to.all.not.match(/code_origin/)
40+
}),
41+
axios.get(`http://localhost:${listener.address().port}/user`)
42+
]).then(() => done(), done)
43+
})
44+
})
45+
})
46+
})
47+
48+
describe('code origin for spans enabled', () => {
49+
const configs = [{}, { codeOriginForSpans: { enabled: true } }]
50+
51+
for (const config of configs) {
52+
describe(`with tracer config ${JSON.stringify(config)}`, () => {
53+
before(() => agent.load(['express', 'http'], [{}, {}, { client: false }], config))
54+
55+
after(() => agent.close({ ritmReset: false, wipe: true }))
56+
57+
it('should add code_origin tag on entry spans when feature is enabled', (done) => {
58+
let routeRegisterLine
59+
60+
// Wrap in a named function to have at least one frame with a function name
61+
function wrapperFunction () {
62+
app.get('/route_before', (req, res) => res.end())
63+
routeRegisterLine = String(getNextLineNumber())
64+
app.get('/user', function userHandler (req, res) {
65+
res.end()
66+
})
67+
app.get('/route_after', (req, res) => res.end())
68+
}
69+
70+
const callWrapperLine = String(getNextLineNumber())
71+
wrapperFunction()
72+
73+
listener = app.listen(0, host, () => {
74+
Promise.all([
75+
agent.assertSomeTraces(traces => {
76+
const spans = traces[0]
77+
// console.log(spans)
78+
const tags = spans[0].meta
79+
80+
expect(tags).to.have.property('_dd.code_origin.type', 'entry')
81+
82+
expect(tags).to.have.property('_dd.code_origin.frames.0.file', __filename)
83+
expect(tags).to.have.property('_dd.code_origin.frames.0.line', routeRegisterLine)
84+
expect(tags).to.have.property('_dd.code_origin.frames.0.column').to.match(/^\d+$/)
85+
expect(tags).to.have.property('_dd.code_origin.frames.0.method', 'wrapperFunction')
86+
expect(tags).to.not.have.property('_dd.code_origin.frames.0.type')
87+
88+
expect(tags).to.have.property('_dd.code_origin.frames.1.file', __filename)
89+
expect(tags).to.have.property('_dd.code_origin.frames.1.line', callWrapperLine)
90+
expect(tags).to.have.property('_dd.code_origin.frames.1.column').to.match(/^\d+$/)
91+
expect(tags).to.not.have.property('_dd.code_origin.frames.1.method')
92+
expect(tags).to.have.property('_dd.code_origin.frames.1.type', 'Context')
93+
94+
expect(tags).to.not.have.property('_dd.code_origin.frames.2.file')
95+
}),
96+
axios.get(`http://localhost:${listener.address().port}/user`)
97+
]).then(() => done(), done)
98+
})
99+
})
100+
101+
it('should point to where actual route handler is configured, not the router', function testCase (done) {
102+
const router = express.Router()
103+
104+
router.get('/route_before', (req, res) => res.end())
105+
const routeRegisterLine = String(getNextLineNumber())
106+
router.get('/user', function userHandler (req, res) {
107+
res.end()
108+
})
109+
router.get('/route_after', (req, res) => res.end())
110+
app.get('/route_before', (req, res) => res.end())
111+
app.use('/v1', router)
112+
app.get('/route_after', (req, res) => res.end())
113+
114+
listener = app.listen(0, host, () => {
115+
Promise.all([
116+
agent.assertSomeTraces(traces => {
117+
const spans = traces[0]
118+
const tags = spans[0].meta
119+
120+
expect(tags).to.have.property('_dd.code_origin.type', 'entry')
121+
122+
expect(tags).to.have.property('_dd.code_origin.frames.0.file', __filename)
123+
expect(tags).to.have.property('_dd.code_origin.frames.0.line', routeRegisterLine)
124+
expect(tags).to.have.property('_dd.code_origin.frames.0.column').to.match(/^\d+$/)
125+
expect(tags).to.have.property('_dd.code_origin.frames.0.type', 'Context')
126+
expect(tags).to.have.property('_dd.code_origin.frames.0.method', 'testCase')
127+
128+
expect(tags).to.not.have.property('_dd.code_origin.frames.1.file')
129+
}),
130+
axios.get(`http://localhost:${listener.address().port}/v1/user`)
131+
]).then(() => done(), done)
132+
})
133+
})
134+
135+
it('should point to route handler even if passed through a middleware', function testCase (done) {
136+
app.use(function middleware (req, res, next) {
137+
next()
138+
})
139+
140+
const routeRegisterLine = String(getNextLineNumber())
141+
app.get('/user', function userHandler (req, res) {
142+
res.end()
143+
})
144+
145+
listener = app.listen(0, host, () => {
146+
Promise.all([
147+
agent.assertSomeTraces(traces => {
148+
const spans = traces[0]
149+
const tags = spans[0].meta
150+
151+
expect(tags).to.have.property('_dd.code_origin.type', 'entry')
152+
153+
expect(tags).to.have.property('_dd.code_origin.frames.0.file', __filename)
154+
expect(tags).to.have.property('_dd.code_origin.frames.0.line', routeRegisterLine)
155+
expect(tags).to.have.property('_dd.code_origin.frames.0.column').to.match(/^\d+$/)
156+
expect(tags).to.have.property('_dd.code_origin.frames.0.method', 'testCase')
157+
expect(tags).to.have.property('_dd.code_origin.frames.0.type', 'Context')
158+
159+
expect(tags).to.not.have.property('_dd.code_origin.frames.1.file')
160+
}),
161+
axios.get(`http://localhost:${listener.address().port}/user`)
162+
]).then(() => done(), done)
163+
})
164+
})
165+
166+
it('should point to middleware if middleware responds early', function testCase (done) {
167+
const middlewareRegisterLine = String(getNextLineNumber())
168+
app.use(function middleware (req, res, next) {
169+
res.end()
170+
})
171+
172+
app.get('/user', function userHandler (req, res) {
173+
res.end()
174+
})
175+
176+
listener = app.listen(0, host, () => {
177+
Promise.all([
178+
agent.assertSomeTraces(traces => {
179+
const spans = traces[0]
180+
const tags = spans[0].meta
181+
182+
expect(tags).to.have.property('_dd.code_origin.type', 'entry')
183+
184+
expect(tags).to.have.property('_dd.code_origin.frames.0.file', __filename)
185+
expect(tags).to.have.property('_dd.code_origin.frames.0.line', middlewareRegisterLine)
186+
expect(tags).to.have.property('_dd.code_origin.frames.0.column').to.match(/^\d+$/)
187+
expect(tags).to.have.property('_dd.code_origin.frames.0.method', 'testCase')
188+
expect(tags).to.have.property('_dd.code_origin.frames.0.type', 'Context')
189+
190+
expect(tags).to.not.have.property('_dd.code_origin.frames.1.file')
191+
}),
192+
axios.get(`http://localhost:${listener.address().port}/user`)
193+
]).then(() => done(), done)
194+
})
195+
})
196+
})
197+
}
198+
})
199+
})
200+
})
201+
})

0 commit comments

Comments
 (0)