Skip to content

Commit

Permalink
Merge branch 'automatic_userid_blocking' into appsec_session_id
Browse files Browse the repository at this point in the history
  • Loading branch information
simon-id authored Jan 20, 2025
2 parents a0031d6 + 03e18df commit bb5b2e7
Show file tree
Hide file tree
Showing 17 changed files with 136 additions and 94 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
/* eslint-disable */
const sum = require('./dependency')
const isJest = require('./is-jest')
const { expect } = require('chai')

// TODO: instead of retrying through jest, this should be retried with auto test retries
if (isJest()) {
jest.retryTimes(1)
}

let count = 0
describe('dynamic-instrumentation', () => {
it('retries with DI', function () {
if (this.retries) {
this.retries(1)
if (process.env.TEST_SHOULD_PASS_AFTER_RETRY && count++ === 1) {
// Passes after a retry if TEST_SHOULD_PASS_AFTER_RETRY is passed
expect(sum(1, 3)).to.equal(4)
} else {
expect(sum(11, 3)).to.equal(14)
}
expect(sum(11, 3)).to.equal(14)
})

it('is not retried', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,10 @@
/* eslint-disable */
const sum = require('./dependency')
const isJest = require('./is-jest')
const { expect } = require('chai')

// TODO: instead of retrying through jest, this should be retried with auto test retries
if (isJest()) {
jest.retryTimes(1)
}

let count = 0
describe('dynamic-instrumentation', () => {
it('retries with DI', function () {
if (this.retries) {
this.retries(1)
}
const willFail = count++ === 0
if (willFail) {
expect(sum(11, 3)).to.equal(14) // only throws the first time
Expand Down
53 changes: 48 additions & 5 deletions integration-tests/jest/jest.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ describe('jest CommonJS', () => {
const retriedTests = tests.filter(test => test.meta[TEST_IS_RETRY] === 'true')

assert.equal(retriedTests.length, 2)
const [retriedTest] = retriedTests
const retriedTest = retriedTests.find(test => test.meta[TEST_SUITE].includes('test-hit-breakpoint.js'))

assert.propertyVal(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED, 'true')

Expand Down Expand Up @@ -560,6 +560,7 @@ describe('jest CommonJS', () => {
...getCiVisAgentlessConfig(receiver.port),
TESTS_TO_RUN: 'dynamic-instrumentation/test-',
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1',
RUN_IN_PARALLEL: true
},
stdio: 'inherit'
Expand Down Expand Up @@ -2518,7 +2519,8 @@ describe('jest CommonJS', () => {
cwd,
env: {
...getCiVisAgentlessConfig(receiver.port),
TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint'
TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
},
stdio: 'inherit'
}
Expand Down Expand Up @@ -2565,7 +2567,8 @@ describe('jest CommonJS', () => {
env: {
...getCiVisAgentlessConfig(receiver.port),
TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
},
stdio: 'inherit'
}
Expand Down Expand Up @@ -2649,7 +2652,8 @@ describe('jest CommonJS', () => {
env: {
...getCiVisAgentlessConfig(receiver.port),
TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
},
stdio: 'inherit'
}
Expand Down Expand Up @@ -2698,7 +2702,8 @@ describe('jest CommonJS', () => {
env: {
...getCiVisAgentlessConfig(receiver.port),
TESTS_TO_RUN: 'dynamic-instrumentation/test-not-hit-breakpoint',
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
},
stdio: 'inherit'
}
Expand All @@ -2711,6 +2716,44 @@ describe('jest CommonJS', () => {
}).catch(done)
})
})

it('does not wait for breakpoint for a passed test', (done) => {
receiver.setSettings({
flaky_test_retries_enabled: true,
di_enabled: true
})

const eventsPromise = receiver
.gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/citestcycle'), (payloads) => {
const events = payloads.flatMap(({ payload }) => payload.events)

const tests = events.filter(event => event.type === 'test').map(event => event.content)
const retriedTests = tests.filter(test => test.meta[TEST_IS_RETRY] === 'true')

assert.equal(retriedTests.length, 1)
const [retriedTest] = retriedTests
// Duration is in nanoseconds, so 200 * 1e6 is 200ms
assert.equal(retriedTest.duration < 200 * 1e6, true)
})

childProcess = exec(runTestsWithCoverageCommand,
{
cwd,
env: {
...getCiVisAgentlessConfig(receiver.port),
TESTS_TO_RUN: 'dynamic-instrumentation/test-hit-breakpoint',
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1',
TEST_SHOULD_PASS_AFTER_RETRY: '1'
},
stdio: 'inherit'
}
)

childProcess.on('exit', () => {
eventsPromise.then(() => done()).catch(done)
})
})
})

// This happens when using office-addin-mock
Expand Down
12 changes: 8 additions & 4 deletions integration-tests/mocha/mocha.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2188,7 +2188,8 @@ describe('mocha CommonJS', function () {
...getCiVisAgentlessConfig(receiver.port),
TESTS_TO_RUN: JSON.stringify([
'./dynamic-instrumentation/test-hit-breakpoint'
])
]),
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
},
stdio: 'inherit'
}
Expand Down Expand Up @@ -2240,7 +2241,8 @@ describe('mocha CommonJS', function () {
TESTS_TO_RUN: JSON.stringify([
'./dynamic-instrumentation/test-hit-breakpoint'
]),
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
},
stdio: 'inherit'
}
Expand Down Expand Up @@ -2329,7 +2331,8 @@ describe('mocha CommonJS', function () {
TESTS_TO_RUN: JSON.stringify([
'./dynamic-instrumentation/test-hit-breakpoint'
]),
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
},
stdio: 'inherit'
}
Expand Down Expand Up @@ -2382,7 +2385,8 @@ describe('mocha CommonJS', function () {
TESTS_TO_RUN: JSON.stringify([
'./dynamic-instrumentation/test-not-hit-breakpoint'
]),
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true',
DD_CIVISIBILITY_FLAKY_RETRY_COUNT: '1'
},
stdio: 'inherit'
}
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/vitest/vitest.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const { DD_HOST_CPU_COUNT } = require('../../packages/dd-trace/src/plugins/util/

const NUM_RETRIES_EFD = 3

const versions = ['2.1.8'] // was previously 'latest', but v3 breaks this test
const versions = ['1.6.0', 'latest']

const linePctMatchRegex = /Lines\s+:\s+([\d.]+)%/

Expand Down
4 changes: 2 additions & 2 deletions packages/datadog-instrumentations/src/jest.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ function getWrappedEnvironment (BaseEnvironment, jestVersion) {
const numRetries = this.global[RETRY_TIMES]
const numTestExecutions = event.test?.invocations
const willBeRetried = numRetries > 0 && numTestExecutions - 1 < numRetries
const mightHitBreakpoint = this.isDiEnabled && numTestExecutions >= 1
const mightHitBreakpoint = this.isDiEnabled && numTestExecutions >= 2

const asyncResource = asyncResources.get(event.test)

Expand All @@ -319,7 +319,7 @@ function getWrappedEnvironment (BaseEnvironment, jestVersion) {

// After finishing it might take a bit for the snapshot to be handled.
// This means that tests retried with DI are BREAKPOINT_HIT_GRACE_PERIOD_MS slower at least.
if (mightHitBreakpoint) {
if (status === 'fail' && mightHitBreakpoint) {
await new Promise(resolve => {
setTimeout(() => {
resolve()
Expand Down
37 changes: 25 additions & 12 deletions packages/datadog-instrumentations/src/vitest.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,9 @@ function getSortWrapper (sort) {
const knownTestsResponse = await getChannelPromise(knownTestsCh)
if (!knownTestsResponse.err) {
knownTests = knownTestsResponse.knownTests
const testFilepaths = await this.ctx.getTestFilepaths()
const getFilePaths = this.ctx.getTestFilepaths || this.ctx._globTestFilepaths

const testFilepaths = await getFilePaths.call(this.ctx)

isEarlyFlakeDetectionFaultyCh.publish({
knownTests: knownTests.vitest || {},
Expand Down Expand Up @@ -492,15 +494,6 @@ addHook({
return vitestPackage
})

addHook({
name: 'vitest',
versions: ['>=2.1.0'],
filePattern: 'dist/chunks/RandomSequencer.*'
}, (randomSequencerPackage) => {
shimmer.wrap(randomSequencerPackage.B.prototype, 'sort', getSortWrapper)
return randomSequencerPackage
})

addHook({
name: 'vitest',
versions: ['>=2.0.5 <2.1.0'],
Expand All @@ -513,6 +506,24 @@ addHook({
return vitestPackage
})

addHook({
name: 'vitest',
versions: ['>=2.1.0 <3.0.0'],
filePattern: 'dist/chunks/RandomSequencer.*'
}, (randomSequencerPackage) => {
shimmer.wrap(randomSequencerPackage.B.prototype, 'sort', getSortWrapper)
return randomSequencerPackage
})

addHook({
name: 'vitest',
versions: ['>=3.0.0'],
filePattern: 'dist/chunks/resolveConfig.*'
}, (randomSequencerPackage) => {
shimmer.wrap(randomSequencerPackage.B.prototype, 'sort', getSortWrapper)
return randomSequencerPackage
})

// Can't specify file because compiled vitest includes hashes in their files
addHook({
name: 'vitest',
Expand All @@ -533,15 +544,17 @@ addHook({
versions: ['>=1.6.0'],
file: 'dist/index.js'
}, (vitestPackage, frameworkVersion) => {
shimmer.wrap(vitestPackage, 'startTests', startTests => async function (testPath) {
shimmer.wrap(vitestPackage, 'startTests', startTests => async function (testPaths) {
let testSuiteError = null
if (!testSuiteStartCh.hasSubscribers) {
return startTests.apply(this, arguments)
}
// From >=3.0.1, the first arguments changes from a string to an object containing the filepath
const testSuiteAbsolutePath = testPaths[0]?.filepath || testPaths[0]

const testSuiteAsyncResource = new AsyncResource('bound-anonymous-fn')
testSuiteAsyncResource.runInAsyncScope(() => {
testSuiteStartCh.publish({ testSuiteAbsolutePath: testPath[0], frameworkVersion })
testSuiteStartCh.publish({ testSuiteAbsolutePath, frameworkVersion })
})
const startTestsResponse = await startTests.apply(this, arguments)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,16 @@ class TestApiManualPlugin extends CiPlugin {

constructor (...args) {
super(...args)
this._isEnvDataCalcualted = false
this.sourceRoot = process.cwd()

this.addSub('dd-trace:ci:manual:test:start', ({ testName, testSuite }) => {
this.unconfiguredAddSub('dd-trace:ci:manual:test:start', ({ testName, testSuite }) => {
const store = storage.getStore()
const testSuiteRelative = getTestSuitePath(testSuite, this.sourceRoot)
const testSpan = this.startTestSpan(testName, testSuiteRelative)
this.enter(testSpan, store)
})
this.addSub('dd-trace:ci:manual:test:finish', ({ status, error }) => {
this.unconfiguredAddSub('dd-trace:ci:manual:test:finish', ({ status, error }) => {
const store = storage.getStore()
const testSpan = store && store.span
if (testSpan) {
Expand All @@ -33,14 +34,30 @@ class TestApiManualPlugin extends CiPlugin {
finishAllTraceSpans(testSpan)
}
})
this.addSub('dd-trace:ci:manual:test:addTags', (tags) => {
this.unconfiguredAddSub('dd-trace:ci:manual:test:addTags', (tags) => {
const store = storage.getStore()
const testSpan = store && store.span
if (testSpan) {
testSpan.addTags(tags)
}
})
}

// To lazily calculate env data.
unconfiguredAddSub (channelName, handler) {
this.addSub(channelName, (...args) => {
if (!this._isEnvDataCalcualted) {
this._isEnvDataCalcualted = true
this.configure(this._config, true)
}
return handler(...args)
})
}

configure (config, shouldGetEnvironmentData) {
this._config = config
super.configure(config, shouldGetEnvironmentData)
}
}

module.exports = TestApiManualPlugin
5 changes: 2 additions & 3 deletions packages/dd-trace/src/debugger/devtools_client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,9 @@ session.on('Debugger.paused', async ({ params }) => {
}
}

ackEmitting(probe)
// TODO: Process template (DEBUG-2628)
send(probe.template, logger, dd, snapshot, () => {
ackEmitting(probe)
})
send(probe.template, logger, dd, snapshot)
}
})

Expand Down
10 changes: 2 additions & 8 deletions packages/dd-trace/src/debugger/devtools_client/send.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ const ddtags = [

const path = `/debugger/v1/input?${stringify({ ddtags })}`

let callbacks = []
const jsonBuffer = new JSONBuffer({ size: config.maxTotalPayloadSize, timeout: 1000, onFlush })

function send (message, logger, dd, snapshot, cb) {
function send (message, logger, dd, snapshot) {
const payload = {
ddsource,
hostname,
Expand All @@ -58,7 +57,6 @@ function send (message, logger, dd, snapshot, cb) {
}

jsonBuffer.write(json, size)
callbacks.push(cb)
}

function onFlush (payload) {
Expand All @@ -69,11 +67,7 @@ function onFlush (payload) {
headers: { 'Content-Type': 'application/json; charset=utf-8' }
}

const _callbacks = callbacks
callbacks = []

request(payload, opts, (err) => {
if (err) log.error('Could not send debugger payload', err)
else _callbacks.forEach(cb => cb())
if (err) log.error('[debugger:devtools_client] Error sending probe payload', err)
})
}
Loading

0 comments on commit bb5b2e7

Please sign in to comment.