feat(cypress): upload failure screenshots to the v2 media endpoint (#8981)

Author: xinyeji

integration-tests/ci-visibility-intake.js

@@ -47,6 +47,7 @@ const DEFAULT_TEST_MANAGEMENT_TESTS_RESPONSE_STATUS = 200
 class FakeCiVisIntake extends FakeAgent {
   #settings = DEFAULT_SETTINGS
   #settingsResponseStatusCode = 200
+  #mediaResponseStatusCode = 201
   #suitesToSkip = DEFAULT_SUITES_TO_SKIP
   #skippableCoverage = DEFAULT_SKIPPABLE_COVERAGE
   #gitUploadStatus = DEFAULT_GIT_UPLOAD_STATUS
@@ -104,6 +105,12 @@ class FakeCiVisIntake extends FakeAgent {
     this.#settingsResponseStatusCode = statusCode
   }
 
+  // Lets a test simulate the media endpoint failing (e.g. 500) to verify the
+  // cypress run still completes and reports normally when an upload fails.
+  setMediaResponseStatusCode (statusCode) {
+    this.#mediaResponseStatusCode = statusCode
+  }
+
   setWaitingTime (newWaitingTime) {
     this.#waitingTime = newWaitingTime
   }
@@ -225,6 +232,23 @@ class FakeCiVisIntake extends FakeAgent {
       })
     })
 
+    app.post('/api/v2/ci/test-runs/:traceId/media', express.raw({ limit: Infinity, type: '*/*' }), (req, res) => {
+      res.status(this.#mediaResponseStatusCode).send()
+      this.emit('message', {
+        headers: req.headers,
+        media: {
+          traceId: req.params.traceId,
+          contentType: req.headers['content-type'],
+          // Metadata is carried as query params (not X-Dd-* headers) so it survives the Agent's
+          // evp_proxy, which forwards only an allow-listed header set.
+          idempotencyKey: req.query.idempotency_key,
+          capturedAt: req.query.captured_at_ms,
+          content: req.body,
+        },
+        url: req.url,
+      })
+    })
+
     app.post([
       '/api/v2/libraries/tests/services/setting',
       '/evp_proxy/:version/api/v2/libraries/tests/services/setting',

integration-tests/cypress-esm-config.mjs

@@ -23,8 +23,13 @@ async function runCypress () {
         },
         specPattern: process.env.SPEC_PATTERN || 'cypress/e2e/**/*.cy.js',
       },
+      // Mirror the env-driven gating in cypress.config.js: off by default so most
+      // specs do not capture screenshots; the failure-screenshot upload tests set
+      // CYPRESS_ENABLE_FAILURE_SCREENSHOTS=true for their runs.
+      // The 'esm' module type runs Cypress through this programmatic config rather
+      // than cypress.config.js, so the same gating has to live here too.
       video: false,
-      screenshotOnRunFailure: false,
+      screenshotOnRunFailure: process.env.CYPRESS_ENABLE_FAILURE_SCREENSHOTS === 'true',
     },
   })
 

integration-tests/cypress.config.js

@@ -60,6 +60,8 @@ module.exports = defineConfig({
     },
     specPattern: process.env.SPEC_PATTERN || 'cypress/e2e/**/*.cy.js',
   },
+  // Off by default so most specs do not capture screenshots; the failure-screenshot
+  // upload tests set CYPRESS_ENABLE_FAILURE_SCREENSHOTS=true for their runs.
   video: false,
-  screenshotOnRunFailure: false,
+  screenshotOnRunFailure: process.env.CYPRESS_ENABLE_FAILURE_SCREENSHOTS === 'true',
 })

integration-tests/cypress/cypress-reporting.spec.js

@@ -915,5 +915,198 @@ moduleTypes.forEach(({
         fs.rmSync(subprojectDir, { recursive: true, force: true })
       }
     })
