[ruby] Enable SymDB system tests#6962
Open
p-datadog wants to merge 13 commits into
Open
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ruby uses FILE root scopes whose path names can match the DebuggerController regex via substring. The old logic short-circuited on name match — if the scope_type wasn't CLASS/MODULE, it returned False without checking children. Now it always recurses when the current scope doesn't match both name and type. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
|
The RC diagnostics commit uses the boolean return value of wait_for to log timeout state and assert the wait succeeded. Update the wait_for return type from None to bool so mypy accepts the new diagnostics. Other callers ignore the return value, which remains valid.
Document the Ruby-specific reasons for the two test edits: - initialize_weblog_remote_config() is needed because Ruby's RC poller starts lazily from the Rack middleware - check_scope must always recurse because Ruby's FILE root scope name matches the regex but its scope_type does not
p-datadog
force-pushed
the
ddsign/symdb-ruby-tests
branch
from
e1bf829 to
ce1978e
Compare
|
The assert added in b033a47 was reached by every tracer's debugger scenarios via the shared send_state call site, and turned a previously tolerated silent-timeout into a hard collection-time error in 75 jobs across dotnet / nodejs / python / golang / java. Root cause is a real cross-tracer state-reporting gap (Go omits apply_state from config_states; Python drops the entry on a version update) but fixing those is out of scope for this PR. Gate the assert on context.library == "ruby" so the diagnostic stays loud for the SymDB tests this PR enables, and other tracers go back to their pre-PR silent behavior. The logger.error diagnostic lines remain always-on. Refs: DEBUG-5365
![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: 8a76b1f616
ℹ️ 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".
nccatoni
approved these changes
May 20, 2026
Address review comment: setup methods must not fail (.cursor/rules/pr-review.mdc §4). The `assert rv` added for Ruby was reachable from `Test_Debugger_SymDb._setup` via `send_rc_symdb` -> `send_symdb_command` -> `send_state`, violating the rule in CI. - The Ruby-specific raise is now gated on SYSTEM_TESTS_FAIL_FAST=true; default off preserves CI's setup-can't-raise invariant. - The two logger.error lines remain, so RC timeouts still surface in CI logs with the last-known targets_version and config_states. The test then fails downstream in _assert_symbols_uploaded as it did pre-PR. - Local debugging of Ruby RC plumbing: export SYSTEM_TESTS_FAIL_FAST=true to fail at the timeout point. Verified: ast.parse OK; no other asserts added in setup-reachable paths in the PR diff.
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e7a33c70e
ℹ️ 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".
The per-test missing_feature override for `test_event_metadata` is removed; the test now inherits the parent `Test_Debugger_SymDb` declaration (v2.34.0-dev for the three Rails weblogs). Also drops the stale `# TODO: drop the -dev suffix when v2.34.0 is released` comment. Verified: YAML parses; manifest entry now matches the parent's structure.
The multi-line `if` block introduced in 3e7a33c tripped `ruff format --check` in the `lint / lint` GitHub Actions job. Ruff prefers a single-line conditional here. Behavior is unchanged. Verified: `ruff check`, `ruff format --check`, and `mypy --config pyproject.toml` all pass on utils/_remote_config.py.
Strech
approved these changes
May 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enables
tests/debugger/test_debugger_symdb.py::Test_Debugger_SymDbfor the Ruby Rails weblogs (rails72,rails80,uds-rails), targeting gemv2.34.0-dev— the next dd-trace-rb release, where the SymDB component wiring lands (see DataDog/dd-trace-rb#5717, merged to master afterv2.33.0).Refs: DEBUG-5365.
Changes
manifests/ruby.yml— flipTest_Debugger_SymDbfrommissing_featuretov2.34.0-devfor the three Rails weblogs (includingTest_Debugger_SymDb::test_event_metadata).tests/debugger/test_debugger_symdb.py— two fixes that matter for Ruby:initialize_weblog_remote_config()in_setupso Ruby's lazy RC poller (Remote::Tie.boot, started from the Rack middleware on first request) gets a chance to start before the RC command is sent. No-op for other tracers.check_scopeto keep recursing into nested scopes when the name regex matches butscope_typeis not in the accepted list. This was short-circuiting on Ruby'sFILEroot scope (path containsdebugger_controller) and missing the nestedCLASS(DebuggerController).utils/_remote_config.py— add log lines for RC poll progress and a logged error on timeout so failures point at a state rather than a silent timeout. For Ruby, an additionalAssertionErroris raised at the timeout point whenSYSTEM_TESTS_FAIL_FAST=trueis set (default off), to fail loudly during local debugging of the RC poller. The default-off gate keeps setup paths from raising in CI per.cursor/rules/pr-review.mdc§4.Test plan
check_scoperecursion fix is a no-op for languages whoseDebuggerController-like scope already matches name+type at the same nodesend_statetiming — resolved by gating the Ruby fail-fast behindSYSTEM_TESTS_FAIL_FAST(default off); CI behavior for all scenarios is unchanged from pre-PRVerification — SymDB scenario ran the enabled tests
Run 26055212800 (commit
ce1978e8b). DEBUGGER_SYMDB is scheduled into End-to-end #2 —logs_debugger_symdb/is only present in the*_dev_2artifacts, andreport.jsonshows the tests actually executed (not deselected, not skipped):test_symdb_uploadtest_event_metadatarails72rails80uds-railsSanity checks:
collected: 2495, deselected: 2491in the DEBUGGER_SYMDB report → 4 tests executed (the 2 SymDB above + 2 unrelatedTest_DdtraceSchemasruntime checks).prodside (released gemv2.33.0<v2.34.0-dev) collected 0 SymDB tests in all three Rails weblogs — thev2.34.0-devgate keeps them onmissing_featureagainst the released gem, as expected.dev_2jobs is fromtests/schemas/test_schemas.py::Test_DdtraceSchemas::test_library, unrelated to SymDB.