[Debugger] Add system-test coverage for the debugger guardrails#7287
[Debugger] Add system-test coverage for the debugger guardrails#7287dudikeleti wants to merge 18 commits into
Conversation
|
|
🎉 All green!🧪 All tests passed 🔄 Datadog auto-retried 1 job - 1 passed on retry 🔗 Commit SHA: 3253db3 | Docs | Datadog PR Page | Give us feedback! |
Bits couldn’t fix the CI🟢 Investigated · 🔴 Fix failed · ⚪ Validate · ⚪ Ready The newly enabled View in Datadog | Reviewed commit 2c71ee0 |
dudikeleti
changed the title
3babba7 to
f2c565e
Compare
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Adds focused end-to-end system-test coverage for debugger “guardrails” (evaluation timeout, snapshot size cap, and capture-timeout reporting) in the system-tests framework, wiring up new fixtures/endpoints across multiple weblogs and gating execution via orchestrator support checks and per-language manifests.
Changes:
- Introduces new debugger guardrail tests (evaluation-timeout, snapshot-size cap, capture-timeout reason) plus new probe fixtures and a dedicated
DEBUGGER_CAPTURE_TIMEOUTscenario. - Extends Node.js/Java/.NET test apps with a
/debugger/snapshot/capture-timeoutendpoint and updates Dockerfiles to include the debugger route modules. - Updates CI orchestration and “test-the-test” coverage (scenario mapping, workflow step, and
_is_supportedgating) and marks tests appropriately in language manifests.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/scripts/ci_orchestrators/workflow_data.py | Restricts DEBUGGER_CAPTURE_TIMEOUT scenario to weblogs that include the new fixture. |
| utils/build/docker/nodejs/uds-express4.Dockerfile | Copies the Express debugger routes into the UDS Express4 image. |
| utils/build/docker/nodejs/fastify/debugger/index.js | Adds capture-timeout fixture route and local variable construction for snapshot capture. |
| utils/build/docker/nodejs/fastify.Dockerfile | Copies fastify debugger routes into the image. |
| utils/build/docker/nodejs/express5.Dockerfile | Copies Express debugger routes into the Express5 image. |
| utils/build/docker/nodejs/express4.Dockerfile | Copies Express debugger routes into the Express4 image. |
| utils/build/docker/nodejs/express4-typescript/debugger/index.ts | Adds capture-timeout fixture route for the TS Express4 app. |
| utils/build/docker/nodejs/express4-typescript.Dockerfile | Copies TS debugger routes into the image. |
| utils/build/docker/nodejs/express/debugger/index.js | Adds capture-timeout fixture route for Express apps. |
| utils/build/docker/java/spring-boot/src/main/java/com/datadoghq/system_tests/springboot/debugger/DebuggerController.java | Adds capture-timeout endpoint/fixture for Java Spring Boot. |
| utils/build/docker/dotnet/weblog/Controllers/DebuggerController.cs | Adds capture-timeout endpoint/fixture for .NET weblog. |
| utils/_context/_scenarios/init.py | Adds DEBUGGER_CAPTURE_TIMEOUT scenario with tight capture/serialization budgets. |
| tests/test_the_test/test_compute_libraries_and_scenarios.py | Updates expected scenario list to include DEBUGGER_CAPTURE_TIMEOUT. |
| tests/test_the_test/test_ci_orchestrator.py | Adds regression test ensuring _is_supported gating matches expected fixture-enabled weblogs. |
| tests/test_the_test/scenarios.json | Maps new guardrail tests to scenarios (DEBUGGER_CAPTURE_TIMEOUT and DEBUGGER_PROBES_SNAPSHOT). |
| tests/debugger/utils/probes/probe_snapshot_size_cap.json | New probe fixture for snapshot-size cap behavior. |
| tests/debugger/utils/probes/probe_evaluation_timeout_regex.json | New probe fixture for ReDoS-style regex evaluation timeout. |
| tests/debugger/utils/probes/probe_evaluation_timeout_collection_filter.json | New probe fixture for expensive collection-filter evaluation timeout. |
| tests/debugger/utils/probes/probe_capture_timeout_reason.json | New probe fixture to validate notCapturedReason='timeout'. |
| tests/debugger/utils.py | Extracts captures_contain_data helper and adds line mapping for CaptureTimeout. |
| tests/debugger/test_debugger_probe_snapshot.py | Extends default max-length test expectations (string truncation metadata). |
| tests/debugger/test_debugger_guardrails.py | Adds new guardrail-focused debugger compliance tests. |
| tests/debugger/test_debugger_condition_errors.py | Switches condition-error tests to use the shared captures_contain_data helper. |
| manifests/ruby.yml | Marks new guardrail tests as missing_feature for Ruby. |
| manifests/python.yml | Marks new guardrail tests as missing_feature for Python. |
| manifests/php.yml | Marks new guardrail tests as missing_feature / incomplete_test_app where applicable. |
| manifests/nodejs.yml | Adds per-weblog declarations for the capture-timeout fixture availability; marks others missing_feature. |
| manifests/java.yml | Enables capture-timeout guardrail test for select Java weblogs; marks other guardrails missing_feature. |
| manifests/golang.yml | Marks new guardrail tests missing_feature/incomplete_test_app for Go. |
| manifests/dotnet.yml | Enables capture-timeout guardrail test for .NET; marks other guardrails missing_feature. |
| docs/understand/weblogs/end-to-end_weblog.md | Documents the new /debugger/snapshot/capture-timeout endpoint semantics. |
| .github/workflows/run-end-to-end.yml | Adds CI workflow step to execute the new DEBUGGER_CAPTURE_TIMEOUT scenario. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: f2c565e096
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
19a1599 to
273ee3c
Compare
…reasons and evaluation timeout behavior, with new rows disabled in manifests until tracer support is ready.
…re timeout reporting. 1MB snapshot cap and notCapturedReason=timeout behavior.
…ves the setup-time negative snapshot wait so evaluation-timeout assertions rely on collected scenario output instead of a fixed sleep.
…upported `notCapturedReason`.
…t relying on runner timing.
Detect delayed duplicate snapshots, validate complete backend request sizes, and avoid tracer-specific error messages. Tune probe limits and fixtures for reliable timeout and pruning coverage.
Preserve the fixture-specific scenario matrix after the orchestrator refactor and remove the unreachable legacy allowlist.
ef3399b to
7bef3e3
Compare
![datadog-datadog-prod-us1-2[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
Assertions in the new guardrail tests are internally consistent and correctly layered: _contains_payload_pruning and _contains_not_captured_reason recurse safely through all nested tracer shapes, _get_captured_local correctly targets captures.lines (the probe is converted to a line probe by _setup_snapshot_guardrail), and the probe capture limits in probe_snapshot_size_cap.json are set just above the test inputs so the snapshot-size guardrail rather than the capture-level limit fires. No behavioral regressions found.
📊 Validated against 12 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit ef3399b · What is Autotest? · Any feedback? Reach out in #autotest
| "DD_DYNAMIC_INSTRUMENTATION_ENABLED": "1", | ||
| }, | ||
| doc="Test that debugger snapshot capture reports when its time budget is exceeded", | ||
| weblog_categories=[], |
There was a problem hiding this comment.
This means the scenario will never be executed. is it what you want ?
There was a problem hiding this comment.
Ok, I see you've added it in weblog_metadata. In theory, those metadata should not be used for test activation, as it's the purpose of manifests. Do you have a reason to do so ?
There was a problem hiding this comment.
There is a cleaner approach if ever you'd like to run only on some weblog, it's using the other way around :
- the scenario applies for weblog shipping a
ddtracelibrary, so we must keep this property : keep weblog categories as it. - Add the scenario in weblog_metadata
excluded_scenariosproperty.
With that, we don't hack the weblog_categories semantic.
| "DD_DYNAMIC_INSTRUMENTATION_ENABLED": "1", | ||
| }, | ||
| doc="Test that debugger snapshot capture reports when its time budget is exceeded", | ||
| weblog_categories=[], |
There was a problem hiding this comment.
There is a cleaner approach if ever you'd like to run only on some weblog, it's using the other way around :
- the scenario applies for weblog shipping a
ddtracelibrary, so we must keep this property : keep weblog categories as it. - Add the scenario in weblog_metadata
excluded_scenariosproperty.
With that, we don't hack the weblog_categories semantic.
Motivation
Add focused system-test coverage for the debugger guardrails required for GA.
Changes
evaluationErrors[]populated and no captured data.notCapturedReason=timeout, without incorrectly satisfying the snapshot-size guardrail.DEBUGGER_CAPTURE_TIMEOUTscenario with a 10 ms capture budget, CI orchestration, and compatible weblog filtering./debugger/snapshot/capture-timeoutfixture to supported .NET, Java, and Node.js weblogs and document the endpoint.test_default_max_lengthto verify that truncated strings reporttruncated=true.captures_contain_datahelper intotests.debugger.utils.Workflow
🚀 Once your PR is reviewed and the CI green, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
tests/ormanifests/is modified ? I have the approval from R&P teambuild-XXX-imagelabel is present