+
+    const reportMethods = [
+      { name: 'evp proxy', getEnvVars: getCiVisEvpProxyConfig },
+      { name: 'agentless', getEnvVars: getCiVisAgentlessConfig },
+    ]
+
+    reportMethods.forEach(({ name: reportMethod, getEnvVars }) => {
+      context(`reporting with ${reportMethod}`, () => {
+        // 6.7.0 uses a flat JSON config that can't read env to enable screenshots, so over10It skips it.
+        // TODO: use over10It for evp proxy too once the Agent can forward the media endpoint.
+        const onlyAgentlessIt = reportMethod === 'agentless' ? over10It : it.skip
+
+        function runCypressWithFailureScreenshots (specToRun) {
+          let testOutput = ''
+          childProcess = exec(
+            testCommand,
+            {
+              cwd,
+              env: {
+                ...getEnvVars(receiver.port),
+                CYPRESS_BASE_URL: webAppBaseUrl,
+                SPEC_PATTERN: specToRun,
+                CYPRESS_ENABLE_FAILURE_SCREENSHOTS: 'true',
+                DD_TEST_FAILURE_SCREENSHOTS_ENABLED: 'true',
+              },
+            }
+          )
+          childProcess.stdout?.on('data', (d) => { testOutput += d.toString() })
+          childProcess.stderr?.on('data', (d) => { testOutput += d.toString() })
+          return () => testOutput
+        }
+
+        onlyAgentlessIt('uploads failure screenshots to the v2 media endpoint', async function () {
+          const getTestOutput = runCypressWithFailureScreenshots('cypress/e2e/basic-fail.js')
+
+          const receiverPromise = receiver
+            .gatherPayloadsUntilChildExit(
+              childProcess,
+              ({ url }) => url.startsWith('/api/v2/ci/test-runs/') || url.endsWith('/api/v2/citestcycle'),
+              (payloads) => {
+                const testOutput = getTestOutput()
+                const mediaPayloads = payloads.filter(({ url }) => url.startsWith('/api/v2/ci/test-runs/'))
+                const failedTest = payloads
+                  .filter(({ url }) => url.endsWith('/api/v2/citestcycle'))
+                  .flatMap(({ payload }) => payload.events)
+                  .filter(event => event.type === 'test')
+                  .find(event => event.content.resource === 'cypress/e2e/basic-fail.js.basic fail suite can fail')
+
+                assert.ok(failedTest, `failed test event should be reported\n${testOutput}`)
+                const expectedTraceId = failedTest.content.trace_id.toString()
+
+                const screenshotPayload = mediaPayloads.find(({ media }) => media.contentType === 'image/png')
+                assert.ok(screenshotPayload, `a screenshot should be uploaded to the v2 media endpoint\n${testOutput}`)
+                assert.strictEqual(
+                  screenshotPayload.url.split('?')[0],
+                  `/api/v2/ci/test-runs/${expectedTraceId}/media`
+                )
+                assert.strictEqual(screenshotPayload.media.traceId, expectedTraceId)
+                assert.strictEqual(screenshotPayload.headers['dd-api-key'], '1')
+
+                // v2 metadata rides the query string, not X-Dd-* headers, so it survives the
+                // Agent's evp_proxy (which strips non-allow-listed headers). The idempotency key is
+                // `<traceId>:<hex(filename)>` (filename hex-encoded so a non-ASCII title and the
+                // proxy's query-charset validation can't break it), reused on retry so the media
+                // service overwrites instead of duplicating the stored object.
+                const { idempotencyKey } = screenshotPayload.media
+                assert.ok(idempotencyKey, 'media upload should send an idempotency_key query param')
+                assert.match(
+                  idempotencyKey,
+                  new RegExp(`^${expectedTraceId}:`),
+                  `idempotency key ${idempotencyKey} should start with the trace id`
+                )
+
+                const capturedAt = Number(screenshotPayload.media.capturedAt)
+                assert.ok(
+                  Number.isInteger(capturedAt) && capturedAt > 0,
+                  `captured_at_ms should be a positive integer, got ${screenshotPayload.media.capturedAt}`
+                )
+                assert.ok(
+                  !('x-dd-idempotency-key' in screenshotPayload.headers),
+                  'v2 must not send metadata as X-Dd-* headers (the Agent evp_proxy strips them)'
+                )
+                assert.ok(
+                  !('test-drive-test-failure-media-bucket' in screenshotPayload.headers),
+                  'v2 must not send the v1 test-drive-test-failure-media-bucket header'
+                )
+                assert.deepStrictEqual(
+                  [...screenshotPayload.media.content.subarray(0, 8)],
+                  [137, 80, 78, 71, 13, 10, 26, 10]
+                )
+              }, { hardTimeout: 60000 })
+            .catch((error) => {
+              error.message += `\nCypress output:\n${getTestOutput()}`
+              throw error
+            })
+
+          await Promise.all([
+            once(childProcess, 'exit'),
+            receiverPromise,
+          ])
+        })
+
+        onlyAgentlessIt('uploads only the auto failure frame, not a manual cy.screenshot()', async function () {
+          const getTestOutput = runCypressWithFailureScreenshots('cypress/e2e/manual-screenshot-before-fail.js')
+
+          const receiverPromise = receiver
+            .gatherPayloadsUntilChildExit(
+              childProcess,
+              ({ url }) => url.startsWith('/api/v2/ci/test-runs/') || url.endsWith('/api/v2/citestcycle'),
+              (payloads) => {
+                const testOutput = getTestOutput()
+                const mediaPayloads = payloads.filter(({ url }) => url.startsWith('/api/v2/ci/test-runs/'))
+                const failedTest = payloads
+                  .filter(({ url }) => url.endsWith('/api/v2/citestcycle'))
+                  .flatMap(({ payload }) => payload.events)
+                  .filter(event => event.type === 'test')
+                  .find(event =>
+                    event.content.resource ===
+                      'cypress/e2e/manual-screenshot-before-fail.js.manual screenshot before fail suite ' +
+                      'takes a manual screenshot then fails'
+                  )
+
+                assert.ok(failedTest, `failed test event should be reported\n${testOutput}`)
+                const expectedTraceId = failedTest.content.trace_id.toString()
+
+                const screenshotPayloads = mediaPayloads.filter(({ media }) => media.contentType === 'image/png')
+                assert.strictEqual(
+                  screenshotPayloads.length,
+                  1,
+                  `only the auto failure screenshot should be uploaded\n${testOutput}`
+                )
+                const [screenshotPayload] = screenshotPayloads
+                assert.strictEqual(screenshotPayload.media.traceId, expectedTraceId)
+
+                const decodeKeyFilename = (key) => {
+                  const [, hexFilename] = (key || '').split(':')
+                  return hexFilename ? Buffer.from(hexFilename, 'hex').toString('utf8') : ''
+                }
+                assert.match(
+                  decodeKeyFilename(screenshotPayload.media.idempotencyKey),
+                  /\(failed\)/,
+                  `the uploaded screenshot should be the auto failure frame\n${testOutput}`
+                )
+
+                const manualUpload = mediaPayloads.find(({ media }) =>
+                  decodeKeyFilename(media.idempotencyKey).includes('before-failure')
+                )
+                assert.ok(
+                  !manualUpload,
+                  `the manual cy.screenshot() must not be uploaded to the media endpoint\n${testOutput}`
+                )
+              }, { hardTimeout: 60000 })
+            .catch((error) => {
+              error.message += `\nCypress output:\n${getTestOutput()}`
+              throw error
+            })
+
+          await Promise.all([
+            once(childProcess, 'exit'),
+            receiverPromise,
+          ])
+        })
+
+        onlyAgentlessIt('continues normally when the media upload endpoint fails', async function () {
+          receiver.setMediaResponseStatusCode(500)
+          const getTestOutput = runCypressWithFailureScreenshots('cypress/e2e/basic-fail.js')
+
+          const receiverPromise = receiver
+            .gatherPayloadsUntilChildExit(
+              childProcess,
+              ({ url }) => url.endsWith('/api/v2/citestcycle'),
+              (payloads) => {
+                const testOutput = getTestOutput()
+                const failedTest = payloads
+                  .flatMap(({ payload }) => payload.events)
+                  .filter(event => event.type === 'test')
+                  .find(event => event.content.resource === 'cypress/e2e/basic-fail.js.basic fail suite can fail')
+
+                assert.ok(failedTest, `the failed test should still be reported when media upload fails\n${testOutput}`)
+                assert.strictEqual(failedTest.content.meta[TEST_STATUS], 'fail')
+              }, { hardTimeout: 60000 })
+            .catch((error) => {
+              error.message += `\nCypress output:\n${getTestOutput()}`
+              throw error
+            })
+
+          await Promise.all([
+            once(childProcess, 'exit'),
+            receiverPromise,
+          ])
+        })
+      })
+    })
   })
 })

