Skip to content

Commit ccc13e2

Browse files
juan-fernandezjuan-fernandez
authored
[test optimization] Add Dynamic Instrumentation to mocha retries (#4944)
1 parent b6c11a6 commit ccc13e2

File tree

10 files changed

+330
-52
lines changed

10 files changed

+330
-52
lines changed

‎integration-tests/ci-visibility/dynamic-instrumentation/is-jest.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
module.exports = function () {
2+
try {
3+
return typeof jest !== 'undefined'
4+
} catch (e) {
5+
return false
6+
}
7+
}

‎integration-tests/ci-visibility/dynamic-instrumentation/test-hit-breakpoint.js

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,22 @@
11
/* eslint-disable */
22
const sum = require('./dependency')
3+
const isJest = require('./is-jest')
4+
const { expect } = require('chai')
35

46
// TODO: instead of retrying through jest, this should be retried with auto test retries
5-
jest.retryTimes(1)
7+
if (isJest()) {
8+
jest.retryTimes(1)
9+
}
610

711
describe('dynamic-instrumentation', () => {
8-
it('retries with DI', () => {
9-
expect(sum(11, 3)).toEqual(14)
12+
it('retries with DI', function () {
13+
if (this.retries) {
14+
this.retries(1)
15+
}
16+
expect(sum(11, 3)).to.equal(14)
1017
})
1118

1219
it('is not retried', () => {
13-
expect(sum(1, 2)).toEqual(3)
20+
expect(1 + 2).to.equal(3)
1421
})
1522
})

‎integration-tests/ci-visibility/dynamic-instrumentation/test-not-hit-breakpoint.js

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,24 @@
11
/* eslint-disable */
22
const sum = require('./dependency')
3+
const isJest = require('./is-jest')
4+
const { expect } = require('chai')
35

46
// TODO: instead of retrying through jest, this should be retried with auto test retries
5-
jest.retryTimes(1)
7+
if (isJest()) {
8+
jest.retryTimes(1)
9+
}
610

711
let count = 0
812
describe('dynamic-instrumentation', () => {
9-
it('retries with DI', () => {
13+
it('retries with DI', function () {
14+
if (this.retries) {
15+
this.retries(1)
16+
}
1017
const willFail = count++ === 0
1118
if (willFail) {
12-
expect(sum(11, 3)).toEqual(14) // only throws the first time
19+
expect(sum(11, 3)).to.equal(14) // only throws the first time
1320
} else {
14-
expect(sum(1, 2)).toEqual(3)
21+
expect(sum(1, 2)).to.equal(3)
1522
}
1623
})
1724
})

‎integration-tests/jest/jest.spec.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2416,7 +2416,7 @@ describe('jest CommonJS', () => {
24162416
itr_enabled: false,
24172417
code_coverage: false,
24182418
tests_skipping: false,
2419-
flaky_test_retries_enabled: true,
2419+
flaky_test_retries_enabled: false,
24202420
early_flake_detection: {
24212421
enabled: false
24222422
}
@@ -2468,7 +2468,7 @@ describe('jest CommonJS', () => {
24682468
itr_enabled: false,
24692469
code_coverage: false,
24702470
tests_skipping: false,
2471-
flaky_test_retries_enabled: true,
2471+
flaky_test_retries_enabled: false,
24722472
early_flake_detection: {
24732473
enabled: false
24742474
}
@@ -2558,7 +2558,7 @@ describe('jest CommonJS', () => {
25582558
itr_enabled: false,
25592559
code_coverage: false,
25602560
tests_skipping: false,
2561-
flaky_test_retries_enabled: true,
2561+
flaky_test_retries_enabled: false,
25622562
early_flake_detection: {
25632563
enabled: false
25642564
}

‎integration-tests/mocha/mocha.spec.js

Lines changed: 219 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,11 @@ const {
3535
TEST_CODE_OWNERS,
3636
TEST_SESSION_NAME,
3737
TEST_LEVEL_EVENT_TYPES,
38-
TEST_EARLY_FLAKE_ABORT_REASON
38+
TEST_EARLY_FLAKE_ABORT_REASON,
39+
DI_ERROR_DEBUG_INFO_CAPTURED,
40+
DI_DEBUG_ERROR_FILE,
41+
DI_DEBUG_ERROR_SNAPSHOT_ID,
42+
DI_DEBUG_ERROR_LINE
3943
} = require('../../packages/dd-trace/src/plugins/util/test')
4044
const { DD_HOST_CPU_COUNT } = require('../../packages/dd-trace/src/plugins/util/env')
4145
const { ERROR_MESSAGE } = require('../../packages/dd-trace/src/constants')
@@ -2144,4 +2148,218 @@ describe('mocha CommonJS', function () {
21442148
})
21452149
})
21462150
})
2151+
2152+
context('dynamic instrumentation', () => {
2153+
it('does not activate dynamic instrumentation if DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED is not set', (done) => {
2154+
receiver.setSettings({
2155+
itr_enabled: false,
2156+
code_coverage: false,
2157+
tests_skipping: false,
2158+
flaky_test_retries_enabled: false,
2159+
early_flake_detection: {
2160+
enabled: false
2161+
}
2162+
// di_enabled: true // TODO
2163+
})
2164+
2165+
const eventsPromise = receiver
2166+
.gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/citestcycle'), (payloads) => {
2167+
const events = payloads.flatMap(({ payload }) => payload.events)
2168+
2169+
const tests = events.filter(event => event.type === 'test').map(event => event.content)
2170+
const retriedTests = tests.filter(test => test.meta[TEST_IS_RETRY] === 'true')
2171+
2172+
assert.equal(retriedTests.length, 1)
2173+
const [retriedTest] = retriedTests
2174+
2175+
assert.notProperty(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED)
2176+
assert.notProperty(retriedTest.meta, DI_DEBUG_ERROR_FILE)
2177+
assert.notProperty(retriedTest.metrics, DI_DEBUG_ERROR_LINE)
2178+
assert.notProperty(retriedTest.meta, DI_DEBUG_ERROR_SNAPSHOT_ID)
2179+
})
2180+
2181+
const logsPromise = receiver
2182+
.gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/logs'), (payloads) => {
2183+
if (payloads.length > 0) {
2184+
throw new Error('Unexpected logs')
2185+
}
2186+
}, 5000)
2187+
2188+
childProcess = exec(
2189+
runTestsWithCoverageCommand,
2190+
{
2191+
cwd,
2192+
env: {
2193+
...getCiVisAgentlessConfig(receiver.port),
2194+
TESTS_TO_RUN: JSON.stringify([
2195+
'./dynamic-instrumentation/test-hit-breakpoint'
2196+
])
2197+
},
2198+
stdio: 'inherit'
2199+
}
2200+
)
2201+
2202+
childProcess.on('exit', (code) => {
2203+
Promise.all([eventsPromise, logsPromise]).then(() => {
2204+
assert.equal(code, 0)
2205+
done()
2206+
}).catch(done)
2207+
})
2208+
})
2209+
2210+
it('runs retries with dynamic instrumentation', (done) => {
2211+
receiver.setSettings({
2212+
itr_enabled: false,
2213+
code_coverage: false,
2214+
tests_skipping: false,
2215+
flaky_test_retries_enabled: false,
2216+
early_flake_detection: {
2217+
enabled: false
2218+
}
2219+
// di_enabled: true // TODO
2220+
})
2221+
2222+
let snapshotIdByTest, snapshotIdByLog
2223+
let spanIdByTest, spanIdByLog, traceIdByTest, traceIdByLog
2224+
2225+
const eventsPromise = receiver
2226+
.gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/citestcycle'), (payloads) => {
2227+
const events = payloads.flatMap(({ payload }) => payload.events)
2228+
2229+
const tests = events.filter(event => event.type === 'test').map(event => event.content)
2230+
const retriedTests = tests.filter(test => test.meta[TEST_IS_RETRY] === 'true')
2231+
2232+
assert.equal(retriedTests.length, 1)
2233+
const [retriedTest] = retriedTests
2234+
2235+
assert.propertyVal(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED, 'true')
2236+
assert.propertyVal(
2237+
retriedTest.meta,
2238+
DI_DEBUG_ERROR_FILE,
2239+
'ci-visibility/dynamic-instrumentation/dependency.js'
2240+
)
2241+
assert.equal(retriedTest.metrics[DI_DEBUG_ERROR_LINE], 4)
2242+
assert.exists(retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID])
2243+
2244+
snapshotIdByTest = retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID]
2245+
spanIdByTest = retriedTest.span_id.toString()
2246+
traceIdByTest = retriedTest.trace_id.toString()
2247+
2248+
const notRetriedTest = tests.find(test => test.meta[TEST_NAME].includes('is not retried'))
2249+
2250+
assert.notProperty(notRetriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED)
2251+
})
2252+
2253+
const logsPromise = receiver
2254+
.gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/logs'), (payloads) => {
2255+
const [{ logMessage: [diLog] }] = payloads
2256+
assert.deepInclude(diLog, {
2257+
ddsource: 'dd_debugger',
2258+
level: 'error'
2259+
})
2260+
assert.equal(diLog.debugger.snapshot.language, 'javascript')
2261+
assert.deepInclude(diLog.debugger.snapshot.captures.lines['4'].locals, {
2262+
a: {
2263+
type: 'number',
2264+
value: '11'
2265+
},
2266+
b: {
2267+
type: 'number',
2268+
value: '3'
2269+
},
2270+
localVariable: {
2271+
type: 'number',
2272+
value: '2'
2273+
}
2274+
})
2275+
spanIdByLog = diLog.dd.span_id
2276+
traceIdByLog = diLog.dd.trace_id
2277+
snapshotIdByLog = diLog.debugger.snapshot.id
2278+
}, 5000)
2279+
2280+
childProcess = exec(
2281+
'node ./ci-visibility/run-mocha.js',
2282+
{
2283+
cwd,
2284+
env: {
2285+
...getCiVisAgentlessConfig(receiver.port),
2286+
TESTS_TO_RUN: JSON.stringify([
2287+
'./dynamic-instrumentation/test-hit-breakpoint'
2288+
]),
2289+
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
2290+
},
2291+
stdio: 'inherit'
2292+
}
2293+
)
2294+
2295+
childProcess.on('exit', () => {
2296+
Promise.all([eventsPromise, logsPromise]).then(() => {
2297+
assert.equal(snapshotIdByTest, snapshotIdByLog)
2298+
assert.equal(spanIdByTest, spanIdByLog)
2299+
assert.equal(traceIdByTest, traceIdByLog)
2300+
done()
2301+
}).catch(done)
2302+
})
2303+
})
2304+
2305+
it('does not crash if the retry does not hit the breakpoint', (done) => {
2306+
receiver.setSettings({
2307+
itr_enabled: false,
2308+
code_coverage: false,
2309+
tests_skipping: false,
2310+
flaky_test_retries_enabled: false,
2311+
early_flake_detection: {
2312+
enabled: false
2313+
}
2314+
// di_enabled: true // TODO
2315+
})
2316+
2317+
const eventsPromise = receiver
2318+
.gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/citestcycle'), (payloads) => {
2319+
const events = payloads.flatMap(({ payload }) => payload.events)
2320+
2321+
const tests = events.filter(event => event.type === 'test').map(event => event.content)
2322+
const retriedTests = tests.filter(test => test.meta[TEST_IS_RETRY] === 'true')
2323+
2324+
assert.equal(retriedTests.length, 1)
2325+
const [retriedTest] = retriedTests
2326+
2327+
assert.propertyVal(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED, 'true')
2328+
assert.propertyVal(
2329+
retriedTest.meta,
2330+
DI_DEBUG_ERROR_FILE,
2331+
'ci-visibility/dynamic-instrumentation/dependency.js'
2332+
)
2333+
assert.equal(retriedTest.metrics[DI_DEBUG_ERROR_LINE], 4)
2334+
assert.exists(retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID])
2335+
})
2336+
const logsPromise = receiver
2337+
.gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/logs'), (payloads) => {
2338+
if (payloads.length > 0) {
2339+
throw new Error('Unexpected logs')
2340+
}
2341+
}, 5000)
2342+
2343+
childProcess = exec(
2344+
'node ./ci-visibility/run-mocha.js',
2345+
{
2346+
cwd,
2347+
env: {
2348+
...getCiVisAgentlessConfig(receiver.port),
2349+
TESTS_TO_RUN: JSON.stringify([
2350+
'./dynamic-instrumentation/test-not-hit-breakpoint'
2351+
]),
2352+
DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
2353+
},
2354+
stdio: 'inherit'
2355+
}
2356+
)
2357+
2358+
childProcess.on('exit', () => {
2359+
Promise.all([eventsPromise, logsPromise]).then(() => {
2360+
done()
2361+
}).catch(done)
2362+
})
2363+
})
2364+
})
21472365
})

