[test optimization] Add Dynamic Instrumentation to mocha retries (#4944)

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

@@ -0,0 +1,7 @@
+module.exports = function () {
+  try {
+    return typeof jest !== 'undefined'
+  } catch (e) {
+    return false
+  }
+}

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

@@ -1,15 +1,22 @@
 /* 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
-jest.retryTimes(1)
+if (isJest()) {
+  jest.retryTimes(1)
+}
 
 describe('dynamic-instrumentation', () => {
-  it('retries with DI', () => {
-    expect(sum(11, 3)).toEqual(14)
+  it('retries with DI', function () {
+    if (this.retries) {
+      this.retries(1)
+    }
+    expect(sum(11, 3)).to.equal(14)
   })
 
   it('is not retried', () => {
-    expect(sum(1, 2)).toEqual(3)
+    expect(1 + 2).to.equal(3)
   })
 })

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

@@ -1,17 +1,24 @@
 /* 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
-jest.retryTimes(1)
+if (isJest()) {
+  jest.retryTimes(1)
+}
 
 let count = 0
 describe('dynamic-instrumentation', () => {
-  it('retries with DI', () => {
+  it('retries with DI', function () {
+    if (this.retries) {
+      this.retries(1)
+    }
     const willFail = count++ === 0
     if (willFail) {
-      expect(sum(11, 3)).toEqual(14) // only throws the first time
+      expect(sum(11, 3)).to.equal(14) // only throws the first time
     } else {
-      expect(sum(1, 2)).toEqual(3)
+      expect(sum(1, 2)).to.equal(3)
     }
   })
 })

integration-tests/jest/jest.spec.js

@@ -2416,7 +2416,7 @@ describe('jest CommonJS', () => {
         itr_enabled: false,
         code_coverage: false,
         tests_skipping: false,
-        flaky_test_retries_enabled: true,
+        flaky_test_retries_enabled: false,
         early_flake_detection: {
           enabled: false
         }
@@ -2468,7 +2468,7 @@ describe('jest CommonJS', () => {
         itr_enabled: false,
         code_coverage: false,
         tests_skipping: false,
-        flaky_test_retries_enabled: true,
+        flaky_test_retries_enabled: false,
         early_flake_detection: {
           enabled: false
         }
@@ -2558,7 +2558,7 @@ describe('jest CommonJS', () => {
         itr_enabled: false,
         code_coverage: false,
         tests_skipping: false,
-        flaky_test_retries_enabled: true,
+        flaky_test_retries_enabled: false,
         early_flake_detection: {
           enabled: false
         }

integration-tests/mocha/mocha.spec.js

@@ -35,7 +35,11 @@ const {
   TEST_CODE_OWNERS,
   TEST_SESSION_NAME,
   TEST_LEVEL_EVENT_TYPES,
-  TEST_EARLY_FLAKE_ABORT_REASON
+  TEST_EARLY_FLAKE_ABORT_REASON,
+  DI_ERROR_DEBUG_INFO_CAPTURED,
+  DI_DEBUG_ERROR_FILE,
+  DI_DEBUG_ERROR_SNAPSHOT_ID,
+  DI_DEBUG_ERROR_LINE
 } = require('../../packages/dd-trace/src/plugins/util/test')
 const { DD_HOST_CPU_COUNT } = require('../../packages/dd-trace/src/plugins/util/env')
 const { ERROR_MESSAGE } = require('../../packages/dd-trace/src/constants')
@@ -2144,4 +2148,218 @@ describe('mocha CommonJS', function () {
       })
     })
   })
+
+  context('dynamic instrumentation', () => {
+    it('does not activate dynamic instrumentation if DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED is not set', (done) => {
+      receiver.setSettings({
+        itr_enabled: false,
+        code_coverage: false,
+        tests_skipping: false,
+        flaky_test_retries_enabled: false,
+        early_flake_detection: {
+          enabled: false
+        }
+        // di_enabled: true // TODO
+      })
+
+      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
+
+          assert.notProperty(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED)
+          assert.notProperty(retriedTest.meta, DI_DEBUG_ERROR_FILE)
+          assert.notProperty(retriedTest.metrics, DI_DEBUG_ERROR_LINE)
+          assert.notProperty(retriedTest.meta, DI_DEBUG_ERROR_SNAPSHOT_ID)
+        })
+
+      const logsPromise = receiver
+        .gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/logs'), (payloads) => {
+          if (payloads.length > 0) {
+            throw new Error('Unexpected logs')
+          }
+        }, 5000)
+
+      childProcess = exec(
+        runTestsWithCoverageCommand,
+        {
+          cwd,
+          env: {
+            ...getCiVisAgentlessConfig(receiver.port),
+            TESTS_TO_RUN: JSON.stringify([
+              './dynamic-instrumentation/test-hit-breakpoint'
+            ])
+          },
+          stdio: 'inherit'
+        }
+      )
+
+      childProcess.on('exit', (code) => {
+        Promise.all([eventsPromise, logsPromise]).then(() => {
+          assert.equal(code, 0)
+          done()
+        }).catch(done)
+      })
+    })
+
+    it('runs retries with dynamic instrumentation', (done) => {
+      receiver.setSettings({
+        itr_enabled: false,
+        code_coverage: false,
+        tests_skipping: false,
+        flaky_test_retries_enabled: false,
+        early_flake_detection: {
+          enabled: false
+        }
+        // di_enabled: true // TODO
+      })
+
+      let snapshotIdByTest, snapshotIdByLog
+      let spanIdByTest, spanIdByLog, traceIdByTest, traceIdByLog
+
+      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
+
+          assert.propertyVal(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED, 'true')
+          assert.propertyVal(
+            retriedTest.meta,
+            DI_DEBUG_ERROR_FILE,
+            'ci-visibility/dynamic-instrumentation/dependency.js'
+          )
+          assert.equal(retriedTest.metrics[DI_DEBUG_ERROR_LINE], 4)
+          assert.exists(retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID])
+
+          snapshotIdByTest = retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID]
+          spanIdByTest = retriedTest.span_id.toString()
+          traceIdByTest = retriedTest.trace_id.toString()
+
+          const notRetriedTest = tests.find(test => test.meta[TEST_NAME].includes('is not retried'))
+
+          assert.notProperty(notRetriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED)
+        })
+
+      const logsPromise = receiver
+        .gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/logs'), (payloads) => {
+          const [{ logMessage: [diLog] }] = payloads
+          assert.deepInclude(diLog, {
+            ddsource: 'dd_debugger',
+            level: 'error'
+          })
+          assert.equal(diLog.debugger.snapshot.language, 'javascript')
+          assert.deepInclude(diLog.debugger.snapshot.captures.lines['4'].locals, {
+            a: {
+              type: 'number',
+              value: '11'
+            },
+            b: {
+              type: 'number',
+              value: '3'
+            },
+            localVariable: {
+              type: 'number',
+              value: '2'
+            }
+          })
+          spanIdByLog = diLog.dd.span_id
+          traceIdByLog = diLog.dd.trace_id
+          snapshotIdByLog = diLog.debugger.snapshot.id
+        }, 5000)
+
+      childProcess = exec(
+        'node ./ci-visibility/run-mocha.js',
+        {
+          cwd,
+          env: {
+            ...getCiVisAgentlessConfig(receiver.port),
+            TESTS_TO_RUN: JSON.stringify([
+              './dynamic-instrumentation/test-hit-breakpoint'
+            ]),
+            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
+          },
+          stdio: 'inherit'
+        }
+      )
+
+      childProcess.on('exit', () => {
+        Promise.all([eventsPromise, logsPromise]).then(() => {
+          assert.equal(snapshotIdByTest, snapshotIdByLog)
+          assert.equal(spanIdByTest, spanIdByLog)
+          assert.equal(traceIdByTest, traceIdByLog)
+          done()
+        }).catch(done)
+      })
+    })
+
+    it('does not crash if the retry does not hit the breakpoint', (done) => {
+      receiver.setSettings({
+        itr_enabled: false,
+        code_coverage: false,
+        tests_skipping: false,
+        flaky_test_retries_enabled: false,
+        early_flake_detection: {
+          enabled: false
+        }
+        // di_enabled: true // TODO
+      })
+
+      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
+
+          assert.propertyVal(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED, 'true')
+          assert.propertyVal(
+            retriedTest.meta,
+            DI_DEBUG_ERROR_FILE,
+            'ci-visibility/dynamic-instrumentation/dependency.js'
+          )
+          assert.equal(retriedTest.metrics[DI_DEBUG_ERROR_LINE], 4)
+          assert.exists(retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID])
+        })
+      const logsPromise = receiver
+        .gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/logs'), (payloads) => {
+          if (payloads.length > 0) {
+            throw new Error('Unexpected logs')
+          }
+        }, 5000)
+
+      childProcess = exec(
+        'node ./ci-visibility/run-mocha.js',
+        {
+          cwd,
+          env: {
+            ...getCiVisAgentlessConfig(receiver.port),
+            TESTS_TO_RUN: JSON.stringify([
+              './dynamic-instrumentation/test-not-hit-breakpoint'
+            ]),
+            DD_TEST_DYNAMIC_INSTRUMENTATION_ENABLED: 'true'
+          },
+          stdio: 'inherit'
+        }
+      )
+
+      childProcess.on('exit', () => {
+        Promise.all([eventsPromise, logsPromise]).then(() => {
+          done()
+        }).catch(done)
+      })
+    })
+  })
 })

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

@@ -284,8 +284,9 @@ function getOnTestRetryHandler () {
     const asyncResource = getTestAsyncResource(test)
     if (asyncResource) {
       const isFirstAttempt = test._currentRetry === 0
+      const willBeRetried = test._currentRetry < test._retries
       asyncResource.runInAsyncScope(() => {
-        testRetryCh.publish({ isFirstAttempt, err })
+        testRetryCh.publish({ isFirstAttempt, err, willBeRetried })
       })
     }
     const key = getTestToArKey(test)

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

@@ -23,12 +23,10 @@ const {
   JEST_DISPLAY_NAME,
   TEST_IS_RUM_ACTIVE,
   TEST_BROWSER_DRIVER,
-  getFileAndLineNumberFromError,
   DI_ERROR_DEBUG_INFO_CAPTURED,
   DI_DEBUG_ERROR_SNAPSHOT_ID,
   DI_DEBUG_ERROR_FILE,
   DI_DEBUG_ERROR_LINE,
-  getTestSuitePath,
   TEST_NAME
 } = require('../../dd-trace/src/plugins/util/test')
 const { COMPONENT } = require('../../dd-trace/src/constants')
@@ -381,39 +379,6 @@ class JestPlugin extends CiPlugin {
     })
   }
 
-  // TODO: If the test finishes and the probe is not hit, we should remove the breakpoint
-  addDiProbe (err, probe) {
-    const [file, line] = getFileAndLineNumberFromError(err)
-
-    const relativePath = getTestSuitePath(file, this.repositoryRoot)
-
-    const [
-      snapshotId,
-      setProbePromise,
-      hitProbePromise
-    ] = this.di.addLineProbe({ file: relativePath, line })
-
-    probe.setProbePromise = setProbePromise
-
-    hitProbePromise.then(({ snapshot }) => {
-      // TODO: handle race conditions for this.retriedTestIds
-      const { traceId, spanId } = this.retriedTestIds
-      this.tracer._exporter.exportDiLogs(this.testEnvironmentMetadata, {
-        debugger: { snapshot },
-        dd: {
-          trace_id: traceId,
-          span_id: spanId
-        }
-      })
-    })
-
-    return {
-      snapshotId,
-      file: relativePath,
-      line
-    }
-  }
-
   startTestSpan (test) {
     const {
       suite,