integration-tests/cypress/e2e/manual-screenshot-before-fail.js

@@ -0,0 +1,13 @@
+/* eslint-disable */
+describe('manual screenshot before fail suite', () => {
+  beforeEach(() => {
+    cy.visit('/')
+  })
+
+  it('takes a manual screenshot then fails', () => {
+    // Manual capture (not a failure frame): must NOT be uploaded to the failure-screenshot endpoint.
+    cy.screenshot('before-failure')
+    cy.get('.hello-world')
+      .should('have.text', 'Hello warld')
+  })
+})

packages/datadog-instrumentations/src/cypress-config.js

@@ -113,9 +113,10 @@ function injectSupportFile (config) {
  * @param {object} config Cypress resolved config object
  * @param {Function[]} userAfterSpecHandlers user's after:spec handlers collected from wrappedOn
  * @param {Function[]} userAfterRunHandlers user's after:run handlers collected from wrappedOn
+ * @param {Function[]} userAfterScreenshotHandlers user's after:screenshot handlers collected from wrappedOn
  * @returns {object} the config object (possibly modified)
  */
-function registerDdTraceHooks (on, config, userAfterSpecHandlers, userAfterRunHandlers) {
+function registerDdTraceHooks (on, config, userAfterSpecHandlers, userAfterRunHandlers, userAfterScreenshotHandlers) {
   const wrapperFile = injectSupportFile(config)
 
   const cleanupWrapper = () => {
@@ -136,6 +137,7 @@ function registerDdTraceHooks (on, config, userAfterSpecHandlers, userAfterRunHa
 
   const registerNoopHandlers = () => {
     for (const h of userAfterSpecHandlers) on('after:spec', h)
+    for (const h of userAfterScreenshotHandlers) on('after:screenshot', h)
     registerAfterRunWithCleanup()
     on('task', noopTask)
   }
@@ -153,6 +155,7 @@ function registerDdTraceHooks (on, config, userAfterSpecHandlers, userAfterRunHa
     config,
     userAfterSpecHandlers,
     userAfterRunHandlers,
+    userAfterScreenshotHandlers,
     cleanupWrapper,
     registered: false,
     configPromise: undefined,
@@ -176,12 +179,15 @@ function wrapSetupNodeEvents (originalSetupNodeEvents) {
   return function ddSetupNodeEvents (on, config) {
     const userAfterSpecHandlers = []
     const userAfterRunHandlers = []
+    const userAfterScreenshotHandlers = []
 
     const wrappedOn = (event, handler) => {
       if (event === 'after:spec') {
         userAfterSpecHandlers.push(handler)
       } else if (event === 'after:run') {
         userAfterRunHandlers.push(handler)
+      } else if (event === 'after:screenshot') {
+        userAfterScreenshotHandlers.push(handler)
       } else {
         on(event, handler)
       }
@@ -197,7 +203,8 @@ function wrapSetupNodeEvents (originalSetupNodeEvents) {
           on,
           mergeReturnedConfig(config, result),
           userAfterSpecHandlers,
-          userAfterRunHandlers
+          userAfterRunHandlers,
+          userAfterScreenshotHandlers
         )
       })
     }
@@ -206,7 +213,8 @@ function wrapSetupNodeEvents (originalSetupNodeEvents) {
       on,
       mergeReturnedConfig(config, maybePromise),
       userAfterSpecHandlers,
-      userAfterRunHandlers
+      userAfterRunHandlers,
+      userAfterScreenshotHandlers
     )
   }
 }