fix(cypress): failure-media metadata via query params + agent (evp_proxy) upload#9186
Conversation
…ent (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]>
Overall package sizeSelf size: 6.44 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 |
|
Paired backend change: ddoghq/dd-source#9350 — reads |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: cf0808f | Docs | Datadog PR Page | Give us feedback! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## xinye.ji/cypress-failure-media-v2 #9186 +/- ##
==================================================================
Coverage 93.69% 93.69%
==================================================================
Files 893 893
Lines 51427 51432 +5
Branches 11973 11976 +3
==================================================================
+ Hits 48182 48187 +5
Misses 3245 3245 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:
|
…vp_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]>
BenchmarksBenchmark execution time: 2026-07-02 14:22:14 Comparing candidate commit cf0808f in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2252 metrics, 34 unstable metrics.
|
… 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]>
![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: cf0808f67a
ℹ️ 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".
| this._codeCoverageReportUrl = this._url | ||
| // Screenshot media uploads go through the Agent's evp_proxy: the uploader prefixes the | ||
| // path with evpProxyPrefix and sets X-Datadog-EVP-Subdomain: api (see uploadTestScreenshot). | ||
| this._testScreenshotUploadUrl = this._url |
There was a problem hiding this comment.
When tracer.setUrl() is called after the agent exporter has initialized, _setUrl() moves the trace and coverage writers to the new agent URL, but the screenshot upload URL remains the old this._url captured here. In agent/EVP mode with DD_TEST_FAILURE_SCREENSHOTS_ENABLED, Cypress media uploads will keep POSTing to the stale agent while the rest of CI Visibility uses the updated URL, so screenshots can be lost for users/tests that reconfigure the agent URL. Mirror the agentless exporter by updating _testScreenshotUploadUrl from setUrl() as well.
Useful? React with 👍 / 👎.
![datadog-prod-us1-3[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
The metadata-to-query-params migration and EVP proxy path construction are correct for all production code paths. One asymmetry found: the function guards every other invalid input (missing traceId, DD_API_KEY, idempotencyKey, invalid capturedAtMs) but has no guard for isEvpProxy: true with a missing/undefined evpProxyPrefix, which silently templates "undefined/api/v2/..." into the request path instead of returning an error. The production call site in agent-proxy always sets both fields together, so real users are unaffected today — but the missing guard is inconsistent with the function's own pattern.
📊 Validated against 5 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit cf0808f · What is Autotest? · Any feedback? Reach out in #autotest
| if (isEvpProxy) { | ||
| // Agent mode: prefix the evp_proxy path, tell the proxy which subdomain to forward to, and | ||
| // drop the API key — the Agent injects it. The query params survive the proxy. | ||
| options.path = `${evpProxyPrefix}${basePath}?${query}` | ||
| options.headers['X-Datadog-EVP-Subdomain'] = 'api' | ||
| } else { | ||
| options.headers['DD-API-KEY'] = DD_API_KEY | ||
| } |
There was a problem hiding this comment.
Any caller that sets isEvpProxy:true without evpProxyPrefix (e.g. a future test or subclass) silently issues an HTTP request to a broken path; the upload fails with a confusing 404 or ECONNREFUSED rather than a clear configuration error.
Assertion details
- Input: uploadTestScreenshot({ ..., isEvpProxy: true, evpProxyPrefix: undefined }, cb) — calling the function with EVP proxy enabled but no prefix supplied
- Expected:
callback called with a clear Error ("evpProxyPrefix is required"), request not sent - Actual: request is sent to path
"undefined/api/v2/ci/test-runs/.../media?..."— the literal string "undefined" is used as the path prefix, silently misrouting to a non-existent endpoint
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · Any feedback? Reach out in #autotest
| if (isEvpProxy) { | |
| // Agent mode: prefix the evp_proxy path, tell the proxy which subdomain to forward to, and | |
| // drop the API key — the Agent injects it. The query params survive the proxy. | |
| options.path = `${evpProxyPrefix}${basePath}?${query}` | |
| options.headers['X-Datadog-EVP-Subdomain'] = 'api' | |
| } else { | |
| options.headers['DD-API-KEY'] = DD_API_KEY | |
| } | |
| if (isEvpProxy) { | |
| if (!evpProxyPrefix) { | |
| return callback(new Error('evpProxyPrefix is required for Agent evp_proxy upload')) | |
| } | |
| // Agent mode: prefix the evp_proxy path, tell the proxy which subdomain to forward to, and | |
| // drop the API key — the Agent injects it. The query params survive the proxy. | |
| options.path = `${evpProxyPrefix}${basePath}?${query}` | |
| options.headers['X-Datadog-EVP-Subdomain'] = 'api' | |
| } else { | |
| options.headers['DD-API-KEY'] = DD_API_KEY | |
| } |
2 participants
What does this PR do?
Two coupled changes to the Cypress failure-media upload, both required to make agent-mode uploads work:
X-Dd-*headers to query params. The uploader now appends?idempotency_key=<k>&captured_at_ms=<ms>(built withURLSearchParams) to the media path in both agentless and agent modes, and no longer setsX-Dd-Idempotency-Key/X-Dd-Media-Captured-At._testScreenshotUploadUrl = undefined, socanUploadTestScreenshots()always returnedfalsein agent mode. It now points at the agent URL (in the evp-compatible branch), and the uploader builds theevp_proxypath +X-Datadog-EVP-Subdomain: apiand drops the clientDD-API-KEY(the Agent injects it).Motivation
The Datadog Agent's
evp_proxyforwards only an allow-listed set of headers and stripsX-Dd-Idempotency-Key/X-Dd-Media-Captured-At. Agent-mode uploads therefore reached the backend with an empty idempotency key, which the media service rejected asMALFORMED. Query params pass through the proxy untouched.This pairs with a dd-source backend change that now reads
idempotency_keyandcaptured_at_msfrom the query string.Because the values are URL-encoded via
URLSearchParams, the header-safety hex hack (toIdempotencyHeaderValue) is no longer needed and has been removed — a non-ASCII filename in the idempotency key can no longer throwERR_INVALID_CHAR.End state
POST https://api.<site>/api/v2/ci/test-runs/<trace>/media?idempotency_key=...&captured_at_ms=...withDD-API-KEY, no EVP subdomain header.POST <agent>/evp_proxy/v{N}/api/v2/ci/test-runs/<trace>/media?idempotency_key=...&captured_at_ms=...withX-Datadog-EVP-Subdomain: apiand no clientDD-API-KEY.Stacked on
Base branch is
xinye.ji/cypress-failure-media-v2.Testing
eslintclean on all four changed files.upload-test-screenshot.spec.jsrewritten to assert query params (both modes) + the evp-proxy path/header/key behaviour — 5 passing.ci-visibility-exporter.spec.js+agent-proxy.spec.js— 60 passing.🤖 Generated with Claude Code