‎packages/datadog-instrumentations/src/mocha/utils.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,8 +284,9 @@ function getOnTestRetryHandler () {
284284
const asyncResource = getTestAsyncResource(test)
285285
if (asyncResource) {
286286
const isFirstAttempt = test._currentRetry === 0
287+
const willBeRetried = test._currentRetry < test._retries
287288
asyncResource.runInAsyncScope(() => {
288-
testRetryCh.publish({ isFirstAttempt, err })
289+
testRetryCh.publish({ isFirstAttempt, err, willBeRetried })
289290
})
290291
}
291292
const key = getTestToArKey(test)

‎packages/datadog-plugin-jest/src/index.js

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,10 @@ const {
2323
JEST_DISPLAY_NAME,
2424
TEST_IS_RUM_ACTIVE,
2525
TEST_BROWSER_DRIVER,
26-
getFileAndLineNumberFromError,
2726
DI_ERROR_DEBUG_INFO_CAPTURED,
2827
DI_DEBUG_ERROR_SNAPSHOT_ID,
2928
DI_DEBUG_ERROR_FILE,
3029
DI_DEBUG_ERROR_LINE,
31-
getTestSuitePath,
3230
TEST_NAME
3331
} = require('../../dd-trace/src/plugins/util/test')
3432
const { COMPONENT } = require('../../dd-trace/src/constants')
@@ -381,39 +379,6 @@ class JestPlugin extends CiPlugin {
381379
})
382380
}
383381

384-
// TODO: If the test finishes and the probe is not hit, we should remove the breakpoint
385-
addDiProbe (err, probe) {
386-
const [file, line] = getFileAndLineNumberFromError(err)
387-
388-
const relativePath = getTestSuitePath(file, this.repositoryRoot)
389-
390-
const [
391-
snapshotId,
392-
setProbePromise,
393-
hitProbePromise
394-
] = this.di.addLineProbe({ file: relativePath, line })
395-
396-
probe.setProbePromise = setProbePromise
397-
398-
hitProbePromise.then(({ snapshot }) => {
399-
// TODO: handle race conditions for this.retriedTestIds
400-
const { traceId, spanId } = this.retriedTestIds
401-
this.tracer._exporter.exportDiLogs(this.testEnvironmentMetadata, {
402-
debugger: { snapshot },
403-
dd: {
404-
trace_id: traceId,
405-
span_id: spanId
406-
}
407-
})
408-
})
409-
410-
return {
411-
snapshotId,
412-
file: relativePath,
413-
line
414-
}
415-
}
416-
417382
startTestSpan (test) {
418383
const {
419384
suite,

0 commit comments

Comments
 (0)