diff --git a/integration-tests/cucumber/cucumber.spec.js b/integration-tests/cucumber/cucumber.spec.js index f7925210a87..b46205fcb05 100644 --- a/integration-tests/cucumber/cucumber.spec.js +++ b/integration-tests/cucumber/cucumber.spec.js @@ -39,9 +39,10 @@ const { TEST_SESSION_NAME, TEST_LEVEL_EVENT_TYPES, DI_ERROR_DEBUG_INFO_CAPTURED, - DI_DEBUG_ERROR_FILE, - DI_DEBUG_ERROR_SNAPSHOT_ID, - DI_DEBUG_ERROR_LINE + DI_DEBUG_ERROR_PREFIX, + DI_DEBUG_ERROR_FILE_SUFFIX, + DI_DEBUG_ERROR_SNAPSHOT_ID_SUFFIX, + DI_DEBUG_ERROR_LINE_SUFFIX } = require('../../packages/dd-trace/src/plugins/util/test') const { DD_HOST_CPU_COUNT } = require('../../packages/dd-trace/src/plugins/util/env') @@ -1559,10 +1560,12 @@ versions.forEach(version => { 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 hasDebugTags = Object.keys(retriedTest.meta) + .some(property => + property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED + ) + + assert.isFalse(hasDebugTags) }) const logsPromise = receiver .gatherPayloadsMaxTimeout(({ url }) => url === logsEndpoint, (payloads) => { @@ -1602,11 +1605,12 @@ versions.forEach(version => { assert.equal(retriedTests.length, 1) const [retriedTest] = retriedTests + const hasDebugTags = Object.keys(retriedTest.meta) + .some(property => + property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED + ) - 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) + assert.isFalse(hasDebugTags) }) const logsPromise = receiver .gatherPayloadsMaxTimeout(({ url }) => url === logsEndpoint, (payloads) => { @@ -1655,15 +1659,17 @@ versions.forEach(version => { const [retriedTest] = retriedTests assert.propertyVal(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED, 'true') - assert.propertyVal( - retriedTest.meta, - DI_DEBUG_ERROR_FILE, - 'ci-visibility/features-di/support/sum.js' + + assert.isTrue( + retriedTest.meta[`${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_FILE_SUFFIX}`] + .endsWith('ci-visibility/features-di/support/sum.js') ) - assert.equal(retriedTest.metrics[DI_DEBUG_ERROR_LINE], 4) - assert.exists(retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID]) + assert.equal(retriedTest.metrics[`${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_LINE_SUFFIX}`], 4) + + const snapshotIdKey = `${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_SNAPSHOT_ID_SUFFIX}` + assert.exists(retriedTest.meta[snapshotIdKey]) - snapshotIdByTest = retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID] + snapshotIdByTest = retriedTest.meta[snapshotIdKey] spanIdByTest = retriedTest.span_id.toString() traceIdByTest = retriedTest.trace_id.toString() }) @@ -1733,14 +1739,12 @@ versions.forEach(version => { 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/features-di/support/sum.js' - ) - assert.equal(retriedTest.metrics[DI_DEBUG_ERROR_LINE], 4) - assert.exists(retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID]) + const hasDebugTags = Object.keys(retriedTest.meta) + .some(property => + property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED + ) + + assert.isFalse(hasDebugTags) }) const logsPromise = receiver .gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/logs'), (payloads) => { diff --git a/integration-tests/jest/jest.spec.js b/integration-tests/jest/jest.spec.js index d8d9f8231a6..fa1e566be31 100644 --- a/integration-tests/jest/jest.spec.js +++ b/integration-tests/jest/jest.spec.js @@ -35,9 +35,10 @@ const { TEST_SESSION_NAME, TEST_LEVEL_EVENT_TYPES, DI_ERROR_DEBUG_INFO_CAPTURED, - DI_DEBUG_ERROR_FILE, - DI_DEBUG_ERROR_SNAPSHOT_ID, - DI_DEBUG_ERROR_LINE + DI_DEBUG_ERROR_PREFIX, + DI_DEBUG_ERROR_FILE_SUFFIX, + DI_DEBUG_ERROR_SNAPSHOT_ID_SUFFIX, + DI_DEBUG_ERROR_LINE_SUFFIX } = 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') @@ -2426,11 +2427,12 @@ describe('jest CommonJS', () => { 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 hasDebugTags = Object.keys(retriedTest.meta) + .some(property => property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED) + + assert.isFalse(hasDebugTags) }) + const logsPromise = receiver .gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/logs'), (payloads) => { if (payloads.length > 0) { @@ -2472,10 +2474,10 @@ describe('jest CommonJS', () => { 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 hasDebugTags = Object.keys(retriedTest.meta) + .some(property => property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED) + + assert.isFalse(hasDebugTags) }) const logsPromise = receiver .gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/logs'), (payloads) => { @@ -2522,15 +2524,17 @@ describe('jest CommonJS', () => { 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.isTrue( + retriedTest.meta[`${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_FILE_SUFFIX}`] + .endsWith('ci-visibility/dynamic-instrumentation/dependency.js') ) - assert.equal(retriedTest.metrics[DI_DEBUG_ERROR_LINE], 4) - assert.exists(retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID]) + assert.equal(retriedTest.metrics[`${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_LINE_SUFFIX}`], 4) - snapshotIdByTest = retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID] + const snapshotIdKey = `${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_SNAPSHOT_ID_SUFFIX}` + assert.exists(retriedTest.meta[snapshotIdKey]) + + snapshotIdByTest = retriedTest.meta[snapshotIdKey] spanIdByTest = retriedTest.span_id.toString() traceIdByTest = retriedTest.trace_id.toString() @@ -2603,14 +2607,10 @@ describe('jest CommonJS', () => { 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 hasDebugTags = Object.keys(retriedTest.meta) + .some(property => property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED) + + assert.isFalse(hasDebugTags) }) const logsPromise = receiver .gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/logs'), (payloads) => { diff --git a/integration-tests/mocha/mocha.spec.js b/integration-tests/mocha/mocha.spec.js index d6d13673485..1bb369c0627 100644 --- a/integration-tests/mocha/mocha.spec.js +++ b/integration-tests/mocha/mocha.spec.js @@ -37,9 +37,10 @@ const { TEST_LEVEL_EVENT_TYPES, TEST_EARLY_FLAKE_ABORT_REASON, DI_ERROR_DEBUG_INFO_CAPTURED, - DI_DEBUG_ERROR_FILE, - DI_DEBUG_ERROR_SNAPSHOT_ID, - DI_DEBUG_ERROR_LINE + DI_DEBUG_ERROR_PREFIX, + DI_DEBUG_ERROR_FILE_SUFFIX, + DI_DEBUG_ERROR_SNAPSHOT_ID_SUFFIX, + DI_DEBUG_ERROR_LINE_SUFFIX } = 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') @@ -2166,10 +2167,10 @@ describe('mocha CommonJS', function () { 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 hasDebugTags = Object.keys(retriedTest.meta) + .some(property => property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED) + + assert.isFalse(hasDebugTags) }) const logsPromise = receiver @@ -2217,10 +2218,10 @@ describe('mocha CommonJS', function () { 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 hasDebugTags = Object.keys(retriedTest.meta) + .some(property => property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED) + + assert.isFalse(hasDebugTags) }) const logsPromise = receiver @@ -2273,15 +2274,17 @@ describe('mocha CommonJS', function () { 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.isTrue( + retriedTest.meta[`${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_FILE_SUFFIX}`] + .endsWith('ci-visibility/dynamic-instrumentation/dependency.js') ) - assert.equal(retriedTest.metrics[DI_DEBUG_ERROR_LINE], 4) - assert.exists(retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID]) + assert.equal(retriedTest.metrics[`${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_LINE_SUFFIX}`], 4) + + const snapshotIdKey = `${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_SNAPSHOT_ID_SUFFIX}` + + assert.exists(retriedTest.meta[snapshotIdKey]) - snapshotIdByTest = retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID] + snapshotIdByTest = retriedTest.meta[snapshotIdKey] spanIdByTest = retriedTest.span_id.toString() traceIdByTest = retriedTest.trace_id.toString() @@ -2358,14 +2361,10 @@ describe('mocha CommonJS', function () { 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 hasDebugTags = Object.keys(retriedTest.meta) + .some(property => property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED) + + assert.isFalse(hasDebugTags) }) const logsPromise = receiver .gatherPayloadsMaxTimeout(({ url }) => url.endsWith('/api/v2/logs'), (payloads) => { diff --git a/integration-tests/vitest/vitest.spec.js b/integration-tests/vitest/vitest.spec.js index 2007baefd52..c4b21e4fa20 100644 --- a/integration-tests/vitest/vitest.spec.js +++ b/integration-tests/vitest/vitest.spec.js @@ -26,15 +26,16 @@ const { TEST_EARLY_FLAKE_ABORT_REASON, TEST_SUITE, DI_ERROR_DEBUG_INFO_CAPTURED, - DI_DEBUG_ERROR_FILE, - DI_DEBUG_ERROR_LINE, - DI_DEBUG_ERROR_SNAPSHOT_ID + DI_DEBUG_ERROR_PREFIX, + DI_DEBUG_ERROR_FILE_SUFFIX, + DI_DEBUG_ERROR_SNAPSHOT_ID_SUFFIX, + DI_DEBUG_ERROR_LINE_SUFFIX } = require('../../packages/dd-trace/src/plugins/util/test') const { DD_HOST_CPU_COUNT } = require('../../packages/dd-trace/src/plugins/util/env') const NUM_RETRIES_EFD = 3 -const versions = ['1.6.0', 'latest'] +const versions = ['latest'] const linePctMatchRegex = /Lines\s+:\s+([\d.]+)%/ @@ -920,10 +921,12 @@ versions.forEach((version) => { 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 hasDebugTags = Object.keys(retriedTest.meta) + .some(property => + property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED + ) + + assert.isFalse(hasDebugTags) }) const logsPromise = receiver @@ -968,11 +971,12 @@ versions.forEach((version) => { assert.equal(retriedTests.length, 1) const [retriedTest] = retriedTests + const hasDebugTags = Object.keys(retriedTest.meta) + .some(property => + property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED + ) - 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) + assert.isFalse(hasDebugTags) }) const logsPromise = receiver @@ -1023,15 +1027,17 @@ versions.forEach((version) => { const [retriedTest] = retriedTests assert.propertyVal(retriedTest.meta, DI_ERROR_DEBUG_INFO_CAPTURED, 'true') - assert.propertyVal( - retriedTest.meta, - DI_DEBUG_ERROR_FILE, - 'ci-visibility/vitest-tests/bad-sum.mjs' + + assert.isTrue( + retriedTest.meta[`${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_FILE_SUFFIX}`] + .endsWith('ci-visibility/vitest-tests/bad-sum.mjs') ) - assert.equal(retriedTest.metrics[DI_DEBUG_ERROR_LINE], 4) - assert.exists(retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID]) + assert.equal(retriedTest.metrics[`${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_LINE_SUFFIX}`], 4) + + const snapshotIdKey = `${DI_DEBUG_ERROR_PREFIX}.0.${DI_DEBUG_ERROR_SNAPSHOT_ID_SUFFIX}` + assert.exists(retriedTest.meta[snapshotIdKey]) - snapshotIdByTest = retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID] + snapshotIdByTest = retriedTest.meta[snapshotIdKey] spanIdByTest = retriedTest.span_id.toString() traceIdByTest = retriedTest.trace_id.toString() @@ -1107,14 +1113,12 @@ versions.forEach((version) => { 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/vitest-tests/bad-sum.mjs' - ) - assert.equal(retriedTest.metrics[DI_DEBUG_ERROR_LINE], 4) - assert.exists(retriedTest.meta[DI_DEBUG_ERROR_SNAPSHOT_ID]) + const hasDebugTags = Object.keys(retriedTest.meta) + .some(property => + property.startsWith(DI_DEBUG_ERROR_PREFIX) || property === DI_ERROR_DEBUG_INFO_CAPTURED + ) + + assert.isFalse(hasDebugTags) }) const logsPromise = receiver diff --git a/packages/datadog-instrumentations/src/cucumber.js b/packages/datadog-instrumentations/src/cucumber.js index 7b9a2db5a02..a3a5ae105fd 100644 --- a/packages/datadog-instrumentations/src/cucumber.js +++ b/packages/datadog-instrumentations/src/cucumber.js @@ -238,8 +238,9 @@ function wrapRun (pl, isLatestVersion) { asyncResource.runInAsyncScope(() => { testStartCh.publish(testStartPayload) }) + const promises = {} try { - this.eventBroadcaster.on('envelope', shimmer.wrapFunction(null, () => (testCase) => { + this.eventBroadcaster.on('envelope', shimmer.wrapFunction(null, () => async (testCase) => { // Only supported from >=8.0.0 if (testCase?.testCaseFinished) { const { testCaseFinished: { willBeRetried } } = testCase @@ -253,17 +254,22 @@ function wrapRun (pl, isLatestVersion) { } const failedAttemptAsyncResource = numAttemptToAsyncResource.get(numAttempt) - const isRetry = numAttempt++ > 0 + const isFirstAttempt = numAttempt++ === 0 + + if (promises.hitBreakpointPromise) { + await promises.hitBreakpointPromise + } + failedAttemptAsyncResource.runInAsyncScope(() => { // the current span will be finished and a new one will be created - testRetryCh.publish({ isRetry, error }) + testRetryCh.publish({ isFirstAttempt, error }) }) const newAsyncResource = new AsyncResource('bound-anonymous-fn') numAttemptToAsyncResource.set(numAttempt, newAsyncResource) newAsyncResource.runInAsyncScope(() => { - testStartCh.publish(testStartPayload) // a new span will be created + testStartCh.publish({ ...testStartPayload, promises }) // a new span will be created }) } } @@ -273,7 +279,7 @@ function wrapRun (pl, isLatestVersion) { asyncResource.runInAsyncScope(() => { promise = run.apply(this, arguments) }) - promise.finally(() => { + promise.finally(async () => { const result = this.getWorstStepResult() const { status, skipReason } = isLatestVersion ? getStatusFromResultLatest(result) @@ -296,6 +302,9 @@ function wrapRun (pl, isLatestVersion) { const error = getErrorFromCucumberResult(result) + if (promises.hitBreakpointPromise) { + await promises.hitBreakpointPromise + } attemptAsyncResource.runInAsyncScope(() => { testFinishCh.publish({ status, skipReason, error, isNew, isEfdRetry, isFlakyRetry: numAttempt > 0 }) }) diff --git a/packages/datadog-instrumentations/src/jest.js b/packages/datadog-instrumentations/src/jest.js index 2d27fdc0acb..2f8a15fd1aa 100644 --- a/packages/datadog-instrumentations/src/jest.js +++ b/packages/datadog-instrumentations/src/jest.js @@ -35,7 +35,7 @@ const testSuiteCodeCoverageCh = channel('ci:jest:test-suite:code-coverage') const testStartCh = channel('ci:jest:test:start') const testSkippedCh = channel('ci:jest:test:skip') -const testRunFinishCh = channel('ci:jest:test:finish') +const testFinishCh = channel('ci:jest:test:finish') const testErrCh = channel('ci:jest:test:err') const skippableSuitesCh = channel('ci:jest:test-suite:skippable') @@ -75,6 +75,8 @@ const originalTestFns = new WeakMap() const retriedTestsToNumAttempts = new Map() const newTestsTestStatuses = new Map() +const BREAKPOINT_HIT_GRACE_PERIOD_MS = 200 + // based on https://github.com/facebook/jest/blob/main/packages/jest-circus/src/formatNodeAssertErrors.ts#L41 function formatJestError (errors) { let error @@ -274,46 +276,70 @@ function getWrappedEnvironment (BaseEnvironment, jestVersion) { } } if (event.name === 'test_done') { - const probe = {} + let status = 'pass' + if (event.test.errors && event.test.errors.length) { + status = 'fail' + } + // restore in case it is retried + event.test.fn = originalTestFns.get(event.test) + + // We'll store the test statuses of the retries + if (this.isEarlyFlakeDetectionEnabled) { + const testName = getJestTestName(event.test) + const originalTestName = removeEfdStringFromTestName(testName) + const isNewTest = retriedTestsToNumAttempts.has(originalTestName) + if (isNewTest) { + if (newTestsTestStatuses.has(originalTestName)) { + newTestsTestStatuses.get(originalTestName).push(status) + } else { + newTestsTestStatuses.set(originalTestName, [status]) + } + } + } + + const promises = {} + 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 asyncResource = asyncResources.get(event.test) - asyncResource.runInAsyncScope(() => { - let status = 'pass' - if (event.test.errors && event.test.errors.length) { - status = 'fail' - const numRetries = this.global[RETRY_TIMES] - const numTestExecutions = event.test?.invocations - const willBeRetried = numRetries > 0 && numTestExecutions - 1 < numRetries - - const error = formatJestError(event.test.errors[0]) + + if (status === 'fail') { + asyncResource.runInAsyncScope(() => { testErrCh.publish({ - error, - willBeRetried, - probe, - isDiEnabled: this.isDiEnabled + error: formatJestError(event.test.errors[0]), + shouldSetProbe: this.isDiEnabled && willBeRetried && numTestExecutions === 1, + promises }) - } - testRunFinishCh.publish({ + }) + } + + // 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) { + await new Promise(resolve => { + setTimeout(() => { + resolve() + }, BREAKPOINT_HIT_GRACE_PERIOD_MS) + }) + } + + asyncResource.runInAsyncScope(() => { + testFinishCh.publish({ status, - testStartLine: getTestLineStart(event.test.asyncError, this.testSuite) + testStartLine: getTestLineStart(event.test.asyncError, this.testSuite), + promises, + shouldRemoveProbe: this.isDiEnabled && !willBeRetried }) - // restore in case it is retried - event.test.fn = originalTestFns.get(event.test) - // We'll store the test statuses of the retries - if (this.isEarlyFlakeDetectionEnabled) { - const testName = getJestTestName(event.test) - const originalTestName = removeEfdStringFromTestName(testName) - const isNewTest = retriedTestsToNumAttempts.has(originalTestName) - if (isNewTest) { - if (newTestsTestStatuses.has(originalTestName)) { - newTestsTestStatuses.get(originalTestName).push(status) - } else { - newTestsTestStatuses.set(originalTestName, [status]) - } - } - } }) - if (probe.setProbePromise) { - await probe.setProbePromise + + if (promises.isProbeReady) { + await promises.isProbeReady + } + + if (promises.isProbeRemoved) { + await promises.isProbeRemoved } } if (event.name === 'test_skip' || event.name === 'test_todo') { diff --git a/packages/datadog-instrumentations/src/mocha/utils.js b/packages/datadog-instrumentations/src/mocha/utils.js index ce462f13256..97b5f2d1209 100644 --- a/packages/datadog-instrumentations/src/mocha/utils.js +++ b/packages/datadog-instrumentations/src/mocha/utils.js @@ -19,6 +19,7 @@ const skipCh = channel('ci:mocha:test:skip') // suite channels const testSuiteErrorCh = channel('ci:mocha:test-suite:error') +const BREAKPOINT_HIT_GRACE_PERIOD_MS = 200 const testToAr = new WeakMap() const originalFns = new WeakMap() const testToStartLine = new WeakMap() @@ -73,7 +74,7 @@ function isMochaRetry (test) { return test._currentRetry !== undefined && test._currentRetry !== 0 } -function isLastRetry (test) { +function getIsLastRetry (test) { return test._currentRetry === test._retries } @@ -203,14 +204,28 @@ function getOnTestHandler (isMain) { } function getOnTestEndHandler () { - return function (test) { + return async function (test) { const asyncResource = getTestAsyncResource(test) const status = getTestStatus(test) + // 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 (test._ddShouldWaitForHitProbe || test._retriedTest?._ddShouldWaitForHitProbe) { + await new Promise((resolve) => { + setTimeout(() => { + resolve() + }, BREAKPOINT_HIT_GRACE_PERIOD_MS) + }) + } + // if there are afterEach to be run, we don't finish the test yet if (asyncResource && !test.parent._afterEach.length) { asyncResource.runInAsyncScope(() => { - testFinishCh.publish({ status, hasBeenRetried: isMochaRetry(test) }) + testFinishCh.publish({ + status, + hasBeenRetried: isMochaRetry(test), + isLastRetry: getIsLastRetry(test) + }) }) } } @@ -220,16 +235,17 @@ function getOnHookEndHandler () { return function (hook) { const test = hook.ctx.currentTest if (test && hook.parent._afterEach.includes(hook)) { // only if it's an afterEach - const isLastAfterEach = hook.parent._afterEach.indexOf(hook) === hook.parent._afterEach.length - 1 - if (test._retries > 0 && !isLastRetry(test)) { + const isLastRetry = getIsLastRetry(test) + if (test._retries > 0 && !isLastRetry) { return } + const isLastAfterEach = hook.parent._afterEach.indexOf(hook) === hook.parent._afterEach.length - 1 if (isLastAfterEach) { const status = getTestStatus(test) const asyncResource = getTestAsyncResource(test) if (asyncResource) { asyncResource.runInAsyncScope(() => { - testFinishCh.publish({ status, hasBeenRetried: isMochaRetry(test) }) + testFinishCh.publish({ status, hasBeenRetried: isMochaRetry(test), isLastRetry }) }) } } @@ -286,7 +302,7 @@ function getOnTestRetryHandler () { const isFirstAttempt = test._currentRetry === 0 const willBeRetried = test._currentRetry < test._retries asyncResource.runInAsyncScope(() => { - testRetryCh.publish({ isFirstAttempt, err, willBeRetried }) + testRetryCh.publish({ isFirstAttempt, err, willBeRetried, test }) }) } const key = getTestToArKey(test) diff --git a/packages/datadog-instrumentations/src/vitest.js b/packages/datadog-instrumentations/src/vitest.js index de7c6d2dc30..f623882352e 100644 --- a/packages/datadog-instrumentations/src/vitest.js +++ b/packages/datadog-instrumentations/src/vitest.js @@ -28,6 +28,42 @@ const newTasks = new WeakSet() const switchedStatuses = new WeakSet() const sessionAsyncResource = new AsyncResource('bound-anonymous-fn') +const BREAKPOINT_HIT_GRACE_PERIOD_MS = 400 + +function waitForHitProbe () { + return new Promise(resolve => { + setTimeout(() => { + resolve() + }, BREAKPOINT_HIT_GRACE_PERIOD_MS) + }) +} + +function getProvidedContext () { + try { + const { + _ddIsEarlyFlakeDetectionEnabled, + _ddIsDiEnabled, + _ddKnownTests: knownTests, + _ddEarlyFlakeDetectionNumRetries: numRepeats + } = globalThis.__vitest_worker__.providedContext + + return { + isDiEnabled: _ddIsDiEnabled, + isEarlyFlakeDetectionEnabled: _ddIsEarlyFlakeDetectionEnabled, + knownTests, + numRepeats + } + } catch (e) { + log.error('Vitest workers could not parse provided context, so some features will not work.') + return { + isDiEnabled: false, + isEarlyFlakeDetectionEnabled: false, + knownTests: {}, + numRepeats: 0 + } + } +} + function isReporterPackage (vitestPackage) { return vitestPackage.B?.name === 'BaseSequencer' } @@ -253,29 +289,26 @@ addHook({ // `onBeforeRunTask` is run before any repetition or attempt is run shimmer.wrap(VitestTestRunner.prototype, 'onBeforeRunTask', onBeforeRunTask => async function (task) { const testName = getTestName(task) - try { - const { - _ddKnownTests: knownTests, - _ddIsEarlyFlakeDetectionEnabled: isEarlyFlakeDetectionEnabled, - _ddEarlyFlakeDetectionNumRetries: numRepeats - } = globalThis.__vitest_worker__.providedContext - - if (isEarlyFlakeDetectionEnabled) { - isNewTestCh.publish({ - knownTests, - testSuiteAbsolutePath: task.file.filepath, - testName, - onDone: (isNew) => { - if (isNew) { - task.repeats = numRepeats - newTasks.add(task) - taskToStatuses.set(task, []) - } + + const { + knownTests, + isEarlyFlakeDetectionEnabled, + numRepeats + } = getProvidedContext() + + if (isEarlyFlakeDetectionEnabled) { + isNewTestCh.publish({ + knownTests, + testSuiteAbsolutePath: task.file.filepath, + testName, + onDone: (isNew) => { + if (isNew) { + task.repeats = numRepeats + newTasks.add(task) + taskToStatuses.set(task, []) } - }) - } - } catch (e) { - log.error('Vitest workers could not parse known tests, so Early Flake Detection will not work.') + } + }) } return onBeforeRunTask.apply(this, arguments) @@ -283,9 +316,7 @@ addHook({ // `onAfterRunTask` is run after all repetitions or attempts are run shimmer.wrap(VitestTestRunner.prototype, 'onAfterRunTask', onAfterRunTask => async function (task) { - const { - _ddIsEarlyFlakeDetectionEnabled: isEarlyFlakeDetectionEnabled - } = globalThis.__vitest_worker__.providedContext + const { isEarlyFlakeDetectionEnabled } = getProvidedContext() if (isEarlyFlakeDetectionEnabled && taskToStatuses.has(task)) { const statuses = taskToStatuses.get(task) @@ -309,43 +340,40 @@ addHook({ } const testName = getTestName(task) let isNew = false - let isEarlyFlakeDetectionEnabled = false - let isDiEnabled = false - - try { - const { - _ddIsEarlyFlakeDetectionEnabled, - _ddIsDiEnabled - } = globalThis.__vitest_worker__.providedContext - isEarlyFlakeDetectionEnabled = _ddIsEarlyFlakeDetectionEnabled - isDiEnabled = _ddIsDiEnabled + const { + isEarlyFlakeDetectionEnabled, + isDiEnabled + } = getProvidedContext() - if (isEarlyFlakeDetectionEnabled) { - isNew = newTasks.has(task) - } - } catch (e) { - log.error('Vitest workers could not parse known tests, so Early Flake Detection will not work.') + if (isEarlyFlakeDetectionEnabled) { + isNew = newTasks.has(task) } + const { retry: numAttempt, repeats: numRepetition } = retryInfo // We finish the previous test here because we know it has failed already if (numAttempt > 0) { - const probe = {} + const shouldWaitForHitProbe = isDiEnabled && numAttempt > 1 + if (shouldWaitForHitProbe) { + await waitForHitProbe() + } + + const promises = {} + const shouldSetProbe = isDiEnabled && numAttempt === 1 const asyncResource = taskToAsync.get(task) const testError = task.result?.errors?.[0] if (asyncResource) { asyncResource.runInAsyncScope(() => { testErrorCh.publish({ error: testError, - willBeRetried: true, - probe, - isDiEnabled + shouldSetProbe, + promises }) }) // We wait for the probe to be set - if (probe.setProbePromise) { - await probe.setProbePromise + if (promises.setProbePromise) { + await promises.setProbePromise } } } @@ -401,7 +429,8 @@ addHook({ testName, testSuiteAbsolutePath: task.file.filepath, isRetry: numAttempt > 0 || numRepetition > 0, - isNew + isNew, + mightHitProbe: isDiEnabled && numAttempt > 0 }) }) return onBeforeTryTask.apply(this, arguments) @@ -418,6 +447,12 @@ addHook({ const status = getVitestTestStatus(task, retryCount) const asyncResource = taskToAsync.get(task) + const { isDiEnabled } = getProvidedContext() + + if (isDiEnabled && retryCount > 1) { + await waitForHitProbe() + } + if (asyncResource) { // We don't finish here because the test might fail in a later hook (afterEach) asyncResource.runInAsyncScope(() => { diff --git a/packages/datadog-plugin-cucumber/src/index.js b/packages/datadog-plugin-cucumber/src/index.js index 1c4403b7ce6..16cca8b6b59 100644 --- a/packages/datadog-plugin-cucumber/src/index.js +++ b/packages/datadog-plugin-cucumber/src/index.js @@ -26,12 +26,7 @@ const { TEST_MODULE, TEST_MODULE_ID, TEST_SUITE, - CUCUMBER_IS_PARALLEL, - TEST_NAME, - DI_ERROR_DEBUG_INFO_CAPTURED, - DI_DEBUG_ERROR_SNAPSHOT_ID, - DI_DEBUG_ERROR_FILE, - DI_DEBUG_ERROR_LINE + CUCUMBER_IS_PARALLEL } = require('../../dd-trace/src/plugins/util/test') const { RESOURCE_NAME } = require('../../../ext/tags') const { COMPONENT, ERROR_MESSAGE } = require('../../dd-trace/src/constants') @@ -50,8 +45,8 @@ const { } = require('../../dd-trace/src/ci-visibility/telemetry') const id = require('../../dd-trace/src/id') +const BREAKPOINT_HIT_GRACE_PERIOD_MS = 200 const isCucumberWorker = !!process.env.CUCUMBER_WORKER_ID -const debuggerParameterPerTest = new Map() function getTestSuiteTags (testSuiteSpan) { const suiteTags = { @@ -210,7 +205,13 @@ class CucumberPlugin extends CiPlugin { this.telemetry.ciVisEvent(TELEMETRY_CODE_COVERAGE_FINISHED, 'suite', { library: 'istanbul' }) }) - this.addSub('ci:cucumber:test:start', ({ testName, testFileAbsolutePath, testSourceLine, isParallel }) => { + this.addSub('ci:cucumber:test:start', ({ + testName, + testFileAbsolutePath, + testSourceLine, + isParallel, + promises + }) => { const store = storage.getStore() const testSuite = getTestSuitePath(testFileAbsolutePath, this.sourceRoot) const testSourceFile = getTestSuitePath(testFileAbsolutePath, this.repositoryRoot) @@ -227,38 +228,30 @@ class CucumberPlugin extends CiPlugin { this.enter(testSpan, store) - const debuggerParameters = debuggerParameterPerTest.get(testName) - - if (debuggerParameters) { - const spanContext = testSpan.context() - - // TODO: handle race conditions with this.retriedTestIds - this.retriedTestIds = { - spanId: spanContext.toSpanId(), - traceId: spanContext.toTraceId() - } - const { snapshotId, file, line } = debuggerParameters - - // TODO: should these be added on test:end if and only if the probe is hit? - // Sync issues: `hitProbePromise` might be resolved after the test ends - testSpan.setTag(DI_ERROR_DEBUG_INFO_CAPTURED, 'true') - testSpan.setTag(DI_DEBUG_ERROR_SNAPSHOT_ID, snapshotId) - testSpan.setTag(DI_DEBUG_ERROR_FILE, file) - testSpan.setTag(DI_DEBUG_ERROR_LINE, line) + this.activeTestSpan = testSpan + // Time we give the breakpoint to be hit + if (promises && this.runningTestProbeId) { + promises.hitBreakpointPromise = new Promise((resolve) => { + setTimeout(resolve, BREAKPOINT_HIT_GRACE_PERIOD_MS) + }) } }) - this.addSub('ci:cucumber:test:retry', ({ isRetry, error }) => { + this.addSub('ci:cucumber:test:retry', ({ isFirstAttempt, error }) => { const store = storage.getStore() const span = store.span - if (isRetry) { + if (!isFirstAttempt) { span.setTag(TEST_IS_RETRY, 'true') } span.setTag('error', error) - if (this.di && error && this.libraryConfig?.isDiEnabled) { - const testName = span.context()._tags[TEST_NAME] - const debuggerParameters = this.addDiProbe(error) - debuggerParameterPerTest.set(testName, debuggerParameters) + if (isFirstAttempt && this.di && error && this.libraryConfig?.isDiEnabled) { + const probeInformation = this.addDiProbe(error) + if (probeInformation) { + const { probeId, stackIndex } = probeInformation + this.runningTestProbeId = probeId + this.testErrorStackIndex = stackIndex + // TODO: we're not waiting for setProbePromise to be resolved, so there might be race conditions + } } span.setTag(TEST_STATUS, 'fail') span.finish() @@ -363,6 +356,11 @@ class CucumberPlugin extends CiPlugin { if (isCucumberWorker) { this.tracer._exporter.flush() } + this.activeTestSpan = null + if (this.runningTestProbeId) { + this.removeDiProbe(this.runningTestProbeId) + this.runningTestProbeId = null + } } }) diff --git a/packages/datadog-plugin-jest/src/index.js b/packages/datadog-plugin-jest/src/index.js index 0287f837653..0a6c23ac7d8 100644 --- a/packages/datadog-plugin-jest/src/index.js +++ b/packages/datadog-plugin-jest/src/index.js @@ -23,11 +23,7 @@ const { JEST_DISPLAY_NAME, TEST_IS_RUM_ACTIVE, TEST_BROWSER_DRIVER, - DI_ERROR_DEBUG_INFO_CAPTURED, - DI_DEBUG_ERROR_SNAPSHOT_ID, - DI_DEBUG_ERROR_FILE, - DI_DEBUG_ERROR_LINE, - TEST_NAME + getFormattedError } = require('../../dd-trace/src/plugins/util/test') const { COMPONENT } = require('../../dd-trace/src/constants') const id = require('../../dd-trace/src/id') @@ -44,11 +40,20 @@ const { } = require('../../dd-trace/src/ci-visibility/telemetry') const isJestWorker = !!process.env.JEST_WORKER_ID -const debuggerParameterPerTest = new Map() // https://github.com/facebook/jest/blob/d6ad15b0f88a05816c2fe034dd6900d28315d570/packages/jest-worker/src/types.ts#L38 const CHILD_MESSAGE_END = 2 +function withTimeout (promise, timeoutMs) { + return new Promise(resolve => { + // Set a timeout to resolve after 1s + setTimeout(resolve, timeoutMs) + + // Also resolve if the original promise resolves + promise.then(resolve) + }) +} + class JestPlugin extends CiPlugin { static get id () { return 'jest' @@ -308,32 +313,10 @@ class JestPlugin extends CiPlugin { const span = this.startTestSpan(test) this.enter(span, store) - - const { name: testName } = test - - const debuggerParameters = debuggerParameterPerTest.get(testName) - - // If we have a debugger probe, we need to add the snapshot id to the span - if (debuggerParameters) { - const spanContext = span.context() - - // TODO: handle race conditions with this.retriedTestIds - this.retriedTestIds = { - spanId: spanContext.toSpanId(), - traceId: spanContext.toTraceId() - } - const { snapshotId, file, line } = debuggerParameters - - // TODO: should these be added on test:end if and only if the probe is hit? - // Sync issues: `hitProbePromise` might be resolved after the test ends - span.setTag(DI_ERROR_DEBUG_INFO_CAPTURED, 'true') - span.setTag(DI_DEBUG_ERROR_SNAPSHOT_ID, snapshotId) - span.setTag(DI_DEBUG_ERROR_FILE, file) - span.setTag(DI_DEBUG_ERROR_LINE, line) - } + this.activeTestSpan = span }) - this.addSub('ci:jest:test:finish', ({ status, testStartLine }) => { + this.addSub('ci:jest:test:finish', ({ status, testStartLine, promises, shouldRemoveProbe }) => { const span = storage.getStore().span span.setTag(TEST_STATUS, status) if (testStartLine) { @@ -354,20 +337,28 @@ class JestPlugin extends CiPlugin { span.finish() finishAllTraceSpans(span) + this.activeTestSpan = null + if (shouldRemoveProbe && this.runningTestProbeId) { + promises.isProbeRemoved = withTimeout(this.removeDiProbe(this.runningTestProbeId), 2000) + this.runningTestProbeId = null + } }) - this.addSub('ci:jest:test:err', ({ error, willBeRetried, probe, isDiEnabled }) => { + this.addSub('ci:jest:test:err', ({ error, shouldSetProbe, promises }) => { if (error) { const store = storage.getStore() if (store && store.span) { const span = store.span span.setTag(TEST_STATUS, 'fail') - span.setTag('error', error) - if (willBeRetried && this.di && isDiEnabled) { - // if we use numTestExecutions, we have to remove the breakpoint after each execution - const testName = span.context()._tags[TEST_NAME] - const debuggerParameters = this.addDiProbe(error, probe) - debuggerParameterPerTest.set(testName, debuggerParameters) + span.setTag('error', getFormattedError(error, this.repositoryRoot)) + if (shouldSetProbe) { + const probeInformation = this.addDiProbe(error) + if (probeInformation) { + const { probeId, setProbePromise, stackIndex } = probeInformation + this.runningTestProbeId = probeId + this.testErrorStackIndex = stackIndex + promises.isProbeReady = withTimeout(setProbePromise, 2000) + } } } } diff --git a/packages/datadog-plugin-mocha/src/index.js b/packages/datadog-plugin-mocha/src/index.js index 1b40b9c5a1c..bea9400b083 100644 --- a/packages/datadog-plugin-mocha/src/index.js +++ b/packages/datadog-plugin-mocha/src/index.js @@ -30,12 +30,7 @@ const { TEST_SUITE, MOCHA_IS_PARALLEL, TEST_IS_RUM_ACTIVE, - TEST_BROWSER_DRIVER, - TEST_NAME, - DI_ERROR_DEBUG_INFO_CAPTURED, - DI_DEBUG_ERROR_SNAPSHOT_ID, - DI_DEBUG_ERROR_FILE, - DI_DEBUG_ERROR_LINE + TEST_BROWSER_DRIVER } = require('../../dd-trace/src/plugins/util/test') const { COMPONENT } = require('../../dd-trace/src/constants') const { @@ -52,8 +47,6 @@ const { const id = require('../../dd-trace/src/id') const log = require('../../dd-trace/src/log') -const debuggerParameterPerTest = new Map() - function getTestSuiteLevelVisibilityTags (testSuiteSpan) { const testSuiteSpanContext = testSuiteSpan.context() const suiteTags = { @@ -192,36 +185,15 @@ class MochaPlugin extends CiPlugin { const store = storage.getStore() const span = this.startTestSpan(testInfo) - const { testName } = testInfo - - const debuggerParameters = debuggerParameterPerTest.get(testName) - - if (debuggerParameters) { - const spanContext = span.context() - - // TODO: handle race conditions with this.retriedTestIds - this.retriedTestIds = { - spanId: spanContext.toSpanId(), - traceId: spanContext.toTraceId() - } - const { snapshotId, file, line } = debuggerParameters - - // TODO: should these be added on test:end if and only if the probe is hit? - // Sync issues: `hitProbePromise` might be resolved after the test ends - span.setTag(DI_ERROR_DEBUG_INFO_CAPTURED, 'true') - span.setTag(DI_DEBUG_ERROR_SNAPSHOT_ID, snapshotId) - span.setTag(DI_DEBUG_ERROR_FILE, file) - span.setTag(DI_DEBUG_ERROR_LINE, line) - } - this.enter(span, store) + this.activeTestSpan = span }) this.addSub('ci:mocha:worker:finish', () => { this.tracer._exporter.flush() }) - this.addSub('ci:mocha:test:finish', ({ status, hasBeenRetried }) => { + this.addSub('ci:mocha:test:finish', ({ status, hasBeenRetried, isLastRetry }) => { const store = storage.getStore() const span = store?.span @@ -245,6 +217,11 @@ class MochaPlugin extends CiPlugin { span.finish() finishAllTraceSpans(span) + this.activeTestSpan = null + if (this.di && this.libraryConfig?.isDiEnabled && this.runningTestProbeId && isLastRetry) { + this.removeDiProbe(this.runningTestProbeId) + this.runningTestProbeId = null + } } }) @@ -271,7 +248,7 @@ class MochaPlugin extends CiPlugin { } }) - this.addSub('ci:mocha:test:retry', ({ isFirstAttempt, willBeRetried, err }) => { + this.addSub('ci:mocha:test:retry', ({ isFirstAttempt, willBeRetried, err, test }) => { const store = storage.getStore() const span = store?.span if (span) { @@ -294,10 +271,15 @@ class MochaPlugin extends CiPlugin { browserDriver: spanTags[TEST_BROWSER_DRIVER] } ) - if (willBeRetried && this.di && this.libraryConfig?.isDiEnabled) { - const testName = span.context()._tags[TEST_NAME] - const debuggerParameters = this.addDiProbe(err) - debuggerParameterPerTest.set(testName, debuggerParameters) + if (isFirstAttempt && willBeRetried && this.di && this.libraryConfig?.isDiEnabled) { + const probeInformation = this.addDiProbe(err) + if (probeInformation) { + const { probeId, stackIndex } = probeInformation + this.runningTestProbeId = probeId + this.testErrorStackIndex = stackIndex + test._ddShouldWaitForHitProbe = true + // TODO: we're not waiting for setProbePromise to be resolved, so there might be race conditions + } } span.finish() diff --git a/packages/datadog-plugin-vitest/src/index.js b/packages/datadog-plugin-vitest/src/index.js index ba2554bf9f9..5b8bc9e865e 100644 --- a/packages/datadog-plugin-vitest/src/index.js +++ b/packages/datadog-plugin-vitest/src/index.js @@ -17,12 +17,7 @@ const { TEST_SOURCE_START, TEST_IS_NEW, TEST_EARLY_FLAKE_ENABLED, - TEST_EARLY_FLAKE_ABORT_REASON, - TEST_NAME, - DI_ERROR_DEBUG_INFO_CAPTURED, - DI_DEBUG_ERROR_SNAPSHOT_ID, - DI_DEBUG_ERROR_FILE, - DI_DEBUG_ERROR_LINE + TEST_EARLY_FLAKE_ABORT_REASON } = require('../../dd-trace/src/plugins/util/test') const { COMPONENT } = require('../../dd-trace/src/constants') const { @@ -36,8 +31,6 @@ const { // This is because there's some loss of resolution. const MILLISECONDS_TO_SUBTRACT_FROM_FAILED_TEST_DURATION = 5 -const debuggerParameterPerTest = new Map() - class VitestPlugin extends CiPlugin { static get id () { return 'vitest' @@ -67,7 +60,7 @@ class VitestPlugin extends CiPlugin { onDone(isFaulty) }) - this.addSub('ci:vitest:test:start', ({ testName, testSuiteAbsolutePath, isRetry, isNew }) => { + this.addSub('ci:vitest:test:start', ({ testName, testSuiteAbsolutePath, isRetry, isNew, mightHitProbe }) => { const testSuite = getTestSuitePath(testSuiteAbsolutePath, this.repositoryRoot) const store = storage.getStore() @@ -88,27 +81,13 @@ class VitestPlugin extends CiPlugin { extraTags ) - const debuggerParameters = debuggerParameterPerTest.get(testName) - - if (debuggerParameters) { - const spanContext = span.context() - - // TODO: handle race conditions with this.retriedTestIds - this.retriedTestIds = { - spanId: spanContext.toSpanId(), - traceId: spanContext.toTraceId() - } - const { snapshotId, file, line } = debuggerParameters + this.enter(span, store) - // TODO: should these be added on test:end if and only if the probe is hit? - // Sync issues: `hitProbePromise` might be resolved after the test ends - span.setTag(DI_ERROR_DEBUG_INFO_CAPTURED, 'true') - span.setTag(DI_DEBUG_ERROR_SNAPSHOT_ID, snapshotId) - span.setTag(DI_DEBUG_ERROR_FILE, file) - span.setTag(DI_DEBUG_ERROR_LINE, line) + // TODO: there might be multiple tests for which mightHitProbe is true, so activeTestSpan + // might be wrongly overwritten. + if (mightHitProbe) { + this.activeTestSpan = span } - - this.enter(span, store) }) this.addSub('ci:vitest:test:finish-time', ({ status, task }) => { @@ -137,15 +116,19 @@ class VitestPlugin extends CiPlugin { } }) - this.addSub('ci:vitest:test:error', ({ duration, error, willBeRetried, probe, isDiEnabled }) => { + this.addSub('ci:vitest:test:error', ({ duration, error, shouldSetProbe, promises }) => { const store = storage.getStore() const span = store?.span if (span) { - if (willBeRetried && this.di && isDiEnabled) { - const testName = span.context()._tags[TEST_NAME] - const debuggerParameters = this.addDiProbe(error, probe) - debuggerParameterPerTest.set(testName, debuggerParameters) + if (shouldSetProbe && this.di) { + const probeInformation = this.addDiProbe(error) + if (probeInformation) { + const { probeId, stackIndex, setProbePromise } = probeInformation + this.runningTestProbeId = probeId + this.testErrorStackIndex = stackIndex + promises.setProbePromise = setProbePromise + } } this.telemetry.ciVisEvent(TELEMETRY_EVENT_FINISHED, 'test', { hasCodeowners: !!span.context()._tags[TEST_CODE_OWNERS] @@ -158,7 +141,7 @@ class VitestPlugin extends CiPlugin { if (duration) { span.finish(span._startTime + duration - MILLISECONDS_TO_SUBTRACT_FROM_FAILED_TEST_DURATION) // milliseconds } else { - span.finish() // retries will not have a duration + span.finish() // `duration` is empty for retries, so we'll use clock time } finishAllTraceSpans(span) } @@ -242,6 +225,9 @@ class VitestPlugin extends CiPlugin { this.telemetry.ciVisEvent(TELEMETRY_EVENT_FINISHED, 'suite') // TODO: too frequent flush - find for method in worker to decrease frequency this.tracer._exporter.flush(onFinish) + if (this.runningTestProbeId) { + this.removeDiProbe(this.runningTestProbeId) + } }) this.addSub('ci:vitest:test-suite:error', ({ error }) => { diff --git a/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js b/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js index 8cf52e709f6..ebae4bed0d2 100644 --- a/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js +++ b/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js @@ -6,7 +6,7 @@ const { randomUUID } = require('crypto') const log = require('../../log') const probeIdToResolveBreakpointSet = new Map() -const probeIdToResolveBreakpointHit = new Map() +const probeIdToResolveBreakpointRemove = new Map() class TestVisDynamicInstrumentation { constructor () { @@ -16,28 +16,34 @@ class TestVisDynamicInstrumentation { }) this.breakpointSetChannel = new MessageChannel() this.breakpointHitChannel = new MessageChannel() + this.breakpointRemoveChannel = new MessageChannel() + this.onHitBreakpointByProbeId = new Map() } - // Return 3 elements: - // 1. Snapshot ID + removeProbe (probeId) { + return new Promise(resolve => { + this.breakpointRemoveChannel.port2.postMessage(probeId) + + probeIdToResolveBreakpointRemove.set(probeId, resolve) + }) + } + + // Return 2 elements: + // 1. Probe ID // 2. Promise that's resolved when the breakpoint is set - // 3. Promise that's resolved when the breakpoint is hit - addLineProbe ({ file, line }) { - const snapshotId = randomUUID() + addLineProbe ({ file, line }, onHitBreakpoint) { const probeId = randomUUID() - this.breakpointSetChannel.port2.postMessage({ - snapshotId, - probe: { id: probeId, file, line } - }) + this.breakpointSetChannel.port2.postMessage( + { id: probeId, file, line } + ) + + this.onHitBreakpointByProbeId.set(probeId, onHitBreakpoint) return [ - snapshotId, + probeId, new Promise(resolve => { probeIdToResolveBreakpointSet.set(probeId, resolve) - }), - new Promise(resolve => { - probeIdToResolveBreakpointHit.set(probeId, resolve) }) ] } @@ -67,13 +73,15 @@ class TestVisDynamicInstrumentation { rcPort: rcChannel.port1, configPort: configChannel.port1, breakpointSetChannel: this.breakpointSetChannel.port1, - breakpointHitChannel: this.breakpointHitChannel.port1 + breakpointHitChannel: this.breakpointHitChannel.port1, + breakpointRemoveChannel: this.breakpointRemoveChannel.port1 }, transferList: [ rcChannel.port1, configChannel.port1, this.breakpointSetChannel.port1, - this.breakpointHitChannel.port1 + this.breakpointHitChannel.port1, + this.breakpointRemoveChannel.port1 ] } ) @@ -91,7 +99,7 @@ class TestVisDynamicInstrumentation { // Allow the parent to exit even if the worker is still running this.worker.unref() - this.breakpointSetChannel.port2.on('message', ({ probeId }) => { + this.breakpointSetChannel.port2.on('message', (probeId) => { const resolve = probeIdToResolveBreakpointSet.get(probeId) if (resolve) { resolve() @@ -101,15 +109,19 @@ class TestVisDynamicInstrumentation { this.breakpointHitChannel.port2.on('message', ({ snapshot }) => { const { probe: { id: probeId } } = snapshot - const resolve = probeIdToResolveBreakpointHit.get(probeId) - if (resolve) { - resolve({ snapshot }) - probeIdToResolveBreakpointHit.delete(probeId) + const onHit = this.onHitBreakpointByProbeId.get(probeId) + if (onHit) { + onHit({ snapshot }) } }).unref() - this.worker.on('error', (err) => log.error('ci-visibility DI worker error', err)) - this.worker.on('messageerror', (err) => log.error('ci-visibility DI worker messageerror', err)) + this.breakpointRemoveChannel.port2.on('message', (probeId) => { + const resolve = probeIdToResolveBreakpointRemove.get(probeId) + if (resolve) { + resolve() + probeIdToResolveBreakpointRemove.delete(probeId) + } + }).unref() } } diff --git a/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/worker/index.js b/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/worker/index.js index 952ba1a7cf7..2b20b5703f9 100644 --- a/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/worker/index.js +++ b/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/worker/index.js @@ -1,7 +1,14 @@ 'use strict' -const sourceMap = require('source-map') const path = require('path') -const { workerData: { breakpointSetChannel, breakpointHitChannel } } = require('worker_threads') +const { + workerData: { + breakpointSetChannel, + breakpointHitChannel, + breakpointRemoveChannel + } +} = require('worker_threads') +const { randomUUID } = require('crypto') +const sourceMap = require('source-map') // TODO: move debugger/devtools_client/session to common place const session = require('../../../debugger/devtools_client/session') @@ -16,8 +23,8 @@ const log = require('../../../log') let sessionStarted = false -const breakpointIdToSnapshotId = new Map() const breakpointIdToProbe = new Map() +const probeIdToBreakpointId = new Map() session.on('Debugger.paused', async ({ params: { hitBreakpoints: [hitBreakpoint], callFrames } }) => { const probe = breakpointIdToProbe.get(hitBreakpoint) @@ -32,13 +39,11 @@ session.on('Debugger.paused', async ({ params: { hitBreakpoints: [hitBreakpoint] await session.post('Debugger.resume') - const snapshotId = breakpointIdToSnapshotId.get(hitBreakpoint) - const snapshot = { - id: snapshotId, + id: randomUUID(), timestamp: Date.now(), probe: { - id: probe.probeId, + id: probe.id, version: '0', location: probe.location }, @@ -56,13 +61,32 @@ session.on('Debugger.paused', async ({ params: { hitBreakpoints: [hitBreakpoint] breakpointHitChannel.postMessage({ snapshot }) }) -// TODO: add option to remove breakpoint -breakpointSetChannel.on('message', async ({ snapshotId, probe: { id: probeId, file, line } }) => { - await addBreakpoint(snapshotId, { probeId, file, line }) - breakpointSetChannel.postMessage({ probeId }) +breakpointRemoveChannel.on('message', async (probeId) => { + await removeBreakpoint(probeId) + breakpointRemoveChannel.postMessage(probeId) +}) + +breakpointSetChannel.on('message', async (probe) => { + await addBreakpoint(probe) + breakpointSetChannel.postMessage(probe.id) }) -async function addBreakpoint (snapshotId, probe) { +async function removeBreakpoint (probeId) { + if (!sessionStarted) { + // We should not get in this state, but abort if we do, so the code doesn't fail unexpected + throw Error(`Cannot remove probe ${probeId}: Debugger not started`) + } + + const breakpointId = probeIdToBreakpointId.get(probeId) + if (!breakpointId) { + throw Error(`Unknown probe id: ${probeId}`) + } + await session.post('Debugger.removeBreakpoint', { breakpointId }) + probeIdToBreakpointId.delete(probeId) + breakpointIdToProbe.delete(breakpointId) +} + +async function addBreakpoint (probe) { if (!sessionStarted) await start() const { file, line } = probe @@ -81,7 +105,7 @@ async function addBreakpoint (snapshotId, probe) { try { lineNumber = await processScriptWithInlineSourceMap({ file, line, sourceMapURL }) } catch (err) { - log.error(err) + log.error('Error processing script with inline source map', err) } } @@ -93,7 +117,7 @@ async function addBreakpoint (snapshotId, probe) { }) breakpointIdToProbe.set(breakpointId, probe) - breakpointIdToSnapshotId.set(breakpointId, snapshotId) + probeIdToBreakpointId.set(probe.id, breakpointId) } function start () { @@ -113,14 +137,30 @@ async function processScriptWithInlineSourceMap (params) { // Parse the source map const consumer = await new sourceMap.SourceMapConsumer(decodedSourceMap) - // Map to the generated position - const generatedPosition = consumer.generatedPositionFor({ - source: path.basename(file), // this needs to be the file, not the filepath + let generatedPosition + + // Map to the generated position. We'll attempt with the full file path first, then with the basename. + // TODO: figure out why sometimes the full path doesn't work + generatedPosition = consumer.generatedPositionFor({ + source: file, line, column: 0 }) + if (generatedPosition.line === null) { + generatedPosition = consumer.generatedPositionFor({ + source: path.basename(file), + line, + column: 0 + }) + } consumer.destroy() + // If we can't find the line, just return the original line + if (generatedPosition.line === null) { + log.error(`Could not find generated position for ${file}:${line}`) + return line + } + return generatedPosition.line } diff --git a/packages/dd-trace/src/plugins/ci_plugin.js b/packages/dd-trace/src/plugins/ci_plugin.js index a2f8948bf49..6909cb308b4 100644 --- a/packages/dd-trace/src/plugins/ci_plugin.js +++ b/packages/dd-trace/src/plugins/ci_plugin.js @@ -23,7 +23,11 @@ const { TEST_LEVEL_EVENT_TYPES, TEST_SUITE, getFileAndLineNumberFromError, - getTestSuitePath + DI_ERROR_DEBUG_INFO_CAPTURED, + DI_DEBUG_ERROR_PREFIX, + DI_DEBUG_ERROR_SNAPSHOT_ID_SUFFIX, + DI_DEBUG_ERROR_FILE_SUFFIX, + DI_DEBUG_ERROR_LINE_SUFFIX } = require('./util/test') const Plugin = require('./plugin') const { COMPONENT } = require('../constants') @@ -292,37 +296,58 @@ module.exports = class CiPlugin extends Plugin { return testSpan } - // TODO: If the test finishes and the probe is not hit, we should remove the breakpoint - addDiProbe (err, probe) { - const [file, line] = getFileAndLineNumberFromError(err) + onDiBreakpointHit ({ snapshot }) { + if (!this.activeTestSpan || this.activeTestSpan.context()._isFinished) { + // This is unexpected and is caused by a race condition. + log.warn('Breakpoint snapshot could not be attached to the active test span') + return + } - const relativePath = getTestSuitePath(file, this.repositoryRoot) + const stackIndex = this.testErrorStackIndex + + this.activeTestSpan.setTag(DI_ERROR_DEBUG_INFO_CAPTURED, 'true') + this.activeTestSpan.setTag( + `${DI_DEBUG_ERROR_PREFIX}.${stackIndex}.${DI_DEBUG_ERROR_SNAPSHOT_ID_SUFFIX}`, + snapshot.id + ) + this.activeTestSpan.setTag( + `${DI_DEBUG_ERROR_PREFIX}.${stackIndex}.${DI_DEBUG_ERROR_FILE_SUFFIX}`, + snapshot.probe.location.file + ) + this.activeTestSpan.setTag( + `${DI_DEBUG_ERROR_PREFIX}.${stackIndex}.${DI_DEBUG_ERROR_LINE_SUFFIX}`, + Number(snapshot.probe.location.lines[0]) + ) + + const activeTestSpanContext = this.activeTestSpan.context() + this.tracer._exporter.exportDiLogs(this.testEnvironmentMetadata, { + debugger: { snapshot }, + dd: { + trace_id: activeTestSpanContext.toTraceId(), + span_id: activeTestSpanContext.toSpanId() + } + }) + } - const [ - snapshotId, - setProbePromise, - hitProbePromise - ] = this.di.addLineProbe({ file: relativePath, line }) + removeDiProbe (probeId) { + return this.di.removeProbe(probeId) + } - if (probe) { // not all frameworks may sync with the set probe promise - probe.setProbePromise = setProbePromise + addDiProbe (err) { + const [file, line, stackIndex] = getFileAndLineNumberFromError(err, this.repositoryRoot) + + if (!file || !Number.isInteger(line)) { + log.warn('Could not add breakpoint for dynamic instrumentation') + return } - 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 - } - }) - }) + const [probeId, setProbePromise] = this.di.addLineProbe({ file, line }, this.onDiBreakpointHit.bind(this)) return { - snapshotId, - file: relativePath, + probeId, + setProbePromise, + stackIndex, + file, line } } diff --git a/packages/dd-trace/src/plugins/util/test.js b/packages/dd-trace/src/plugins/util/test.js index 633b1f14361..b47fc95f130 100644 --- a/packages/dd-trace/src/plugins/util/test.js +++ b/packages/dd-trace/src/plugins/util/test.js @@ -108,10 +108,10 @@ const TEST_LEVEL_EVENT_TYPES = [ // Dynamic instrumentation - Test optimization integration tags const DI_ERROR_DEBUG_INFO_CAPTURED = 'error.debug_info_captured' -// TODO: for the moment we'll only use a single snapshot id, so `0` is hardcoded -const DI_DEBUG_ERROR_SNAPSHOT_ID = '_dd.debug.error.0.snapshot_id' -const DI_DEBUG_ERROR_FILE = '_dd.debug.error.0.file' -const DI_DEBUG_ERROR_LINE = '_dd.debug.error.0.line' +const DI_DEBUG_ERROR_PREFIX = '_dd.debug.error' +const DI_DEBUG_ERROR_SNAPSHOT_ID_SUFFIX = 'snapshot_id' +const DI_DEBUG_ERROR_FILE_SUFFIX = 'file' +const DI_DEBUG_ERROR_LINE_SUFFIX = 'line' module.exports = { TEST_CODE_OWNERS, @@ -191,9 +191,11 @@ module.exports = { getNumFromKnownTests, getFileAndLineNumberFromError, DI_ERROR_DEBUG_INFO_CAPTURED, - DI_DEBUG_ERROR_SNAPSHOT_ID, - DI_DEBUG_ERROR_FILE, - DI_DEBUG_ERROR_LINE + DI_DEBUG_ERROR_PREFIX, + DI_DEBUG_ERROR_SNAPSHOT_ID_SUFFIX, + DI_DEBUG_ERROR_FILE_SUFFIX, + DI_DEBUG_ERROR_LINE_SUFFIX, + getFormattedError } // Returns pkg manager and its version, separated by '-', e.g. npm-8.15.0 or yarn-1.22.19 @@ -650,13 +652,30 @@ function getNumFromKnownTests (knownTests) { return totalNumTests } -function getFileAndLineNumberFromError (error) { +const DEPENDENCY_FOLDERS = [ + 'node_modules', + 'node:', + '.pnpm', + '.yarn', + '.pnp' +] + +function getFileAndLineNumberFromError (error, repositoryRoot) { // Split the stack trace into individual lines const stackLines = error.stack.split('\n') - // The top frame is usually the second line - const topFrame = stackLines[1] + // Remove potential messages on top of the stack that are not frames + const frames = stackLines.filter(line => line.includes('at ') && line.includes(repositoryRoot)) + + const topRelevantFrameIndex = frames.findIndex(line => + line.includes(repositoryRoot) && !DEPENDENCY_FOLDERS.some(pattern => line.includes(pattern)) + ) + + if (topRelevantFrameIndex === -1) { + return [] + } + const topFrame = frames[topRelevantFrameIndex] // Regular expression to match the file path, line number, and column number const regex = /\s*at\s+(?:.*\()?(.+):(\d+):(\d+)\)?/ const match = topFrame.match(regex) @@ -664,9 +683,20 @@ function getFileAndLineNumberFromError (error) { if (match) { const filePath = match[1] const lineNumber = Number(match[2]) - const columnNumber = Number(match[3]) - return [filePath, lineNumber, columnNumber] + return [filePath, lineNumber, topRelevantFrameIndex] } return [] } + +// The error.stack property in TestingLibraryElementError includes the message, which results in redundant information +function getFormattedError (error, repositoryRoot) { + if (error.name !== 'TestingLibraryElementError') { + return error + } + const { stack } = error + const newError = new Error(error.message) + newError.stack = stack.split('\n').filter(line => line.includes(repositoryRoot)).join('\n') + + return newError +} diff --git a/packages/dd-trace/test/ci-visibility/dynamic-instrumentation/dynamic-instrumentation.spec.js b/packages/dd-trace/test/ci-visibility/dynamic-instrumentation/dynamic-instrumentation.spec.js index b07ce40533f..6124ef2343d 100644 --- a/packages/dd-trace/test/ci-visibility/dynamic-instrumentation/dynamic-instrumentation.spec.js +++ b/packages/dd-trace/test/ci-visibility/dynamic-instrumentation/dynamic-instrumentation.spec.js @@ -23,8 +23,8 @@ describe('test visibility with dynamic instrumentation', () => { it('can grab local variables', (done) => { childProcess = fork(path.join(__dirname, 'target-app', 'test-visibility-dynamic-instrumentation-script.js')) - childProcess.on('message', ({ snapshot: { language, stack, probe, captures }, snapshotId }) => { - assert.exists(snapshotId) + childProcess.on('message', ({ snapshot: { language, stack, probe, captures }, probeId }) => { + assert.exists(probeId) assert.exists(probe) assert.exists(stack) assert.equal(language, 'javascript') diff --git a/packages/dd-trace/test/ci-visibility/dynamic-instrumentation/target-app/test-visibility-dynamic-instrumentation-script.js b/packages/dd-trace/test/ci-visibility/dynamic-instrumentation/target-app/test-visibility-dynamic-instrumentation-script.js index 39382ea0089..88dbf230c1b 100644 --- a/packages/dd-trace/test/ci-visibility/dynamic-instrumentation/target-app/test-visibility-dynamic-instrumentation-script.js +++ b/packages/dd-trace/test/ci-visibility/dynamic-instrumentation/target-app/test-visibility-dynamic-instrumentation-script.js @@ -11,17 +11,15 @@ const intervalId = setInterval(() => {}, 5000) tvDynamicInstrumentation.start(new Config()) tvDynamicInstrumentation.isReady().then(() => { - const [ - snapshotId, - breakpointSetPromise, - breakpointHitPromise - ] = tvDynamicInstrumentation.addLineProbe({ file: path.join(__dirname, 'di-dependency.js'), line: 9 }) - - breakpointHitPromise.then(({ snapshot }) => { - // once the breakpoint is hit, we can grab the snapshot and send it to the parent process - process.send({ snapshot, snapshotId }) - clearInterval(intervalId) - }) + const file = path.join(__dirname, 'di-dependency.js') + const [probeId, breakpointSetPromise] = tvDynamicInstrumentation.addLineProbe( + { file, line: 9 }, + ({ snapshot }) => { + // once the breakpoint is hit, we can grab the snapshot and send it to the parent process + process.send({ snapshot, probeId }) + clearInterval(intervalId) + } + ) // We run the code once the breakpoint is set breakpointSetPromise.then(() => {