diff --git a/integration-tests/standalone-asm.spec.js b/integration-tests/standalone-asm.spec.js index d57a96f738..4e57b25bad 100644 --- a/integration-tests/standalone-asm.spec.js +++ b/integration-tests/standalone-asm.spec.js @@ -10,6 +10,7 @@ const { curlAndAssertMessage, curl } = require('./helpers') +const { USER_KEEP, AUTO_REJECT, AUTO_KEEP } = require('../ext/priority') describe('Standalone ASM', () => { let sandbox, cwd, startupTestFile, agent, proc, env @@ -43,22 +44,18 @@ describe('Standalone ASM', () => { await agent.stop() }) - function assertKeep (payload, manual = true) { + function assertKeep (payload) { 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, '_sampling_priority_v1', USER_KEEP) assert.propertyVal(metrics, '_dd.apm.enabled', 0) } function assertDrop (payload) { const { metrics } = payload - assert.propertyVal(metrics, '_sampling_priority_v1', 0) + assert.propertyVal(metrics, '_sampling_priority_v1', AUTO_REJECT) assert.propertyVal(metrics, '_dd.apm.enabled', 0) assert.notProperty(metrics, '_dd.p.appsec') } @@ -103,7 +100,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', AUTO_KEEP) assert.propertyVal(metrics, '_dd.apm.enabled', 0) assertDrop(payload[2][0]) @@ -213,7 +210,7 @@ describe('Standalone ASM', () => { const innerReq = payload.find(p => p[0].resource === 'GET /down') assert.notStrictEqual(innerReq, undefined) - assertKeep(innerReq[0], false) + assertKeep(innerReq[0]) }, undefined, undefined, true) }) diff --git a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js index cc25d51b1e..e2d1619b11 100644 --- a/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js +++ b/packages/dd-trace/src/appsec/iast/vulnerability-reporter.js @@ -1,10 +1,11 @@ 'use strict' -const { MANUAL_KEEP } = require('../../../../../ext/tags') const LRU = require('lru-cache') const vulnerabilitiesFormatter = require('./vulnerabilities-formatter') const { IAST_ENABLED_TAG_KEY, IAST_JSON_TAG_KEY } = require('./tags') const standalone = require('../standalone') +const { SAMPLING_MECHANISM_APPSEC } = require('../../constants') +const { keepTrace } = require('../../priority_sampler') const VULNERABILITIES_KEY = 'vulnerabilities' const VULNERABILITY_HASHES_MAX_SIZE = 1000 @@ -56,9 +57,10 @@ function sendVulnerabilities (vulnerabilities, rootSpan) { const tags = {} // TODO: Store this outside of the span and set the tag in the exporter. tags[IAST_JSON_TAG_KEY] = JSON.stringify(jsonToSend) - tags[MANUAL_KEEP] = 'true' span.addTags(tags) + keepTrace(span, SAMPLING_MECHANISM_APPSEC) + standalone.sample(span) if (!rootSpan) span.finish() diff --git a/packages/dd-trace/src/appsec/reporter.js b/packages/dd-trace/src/appsec/reporter.js index dd2bde9fb0..3cd23b1f00 100644 --- a/packages/dd-trace/src/appsec/reporter.js +++ b/packages/dd-trace/src/appsec/reporter.js @@ -13,8 +13,9 @@ const { getRequestMetrics } = require('./telemetry') const zlib = require('zlib') -const { MANUAL_KEEP } = require('../../../../ext/tags') const standalone = require('./standalone') +const { SAMPLING_MECHANISM_APPSEC } = require('../constants') +const { keepTrace } = require('../priority_sampler') // default limiter, configurable with setRateLimit() let limiter = new Limiter(100) @@ -96,8 +97,6 @@ function reportWafInit (wafVersion, rulesVersion, diagnosticsRules = {}) { metricsQueue.set('_dd.appsec.event_rules.errors', JSON.stringify(diagnosticsRules.errors)) } - metricsQueue.set(MANUAL_KEEP, 'true') - incrementWafInitMetric(wafVersion, rulesVersion) } @@ -129,7 +128,7 @@ function reportAttack (attackData) { } if (limiter.isAllowed()) { - newTags[MANUAL_KEEP] = 'true' + keepTrace(rootSpan, SAMPLING_MECHANISM_APPSEC) standalone.sample(rootSpan) } @@ -184,6 +183,8 @@ function finishRequest (req, res) { if (metricsQueue.size) { rootSpan.addTags(Object.fromEntries(metricsQueue)) + keepTrace(rootSpan, SAMPLING_MECHANISM_APPSEC) + standalone.sample(rootSpan) metricsQueue.clear() diff --git a/packages/dd-trace/src/appsec/sdk/track_event.js b/packages/dd-trace/src/appsec/sdk/track_event.js index 36c40093b1..e95081314d 100644 --- a/packages/dd-trace/src/appsec/sdk/track_event.js +++ b/packages/dd-trace/src/appsec/sdk/track_event.js @@ -2,10 +2,11 @@ const log = require('../../log') const { getRootSpan } = require('./utils') -const { MANUAL_KEEP } = require('../../../../../ext/tags') const { setUserTags } = require('./set_user') const standalone = require('../standalone') const waf = require('../waf') +const { SAMPLING_MECHANISM_APPSEC } = require('../../constants') +const { keepTrace } = require('../../priority_sampler') function trackUserLoginSuccessEvent (tracer, user, metadata) { // TODO: better user check here and in _setUser() ? @@ -55,9 +56,10 @@ function trackEvent (eventName, fields, sdkMethodName, rootSpan, mode) { return } + keepTrace(rootSpan, SAMPLING_MECHANISM_APPSEC) + const tags = { - [`appsec.events.${eventName}.track`]: 'true', - [MANUAL_KEEP]: 'true' + [`appsec.events.${eventName}.track`]: 'true' } if (mode === 'sdk') { diff --git a/packages/dd-trace/src/priority_sampler.js b/packages/dd-trace/src/priority_sampler.js index aae366c262..f9968a4119 100644 --- a/packages/dd-trace/src/priority_sampler.js +++ b/packages/dd-trace/src/priority_sampler.js @@ -108,6 +108,18 @@ class PrioritySampler { } } + setPriority (span, samplingPriority, mechanism = SAMPLING_MECHANISM_MANUAL) { + if (!span || !this.validate(samplingPriority)) return + + const context = this._getContext(span) + + context._sampling.priority = samplingPriority + context._sampling.mechanism = mechanism + + const root = context._trace.started[0] + this._addDecisionMaker(root) + } + _getContext (span) { return typeof span.context === 'function' ? span.context() : span } @@ -201,6 +213,10 @@ class PrioritySampler { if (rule.match(span)) return rule } } + + static keepTrace (span, mechanism) { + span?._prioritySampler?.setPriority(span, USER_KEEP, mechanism) + } } module.exports = PrioritySampler diff --git a/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js b/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js index f498ef6e12..1f4516218a 100644 --- a/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js +++ b/packages/dd-trace/test/appsec/iast/vulnerability-reporter.spec.js @@ -2,7 +2,8 @@ const { addVulnerability, sendVulnerabilities, clearCache, start, stop } = require('../../../src/appsec/iast/vulnerability-reporter') const VulnerabilityAnalyzer = require('../../../../dd-trace/src/appsec/iast/analyzers/vulnerability-analyzer') const appsecStandalone = require('../../../src/appsec/standalone') -const { APPSEC_PROPAGATION_KEY } = require('../../../src/constants') +const { APPSEC_PROPAGATION_KEY, SAMPLING_MECHANISM_APPSEC } = require('../../../src/constants') +const { USER_KEEP } = require('../../../../../ext/priority') describe('vulnerability-reporter', () => { let vulnerabilityAnalyzer @@ -82,9 +83,14 @@ describe('vulnerability-reporter', () => { describe('without rootSpan', () => { let fakeTracer let onTheFlySpan + let prioritySampler beforeEach(() => { + prioritySampler = { + setPriority: sinon.stub() + } onTheFlySpan = { + _prioritySampler: prioritySampler, finish: sinon.spy(), addTags: sinon.spy(), context () { @@ -120,10 +126,11 @@ describe('vulnerability-reporter', () => { '_dd.iast.enabled': 1 }) expect(onTheFlySpan.addTags.secondCall).to.have.been.calledWithExactly({ - 'manual.keep': 'true', '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3410512655,' + '"evidence":{"value":"sha1"},"location":{"spanId":42,"path":"filename.js","line":73}}]}' }) + expect(prioritySampler.setPriority) + .to.have.been.calledOnceWithExactly(onTheFlySpan, USER_KEEP, SAMPLING_MECHANISM_APPSEC) expect(onTheFlySpan.finish).to.have.been.calledOnce }) @@ -140,10 +147,15 @@ describe('vulnerability-reporter', () => { describe('sendVulnerabilities', () => { let span let context + let prioritySampler beforeEach(() => { context = { _trace: { tags: {} } } + prioritySampler = { + setPriority: sinon.stub() + } span = { + _prioritySampler: prioritySampler, addTags: sinon.stub(), context: sinon.stub().returns(context) } @@ -178,10 +190,10 @@ describe('vulnerability-reporter', () => { vulnerabilityAnalyzer._createVulnerability('INSECURE_HASHING', { value: 'sha1' }, 888)) sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ - 'manual.keep': 'true', '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3254801297,' + '"evidence":{"value":"sha1"},"location":{"spanId":888}}]}' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should send only valid vulnerabilities', () => { @@ -191,10 +203,10 @@ describe('vulnerability-reporter', () => { iastContext.vulnerabilities.push({ invalid: 'vulnerability' }) sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ - 'manual.keep': 'true', '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3254801297,' + '"evidence":{"value":"sha1"},"location":{"spanId":888}}]}' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should send vulnerabilities with evidence, ranges and sources', () => { @@ -239,7 +251,6 @@ describe('vulnerability-reporter', () => { sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ - 'manual.keep': 'true', '_dd.iast.json': '{"sources":[{"origin":"ORIGIN_TYPE_1","name":"PARAMETER_NAME_1","value":"joe"},' + '{"origin":"ORIGIN_TYPE_2","name":"PARAMETER_NAME_2","value":"joe@mail.com"}],' + '"vulnerabilities":[{"type":"SQL_INJECTION","hash":4676753086,' + @@ -249,6 +260,7 @@ describe('vulnerability-reporter', () => { '[{"value":"SELECT id FROM u WHERE email = \'"},{"value":"joe@mail.com","source":1},{"value":"\';"}]},' + '"location":{"spanId":888,"path":"filename.js","line":99}}]}' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should send multiple vulnerabilities with same tainted source', () => { @@ -293,7 +305,6 @@ describe('vulnerability-reporter', () => { sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ - 'manual.keep': 'true', '_dd.iast.json': '{"sources":[{"origin":"ORIGIN_TYPE_1","name":"PARAMETER_NAME_1","value":"joe"}],' + '"vulnerabilities":[{"type":"SQL_INJECTION","hash":4676753086,' + '"evidence":{"valueParts":[{"value":"SELECT * FROM u WHERE name = \'"},{"value":"joe","source":0},' + @@ -302,6 +313,7 @@ describe('vulnerability-reporter', () => { '[{"value":"UPDATE u SET name=\'"},{"value":"joe","source":0},{"value":"\' WHERE id=1;"}]},' + '"location":{"spanId":888,"path":"filename.js","line":99}}]}' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should send once with multiple vulnerabilities', () => { @@ -314,7 +326,6 @@ describe('vulnerability-reporter', () => { { path: '/path/to/file3.js', line: 3 })) sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ - 'manual.keep': 'true', '_dd.iast.json': '{"sources":[],"vulnerabilities":[' + '{"type":"INSECURE_HASHING","hash":1697980169,"evidence":{"value":"sha1"},' + '"location":{"spanId":888,"path":"/path/to/file1.js","line":1}},' + @@ -323,6 +334,7 @@ describe('vulnerability-reporter', () => { '{"type":"INSECURE_HASHING","hash":1755238473,"evidence":{"value":"md5"},' + '"location":{"spanId":-5,"path":"/path/to/file3.js","line":3}}]}' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should send once vulnerability with one vulnerability', () => { @@ -332,10 +344,10 @@ describe('vulnerability-reporter', () => { { path: 'filename.js', line: 88 })) sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ - 'manual.keep': 'true', '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3410512691,' + '"evidence":{"value":"sha1"},"location":{"spanId":888,"path":"filename.js","line":88}}]}' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should not send duplicated vulnerabilities', () => { @@ -348,10 +360,10 @@ describe('vulnerability-reporter', () => { { path: 'filename.js', line: 88 })) sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ - 'manual.keep': 'true', '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3410512691,' + '"evidence":{"value":"sha1"},"location":{"spanId":888,"path":"filename.js","line":88}}]}' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should not send duplicated vulnerabilities in multiple sends', () => { @@ -365,10 +377,10 @@ describe('vulnerability-reporter', () => { sendVulnerabilities(iastContext.vulnerabilities, span) sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ - 'manual.keep': 'true', '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3410512691,' + '"evidence":{"value":"sha1"},"location":{"spanId":888,"path":"filename.js","line":88}}]}' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should not deduplicate vulnerabilities if not enabled', () => { @@ -384,12 +396,12 @@ describe('vulnerability-reporter', () => { { value: 'sha1' }, 888, { path: 'filename.js', line: 88 })) sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ - 'manual.keep': 'true', '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3410512691,' + '"evidence":{"value":"sha1"},"location":{"spanId":888,"path":"filename.js","line":88}},' + '{"type":"INSECURE_HASHING","hash":3410512691,"evidence":{"value":"sha1"},"location":' + '{"spanId":888,"path":"filename.js","line":88}}]}' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should add _dd.p.appsec trace tag with standalone enabled', () => { @@ -401,11 +413,12 @@ describe('vulnerability-reporter', () => { sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ - 'manual.keep': 'true', '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3254801297,' + '"evidence":{"value":"sha1"},"location":{"spanId":999}}]}' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) + expect(span.context()._trace.tags).to.have.property(APPSEC_PROPAGATION_KEY) }) @@ -418,11 +431,12 @@ describe('vulnerability-reporter', () => { sendVulnerabilities(iastContext.vulnerabilities, span) expect(span.addTags).to.have.been.calledOnceWithExactly({ - 'manual.keep': 'true', '_dd.iast.json': '{"sources":[],"vulnerabilities":[{"type":"INSECURE_HASHING","hash":3254801297,' + '"evidence":{"value":"sha1"},"location":{"spanId":999}}]}' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) + expect(span.context()._trace.tags).to.not.have.property(APPSEC_PROPAGATION_KEY) }) }) @@ -441,7 +455,8 @@ describe('vulnerability-reporter', () => { global.setInterval = sinon.spy(global.setInterval) global.clearInterval = sinon.spy(global.clearInterval) span = { - addTags: sinon.stub() + addTags: sinon.stub(), + keep: sinon.stub() } }) diff --git a/packages/dd-trace/test/appsec/reporter.spec.js b/packages/dd-trace/test/appsec/reporter.spec.js index 0860b2c75a..757884c356 100644 --- a/packages/dd-trace/test/appsec/reporter.spec.js +++ b/packages/dd-trace/test/appsec/reporter.spec.js @@ -3,6 +3,8 @@ const proxyquire = require('proxyquire') const { storage } = require('../../../datadog-core') const zlib = require('zlib') +const { SAMPLING_MECHANISM_APPSEC } = require('../../src/constants') +const { USER_KEEP } = require('../../../../ext/priority') describe('reporter', () => { let Reporter @@ -10,14 +12,21 @@ describe('reporter', () => { let web let telemetry let sample + let prioritySampler beforeEach(() => { + prioritySampler = { + setPriority: sinon.stub() + } + span = { + _prioritySampler: prioritySampler, context: sinon.stub().returns({ _tags: {} }), addTags: sinon.stub(), - setTag: sinon.stub() + setTag: sinon.stub(), + keep: sinon.stub() } web = { @@ -105,7 +114,6 @@ describe('reporter', () => { expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.error_count')).to.be.eq(1) expect(Reporter.metricsQueue.get('_dd.appsec.event_rules.errors')) .to.be.eq(JSON.stringify(diagnosticsRules.errors)) - expect(Reporter.metricsQueue.get('manual.keep')).to.be.eq('true') }) it('should call incrementWafInitMetric', () => { @@ -222,11 +230,11 @@ describe('reporter', () => { expect(span.addTags).to.have.been.calledOnceWithExactly({ 'appsec.event': 'true', - 'manual.keep': 'true', '_dd.origin': 'appsec', '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]}]}', 'network.client.ip': '8.8.8.8' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should not add manual.keep when rate limit is reached', (done) => { @@ -234,24 +242,23 @@ describe('reporter', () => { const params = {} expect(Reporter.reportAttack('', params)).to.not.be.false - expect(addTags.getCall(0).firstArg).to.have.property('manual.keep').that.equals('true') expect(Reporter.reportAttack('', params)).to.not.be.false - expect(addTags.getCall(1).firstArg).to.have.property('manual.keep').that.equals('true') expect(Reporter.reportAttack('', params)).to.not.be.false - expect(addTags.getCall(2).firstArg).to.have.property('manual.keep').that.equals('true') + + expect(prioritySampler.setPriority).to.have.callCount(3) Reporter.setRateLimit(1) expect(Reporter.reportAttack('', params)).to.not.be.false expect(addTags.getCall(3).firstArg).to.have.property('appsec.event').that.equals('true') - expect(addTags.getCall(3).firstArg).to.have.property('manual.keep').that.equals('true') + expect(prioritySampler.setPriority).to.have.callCount(4) expect(Reporter.reportAttack('', params)).to.not.be.false expect(addTags.getCall(4).firstArg).to.have.property('appsec.event').that.equals('true') - expect(addTags.getCall(4).firstArg).to.not.have.property('manual.keep') + expect(prioritySampler.setPriority).to.have.callCount(4) setTimeout(() => { expect(Reporter.reportAttack('', params)).to.not.be.false - expect(addTags.getCall(5).firstArg).to.have.property('manual.keep').that.equals('true') + expect(prioritySampler.setPriority).to.have.callCount(5) done() }, 1020) }) @@ -265,10 +272,10 @@ describe('reporter', () => { expect(span.addTags).to.have.been.calledOnceWithExactly({ 'appsec.event': 'true', - 'manual.keep': 'true', '_dd.appsec.json': '{"triggers":[]}', 'network.client.ip': '8.8.8.8' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should merge attacks json', () => { @@ -280,11 +287,11 @@ describe('reporter', () => { expect(span.addTags).to.have.been.calledOnceWithExactly({ 'appsec.event': 'true', - 'manual.keep': 'true', '_dd.origin': 'appsec', '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]},{"rule":{}},{"rule":{},"rule_matches":[{}]}]}', 'network.client.ip': '8.8.8.8' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should call standalone sample', () => { @@ -296,12 +303,13 @@ describe('reporter', () => { expect(span.addTags).to.have.been.calledOnceWithExactly({ 'appsec.event': 'true', - 'manual.keep': 'true', '_dd.origin': 'appsec', '_dd.appsec.json': '{"triggers":[{"rule":{},"rule_matches":[{}]},{"rule":{}},{"rule":{},"rule_matches":[{}]}]}', 'network.client.ip': '8.8.8.8' }) + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) + expect(sample).to.have.been.calledOnceWithExactly(span) }) }) @@ -642,5 +650,16 @@ describe('reporter', () => { expect(span.setTag).to.have.been.calledWithExactly('_dd.appsec.rasp.duration_ext', 321) expect(span.setTag).to.have.been.calledWithExactly('_dd.appsec.rasp.rule.eval', 3) }) + + it('should keep span if there are metrics', () => { + const req = {} + + Reporter.metricsQueue.set('a', 1) + Reporter.metricsQueue.set('b', 2) + + Reporter.finishRequest(req, wafContext, {}) + + expect(prioritySampler.setPriority).to.have.been.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) + }) }) }) diff --git a/packages/dd-trace/test/appsec/sdk/track_event.spec.js b/packages/dd-trace/test/appsec/sdk/track_event.spec.js index e3739488b8..fca01030c0 100644 --- a/packages/dd-trace/test/appsec/sdk/track_event.spec.js +++ b/packages/dd-trace/test/appsec/sdk/track_event.spec.js @@ -5,6 +5,8 @@ const agent = require('../../plugins/agent') const axios = require('axios') const tracer = require('../../../../../index') const { LOGIN_SUCCESS, LOGIN_FAILURE } = require('../../../src/appsec/addresses') +const { SAMPLING_MECHANISM_APPSEC } = require('../../../src/constants') +const { USER_KEEP } = require('../../../../../ext/priority') describe('track_event', () => { describe('Internal API', () => { @@ -16,14 +18,21 @@ describe('track_event', () => { let trackUserLoginSuccessEvent, trackUserLoginFailureEvent, trackCustomEvent, trackEvent let sample let waf + let prioritySampler beforeEach(() => { log = { warn: sinon.stub() } + prioritySampler = { + setPriority: sinon.stub() + } + rootSpan = { - addTags: sinon.stub() + _prioritySampler: prioritySampler, + addTags: sinon.stub(), + keep: sinon.stub() } getRootSpan = sinon.stub().callsFake(() => rootSpan) @@ -96,12 +105,13 @@ describe('track_event', () => { expect(rootSpan.addTags).to.have.been.calledOnceWithExactly( { 'appsec.events.users.login.success.track': 'true', - 'manual.keep': 'true', '_dd.appsec.events.users.login.success.sdk': 'true', 'appsec.events.users.login.success.metakey1': 'metaValue1', 'appsec.events.users.login.success.metakey2': 'metaValue2', 'appsec.events.users.login.success.metakey3': 'metaValue3' }) + expect(prioritySampler.setPriority) + .to.have.been.calledOnceWithExactly(rootSpan, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should call setUser and addTags without metadata', () => { @@ -113,9 +123,10 @@ describe('track_event', () => { expect(setUserTags).to.have.been.calledOnceWithExactly(user, rootSpan) expect(rootSpan.addTags).to.have.been.calledOnceWithExactly({ 'appsec.events.users.login.success.track': 'true', - 'manual.keep': 'true', '_dd.appsec.events.users.login.success.sdk': 'true' }) + expect(prioritySampler.setPriority) + .to.have.been.calledOnceWithExactly(rootSpan, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should call waf run with login success address', () => { @@ -161,7 +172,6 @@ describe('track_event', () => { expect(setUserTags).to.not.have.been.called expect(rootSpan.addTags).to.have.been.calledOnceWithExactly({ 'appsec.events.users.login.failure.track': 'true', - 'manual.keep': 'true', '_dd.appsec.events.users.login.failure.sdk': 'true', 'appsec.events.users.login.failure.usr.id': 'user_id', 'appsec.events.users.login.failure.usr.exists': 'true', @@ -169,6 +179,8 @@ describe('track_event', () => { 'appsec.events.users.login.failure.metakey2': 'metaValue2', 'appsec.events.users.login.failure.metakey3': 'metaValue3' }) + expect(prioritySampler.setPriority) + .to.have.been.calledOnceWithExactly(rootSpan, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should send false `usr.exists` property when the user does not exist', () => { @@ -180,7 +192,6 @@ describe('track_event', () => { expect(setUserTags).to.not.have.been.called expect(rootSpan.addTags).to.have.been.calledOnceWithExactly({ 'appsec.events.users.login.failure.track': 'true', - 'manual.keep': 'true', '_dd.appsec.events.users.login.failure.sdk': 'true', 'appsec.events.users.login.failure.usr.id': 'user_id', 'appsec.events.users.login.failure.usr.exists': 'false', @@ -188,6 +199,8 @@ describe('track_event', () => { 'appsec.events.users.login.failure.metakey2': 'metaValue2', 'appsec.events.users.login.failure.metakey3': 'metaValue3' }) + expect(prioritySampler.setPriority) + .to.have.been.calledOnceWithExactly(rootSpan, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should call addTags without metadata', () => { @@ -197,11 +210,12 @@ describe('track_event', () => { expect(setUserTags).to.not.have.been.called expect(rootSpan.addTags).to.have.been.calledOnceWithExactly({ 'appsec.events.users.login.failure.track': 'true', - 'manual.keep': 'true', '_dd.appsec.events.users.login.failure.sdk': 'true', 'appsec.events.users.login.failure.usr.id': 'user_id', 'appsec.events.users.login.failure.usr.exists': 'true' }) + expect(prioritySampler.setPriority) + .to.have.been.calledOnceWithExactly(rootSpan, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should call waf run with login failure address', () => { @@ -241,11 +255,12 @@ describe('track_event', () => { expect(setUserTags).to.not.have.been.called expect(rootSpan.addTags).to.have.been.calledOnceWithExactly({ 'appsec.events.custom_event.track': 'true', - 'manual.keep': 'true', '_dd.appsec.events.custom_event.sdk': 'true', 'appsec.events.custom_event.metaKey1': 'metaValue1', 'appsec.events.custom_event.metakey2': 'metaValue2' }) + expect(prioritySampler.setPriority) + .to.have.been.calledOnceWithExactly(rootSpan, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should call addTags without metadata', () => { @@ -255,9 +270,10 @@ describe('track_event', () => { expect(setUserTags).to.not.have.been.called expect(rootSpan.addTags).to.have.been.calledOnceWithExactly({ 'appsec.events.custom_event.track': 'true', - 'manual.keep': 'true', '_dd.appsec.events.custom_event.sdk': 'true' }) + expect(prioritySampler.setPriority) + .to.have.been.calledOnceWithExactly(rootSpan, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) }) @@ -266,31 +282,34 @@ describe('track_event', () => { trackEvent('event', { metaKey1: 'metaValue1', metakey2: 'metaValue2' }, 'trackEvent', rootSpan, 'safe') expect(rootSpan.addTags).to.have.been.calledOnceWithExactly({ 'appsec.events.event.track': 'true', - 'manual.keep': 'true', '_dd.appsec.events.event.auto.mode': 'safe', 'appsec.events.event.metaKey1': 'metaValue1', 'appsec.events.event.metakey2': 'metaValue2' }) + expect(prioritySampler.setPriority) + .to.have.been.calledOnceWithExactly(rootSpan, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should call addTags with extended mode', () => { trackEvent('event', { metaKey1: 'metaValue1', metakey2: 'metaValue2' }, 'trackEvent', rootSpan, 'extended') expect(rootSpan.addTags).to.have.been.calledOnceWithExactly({ 'appsec.events.event.track': 'true', - 'manual.keep': 'true', '_dd.appsec.events.event.auto.mode': 'extended', 'appsec.events.event.metaKey1': 'metaValue1', 'appsec.events.event.metakey2': 'metaValue2' }) + expect(prioritySampler.setPriority) + .to.have.been.calledOnceWithExactly(rootSpan, USER_KEEP, SAMPLING_MECHANISM_APPSEC) }) it('should call standalone sample', () => { trackEvent('event', undefined, 'trackEvent', rootSpan, undefined) expect(rootSpan.addTags).to.have.been.calledOnceWithExactly({ - 'appsec.events.event.track': 'true', - 'manual.keep': 'true' + 'appsec.events.event.track': 'true' }) + expect(prioritySampler.setPriority) + .to.have.been.calledOnceWithExactly(rootSpan, USER_KEEP, SAMPLING_MECHANISM_APPSEC) expect(sample).to.have.been.calledOnceWithExactly(rootSpan) }) }) @@ -339,7 +358,7 @@ describe('track_event', () => { expect(traces[0][0].meta).to.have.property('appsec.events.users.login.success.track', 'true') expect(traces[0][0].meta).to.have.property('usr.id', 'test_user_id') expect(traces[0][0].meta).to.have.property('appsec.events.users.login.success.metakey', 'metaValue') - expect(traces[0][0].meta).to.have.property('manual.keep', 'true') + expect(traces[0][0].metrics).to.have.property('_sampling_priority_v1', USER_KEEP) }).then(done).catch(done) axios.get(`http://localhost:${port}/`) }) @@ -377,7 +396,7 @@ describe('track_event', () => { expect(traces[0][0].meta).to.have.property('appsec.events.users.login.failure.usr.id', 'test_user_id') expect(traces[0][0].meta).to.have.property('appsec.events.users.login.failure.usr.exists', 'true') expect(traces[0][0].meta).to.have.property('appsec.events.users.login.failure.metakey', 'metaValue') - expect(traces[0][0].meta).to.have.property('manual.keep', 'true') + expect(traces[0][0].metrics).to.have.property('_sampling_priority_v1', USER_KEEP) }).then(done).catch(done) axios.get(`http://localhost:${port}/`) }) @@ -392,7 +411,7 @@ describe('track_event', () => { expect(traces[0][0].meta).to.have.property('appsec.events.users.login.failure.usr.id', 'test_user_id') expect(traces[0][0].meta).to.have.property('appsec.events.users.login.failure.usr.exists', 'false') expect(traces[0][0].meta).to.have.property('appsec.events.users.login.failure.metakey', 'metaValue') - expect(traces[0][0].meta).to.have.property('manual.keep', 'true') + expect(traces[0][0].metrics).to.have.property('_sampling_priority_v1', USER_KEEP) }).then(done).catch(done) axios.get(`http://localhost:${port}/`) }) @@ -428,7 +447,7 @@ describe('track_event', () => { agent.use(traces => { expect(traces[0][0].meta).to.have.property('appsec.events.my-custom-event.track', 'true') expect(traces[0][0].meta).to.have.property('appsec.events.my-custom-event.metakey', 'metaValue') - expect(traces[0][0].meta).to.have.property('manual.keep', 'true') + expect(traces[0][0].metrics).to.have.property('_sampling_priority_v1', USER_KEEP) }).then(done).catch(done) axios.get(`http://localhost:${port}/`) }) @@ -440,7 +459,7 @@ describe('track_event', () => { res.end() } agent.use(traces => { - expect(traces[0][0].meta).to.not.have.property('manual.keep', 'true') + expect(traces[0][0].metrics).to.not.have.property('_sampling_priority_v1', USER_KEEP) }).then(done).catch(done) axios.get(`http://localhost:${port}/`) }) diff --git a/packages/dd-trace/test/appsec/waf/index.spec.js b/packages/dd-trace/test/appsec/waf/index.spec.js index aff0a7e37a..33c0bfbb3a 100644 --- a/packages/dd-trace/test/appsec/waf/index.spec.js +++ b/packages/dd-trace/test/appsec/waf/index.spec.js @@ -81,7 +81,6 @@ describe('WAF Manager', () => { expect(Reporter.metricsQueue.set).to.been.calledWithExactly('_dd.appsec.event_rules.loaded', 1) expect(Reporter.metricsQueue.set).to.been.calledWithExactly('_dd.appsec.event_rules.error_count', 0) expect(Reporter.metricsQueue.set).not.to.been.calledWith('_dd.appsec.event_rules.errors') - expect(Reporter.metricsQueue.set).to.been.calledWithExactly('manual.keep', 'true') }) it('should set init metrics with errors', () => { @@ -104,7 +103,6 @@ describe('WAF Manager', () => { expect(Reporter.metricsQueue.set).to.been.calledWithExactly('_dd.appsec.event_rules.error_count', 2) expect(Reporter.metricsQueue.set).to.been.calledWithExactly('_dd.appsec.event_rules.errors', '{"error_1":["invalid_1"],"error_2":["invalid_2","invalid_3"]}') - expect(Reporter.metricsQueue.set).to.been.calledWithExactly('manual.keep', 'true') }) }) diff --git a/packages/dd-trace/test/priority_sampler.spec.js b/packages/dd-trace/test/priority_sampler.spec.js index 5000d81ff0..88c134a575 100644 --- a/packages/dd-trace/test/priority_sampler.spec.js +++ b/packages/dd-trace/test/priority_sampler.spec.js @@ -11,7 +11,8 @@ const { SAMPLING_MECHANISM_MANUAL, SAMPLING_MECHANISM_REMOTE_USER, SAMPLING_MECHANISM_REMOTE_DYNAMIC, - DECISION_MAKER_KEY + DECISION_MAKER_KEY, + SAMPLING_MECHANISM_APPSEC } = require('../src/constants') const SERVICE_NAME = ext.tags.SERVICE_NAME @@ -451,4 +452,61 @@ describe('PrioritySampler', () => { expect(context._sampling.mechanism).to.equal(SAMPLING_MECHANISM_AGENT) }) }) + + describe('setPriority', () => { + it('should set sampling priority and default mechanism', () => { + prioritySampler.setPriority(span, USER_KEEP) + + expect(context._sampling.priority).to.equal(USER_KEEP) + expect(context._sampling.mechanism).to.equal(SAMPLING_MECHANISM_MANUAL) + }) + + it('should set sampling priority and mechanism', () => { + prioritySampler.setPriority(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) + + expect(context._sampling.priority).to.equal(USER_KEEP) + expect(context._sampling.mechanism).to.equal(SAMPLING_MECHANISM_APPSEC) + }) + + it('should filter out invalid priorities', () => { + prioritySampler.setPriority(span, 42) + + expect(context._sampling.priority).to.be.undefined + expect(context._sampling.mechanism).to.be.undefined + }) + + it('should add decision maker tag if not set before', () => { + prioritySampler.setPriority(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) + + expect(context._trace.tags[DECISION_MAKER_KEY]).to.equal('-5') + }) + + it('should override previous priority but mantain previous decision maker tag', () => { + prioritySampler.sample(span) + + prioritySampler.setPriority(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) + + expect(context._sampling.priority).to.equal(USER_KEEP) + expect(context._sampling.mechanism).to.equal(SAMPLING_MECHANISM_APPSEC) + expect(context._trace.tags[DECISION_MAKER_KEY]).to.equal('-0') + }) + }) + + describe('keepTrace', () => { + it('should not fail if no _prioritySampler', () => { + expect(() => { + PrioritySampler.keepTrace(span, SAMPLING_MECHANISM_APPSEC) + }).to.not.throw() + }) + + it('should call setPriority with span USER_KEEP and mechanism', () => { + const setPriority = sinon.stub(prioritySampler, 'setPriority') + + span._prioritySampler = prioritySampler + + PrioritySampler.keepTrace(span, SAMPLING_MECHANISM_APPSEC) + + expect(setPriority).to.be.calledOnceWithExactly(span, USER_KEEP, SAMPLING_MECHANISM_APPSEC) + }) + }) })