feat(cypress): upload failure screenshots to the v2 media endpoint#8981
Conversation
Updates the Cypress failure-screenshot upload (from #8574) to the production v2 Test Optimization media contract: - Send the two now-required headers: - X-Dd-Idempotency-Key = `{trace_id}:{filename}` — stable per artifact and reused on retry, so a retried upload overwrites the same stored object instead of creating a duplicate (the Cypress filename encodes test + attempt). - X-Dd-Media-Captured-At = capture time in epoch ms, taken from the screenshot's `takenAt` (mtime / now fallback), stamped once and resent unchanged on retry (it is part of the stored object key). - Drop the temporary PoC bits: the `test-drive-test-failure-media-bucket` header and the `DD_POC_SITE` host override. - Agentless uploads go to the normal api.<site> host with DD-API-KEY. Agent (evp_proxy) mode upload stays disabled until the Datadog Agent allow-lists POST /api/unstable/ci/test-runs/<trace_id>/media (follow-up). Backend: DataDog/dd-source#443668. Builds on Juan's PoC #8574. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Overall package sizeSelf size: 6.53 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.2.0 | 104.26 kB | 843.44 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 577dde6 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-07-02 14:39:49 Comparing candidate commit 577dde6 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2259 metrics, 27 unstable metrics.
|
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (83.00%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #8981 +/- ##
==========================================
- Coverage 93.73% 93.58% -0.15%
==========================================
Files 892 897 +5
Lines 51236 52416 +1180
Branches 11924 12317 +393
==========================================
+ Hits 48026 49056 +1030
- Misses 3210 3360 +150 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Extends the media upload to Cypress's per-spec video:
- getContentType maps .mp4 -> video/mp4 and .webm -> video/webm.
- afterSpec uploads results.video to each failed test run's trace_id (Cypress
records one video per spec, so it is attached to every failed test in the
spec). Capture time falls back to the video file mtime (no per-test takenAt).
- New uploadTestVideo() reuses the same media request + idempotency key
(`{trace_id}:{filename}`), so an upload retry overwrites instead of duplicating.
Backend + UI already support video (mp4/webm); this closes the tracer side for
Cypress. Image (screenshot) and video now both flow end-to-end.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The debug line concatenated url.toString() (trailing '/') with the leading-slash path, logging https://api.<site>//api/unstable/... The actual request was always correct (request helper uses url.hostname + options.path separately); this just makes the log resolve to a single, clean URL. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…RL import - getScreenshotCapturedAtMs: add explicit screenshot !== null guard (typeof null is 'object'; required by eslint-rules/eslint-safe-typeof-object). - agent-proxy: remove the now-unused 'url'.URL import (its only user was the DD_POC_SITE helper removed in the v2 change). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Recovers the failure-media upload assertion dropped when the v2 branch was rebased onto master, adapted to the v2 contract. The intake's /media handler now surfaces the content-type, idempotency key and captured-at header on the emitted message so the test can assert them. The test runs the basic-fail fixture with screenshots and video enabled and asserts the tracer POSTs each artifact to /api/unstable/ci/test-runs/<traceId>/media with the X-Dd-Idempotency-Key (<traceId>:<filename> shape) and a positive-integer X-Dd-Media-Captured-At, sends image/png for the screenshot and video/mp4 for the spec video, and no longer sends the v1 test-drive-test-failure-media-bucket header. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…es it The harness cypress.config.js disabled screenshots+video and a CLI --config override does not win over the file values, so the v2 media test captured nothing (Screenshots: 0). Drive both from CYPRESS_ENABLE_FAILURE_MEDIA (off by default; the test sets it), skip 6.7.0 (flat JSON config can't read env), and fix the lint nit (assert.match over assert.ok(startsWith)). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…oggles Add DD_CIVISIBILITY_TEST_FAILURE_SCREENSHOTS_ENABLED and DD_CIVISIBILITY_TEST_FAILURE_VIDEO_ENABLED (both default off) so failure screenshots and the per-spec video can be uploaded independently. Gate screenshot uploads on canUploadTestScreenshots() and video uploads on the new canUploadTestVideo(); the v2 media integration test opts both in. Adds exporter gate unit tests + config tests. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The esm module type runs Cypress via cypress-esm-config.mjs (programmatic cypress.run), not cypress.config.js, so the env-driven media gating must be mirrored there. It had video hardcoded false and screenshotOnRunFailure keyed off an unset var, so the esm cypress-reporting media test captured nothing. Both now gate on CYPRESS_ENABLE_FAILURE_MEDIA, matching the commonJS config. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
… Optimization convention)
Rename the two not-yet-released config flags from DD_CIVISIBILITY_TEST_FAILURE_{SCREENSHOTS,VIDEO}_ENABLED to DD_TEST_FAILURE_{SCREENSHOTS,VIDEO}_ENABLED, matching the newer DD_TEST_* family used by other test-optimization features (notably DD_TEST_FAILED_TEST_REPLAY_ENABLED, a sibling failure-capture feature) and the product naming. Internal property names (isTestFailure{Screenshots,Video}Enabled) are unchanged, so the exporter gate is untouched; regenerated config types + updated config and integration tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f64f92c0cb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
E2E Test Report: SUCCESS WITH LOCAL CONTRACT VERIFICATIONTested by: Shepherd Agent (autonomous QA for Datadog Test Optimization) Test Environment
Results
Test Methodology
Issues Found
This E2E test was performed by Shepherd - autonomous QA agent for Datadog Test Optimization. |
| it('uploads failure screenshots and the spec video to the v2 media endpoint', async function () { | ||
| const envVars = getCiVisAgentlessConfig(receiver.port) | ||
| const specToRun = 'cypress/e2e/basic-fail.js' | ||
| // 6.7.0 uses a flat JSON config that can't read env to enable media; skip it. | ||
| if (version === '6.7.0') { | ||
| this.skip() | ||
| return | ||
| } |
There was a problem hiding this comment.
you can use over10It instead of it instead of doing the version check here
There was a problem hiding this comment.
Done in 34371da — switched to over10It and dropped the manual version === '6.7.0' check (kept a one-line comment for the why). Thanks!
| class AgentProxyCiVisibilityExporter extends CiVisibilityExporter { | ||
| constructor (config) { | ||
| super(config) | ||
| // Agent-mode media upload is not wired yet: the Datadog Agent's evp_proxy must |
There was a problem hiding this comment.
this sucks. Would this work if the endpoint were /api/v2/ci/test-runs/<trace_id>/media ? If so, there is any plan to move away from unstable?
There was a problem hiding this comment.
Update: we're on /api/unstable because libraries only carry an API key (ValidReportingAPIUser), and lambo only accepts that auth method on /api/intake/ and /api/unstable/ by default. To move to /api/v2 we have to register the route in lambo's API-key allow-list (ApiKeyOnlyAuthRoutes) — I've put that up in https://github.com/ddoghq/dd-source/pull/5434 and I'm checking with the CoreAuthN/aaa-authn team that it fits the use case. Once it lands we can flip the prefix to /api/v2 (which also unblocks agent mode, since evp_proxy forwards /api/v2/* but not /api/unstable/).
| } | ||
|
|
||
| on('before:run', cypressPlugin.beforeRun.bind(cypressPlugin)) | ||
| on('after:screenshot', cypressPlugin.afterScreenshot.bind(cypressPlugin)) |
There was a problem hiding this comment.
[P2] Chain the user after:screenshot handler instead of replacing it
Cypress stores one callback per node event, except for task merging. In the auto path we collect user after:spec/after:run handlers but let user after:screenshot go straight through, then this registration replaces it. That drops existing screenshot hooks that move or redact screenshots, even when media upload is disabled. Please capture user after:screenshot handlers in the wrapper and chain them here, preserving any returned { path, size, dimensions } before uploading.
There was a problem hiding this comment.
Fixed in 34371da — we now capture the user's after:screenshot handler in the setupNodeEvents wrapper (same pattern we already use for after:spec/after:run) and chain it: the user handler runs first, we thread its returned { path, size, dimensions } forward (honoring Cypress's "undefined return ⇒ keep original"), then run our upload on the final details and return them to Cypress. It runs regardless of whether media upload is enabled, so screenshot move/redact hooks are preserved. Also covered the manual-plugin + auto-instrumentation path so the upload isn't dropped when a user handler is present.
| } | ||
|
|
||
| function isFailureScreenshot (screenshot) { | ||
| return !!getScreenshotFilePath(screenshot) && screenshot?.testFailure !== false |
There was a problem hiding this comment.
[P2] Do not treat screenshots without testFailure as failure screenshots
The screenshots in Cypress RunResult only expose name/takenAt/path/height/width, so the after:spec fallback usually cannot see testFailure. With this predicate, any manual cy.screenshot() taken by a test that later fails can be uploaded under the failure-screenshot flag, which is broader than the intended privacy contract. Please make this fallback use the stricter failure-screenshot check, for example requiring testFailure === true or the Cypress failure filename marker, and add a failed-test-with-manual-screenshot case.
There was a problem hiding this comment.
Done in 34371da — tightened the after:spec fallback predicate (isFailureScreenshot) to require an explicit failure signal: testFailure === true || filename includes '(failed)' (matching isFailureScreenshotForUpload). So a manual cy.screenshot() in a test that later fails is no longer swept into the failure-media upload. Added an integration test: a failing test that also takes cy.screenshot('before-failure'), asserting only the auto (failed) frame is uploaded and the manual capture never reaches the media endpoint.
- Hex-encode the idempotency key's filename portion before the X-Dd-Idempotency-Key header so a non-ASCII test title can't throw ERR_INVALID_CHAR and abort the run (deterministic; backend UUIDv5 unchanged). - Tighten the after:spec failure-screenshot predicate to require an explicit failure signal (testFailure === true || the '(failed)' marker), so manual cy.screenshot() captures aren't uploaded as failure media. - Chain the user's after:screenshot handler instead of replacing it, mirroring the after:spec/after:run capture, so user screenshot hooks are preserved. - Use over10It for the media-endpoint test instead of a manual version skip. Adds unit + integration coverage (non-ASCII filename; failing test with a manual screenshot uploads only the auto failure frame). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…ilure-media-v2 # Conflicts: # packages/dd-trace/src/config/generated-config-types.d.ts
juan-fernandez
left a comment
juan-fernandez
left a comment
There was a problem hiding this comment.
looking good! Left some integration test feedback
juan-fernandez
left a comment
juan-fernandez
left a comment
There was a problem hiding this comment.
One inline comment from the review follow-up.
A user-supplied after:screenshot handler that returns only { path, size, dimensions } previously replaced the whole details object, dropping Cypress metadata (testFailure, takenAt) the plugin relies on to decide which screenshots to upload. Merge the handler's return into the running details instead of replacing them.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…ilience Add agentless tests for screenshots-only and video-only flag combinations, a resilience test that the cypress run still completes and reports the failed test when the media endpoint returns 500, and a configurable media response status code on the intake mock. onlyAgentlessIt documents that media upload currently only goes through the agentless transport. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
A config test forbids internalPropertyName in supported-configurations.json. Switch the two DD_TEST_FAILURE_*_ENABLED entries to configurationNames with the same camelCase property name (the established pattern, e.g. DD_ENV -> env), so config.isTestFailureScreenshotsEnabled / isTestFailureVideoEnabled are unchanged and the generated types regenerate identically. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…oxy) upload (#9186) * fix(cypress): send failure-media metadata as query params + enable agent (evp_proxy) upload The Agent's evp_proxy strips X-Dd-* headers, so agent-mode failure-media uploads reached the backend with an empty idempotency key. Move idempotency_key and captured_at_ms to query params (survive the proxy) and enable the agent-mode upload path via evp_proxy (X-Datadog-EVP-Subdomain: api). Pairs with the dd-source backend change reading these from the query string. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * fix(cypress): hex-encode failure-media idempotency key filename for evp_proxy The Agent's evp_proxy validates the forwarded query string against a restrictive charset and rejects a raw Cypress screenshot filename (spaces, parens), so the query-param upload 502'd in agent mode. Restore the reviewed encoding (<traceId>:<hex(filename)>) applied to the query value: trace id stays readable, filename is hex-safe. Verified 201 through evp_proxy end-to-end. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * test(cypress): assert failure-media metadata on the query string, not headers The mock intake and the integration assertions still expected the old header-borne metadata, so the cypress media tests failed after the query-param move. Align them: the intake parses idempotency_key/captured_at_ms from the query, the path assertion ignores the query string, and a guard confirms the X-Dd-* metadata headers are gone. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
|
great job! 👏 |
2 participants
What does this PR do?
Adds opt-in upload of Cypress test-failure screenshots to the production v2 Test Optimization media endpoint (
POST /api/unstable/ci/test-runs/<trace_id>/media), so failure-moment screenshots show up on the test run in the Datadog UI.Builds on the innovation-day PoC #8574 (authored by @juan-fernandez, whose authorship is preserved on the first commits) and brings it up to the real bucket-backed v2 media contract. The remaining work was scoped and refined together with @juan-fernandez (#9154).
What's wired in:
DD_TEST_FAILURE_SCREENSHOTS_ENABLED(config.testOptimization.DD_TEST_FAILURE_SCREENSHOTS_ENABLED), default unset (off). Customer opt-in only; remote (library-settings) control is a follow-up. Naming follows theDD_TEST_*Test Optimization family (sibling ofDD_TEST_FAILED_TEST_REPLAY_ENABLED).packages/dd-trace/src/ci-visibility/requests/upload-test-screenshot.js): POSTs the raw image bytes with two headers the media service requires:X-Dd-Idempotency-Key=<trace_id>:<filename>— stable per artifact and reused on retry, so a retried upload overwrites the same stored object instead of duplicating. The filename portion is hex-encoded so a non-ASCII test title can't throwERR_INVALID_CHARinhttp.requestand abort the run (the transform is deterministic, so the backend's UUIDv5 overwrite-on-retry still holds).X-Dd-Media-Captured-At= capture time in epoch ms (from the screenshot'stakenAt, falling back to file mtime, then now). Stamped once and resent unchanged on retry, as it is part of the stored object key.trace_id(non-zero decimal uint64),DD_API_KEY, idempotency key, captured-at, and a non-empty file before sending; content type inferred from the extension (png/jpg/gif/webp).ci-visibility-exporter.js): newcanUploadTestScreenshots()(true only when the upload URL is configured and the flag is on) anduploadTestScreenshot(). Agentless only for now — the agentless exporter points the upload atapi.<site>withDD-API-KEY; the agent-proxy exporter leaves the URL undefined because the Datadog Agent'sevp_proxydoes not yet forward the media route (tracked as a follow-up).cypress-plugin.js,index.js,plugin.js, instrumentationcypress-config.js):after:screenshotas soon as Cypress writes a failure screenshot, with anafter:specfallback that scans the spec's screenshots per failed test/attempt; uploads are deduped by file path.testFailure === true, or the(failed)filename marker), so manualcy.screenshot()captures are not uploaded (privacy).after:screenshothandlers are preserved: the plugin chains the user handler(s) first and merges their (possibly partial) return into the running details rather than replacing them, so Cypress metadata such astestFailure/takenAtsurvives a handler that only returns{ path, size, dimensions }. Same chaining as the existingafter:spec/after:runwrapping.warnIfMisconfiguredTestFailureScreenshots()logs when the flag is on but CypressscreenshotOnRunFailureis off, or when not in agentless mode.Motivation
Cypress already captures a screenshot at the moment a test fails. Surfacing it on the test run in Datadog makes triaging CI failures much faster. The v2 media backend requires a stable idempotency key and a real capture timestamp from the tracer; this wires those in against the real endpoint (no PoC-only host/header). Backend: DataDog/dd-source#443668. Frontend (Media tab): DataDog/web-ui#320875.
Additional Notes
DD_TEST_FAILURE_VIDEO_ENABLEDflag and no video code path ships here.evp_proxy) mode is intentionally disabled. Agentless works today; agent forwarding of the media route is a follow-up.semver-minor(new opt-in feature).Tests
test/config/index.spec.js): default-off plus parse oftrue/false.ci-visibility-exporter.spec.js):canUploadTestScreenshots()gate matrix (URL × flag).requests/upload-test-screenshot.spec.js): header shape, validation errors, content-type, non-ASCII filename hex-encoding.cypress/cypress-reporting.spec.js,ci-visibility-intake.jsmedia handler with a configurable response status): a failing spec POSTs each failure screenshot to/api/unstable/ci/test-runs/<traceId>/mediawith the correctX-Dd-Idempotency-Keyshape and a positive-integerX-Dd-Media-Captured-At; the flag-on/off matrix; a manualcy.screenshot()is not uploaded as failure media; and the cypress run still completes and reports the failed test when the media endpoint returns 500. ESM and CommonJS configs both gate capture onCYPRESS_ENABLE_FAILURE_SCREENSHOTSso the upload tests capture screenshots.🤖 Generated with Claude Code