use route.path or url to generate the key

integration-tests/standalone-asm.spec.js

@@ -103,7 +103,7 @@ describe('Standalone ASM', () => {
         assert.notProperty(meta, 'manual.keep')
         assert.notProperty(meta, '_dd.p.appsec')
 
-        assert.propertyVal(metrics, '_sampling_priority_v1', 1)
+        assert.propertyVal(metrics, '_sampling_priority_v1', 0)
         assert.propertyVal(metrics, '_dd.apm.enabled', 0)
 
         assertDrop(payload[2][0])

packages/datadog-instrumentations/src/express.js

@@ -29,7 +29,7 @@ function wrapResponseJson (json) {
         obj = arguments[1]
       }
 
-      responseJsonChannel.publish({ req: this.req, body: obj })
+      responseJsonChannel.publish({ req: this.req, res: this, body: obj })
     }
 
     return json.apply(this, arguments)

packages/dd-trace/src/appsec/api_security_sampler.js

@@ -1,72 +1,82 @@
 'use strict'
 
-const crypto = require('node:crypto')
 const LRUCache = require('lru-cache')
-const PrioritySampler = require('../priority_sampler')
 const web = require('../plugins/util/web')
 const log = require('../log')
+const { AUTO_REJECT, USER_REJECT } = require('../../../../ext/priority')
 
 const MAX_SIZE = 4096
 const DEFAULT_DELAY = 30 // 30s
 
 let enabled
 let sampledRequests
-let prioritySampler
 
 function configure ({ apiSecurity }) {
   enabled = apiSecurity.enabled
-  const ttl = parseSampleDelay(apiSecurity.sampleDelay) * 1000
-  sampledRequests = new LRUCache({ max: MAX_SIZE, ttl })
-  prioritySampler = new PrioritySampler()
+  const delay = apiSecurity.sampleDelay || DEFAULT_DELAY
+  sampledRequests = new LRUCache({ max: MAX_SIZE, ttl: delay * 1000 })
 }
 
 function disable () {
   enabled = false
   sampledRequests?.clear()
 }
 
-function sampleRequest (req, res) {
+function sampleRequest (req, res, force = false) {
   if (!enabled) return false
+  if (this.isSampled(req, res)) return false
 
   const rootSpan = web.root(req)
   if (!rootSpan) return false
 
-  const isSampled = prioritySampler.isSampled(rootSpan)
+  const priority = getSpanPriority(rootSpan)
 
-  if (!isSampled) {
+  if (priority === AUTO_REJECT || priority === USER_REJECT) {
     return false
   }
 
-  const key = computeKey(req, res)
-  const alreadySampled = sampledRequests.has(key)
+  if (!priority && !rootSpan._prioritySampler?.isSampled(rootSpan)) {
+    return false
+  }
 
-  if (alreadySampled) return false
+  if (force) {
+    return sample(req, res)
+  }
 
-  sampledRequests.set(key)
+  return true
+}
 
+function sample (req, res) {
+  const key = computeKey(req, res)
+  if (!key) return false
+  if (sampledRequests.has(key)) return false
+
+  sampledRequests.set(key)
   return true
 }
 
 function isSampled (req, res) {
   const key = computeKey(req, res)
-  return !!sampledRequests.has(key)
+  if (!key) return false
+
+  return sampledRequests.has(key)
 }
 
 function computeKey (req, res) {
-  const route = req.route.path
-  const method = req.method.toLowerCase()
-  const statusCode = res.statusCode
-  const str = route + statusCode + method
-  return crypto.createHash('md5').update(str).digest('hex')
-}
+  const route = web.getContext(req).paths.join('') || ''
+  const method = req.method
+  const status = res.statusCode
 
-function parseSampleDelay (delay) {
-  if (typeof delay === 'number' && Number.isFinite(delay) && delay > 0) {
-    return delay
-  } else {
-    log.warn('Invalid delay value. Delay must be a positive number.')
-    return DEFAULT_DELAY
+  if (!method || !status) {
+    log.warn('Unsupported groupkey for API security')
+    return null
   }
+  return method + route + status
+}
+
+function getSpanPriority (span) {
+  const spanContext = span.context?.()
+  return spanContext._sampling?.priority
 }
 
 module.exports = {

packages/dd-trace/src/appsec/index.js

@@ -132,7 +132,7 @@ function incomingHttpEndTranslator ({ req, res }) {
     persistent[addresses.HTTP_INCOMING_QUERY] = req.query
   }
 
-  if (req.route && typeof req.route.path === 'string' && apiSecuritySampler.sampleRequest(req, res)) {
+  if (apiSecuritySampler.sampleRequest(req, res, true)) {
     persistent[addresses.WAF_CONTEXT_PROCESSOR] = { 'extract-schema': true }
   }
 
@@ -200,8 +200,9 @@ function onRequestCookieParser ({ req, res, abortController, cookies }) {
   handleResults(results, req, res, rootSpan, abortController)
 }
 
-function onResponseBody ({ req, body }) {
+function onResponseBody ({ req, res, body }) {
   if (!body || typeof body !== 'object') return
+  if (!apiSecuritySampler.sampleRequest(req, res)) return
 
   // we don't support blocking at this point, so no results needed
   waf.run({

packages/dd-trace/test/appsec/api_security_sampler.spec.js

@@ -2,34 +2,38 @@
 
 const { performance } = require('node:perf_hooks')
 const proxyquire = require('proxyquire')
+const { USER_KEEP, AUTO_KEEP, AUTO_REJECT, USER_REJECT } = require('../../../../ext/priority')
 
 describe('API Security Sampler', () => {
   const req = { route: { path: '/test' }, method: 'GET' }
   const res = { statusCode: 200 }
-  let apiSecuritySampler, performanceNowStub, webStub, sampler, span
+  let apiSecuritySampler, performanceNowStub, webStub, span
 
   beforeEach(() => {
     performanceNowStub = sinon.stub(performance, 'now').returns(0)
 
-    webStub = { root: sinon.stub() }
-
-    sampler = sinon.stub().returns({
-      isSampled: sinon.stub()
-    })
+    webStub = {
+      root: sinon.stub(),
+      getContext: sinon.stub(),
+      _prioritySampler: {
+        isSampled: sinon.stub()
+      }
+    }
 
     apiSecuritySampler = proxyquire('../../src/appsec/api_security_sampler', {
-      '../plugins/util/web': webStub,
-      '../priority_sampler': sampler
+      '../plugins/util/web': webStub
     })
 
     apiSecuritySampler.configure({ apiSecurity: { enabled: true } })
 
     span = {
-      context: sinon.stub().returns({})
+      context: sinon.stub().returns({
+        _sampling: { priority: AUTO_KEEP }
+      })
     }
 
     webStub.root.returns(span)
-    sampler().isSampled.returns(true)
+    webStub.getContext.returns({ paths: ['path'] })
 
     performanceNowStub.returns(performance.now() + 1)
   })
@@ -49,64 +53,67 @@ describe('API Security Sampler', () => {
     expect(apiSecuritySampler.sampleRequest({}, {})).to.be.false
   })
 
-  it('should return true and put request in cache if priority is AUTO_KEEP or USER_KEEP', () => {
+  it('should return false for AUTO_REJECT priority', () => {
+    span.context.returns({ _sampling: { priority: AUTO_REJECT } })
+    expect(apiSecuritySampler.sampleRequest(req, res)).to.be.false
+  })
+
+  it('should return false for USER_REJECT priority', () => {
+    span.context.returns({ _sampling: { priority: USER_REJECT } })
+    expect(apiSecuritySampler.sampleRequest(req, res)).to.be.false
+  })
+
+  it('should sample for AUTO_KEEP priority without checking prioritySampler', () => {
+    span.context.returns({ _sampling: { priority: AUTO_KEEP } })
     expect(apiSecuritySampler.sampleRequest(req, res)).to.be.true
   })
 
-  it('should not sample before 30 seconds', () => {
+  it('should sample for USER_KEEP priority without checking prioritySampler', () => {
+    span.context.returns({ _sampling: { priority: USER_KEEP } })
     expect(apiSecuritySampler.sampleRequest(req, res)).to.be.true
+  })
+
+  it('should not sample before 30 seconds', () => {
+    expect(apiSecuritySampler.sampleRequest(req, res, true)).to.be.true
     performanceNowStub.returns(performance.now() + 25000)
 
-    expect(apiSecuritySampler.sampleRequest(req, res)).to.be.false
+    expect(apiSecuritySampler.sampleRequest(req, res, true)).to.be.false
     expect(apiSecuritySampler.isSampled(req, res)).to.be.true
   })
 
   it('should sample after 30 seconds', () => {
-    expect(apiSecuritySampler.sampleRequest(req, res)).to.be.true
+    expect(apiSecuritySampler.sampleRequest(req, res, true)).to.be.true
 
     performanceNowStub.returns(performance.now() + 35000)
 
-    expect(apiSecuritySampler.sampleRequest(req, res)).to.be.true
-  })
-
-  it('should return false if priority is neither AUTO_KEEP nor USER_KEEP', () => {
-    sampler().isSampled.returns(false)
-    expect(apiSecuritySampler.sampleRequest({}, {})).to.be.false
+    expect(apiSecuritySampler.sampleRequest(req, res, true)).to.be.true
   })
 
   it('should remove oldest entry when max size is exceeded', () => {
-    const method = req.method
     for (let i = 0; i < 4097; i++) {
-      expect(apiSecuritySampler.sampleRequest({ method, route: { path: `/test${i}` } }, res)).to.be.true
+      const path = `/test${i}`
+      webStub.getContext.returns({ paths: [path] })
+      expect(apiSecuritySampler.sampleRequest(req, res, true)).to.be.true
     }
-    expect(apiSecuritySampler.isSampled({ method, route: { path: '/test0' } }, res)).to.be.false
-    expect(apiSecuritySampler.isSampled({ method, route: { path: '/test4096' } }, res)).to.be.true
+    webStub.getContext.returns({ paths: ['/test0'] })
+    expect(apiSecuritySampler.isSampled(req, res)).to.be.false
+    webStub.getContext.returns({ paths: ['/test4096'] })
+    expect(apiSecuritySampler.isSampled(req, res)).to.be.true
   })
 
   it('should set enabled to false and clear the cache', () => {
-    expect(apiSecuritySampler.sampleRequest(req, res)).to.be.true
+    expect(apiSecuritySampler.sampleRequest(req, res, true)).to.be.true
 
     apiSecuritySampler.disable()
 
-    expect(apiSecuritySampler.sampleRequest(req, res)).to.be.false
-  })
-
-  it('should create different keys for different URLs', () => {
-    const req1 = { route: { path: '/test1' }, method: 'GET' }
-    const req2 = { route: { path: '/test2' }, method: 'GET' }
-
-    expect(apiSecuritySampler.sampleRequest(req1, res)).to.be.true
-    expect(apiSecuritySampler.sampleRequest(req2, res)).to.be.true
-    expect(apiSecuritySampler.isSampled(req1, res)).to.be.true
-    expect(apiSecuritySampler.isSampled(req2, res)).to.be.true
+    expect(apiSecuritySampler.sampleRequest(req, res, true)).to.be.false
   })
 
   it('should create different keys for different methods', () => {
-    const getReq = { route: { path: '/test1' }, method: 'GET' }
-    const postReq = { route: { path: '/test1' }, method: 'POST' }
-
-    expect(apiSecuritySampler.sampleRequest(getReq, res)).to.be.true
-    expect(apiSecuritySampler.sampleRequest(postReq, res)).to.be.true
+    const getReq = { method: 'GET' }
+    const postReq = { method: 'POST' }
+    expect(apiSecuritySampler.sampleRequest(getReq, res, true)).to.be.true
+    expect(apiSecuritySampler.sampleRequest(postReq, res, true)).to.be.true
     expect(apiSecuritySampler.isSampled(getReq, res)).to.be.true
     expect(apiSecuritySampler.isSampled(postReq, res)).to.be.true
   })
@@ -115,9 +122,14 @@ describe('API Security Sampler', () => {
     const res200 = { statusCode: 200 }
     const res404 = { statusCode: 404 }
 
-    expect(apiSecuritySampler.sampleRequest(req, res200)).to.be.true
-    expect(apiSecuritySampler.sampleRequest(req, res404)).to.be.true
+    expect(apiSecuritySampler.sampleRequest(req, res200, true)).to.be.true
+    expect(apiSecuritySampler.sampleRequest(req, res404, true)).to.be.true
     expect(apiSecuritySampler.isSampled(req, res200)).to.be.true
     expect(apiSecuritySampler.isSampled(req, res404)).to.be.true
   })
+
+  it('should not sample when method or statusCode is not available', () => {
+    expect(apiSecuritySampler.sampleRequest(req, {}, true)).to.be.false
+    expect(apiSecuritySampler.sampleRequest({}, res, true)).to.be.false
+  })
 })

packages/dd-trace/test/appsec/index.express.plugin.spec.js

@@ -41,7 +41,7 @@ withVersions('express', 'express', version => {
       })
 
       app.post('/json', (req, res) => {
-        res.jsonp({ jsonResKey: 'jsonResValue' })
+        res.json({ jsonResKey: 'jsonResValue' })
       })
 
       server = app.listen(port, () => {