From 3f1e21ab64e007c7ef1d9bb7d9e332ef979718db Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Fri, 5 Jul 2024 10:47:24 +0200 Subject: [PATCH] Standalone ASM priority sampler and tag propagation (#4416) * DD_APM_TRACING_ENABLED and span _dd.apm.enabled tag * clean up * Use MANUAL_KEEP const * Add _dd.p.appsec tag on standalone ASM events * Include apmTracingEnabled checks * Appsec Reporter tests * Appsec sdk track_event test * Use numeric value for _dd.p.appsec * Include appsec standalone config in .ts files * Clean up null and undefined values * Remove not needed config properties * standalone module * Clean up * standalone proxy test * Update packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js Co-authored-by: Ugaitz Urien * appsec reporter test * Use standalone singletone in vulnerability-reporter * continue applying ratelimiter on appsec standalone events * Update packages/dd-trace/src/appsec/reporter.js Co-authored-by: simon-id * Add _dd.apm.enabled:0 in root spans with remote parent * Use a method to add the tag * Remove apmTracingEnabled config property * Add _dd.p.appsec tag in trace tags * Some tests * Set _dd.apm.enabled in root span * configure standalone if _tracingInitialized * Use dd-trace:span:start channel * PrioritySampler and propagation * Clean up * Clean up * use a meta tag * Use dc to modify what is injected and extracted * set USER_KEEP priority * integration tests * hasSubscribers check * test description * hasSubscribers check * standalone tests * Check span context has tags before using them and check if config has changed * clean up * Pass prioritySampler as argument to DatadogTracer * clean up * clean up * protect span context sampling access * Disable span stats if standalone enabled * clean up * clean up * Clean up * Clean up * clean up * Remove all headers from carrier * inject integration tests * remove only * Update packages/dd-trace/test/appsec/sdk/track_event.spec.js Co-authored-by: Ugaitz Urien * Update packages/dd-trace/test/appsec/standalone.spec.js Co-authored-by: Ugaitz Urien * protect sample method * Use assert instead expect * reset sampling prio * unsubscribe after test * clear dd context from tracestate * propagation with and without ASM events * suggestions * test inject and extrach channels * use two services to test propagation * integration tests cleanup * clean up * clean up * Update packages/dd-trace/src/priority_sampler.js Co-authored-by: Ugaitz Urien * Update packages/dd-trace/src/appsec/standalone.js Co-authored-by: Ugaitz Urien * Move hasOwn, remove not used argument and fix test * simplify iast integration-test using weak_hash * Update packages/dd-trace/src/appsec/standalone.js Co-authored-by: Ugaitz Urien * suggestions * Fix integration tests * Update packages/dd-trace/test/span_stats.spec.js Co-authored-by: Ugaitz Urien * Update packages/dd-trace/test/exporters/agent/exporter.spec.js Co-authored-by: Ugaitz Urien * Remove redundant check * Remove standalone option * protect onSpanInject and onSpanExtract * do not set _dd.p.dm * remove _dd.p.dm check from integration tests * increase coverage * increase coverage * more coverage * remove not needed async * set default mechanism * sugestions * Remove throw tests * Remove throw tests * Update packages/dd-trace/test/appsec/standalone.spec.js Co-authored-by: simon-id --------- Co-authored-by: Ugaitz Urien Co-authored-by: simon-id --- integration-tests/standalone-asm.spec.js | 306 +++++++++++++++ integration-tests/standalone-asm/index.js | 101 +++++ packages/dd-trace/src/appsec/standalone.js | 105 +++++- .../dd-trace/src/exporters/agent/index.js | 4 +- .../src/opentracing/propagation/text_map.js | 12 + packages/dd-trace/src/opentracing/tracer.js | 4 +- packages/dd-trace/src/priority_sampler.js | 7 +- packages/dd-trace/src/proxy.js | 4 +- packages/dd-trace/src/rate_limiter.js | 4 +- packages/dd-trace/src/span_stats.js | 7 +- packages/dd-trace/src/tracer.js | 4 +- packages/dd-trace/src/util.js | 7 +- .../dd-trace/test/appsec/standalone.spec.js | 352 +++++++++++++++++- .../test/exporters/agent/exporter.spec.js | 12 + .../opentracing/propagation/text_map.spec.js | 39 ++ .../dd-trace/test/opentracing/tracer.spec.js | 8 + packages/dd-trace/test/span_stats.spec.js | 8 + 17 files changed, 953 insertions(+), 31 deletions(-) create mode 100644 integration-tests/standalone-asm.spec.js create mode 100644 integration-tests/standalone-asm/index.js diff --git a/integration-tests/standalone-asm.spec.js b/integration-tests/standalone-asm.spec.js new file mode 100644 index 0000000000..d57a96f738 --- /dev/null +++ b/integration-tests/standalone-asm.spec.js @@ -0,0 +1,306 @@ +'use strict' + +const { assert } = require('chai') +const path = require('path') + +const { + createSandbox, + FakeAgent, + spawnProc, + curlAndAssertMessage, + curl +} = require('./helpers') + +describe('Standalone ASM', () => { + let sandbox, cwd, startupTestFile, agent, proc, env + + before(async () => { + sandbox = await createSandbox(['express']) + cwd = sandbox.folder + startupTestFile = path.join(cwd, 'standalone-asm/index.js') + }) + + after(async () => { + await sandbox.remove() + }) + + describe('enabled', () => { + beforeEach(async () => { + agent = await new FakeAgent().start() + + env = { + AGENT_PORT: agent.port, + DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED: 'true' + } + + const execArgv = [] + + proc = await spawnProc(startupTestFile, { cwd, env, execArgv }) + }) + + afterEach(async () => { + proc.kill() + await agent.stop() + }) + + function assertKeep (payload, manual = true) { + const { meta, metrics } = payload + if (manual) { + assert.propertyVal(meta, 'manual.keep', 'true') + } else { + assert.notProperty(meta, 'manual.keep') + } + assert.propertyVal(meta, '_dd.p.appsec', '1') + + assert.propertyVal(metrics, '_sampling_priority_v1', 2) + assert.propertyVal(metrics, '_dd.apm.enabled', 0) + } + + function assertDrop (payload) { + const { metrics } = payload + assert.propertyVal(metrics, '_sampling_priority_v1', 0) + assert.propertyVal(metrics, '_dd.apm.enabled', 0) + assert.notProperty(metrics, '_dd.p.appsec') + } + + async function doWarmupRequests (procOrUrl, number = 3) { + for (let i = number; i > 0; i--) { + await curl(procOrUrl) + } + } + + // first req initializes the waf and reports the first appsec event adding manual.keep tag + it('should send correct headers and tags on first req', async () => { + return curlAndAssertMessage(agent, proc, ({ headers, payload }) => { + assert.propertyVal(headers, 'datadog-client-computed-stats', 'yes') + assert.isArray(payload) + assert.strictEqual(payload.length, 1) + assert.isArray(payload[0]) + + // express.request + 4 middlewares + assert.strictEqual(payload[0].length, 5) + + assertKeep(payload[0][0]) + }) + }) + + it('should keep second req because RateLimiter allows 1 req/min and discard the next', async () => { + // 1st req kept because waf init + // 2nd req kept because it's the first one hitting RateLimiter + // next in the first minute are dropped + await doWarmupRequests(proc) + + return curlAndAssertMessage(agent, proc, ({ headers, payload }) => { + assert.propertyVal(headers, 'datadog-client-computed-stats', 'yes') + assert.isArray(payload) + assert.strictEqual(payload.length, 4) + + const secondReq = payload[1] + assert.isArray(secondReq) + assert.strictEqual(secondReq.length, 5) + + const { meta, metrics } = secondReq[0] + assert.notProperty(meta, 'manual.keep') + assert.notProperty(meta, '_dd.p.appsec') + + assert.propertyVal(metrics, '_sampling_priority_v1', 1) + assert.propertyVal(metrics, '_dd.apm.enabled', 0) + + assertDrop(payload[2][0]) + + assertDrop(payload[3][0]) + }) + }) + + it('should keep attack requests', async () => { + await doWarmupRequests(proc) + + const urlAttack = proc.url + '?query=1 or 1=1' + return curlAndAssertMessage(agent, urlAttack, ({ headers, payload }) => { + assert.propertyVal(headers, 'datadog-client-computed-stats', 'yes') + assert.isArray(payload) + assert.strictEqual(payload.length, 4) + + assertKeep(payload[3][0]) + }) + }) + + it('should keep sdk events', async () => { + await doWarmupRequests(proc) + + const url = proc.url + '/login?user=test' + return curlAndAssertMessage(agent, url, ({ headers, payload }) => { + assert.propertyVal(headers, 'datadog-client-computed-stats', 'yes') + assert.isArray(payload) + assert.strictEqual(payload.length, 4) + + assertKeep(payload[3][0]) + }) + }) + + it('should keep custom sdk events', async () => { + await doWarmupRequests(proc) + + const url = proc.url + '/sdk' + return curlAndAssertMessage(agent, url, ({ headers, payload }) => { + assert.propertyVal(headers, 'datadog-client-computed-stats', 'yes') + assert.isArray(payload) + assert.strictEqual(payload.length, 4) + + assertKeep(payload[3][0]) + }) + }) + + it('should keep iast events', async () => { + await doWarmupRequests(proc) + + const url = proc.url + '/vulnerableHash' + return curlAndAssertMessage(agent, url, ({ headers, payload }) => { + assert.propertyVal(headers, 'datadog-client-computed-stats', 'yes') + assert.isArray(payload) + assert.strictEqual(payload.length, 4) + + const expressReq4 = payload[3][0] + assertKeep(expressReq4) + assert.property(expressReq4.meta, '_dd.iast.json') + assert.propertyVal(expressReq4.metrics, '_dd.iast.enabled', 1) + }) + }) + + describe('propagation', () => { + let proc2 + let port2 + + beforeEach(async () => { + const execArgv = [] + + proc2 = await spawnProc(startupTestFile, { cwd, env, execArgv }) + + port2 = parseInt(proc2.url.substring(proc2.url.lastIndexOf(':') + 1), 10) + }) + + afterEach(async () => { + proc2.kill() + }) + + // proc/drop-and-call-sdk: + // after setting a manual.drop calls to downstream proc2/sdk which triggers an appsec event + it('should keep trace even if parent prio is -1 but there is an event in the local trace', async () => { + await doWarmupRequests(proc) + await doWarmupRequests(proc2) + + const url = `${proc.url}/propagation-after-drop-and-call-sdk?port=${port2}` + return curlAndAssertMessage(agent, url, ({ headers, payload }) => { + assert.propertyVal(headers, 'datadog-client-computed-stats', 'yes') + assert.isArray(payload) + + const innerReq = payload.find(p => p[0].resource === 'GET /sdk') + assert.notStrictEqual(innerReq, undefined) + assertKeep(innerReq[0]) + }, undefined, undefined, true) + }) + + // proc/propagation-with-event triggers an appsec event and calls downstream proc2/down with no event + it('should keep if parent trace is (prio:2, _dd.p.appsec:1) but there is no event in the local trace', + async () => { + await doWarmupRequests(proc) + await doWarmupRequests(proc2) + + const url = `${proc.url}/propagation-with-event?port=${port2}` + return curlAndAssertMessage(agent, url, ({ headers, payload }) => { + assert.propertyVal(headers, 'datadog-client-computed-stats', 'yes') + assert.isArray(payload) + + const innerReq = payload.find(p => p[0].resource === 'GET /down') + assert.notStrictEqual(innerReq, undefined) + assertKeep(innerReq[0], false) + }, undefined, undefined, true) + }) + + it('should remove parent trace data if there is no event in the local trace', async () => { + await doWarmupRequests(proc) + await doWarmupRequests(proc2) + + const url = `${proc.url}/propagation-without-event?port=${port2}` + return curlAndAssertMessage(agent, url, ({ headers, payload }) => { + assert.propertyVal(headers, 'datadog-client-computed-stats', 'yes') + assert.isArray(payload) + + const innerReq = payload.find(p => p[0].resource === 'GET /down') + assert.notStrictEqual(innerReq, undefined) + assert.notProperty(innerReq[0].meta, '_dd.p.other') + }, undefined, undefined, true) + }) + + it('should not remove parent trace data if there is event in the local trace', async () => { + await doWarmupRequests(proc) + + const url = `${proc.url}/propagation-with-event?port=${port2}` + return curlAndAssertMessage(agent, url, ({ headers, payload }) => { + assert.propertyVal(headers, 'datadog-client-computed-stats', 'yes') + assert.isArray(payload) + + const innerReq = payload.find(p => p[0].resource === 'GET /down') + assert.notStrictEqual(innerReq, undefined) + assert.property(innerReq[0].meta, '_dd.p.other') + }, undefined, undefined, true) + }) + }) + }) + + describe('disabled', () => { + beforeEach(async () => { + agent = await new FakeAgent().start() + + env = { + AGENT_PORT: agent.port + } + + proc = await spawnProc(startupTestFile, { cwd, env }) + }) + + afterEach(async () => { + proc.kill() + await agent.stop() + }) + + it('should not add standalone related tags in iast events', () => { + const url = proc.url + '/vulnerableHash' + return curlAndAssertMessage(agent, url, ({ headers, payload }) => { + assert.notProperty(headers, 'datadog-client-computed-stats') + assert.isArray(payload) + assert.strictEqual(payload.length, 1) + assert.isArray(payload[0]) + + // express.request + 4 middlewares + assert.strictEqual(payload[0].length, 5) + + const { meta, metrics } = payload[0][0] + assert.property(meta, '_dd.iast.json') // WEAK_HASH and XCONTENTTYPE_HEADER_MISSING reported + + assert.notProperty(meta, '_dd.p.appsec') + assert.notProperty(metrics, '_dd.apm.enabled') + }) + }) + + it('should not add standalone related tags in appsec events', () => { + const urlAttack = proc.url + '?query=1 or 1=1' + + return curlAndAssertMessage(agent, urlAttack, ({ headers, payload }) => { + assert.notProperty(headers, 'datadog-client-computed-stats') + assert.isArray(payload) + assert.strictEqual(payload.length, 1) + assert.isArray(payload[0]) + + // express.request + 4 middlewares + assert.strictEqual(payload[0].length, 5) + + const { meta, metrics } = payload[0][0] + assert.property(meta, '_dd.appsec.json') // crs-942-100 triggered + + assert.notProperty(meta, '_dd.p.appsec') + assert.notProperty(metrics, '_dd.apm.enabled') + }) + }) + }) +}) diff --git a/integration-tests/standalone-asm/index.js b/integration-tests/standalone-asm/index.js new file mode 100644 index 0000000000..52070a50f2 --- /dev/null +++ b/integration-tests/standalone-asm/index.js @@ -0,0 +1,101 @@ +'use strict' + +const options = { + appsec: { + enabled: true + }, + experimental: { + iast: { + enabled: true, + requestSampling: 100 + } + } +} + +if (process.env.AGENT_PORT) { + options.port = process.env.AGENT_PORT +} + +if (process.env.AGENT_URL) { + options.url = process.env.AGENT_URL +} + +const tracer = require('dd-trace') +tracer.init(options) + +const http = require('http') +const express = require('express') +const app = express() + +const valueToHash = 'iast-showcase-demo' +const crypto = require('crypto') + +app.get('/', (req, res) => { + res.status(200).send('hello world') +}) + +app.get('/login', (req, res) => { + tracer.appsec.trackUserLoginSuccessEvent({ id: req.query.user }) + res.status(200).send('login') +}) + +app.get('/sdk', (req, res) => { + tracer.appsec.trackCustomEvent('custom-event') + res.status(200).send('sdk') +}) + +app.get('/vulnerableHash', (req, res) => { + const result = crypto.createHash('sha1').update(valueToHash).digest('hex') + res.status(200).send(result) +}) + +app.get('/propagation-with-event', async (req, res) => { + tracer.appsec.trackCustomEvent('custom-event') + + const span = tracer.scope().active() + span.context()._trace.tags['_dd.p.other'] = '1' + + const port = req.query.port || server.address().port + const url = `http://localhost:${port}/down` + + const resFetch = await fetch(url) + await resFetch.text() + + res.status(200).send('propagation-with-event') +}) + +app.get('/propagation-without-event', async (req, res) => { + const port = req.query.port || server.address().port + const url = `http://localhost:${port}/down` + + const span = tracer.scope().active() + span.context()._trace.tags['_dd.p.other'] = '1' + + const resFetch = await fetch(url) + await resFetch.text() + + res.status(200).send('propagation-without-event') +}) + +app.get('/down', (req, res) => { + res.status(200).send('down') +}) + +app.get('/propagation-after-drop-and-call-sdk', async (req, res) => { + const span = tracer.scope().active() + span?.setTag('manual.drop', 'true') + + const port = req.query.port + + const url = `http://localhost:${port}/sdk` + + const resFetch = await fetch(url) + const sdkRes = await resFetch.text() + + res.status(200).send(`drop-and-call-sdk ${sdkRes}`) +}) + +const server = http.createServer(app).listen(0, () => { + const port = server.address().port + process.send?.({ port }) +}) diff --git a/packages/dd-trace/src/appsec/standalone.js b/packages/dd-trace/src/appsec/standalone.js index d0402d8ab5..9d75dd3626 100644 --- a/packages/dd-trace/src/appsec/standalone.js +++ b/packages/dd-trace/src/appsec/standalone.js @@ -1,12 +1,55 @@ 'use strict' const { channel } = require('dc-polyfill') -const { APM_TRACING_ENABLED_KEY, APPSEC_PROPAGATION_KEY } = require('../constants') +const { USER_KEEP, AUTO_KEEP, AUTO_REJECT } = require('../../../../ext/priority') +const { MANUAL_KEEP } = require('../../../../ext/tags') +const PrioritySampler = require('../priority_sampler') +const RateLimiter = require('../rate_limiter') +const TraceState = require('../opentracing/propagation/tracestate') +const { hasOwn } = require('../util') +const { APM_TRACING_ENABLED_KEY, APPSEC_PROPAGATION_KEY, SAMPLING_MECHANISM_DEFAULT } = require('../constants') const startCh = channel('dd-trace:span:start') +const injectCh = channel('dd-trace:span:inject') +const extractCh = channel('dd-trace:span:extract') let enabled +class StandAloneAsmPrioritySampler extends PrioritySampler { + constructor (env) { + super(env, { sampleRate: 0, rateLimit: 0, rules: [] }) + + // let some regular APM traces go through, 1 per minute to keep alive the service + this._limiter = new RateLimiter(1, 'minute') + } + + configure (env, config) { + // rules not supported + this._env = env + } + + _getPriorityFromTags (tags, context) { + if (hasOwn(tags, MANUAL_KEEP) && + tags[MANUAL_KEEP] !== false && + hasOwn(context._trace.tags, APPSEC_PROPAGATION_KEY) + ) { + return USER_KEEP + } + } + + _getPriorityFromAuto (span) { + const context = this._getContext(span) + + context._sampling.mechanism = SAMPLING_MECHANISM_DEFAULT + + if (hasOwn(context._trace.tags, APPSEC_PROPAGATION_KEY)) { + return USER_KEEP + } + + return this._isSampledByRateLimit(context) ? AUTO_KEEP : AUTO_REJECT + } +} + function onSpanStart ({ span, fields }) { const tags = span.context?.()?._tags if (!tags) return @@ -17,27 +60,71 @@ function onSpanStart ({ span, fields }) { } } +function onSpanInject ({ spanContext, carrier }) { + if (!spanContext?._trace?.tags || !carrier) return + + // do not inject trace and sampling if there is no appsec event + if (!hasOwn(spanContext._trace.tags, APPSEC_PROPAGATION_KEY)) { + for (const key in carrier) { + const lKey = key.toLowerCase() + if (lKey.startsWith('x-datadog')) { + delete carrier[key] + } else if (lKey === 'tracestate') { + const tracestate = TraceState.fromString(carrier[key]) + tracestate.forVendor('dd', state => state.clear()) + carrier[key] = tracestate.toString() + } + } + } +} + +function onSpanExtract ({ spanContext = {} }) { + if (!spanContext._trace?.tags || !spanContext._sampling) return + + // reset upstream priority if _dd.p.appsec is not found + if (!hasOwn(spanContext._trace.tags, APPSEC_PROPAGATION_KEY)) { + spanContext._sampling.priority = undefined + } else if (spanContext._sampling.priority !== USER_KEEP) { + spanContext._sampling.priority = USER_KEEP + } +} + +function sample (span) { + const spanContext = span.context?.() + if (enabled && spanContext?._trace?.tags) { + spanContext._trace.tags[APPSEC_PROPAGATION_KEY] = '1' + + // TODO: ask. can we reset here sampling like this? + if (spanContext._sampling?.priority < AUTO_KEEP) { + spanContext._sampling.priority = undefined + } + } +} + function configure (config) { const configChanged = enabled !== config.appsec?.standalone?.enabled if (!configChanged) return enabled = config.appsec?.standalone?.enabled + let prioritySampler if (enabled) { startCh.subscribe(onSpanStart) + injectCh.subscribe(onSpanInject) + extractCh.subscribe(onSpanExtract) + + prioritySampler = new StandAloneAsmPrioritySampler(config.env) } else { - startCh.unsubscribe(onSpanStart) + if (startCh.hasSubscribers) startCh.unsubscribe(onSpanStart) + if (injectCh.hasSubscribers) injectCh.unsubscribe(onSpanInject) + if (extractCh.hasSubscribers) extractCh.unsubscribe(onSpanExtract) } -} -function sample (span) { - const context = span.context?.() - if (enabled && context._trace?.tags) { - context._trace.tags[APPSEC_PROPAGATION_KEY] = '1' - } + return prioritySampler } module.exports = { configure, - sample + sample, + StandAloneAsmPrioritySampler } diff --git a/packages/dd-trace/src/exporters/agent/index.js b/packages/dd-trace/src/exporters/agent/index.js index c617d27e89..b2f25eeda9 100644 --- a/packages/dd-trace/src/exporters/agent/index.js +++ b/packages/dd-trace/src/exporters/agent/index.js @@ -7,7 +7,7 @@ const Writer = require('./writer') class AgentExporter { constructor (config, prioritySampler) { this._config = config - const { url, hostname, port, lookup, protocolVersion, stats = {} } = config + const { url, hostname, port, lookup, protocolVersion, stats = {}, appsec } = config this._url = url || new URL(format({ protocol: 'http:', hostname: hostname || 'localhost', @@ -15,7 +15,7 @@ class AgentExporter { })) const headers = {} - if (stats.enabled) { + if (stats.enabled || appsec?.standalone?.enabled) { headers['Datadog-Client-Computed-Stats'] = 'yes' } diff --git a/packages/dd-trace/src/opentracing/propagation/text_map.js b/packages/dd-trace/src/opentracing/propagation/text_map.js index 0b74674f21..a183e977d7 100644 --- a/packages/dd-trace/src/opentracing/propagation/text_map.js +++ b/packages/dd-trace/src/opentracing/propagation/text_map.js @@ -6,9 +6,13 @@ const DatadogSpanContext = require('../span_context') const log = require('../../log') const TraceState = require('./tracestate') const tags = require('../../../../../ext/tags') +const { channel } = require('dc-polyfill') const { AUTO_KEEP, AUTO_REJECT, USER_KEEP } = require('../../../../../ext/priority') +const injectCh = channel('dd-trace:span:inject') +const extractCh = channel('dd-trace:span:extract') + const traceKey = 'x-datadog-trace-id' const spanKey = 'x-datadog-parent-id' const originKey = 'x-datadog-origin' @@ -54,6 +58,10 @@ class TextMapPropagator { this._injectB3SingleHeader(spanContext, carrier) this._injectTraceparent(spanContext, carrier) + if (injectCh.hasSubscribers) { + injectCh.publish({ spanContext, carrier }) + } + log.debug(() => `Inject into carrier: ${JSON.stringify(pick(carrier, logKeys))}.`) } @@ -62,6 +70,10 @@ class TextMapPropagator { if (!spanContext) return spanContext + if (extractCh.hasSubscribers) { + extractCh.publish({ spanContext, carrier }) + } + log.debug(() => `Extract from carrier: ${JSON.stringify(pick(carrier, logKeys))}.`) return spanContext diff --git a/packages/dd-trace/src/opentracing/tracer.js b/packages/dd-trace/src/opentracing/tracer.js index 13e6b9c150..56a6f95690 100644 --- a/packages/dd-trace/src/opentracing/tracer.js +++ b/packages/dd-trace/src/opentracing/tracer.js @@ -19,7 +19,7 @@ const REFERENCE_CHILD_OF = 'child_of' const REFERENCE_FOLLOWS_FROM = 'follows_from' class DatadogTracer { - constructor (config) { + constructor (config, prioritySampler) { const Exporter = getExporter(config.experimental.exporter) this._config = config @@ -28,7 +28,7 @@ class DatadogTracer { this._env = config.env this._logInjection = config.logInjection this._debug = config.debug - this._prioritySampler = new PrioritySampler(config.env, config.sampler) + this._prioritySampler = prioritySampler ?? new PrioritySampler(config.env, config.sampler) this._exporter = new Exporter(config, this._prioritySampler) this._processor = new SpanProcessor(this._exporter, this._prioritySampler, config) this._url = this._exporter._url diff --git a/packages/dd-trace/src/priority_sampler.js b/packages/dd-trace/src/priority_sampler.js index b3b7737fc5..aae366c262 100644 --- a/packages/dd-trace/src/priority_sampler.js +++ b/packages/dd-trace/src/priority_sampler.js @@ -4,6 +4,7 @@ const RateLimiter = require('./rate_limiter') const Sampler = require('./sampler') const { setSamplingRules } = require('./startup-log') const SamplingRule = require('./sampling_rule') +const { hasOwn } = require('./util') const { SAMPLING_MECHANISM_DEFAULT, @@ -66,7 +67,7 @@ class PrioritySampler { if (context._sampling.priority !== undefined) return if (!root) return // noop span - const tag = this._getPriorityFromTags(context._tags) + const tag = this._getPriorityFromTags(context._tags, context) if (this.validate(tag)) { context._sampling.priority = tag @@ -202,8 +203,4 @@ class PrioritySampler { } } -function hasOwn (object, prop) { - return Object.prototype.hasOwnProperty.call(object, prop) -} - module.exports = PrioritySampler diff --git a/packages/dd-trace/src/proxy.js b/packages/dd-trace/src/proxy.js index 863c2fdcce..7f3a0e8178 100644 --- a/packages/dd-trace/src/proxy.js +++ b/packages/dd-trace/src/proxy.js @@ -179,9 +179,9 @@ class Tracer extends NoopProxy { this._modules.appsec.enable(config) } if (!this._tracingInitialized) { - this._tracer = new DatadogTracer(config) + const prioritySampler = appsecStandalone.configure(config) + this._tracer = new DatadogTracer(config, prioritySampler) this.appsec = new AppsecSdk(this._tracer, config) - appsecStandalone.configure(config) this._tracingInitialized = true } if (config.iast.enabled) { diff --git a/packages/dd-trace/src/rate_limiter.js b/packages/dd-trace/src/rate_limiter.js index 99b7ceb57e..8417d77789 100644 --- a/packages/dd-trace/src/rate_limiter.js +++ b/packages/dd-trace/src/rate_limiter.js @@ -3,9 +3,9 @@ const limiter = require('limiter') class RateLimiter { - constructor (rateLimit) { + constructor (rateLimit, interval = 'second') { this._rateLimit = parseInt(rateLimit) - this._limiter = new limiter.RateLimiter(this._rateLimit, 'second') + this._limiter = new limiter.RateLimiter(this._rateLimit, interval) this._tokensRequested = 0 this._prevIntervalTokens = 0 this._prevTokensRequested = 0 diff --git a/packages/dd-trace/src/span_stats.js b/packages/dd-trace/src/span_stats.js index 67b8089ec1..3f7b5e34ea 100644 --- a/packages/dd-trace/src/span_stats.js +++ b/packages/dd-trace/src/span_stats.js @@ -126,7 +126,8 @@ class SpanStatsProcessor { port, url, env, - tags + tags, + appsec } = {}) { this.exporter = new SpanStatsExporter({ hostname, @@ -138,12 +139,12 @@ class SpanStatsProcessor { this.bucketSizeNs = interval * 1e9 this.buckets = new TimeBuckets() this.hostname = os.hostname() - this.enabled = enabled + this.enabled = enabled && !appsec?.standalone?.enabled this.env = env this.tags = tags || {} this.sequence = 0 - if (enabled) { + if (this.enabled) { this.timer = setInterval(this.onInterval.bind(this), interval * 1e3) this.timer.unref() } diff --git a/packages/dd-trace/src/tracer.js b/packages/dd-trace/src/tracer.js index 5c36d0ee90..1ed08e2037 100644 --- a/packages/dd-trace/src/tracer.js +++ b/packages/dd-trace/src/tracer.js @@ -20,8 +20,8 @@ const SERVICE_NAME = tags.SERVICE_NAME const MEASURED = tags.MEASURED class DatadogTracer extends Tracer { - constructor (config) { - super(config) + constructor (config, prioritySampler) { + super(config, prioritySampler) this._dataStreamsProcessor = new DataStreamsProcessor(config) this._scope = new Scope() setStartupLogConfig(config) diff --git a/packages/dd-trace/src/util.js b/packages/dd-trace/src/util.js index 77ab79aa53..04048c9b18 100644 --- a/packages/dd-trace/src/util.js +++ b/packages/dd-trace/src/util.js @@ -69,10 +69,15 @@ function calculateDDBasePath (dirname) { return dirSteps.slice(0, packagesIndex + 1).join(path.sep) + path.sep } +function hasOwn (object, prop) { + return Object.prototype.hasOwnProperty.call(object, prop) +} + module.exports = { isTrue, isFalse, isError, globMatch, - calculateDDBasePath + calculateDDBasePath, + hasOwn } diff --git a/packages/dd-trace/test/appsec/standalone.spec.js b/packages/dd-trace/test/appsec/standalone.spec.js index f41ea70f1b..027e23c3b5 100644 --- a/packages/dd-trace/test/appsec/standalone.spec.js +++ b/packages/dd-trace/test/appsec/standalone.spec.js @@ -4,16 +4,33 @@ const { channel } = require('dc-polyfill') const { assert } = require('chai') const standalone = require('../../src/appsec/standalone') const DatadogSpan = require('../../src/opentracing/span') -const { APM_TRACING_ENABLED_KEY } = require('../../src/constants') +const { + APM_TRACING_ENABLED_KEY, + APPSEC_PROPAGATION_KEY, + SAMPLING_MECHANISM_APPSEC, + DECISION_MAKER_KEY +} = require('../../src/constants') +const { USER_KEEP, AUTO_KEEP, AUTO_REJECT, USER_REJECT } = require('../../../../ext/priority') +const TextMapPropagator = require('../../src/opentracing/propagation/text_map') +const TraceState = require('../../src/opentracing/propagation/tracestate') const startCh = channel('dd-trace:span:start') +const injectCh = channel('dd-trace:span:inject') +const extractCh = channel('dd-trace:span:extract') describe('Appsec Standalone', () => { let config let tracer, processor, prioritySampler beforeEach(() => { - config = { appsec: { standalone: { enabled: true } } } + config = { + appsec: { standalone: { enabled: true } }, + + tracePropagationStyle: { + inject: ['datadog', 'tracecontext'], + extract: ['datadog'] + } + } tracer = {} processor = {} @@ -25,16 +42,26 @@ describe('Appsec Standalone', () => { describe('configure', () => { let startChSubscribe let startChUnsubscribe + let injectChSubscribe + let injectChUnsubscribe + let extractChSubscribe + let extractChUnsubscribe beforeEach(() => { startChSubscribe = sinon.stub(startCh, 'subscribe') startChUnsubscribe = sinon.stub(startCh, 'unsubscribe') + injectChSubscribe = sinon.stub(injectCh, 'subscribe') + injectChUnsubscribe = sinon.stub(injectCh, 'unsubscribe') + extractChSubscribe = sinon.stub(extractCh, 'subscribe') + extractChUnsubscribe = sinon.stub(extractCh, 'unsubscribe') }) it('should subscribe to start span if standalone enabled', () => { standalone.configure(config) sinon.assert.calledOnce(startChSubscribe) + sinon.assert.calledOnce(injectChSubscribe) + sinon.assert.calledOnce(extractChSubscribe) }) it('should not subscribe to start span if standalone disabled', () => { @@ -42,7 +69,12 @@ describe('Appsec Standalone', () => { standalone.configure(config) - sinon.assert.calledOnce(startChUnsubscribe) + sinon.assert.notCalled(startChSubscribe) + sinon.assert.notCalled(injectChSubscribe) + sinon.assert.notCalled(extractChSubscribe) + sinon.assert.notCalled(startChUnsubscribe) + sinon.assert.notCalled(injectChUnsubscribe) + sinon.assert.notCalled(extractChUnsubscribe) }) it('should subscribe only once', () => { @@ -52,6 +84,60 @@ describe('Appsec Standalone', () => { sinon.assert.calledOnce(startChSubscribe) }) + + it('should not return a prioritySampler when standalone ASM is disabled', () => { + const prioritySampler = standalone.configure({ appsec: { standalone: { enabled: false } } }) + + assert.isUndefined(prioritySampler) + }) + + it('should return a StandAloneAsmPrioritySampler when standalone ASM is enabled', () => { + const prioritySampler = standalone.configure(config) + + assert.instanceOf(prioritySampler, standalone.StandAloneAsmPrioritySampler) + }) + }) + + describe('sample', () => { + it('should add _dd.p.appsec tag if enabled', () => { + standalone.configure(config) + + const span = new DatadogSpan(tracer, processor, prioritySampler, { + operationName: 'operation' + }) + + standalone.sample(span) + + assert.propertyVal(span.context()._trace.tags, APPSEC_PROPAGATION_KEY, '1') + }) + + it('should reset priority', () => { + standalone.configure(config) + + const span = new DatadogSpan(tracer, processor, prioritySampler, { + operationName: 'operation' + }) + + span.context()._sampling.priority = USER_REJECT + + standalone.sample(span) + + assert.strictEqual(span.context()._sampling.priority, undefined) + }) + + it('should not add _dd.p.appsec tag if disabled', () => { + delete config.appsec.standalone + + standalone.configure(config) + + const span = new DatadogSpan(tracer, processor, prioritySampler, { + operationName: 'operation' + }) + + standalone.sample(span) + + assert.notProperty(span.context()._trace.tags, APPSEC_PROPAGATION_KEY) + }) }) describe('onStartSpan', () => { @@ -77,6 +163,8 @@ describe('Appsec Standalone', () => { }) it('should not add _dd.apm.enabled tag in child spans with local parent', () => { + standalone.configure(config) + const parent = new DatadogSpan(tracer, processor, prioritySampler, { operationName: 'operation' }) @@ -92,6 +180,8 @@ describe('Appsec Standalone', () => { }) it('should add _dd.apm.enabled tag in child spans with remote parent', () => { + standalone.configure(config) + const parent = new DatadogSpan(tracer, processor, prioritySampler, { operationName: 'operation' }) @@ -106,4 +196,260 @@ describe('Appsec Standalone', () => { assert.propertyVal(child.context()._tags, APM_TRACING_ENABLED_KEY, 0) }) }) + + describe('onSpanExtract', () => { + it('should reset priority if _dd.p.appsec not present', () => { + standalone.configure(config) + + const carrier = { + 'x-datadog-trace-id': 123123, + 'x-datadog-parent-id': 345345, + 'x-datadog-sampling-priority': 2 + } + + const propagator = new TextMapPropagator(config) + const spanContext = propagator.extract(carrier) + + assert.isUndefined(spanContext._sampling.priority) + }) + + it('should not reset dm if _dd.p.appsec not present', () => { + standalone.configure(config) + + const carrier = { + 'x-datadog-trace-id': 123123, + 'x-datadog-parent-id': 345345, + 'x-datadog-sampling-priority': 2, + 'x-datadog-tags': '_dd.p.dm=-4' + } + + const propagator = new TextMapPropagator(config) + const spanContext = propagator.extract(carrier) + + assert.propertyVal(spanContext._trace.tags, DECISION_MAKER_KEY, '-4') + }) + + it('should keep priority if _dd.p.appsec is present', () => { + standalone.configure(config) + + const carrier = { + 'x-datadog-trace-id': 123123, + 'x-datadog-parent-id': 345345, + 'x-datadog-sampling-priority': 2, + 'x-datadog-tags': '_dd.p.appsec=1,_dd.p.dm=-5' + } + + const propagator = new TextMapPropagator(config) + const spanContext = propagator.extract(carrier) + + assert.strictEqual(spanContext._sampling.priority, USER_KEEP) + assert.propertyVal(spanContext._trace.tags, DECISION_MAKER_KEY, '-5') + }) + + it('should set USER_KEEP priority if _dd.p.appsec=1 is present', () => { + standalone.configure(config) + + const carrier = { + 'x-datadog-trace-id': 123123, + 'x-datadog-parent-id': 345345, + 'x-datadog-sampling-priority': 1, + 'x-datadog-tags': '_dd.p.appsec=1' + } + + const propagator = new TextMapPropagator(config) + const spanContext = propagator.extract(carrier) + + assert.strictEqual(spanContext._sampling.priority, USER_KEEP) + }) + + it('should keep priority if standalone is disabled', () => { + delete config.appsec.standalone + standalone.configure(config) + + const carrier = { + 'x-datadog-trace-id': 123123, + 'x-datadog-parent-id': 345345, + 'x-datadog-sampling-priority': 2 + } + + const propagator = new TextMapPropagator(config) + const spanContext = propagator.extract(carrier) + + assert.strictEqual(spanContext._sampling.priority, USER_KEEP) + }) + }) + + describe('onSpanInject', () => { + it('should reset priority if standalone enabled and there is no appsec event', () => { + standalone.configure(config) + + const span = new DatadogSpan(tracer, processor, prioritySampler, { + operationName: 'operation' + }) + + span._spanContext._sampling = { + priority: USER_KEEP, + mechanism: SAMPLING_MECHANISM_APPSEC + } + + const carrier = {} + const propagator = new TextMapPropagator(config) + propagator.inject(span._spanContext, carrier) + + assert.notProperty(carrier, 'x-datadog-trace-id') + assert.notProperty(carrier, 'x-datadog-parent-id') + assert.notProperty(carrier, 'x-datadog-sampling-priority') + }) + + it('should keep priority if standalone enabled and there is an appsec event', () => { + standalone.configure(config) + + const span = new DatadogSpan(tracer, processor, prioritySampler, { + operationName: 'operation' + }) + + span._spanContext._sampling = { + priority: USER_KEEP, + mechanism: SAMPLING_MECHANISM_APPSEC + } + + span._spanContext._trace.tags[APPSEC_PROPAGATION_KEY] = '1' + + const carrier = {} + const propagator = new TextMapPropagator(config) + propagator.inject(span._spanContext, carrier) + + assert.property(carrier, 'x-datadog-trace-id') + assert.property(carrier, 'x-datadog-parent-id') + assert.property(carrier, 'x-datadog-sampling-priority') + assert.propertyVal(carrier, 'x-datadog-tags', '_dd.p.appsec=1') + }) + + it('should not reset priority if standalone disabled', () => { + delete config.appsec.standalone + standalone.configure(config) + + const span = new DatadogSpan(tracer, processor, prioritySampler, { + operationName: 'operation' + }) + + span._spanContext._sampling = { + priority: USER_KEEP, + mechanism: SAMPLING_MECHANISM_APPSEC + } + + const carrier = {} + const propagator = new TextMapPropagator(config) + propagator.inject(span._spanContext, carrier) + + assert.property(carrier, 'x-datadog-trace-id') + assert.property(carrier, 'x-datadog-parent-id') + assert.property(carrier, 'x-datadog-sampling-priority') + }) + + it('should clear tracestate datadog info', () => { + standalone.configure(config) + + const span = new DatadogSpan(tracer, processor, prioritySampler, { + operationName: 'operation' + }) + + span._spanContext._sampling = { + priority: USER_KEEP, + mechanism: SAMPLING_MECHANISM_APPSEC + } + + const tracestate = new TraceState() + tracestate.set('dd', 't.tid:666b118100000000;t.dm:-1;s:1;p:73a164d716fcddff') + tracestate.set('other', 'id:0xC0FFEE') + span._spanContext._tracestate = tracestate + + const carrier = {} + const propagator = new TextMapPropagator(config) + propagator.inject(span._spanContext, carrier) + + assert.propertyVal(carrier, 'tracestate', 'other=id:0xC0FFEE') + }) + }) + + describe('StandaloneASMPriorityManager', () => { + let prioritySampler + let tags + let context + let root + + beforeEach(() => { + tags = { 'manual.keep': 'true' } + prioritySampler = new standalone.StandAloneAsmPrioritySampler('test') + + root = {} + context = { + _sampling: {}, + _trace: { + tags: {}, + started: [root] + } + } + sinon.stub(prioritySampler, '_getContext').returns(context) + }) + + describe('sample', () => { + it('should provide the context when invoking _getPriorityFromTags', () => { + const span = new DatadogSpan(tracer, processor, prioritySampler, { + operationName: 'operation' + }) + + const _getPriorityFromTags = sinon.stub(prioritySampler, '_getPriorityFromTags') + + prioritySampler.sample(span, false) + + sinon.assert.calledWithExactly(_getPriorityFromTags, context._tags, context) + }) + }) + + describe('_getPriorityFromTags', () => { + it('should keep the trace if manual.keep and _dd.p.appsec are present', () => { + context._trace.tags[APPSEC_PROPAGATION_KEY] = 1 + assert.strictEqual(prioritySampler._getPriorityFromTags(tags, context), USER_KEEP) + }) + + it('should return undefined if manual.keep or _dd.p.appsec are not present', () => { + assert.isUndefined(prioritySampler._getPriorityFromTags(tags, context)) + }) + }) + + describe('_getPriorityFromAuto', () => { + it('should keep one trace per 1 min', () => { + const span = { + _trace: {} + } + + const clock = sinon.useFakeTimers() + + assert.strictEqual(prioritySampler._getPriorityFromAuto(span), AUTO_KEEP) + + assert.strictEqual(prioritySampler._getPriorityFromAuto(span), AUTO_REJECT) + + clock.tick(30000) + + assert.strictEqual(prioritySampler._getPriorityFromAuto(span), AUTO_REJECT) + + clock.tick(60000) + + assert.strictEqual(prioritySampler._getPriorityFromAuto(span), AUTO_KEEP) + + clock.restore() + }) + + it('should keep trace if it contains _dd.p.appsec tag', () => { + const span = { + _trace: {} + } + + context._trace.tags[APPSEC_PROPAGATION_KEY] = 1 + + assert.strictEqual(prioritySampler._getPriorityFromAuto(span), USER_KEEP) + }) + }) + }) }) diff --git a/packages/dd-trace/test/exporters/agent/exporter.spec.js b/packages/dd-trace/test/exporters/agent/exporter.spec.js index 9bfeb2ec04..a92a4d975d 100644 --- a/packages/dd-trace/test/exporters/agent/exporter.spec.js +++ b/packages/dd-trace/test/exporters/agent/exporter.spec.js @@ -43,6 +43,18 @@ describe('Exporter', () => { }) }) + it('should pass computed stats header through to writer if standalone appsec is enabled', () => { + const stats = { enabled: false } + const appsec = { standalone: { enabled: true } } + exporter = new Exporter({ url, flushInterval, stats, appsec }, prioritySampler) + + expect(Writer).to.have.been.calledWithMatch({ + headers: { + 'Datadog-Client-Computed-Stats': 'yes' + } + }) + }) + it('should support IPv6', () => { const stats = { enabled: true } exporter = new Exporter({ hostname: '::1', flushInterval, stats }, prioritySampler) diff --git a/packages/dd-trace/test/opentracing/propagation/text_map.spec.js b/packages/dd-trace/test/opentracing/propagation/text_map.spec.js index 9bc86bc16f..e6a206a8bb 100644 --- a/packages/dd-trace/test/opentracing/propagation/text_map.spec.js +++ b/packages/dd-trace/test/opentracing/propagation/text_map.spec.js @@ -6,11 +6,15 @@ const Config = require('../../../src/config') const id = require('../../../src/id') const SpanContext = require('../../../src/opentracing/span_context') const TraceState = require('../../../src/opentracing/propagation/tracestate') +const { channel } = require('dc-polyfill') const { AUTO_KEEP, AUTO_REJECT, USER_KEEP } = require('../../../../../ext/priority') const { SAMPLING_MECHANISM_MANUAL } = require('../../../src/constants') const { expect } = require('chai') +const injectCh = channel('dd-trace:span:inject') +const extractCh = channel('dd-trace:span:extract') + describe('TextMapPropagator', () => { let TextMapPropagator let propagator @@ -319,6 +323,26 @@ describe('TextMapPropagator', () => { expect(carrier).to.not.have.property('x-datadog-origin') expect(carrier).to.not.have.property('x-datadog-tags') }) + + it('should publish spanContext and carrier', () => { + const carrier = {} + const spanContext = createContext({ + traceId: id('0000000000000123'), + spanId: id('0000000000000456') + }) + + const onSpanInject = sinon.stub() + injectCh.subscribe(onSpanInject) + + propagator.inject(spanContext, carrier) + + try { + expect(onSpanInject).to.be.calledOnce + expect(onSpanInject.firstCall.args[0]).to.be.deep.equal({ spanContext, carrier }) + } finally { + injectCh.unsubscribe(onSpanInject) + } + }) }) describe('extract', () => { @@ -551,6 +575,21 @@ describe('TextMapPropagator', () => { expect(spanContext._trace.tags).to.have.property('_dd.parent_id', '0000000000000001') }) + it('should publish spanContext and carrier', () => { + const onSpanExtract = sinon.stub() + extractCh.subscribe(onSpanExtract) + + const carrier = textMap + const spanContext = propagator.extract(carrier) + + try { + expect(onSpanExtract).to.be.calledOnce + expect(onSpanExtract.firstCall.args[0]).to.be.deep.equal({ spanContext, carrier }) + } finally { + extractCh.unsubscribe(onSpanExtract) + } + }) + describe('with B3 propagation as multiple headers', () => { beforeEach(() => { config.tracePropagationStyle.extract = ['b3multi'] diff --git a/packages/dd-trace/test/opentracing/tracer.spec.js b/packages/dd-trace/test/opentracing/tracer.spec.js index 2b6927e99d..1a6ae261f0 100644 --- a/packages/dd-trace/test/opentracing/tracer.spec.js +++ b/packages/dd-trace/test/opentracing/tracer.spec.js @@ -103,6 +103,14 @@ describe('Tracer', () => { expect(SpanProcessor).to.have.been.calledWith(agentExporter, prioritySampler, config) }) + it('should allow to configure an alternative prioritySampler', () => { + const sampler = {} + tracer = new Tracer(config, sampler) + + expect(AgentExporter).to.have.been.calledWith(config, sampler) + expect(SpanProcessor).to.have.been.calledWith(agentExporter, sampler, config) + }) + describe('startSpan', () => { it('should start a span', () => { fields.tags = { foo: 'bar' } diff --git a/packages/dd-trace/test/span_stats.spec.js b/packages/dd-trace/test/span_stats.spec.js index 77f59f6864..3ffdd4899a 100644 --- a/packages/dd-trace/test/span_stats.spec.js +++ b/packages/dd-trace/test/span_stats.spec.js @@ -255,6 +255,14 @@ describe('SpanStatsProcessor', () => { expect(processor.tags).to.deep.equal(config.tags) }) + it('should construct a disabled instance if appsec standalone is enabled', () => { + const standaloneConfig = { appsec: { standalone: { enabled: true } }, ...config } + const processor = new SpanStatsProcessor(standaloneConfig) + + expect(processor.enabled).to.be.false + expect(processor.timer).to.be.undefined + }) + it('should track span stats', () => { expect(processor.buckets.size).to.equal(0) for (let i = 0; i < n; i++) {