Skip to content
Draft
68 changes: 42 additions & 26 deletions integration-tests/appsec/endpoints-collection.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('Endpoints collection', () => {
this.timeout(process.platform === 'win32' ? 90000 : 30000)

sandbox = await createSandbox(
['fastify'],
['express', 'fastify'],
false
)

Expand All @@ -31,34 +31,16 @@ describe('Endpoints collection', () => {
{ method: 'PUT', path: '/users/:id' },
{ method: 'DELETE', path: '/users/:id' },
{ method: 'PATCH', path: '/users/:id/:name' },
{ method: 'OPTIONS', path: '/users/:id?' },

// Route with regex
{ method: 'DELETE', path: '/regex/:hour(^\\d{2})h:minute(^\\d{2})m' },

// Additional methods
{ method: 'TRACE', path: '/trace-test' },
{ method: 'HEAD', path: '/head-test' },

// Custom method
{ method: 'MKCOL', path: '/example/near/:lat-:lng/radius/:r' },

// Using app.route()
{ method: 'POST', path: '/multi-method' },
{ method: 'PUT', path: '/multi-method' },
{ method: 'PATCH', path: '/multi-method' },

// All supported methods route
{ method: 'GET', path: '/all-methods' },
{ method: 'HEAD', path: '/all-methods' },
{ method: 'TRACE', path: '/all-methods' },
{ method: 'DELETE', path: '/all-methods' },
{ method: 'OPTIONS', path: '/all-methods' },
{ method: 'PATCH', path: '/all-methods' },
{ method: 'PUT', path: '/all-methods' },
{ method: 'POST', path: '/all-methods' },
{ method: 'MKCOL', path: '/all-methods' }, // Added with addHttpMethod

// Nested routes with Router
{ method: 'PUT', path: '/v1/nested/:id' },

Expand All @@ -69,16 +51,43 @@ describe('Endpoints collection', () => {
{ method: 'HEAD', path: '/api/sub/deep' },
{ method: 'POST', path: '/api/sub/deep/:id' },

// Wildcard routes
{ method: 'GET', path: '/wildcard/*' },
{ method: 'HEAD', path: '/wildcard/*' },
{ method: 'GET', path: '*' },
{ method: 'HEAD', path: '*' },

{ method: 'GET', path: '/later' },
{ method: 'HEAD', path: '/later' },
]

if (framework === 'fastify') {
expectedEndpoints.push({ method: 'OPTIONS', path: '/users/:id?' })

// Route with regex - not supported in express5
expectedEndpoints.push({ method: 'DELETE', path: '/regex/:hour(^\\d{2})h:minute(^\\d{2})m' })
expectedEndpoints.push({ method: 'OPTIONS', path: '/users/:id?' })// Added with addHttpMethod
expectedEndpoints.push({ method: 'MKCOL', path: '/example/near/:lat-:lng/radius/:r' })// Added with addHttpMethod

// All supported methods route
expectedEndpoints.push({ method: 'GET', path: '/all-methods' })
expectedEndpoints.push({ method: 'HEAD', path: '/all-methods' })
expectedEndpoints.push({ method: 'TRACE', path: '/all-methods' })
expectedEndpoints.push({ method: 'DELETE', path: '/all-methods' })
expectedEndpoints.push({ method: 'OPTIONS', path: '/all-methods' })
expectedEndpoints.push({ method: 'PATCH', path: '/all-methods' })
expectedEndpoints.push({ method: 'PUT', path: '/all-methods' })
expectedEndpoints.push({ method: 'POST', path: '/all-methods' })
expectedEndpoints.push({ method: 'GET', path: '/wildcard/*' })
expectedEndpoints.push({ method: 'HEAD', path: '/wildcard/*' })
// Wildcard routes
expectedEndpoints.push({ method: 'GET', path: '*' })
expectedEndpoints.push({ method: 'HEAD', path: '*' })
}

if (framework === 'express') {
expectedEndpoints.push({ method: 'CONNECT', path: '/connect-test' })
expectedEndpoints.push({ method: '*', path: '/multi-method' })
expectedEndpoints.push({ method: '*', path: '/all-methods' })
expectedEndpoints.push({ method: '*', path: '/wildcard/*name' })
expectedEndpoints.push({ method: '*', path: '/^\\/login\\/.*$/i' })
expectedEndpoints.push({ method: 'OPTIONS', path: '/users/:id' })
}

return expectedEndpoints
}

Expand All @@ -93,6 +102,8 @@ describe('Endpoints collection', () => {
const endpointsFound = []
const isFirstFlags = []

const expectedMessageCount = framework === 'express' ? 3 : 4

const telemetryPromise = agent.assertTelemetryReceived(({ payload }) => {
isFirstFlags.push(Boolean(payload.payload.is_first))

Expand All @@ -107,7 +118,7 @@ describe('Endpoints collection', () => {
})
})
}
}, 'app-endpoints', 5_000, 4)
}, 'app-endpoints', 5_000, expectedMessageCount)

proc = await spawnProc(appFile, {
cwd,
Expand All @@ -128,6 +139,7 @@ describe('Endpoints collection', () => {
const found = endpointsFound.find(e =>
e.method === expected.method && e.path === expected.path
)

expect(found).to.exist
expect(found.type).to.equal('REST')
expect(found.operation_name).to.equal('http.request')
Expand All @@ -142,6 +154,10 @@ describe('Endpoints collection', () => {
}
}

it('should send express endpoints via telemetry', async () => {
await runEndpointTest('express')
})

it('should send fastify endpoints via telemetry', async () => {
await runEndpointTest('fastify')
})
Expand Down
63 changes: 63 additions & 0 deletions integration-tests/appsec/endpoints-collection/express.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
'use strict'

const tracer = require('dd-trace')
tracer.init({
flushInterval: 0
})

const express = require('express')
const app = express()

// Basic routes
app.get('/users', (_, res) => res.send('ok'))
app.post('/users/', (_, res) => res.send('ok'))
app.put('/users/:id', (_, res) => res.send('ok'))
app.delete('/users/:id', (_, res) => res.send('ok'))
app.patch('/users/:id/:name', (_, res) => res.send('ok'))
app.options('/users/:id', (_, res) => res.send('ok'))

// Additional methods
app.trace('/trace-test', (_, res) => res.send('ok'))
app.head('/head-test', (_, res) => res.send('ok'))
app.connect('/connect-test', (_, res) => res.send('ok'))

// Using app.route()
app.route('/multi-method')
.post((_, res) => res.send('ok'))
.put((_, res) => res.send('ok'))
.patch((_, res) => res.send('ok'))
.all((_, res) => res.send('ok'))

// All supported methods route
app.all('/all-methods', async (_, res) => res.send('ok'))

// Wildcard routes via array
app.all(['/wildcard/*name'], (_, res) => res.send('ok'))

// RegExp wildcard route
app.all(/^\/login\/.*$/i, (req, res) => {
res.send('ok')
})

// Nested routes with Router
const apiRouter = express.Router()
apiRouter.put('/nested/:id', (_, res) => res.send('ok'))
app.use('/v1', apiRouter)

// Add endpoint during runtime
setTimeout(() => {
// Deeply nested routes
const deepRouter = express.Router()
deepRouter.get('/nested', (_, res) => res.send('ok'))
const subRouter = express.Router()
subRouter.get('/deep', (_, res) => res.send('ok'))
subRouter.post('/deep/:id', (_, res) => res.send('ok'))
deepRouter.use('/sub', subRouter)
app.use('/api', deepRouter)
app.get('/later', (_, res) => res.send('ok'))
}, 1_000)

const server = app.listen(0, '127.0.0.1', () => {
const port = server.address().port
process.send({ port })
})
120 changes: 120 additions & 0 deletions packages/datadog-instrumentations/src/express.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ const shimmer = require('../../datadog-shimmer')
const { addHook, channel } = require('./helpers/instrument')
const tracingChannel = require('dc-polyfill').tracingChannel

const METHODS = require('http').METHODS.map(v => v.toLowerCase())

const handleChannel = channel('apm:express:request:handle')
const routeAddedChannel = channel('apm:express:add:route')

function wrapHandle (handle) {
return function handleWithTrace (req, res) {
Expand Down Expand Up @@ -57,8 +60,125 @@ function wrapResponseRender (render) {
}
}

function wrapAppAll (all) {
return function wrappedAll (path) {
if (routeAddedChannel.hasSubscribers) {
const paths = Array.isArray(path) ? path : [path]

for (const p of paths) {
routeAddedChannel.publish({
method: '*',
path: p instanceof RegExp ? p.toString() : p
})
}
}

return all.apply(this, arguments)
}
}

// Wrap app.route() to instrument Route object
function wrapAppRoute (route) {
return function wrappedRoute (path) {
const routeObj = route.apply(this, arguments)

if (routeAddedChannel.hasSubscribers && typeof path === 'string') {
// Wrap the .all() first
if (typeof routeObj.all === 'function') {
shimmer.wrap(routeObj, 'all', (original) => function wrappedRouteAll () {
routeAddedChannel.publish({
method: '*',
path
})

return original.apply(this, arguments)
})
}

// Wrap each HTTP method
METHODS.forEach(method => {
Copy link
Member

Choose a reason for hiding this comment

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

Is using METHODS needed here? Aren't all methods on the route HTTP methods?

if (typeof routeObj[method] === 'function') {
shimmer.wrap(routeObj, method, (original) => function wrapMethod () {
routeAddedChannel.publish({
method,
path
})

return original.apply(this, arguments)
})
}
})
}

return routeObj
}
}

function joinPath (base, path) {
if (!base || base === '/') return path || '/'
if (!path || path === '/') return base
return base + path
}

function wrapAppUse (use) {
return function wrappedUse () {
if (arguments.length < 2) {
return use.apply(this, arguments)
}

// Check if second argument has a stack (likely a router)
const mountPath = arguments[0]
const router = arguments[1]

if (typeof mountPath === 'string' && router?.stack?.length && routeAddedChannel.hasSubscribers) {
collectRoutesFromRouter(router, mountPath)
}

return use.apply(this, arguments)
}
}

function collectRoutesFromRouter (router, prefix) {
Copy link
Member

Choose a reason for hiding this comment

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

If this operates on the router, doesn't it also apply to the router library? Should it be moved over there like most of the Express instrumentation already is?

if (!router?.stack?.length) return

router.stack.forEach(layer => {
if (layer.route) {
// This layer has a direct route
const route = layer.route
const fullPath = joinPath(prefix, route.path)

Object.keys(route.methods).forEach(method => {
Copy link
Member

Choose a reason for hiding this comment

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

I think this could be done in just one step with a condition only on the method which would result in simpler code. Same for the similar code above.

if (route.methods[method] && method !== '_all') {
routeAddedChannel.publish({
method,
path: fullPath
})
}
})

if (route.methods._all) {
routeAddedChannel.publish({
method: '*',
path: fullPath
})
}
} else if (layer.handle?.stack?.length) {
// This layer contains a nested router
// Prefer matchers (from router.js) when available to resolve the exact mount path
const matchers = layer.ddMatchers
const mountPath = (Array.isArray(matchers) && matchers.length) ? matchers[0].path : ''

const nestedPrefix = joinPath(prefix, mountPath)
collectRoutesFromRouter(layer.handle, nestedPrefix)
}
})
}

addHook({ name: 'express', versions: ['>=4'] }, express => {
shimmer.wrap(express.application, 'handle', wrapHandle)
shimmer.wrap(express.application, 'all', wrapAppAll)
shimmer.wrap(express.application, 'route', wrapAppRoute)
shimmer.wrap(express.application, 'use', wrapAppUse)

shimmer.wrap(express.response, 'json', wrapResponseJson)
shimmer.wrap(express.response, 'jsonp', wrapResponseJson)
Expand Down
3 changes: 3 additions & 0 deletions packages/datadog-instrumentations/src/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ function createWrapRouterMethod (name) {

layerMatchers.set(layer, matchers)

// Save matchers to consume it later on express.js for endpoint discovery
layer.ddMatchers = matchers

if (layer.route) {
METHODS.forEach(method => {
if (typeof layer.route.stack === 'function') {
Expand Down
5 changes: 5 additions & 0 deletions packages/dd-trace/src/plugins/util/stacktrace.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ function parseLine (stack, start, end) {
[fileName, lineNumber, columnNumber, index] = result

if (isNodeModulesFrame(fileName)) return
if (isDDInstrumentationFile(fileName)) return

// parse method name
let methodName, functionName
Expand Down Expand Up @@ -153,6 +154,10 @@ function isNodeModulesFrame (fileName) {
return relativePath.startsWith(NODE_MODULES_PATTERN_START) || relativePath.includes(NODE_MODULES_PATTERN_MIDDLE)
}

function isDDInstrumentationFile (fileName) {
return fileName.includes(`packages${sep}datadog-instrumentations${sep}src`)
}

/**
* A stack trace location can be in one of the following formats:
*
Expand Down
Loading
Loading