SymDB: filter Class objects detached from their constant in extract_all by p-datadog · Pull Request #5872 · DataDog/dd-trace-rb

p-datadog · 2026-06-08T22:15:43Z

What does this PR do?

Filters out leaked Class and Module objects from symbol-database extraction so they don't get uploaded alongside the current bindings.

Motivation:

CRuby caches Module#name. When customer code calls Object.send(:remove_const, :Foo), the Foo Class doesn't disappear — it stays alive in ObjectSpace and still answers "Foo" when asked for its name. If a new class Foo is then defined at a different file, the process has two distinct Class objects both claiming the name "Foo".

Symbol-database extraction walks ObjectSpace and groups entries by Module#name. With two Foos, the hash collision is broken by ObjectSpace iteration order — last-write-wins. The "winner" can pair a MODULE entry from one binding with a CLASS entry from a different binding, placing them in separate FILE trees. The resulting MODULE scope has no nested CLASS. Scope#to_h drops the empty scopes key via h.compact!, and downstream .find on it raises NoMethodError: undefined method 'find' for nil:NilClass.

Where this matters:

Customer impact: Rails reloaders, hot-loaded plugins, and certain DSL idioms use remove_const-then-redefine. Without this fix, stale Class objects ship to the symbol database alongside the current bindings.
CI flakiness: PR #5525 CI run 26135917255 failed spec/datadog/symbol_database/remote_config_integration_spec.rb:76 on Ruby 3.2 [3] and Ruby 4.0 [0]. The same bug is on master but dormant — recent CI seeds happened to iterate ObjectSpace in an order that paired matching MODULE+CLASS entries.

The fix: Extractor#collect_extractable_modules now skips any Class/Module whose cached Module#name no longer resolves to the same object through Ruby's constant table. Stale entries are filtered before the name-keyed hash is built, so the collision never happens.

Change log entry

Yes. Symbol database: stale class definitions are no longer uploaded in applications that use remove_const-then-redefine patterns (Rails reloaders, hot-loaded plugins). Affects autocomplete accuracy in the Dynamic Instrumentation UI.

Additional Notes:

resolves_to_same_module? walks the namespace path segment-by-segment, calling Module#autoload? at each step. A pending autoload at any segment is treated as "does not resolve" without ever calling const_get on the autoload target.

This matters because extraction must not load customer code as a side effect: a naive Object.const_get(mod_name) triggers any autoload registered for mod_name, loading customer files and raising LoadError if the target file is missing. LoadError is ScriptError, not StandardError, so it isn't caught by the rescue blocks in collect_extractable_modules — without the segment walk, extract_all would abort instead of just skipping the leaked Class.

How to test the change?

Two new unit tests in spec/datadog/symbol_database/extractor_spec.rb under describe '.extract_all':

'class detached from its constant via remove_const' — loads a class, captures a hard reference, remove_consts, redefines at a different path, then asserts extract_all produces only the new binding's FILE scope.
'autoload registered after remove_const' — loads a class, captures a reference, remove_consts, registers an autoload at the same name pointing at a non-existent file, then asserts extract_all runs without raising and the autoload remains pending (no require triggered).

Verified locally on Ruby 3.2.3:

Both new unit tests pass.
Re-running the original flaky scope (spec/datadog/symbol_database/**/*_spec.rb,spec/datadog/core/**/*_spec.rb with seed 32729, which deterministically reproduced the symdb failure before this change) no longer reports the failure.
All 351 examples in spec/datadog/symbol_database/**/*_spec.rb pass.
bundle exec steep check lib/datadog/symbol_database/extractor.rb is clean.

CRuby caches `Module#name`. After `Object.send(:remove_const, :Foo)`, the Class stays in `ObjectSpace` and `mod.name` still returns the cached FQN "Foo". Without a guard, two Class objects can report the same name during extraction — a leaked one from a prior `remove_const`-then-redefine and the current binding. `Extractor#collect_extractable_modules` keys `entries` by name, so the iteration-order winner is undefined; the MODULE entry can pair with a CLASS entry from a different binding (different `source_location`), producing FILE scopes whose MODULE child has no nested CLASS — and `Scope#to_h` then drops the empty `scopes` key via `h.compact!`. This caused a flaky failure on PR 5525's CI run 26135917255 (Ruby 3.2 [3] and Ruby 4.0 [0]): `spec/datadog/symbol_database/remote_config_integration_spec.rb:76` raised `NoMethodError: undefined method 'find' for nil:NilClass` at line 123 because `module_scope['scopes']` was absent. The bug also exists on master; the flake there is dormant only because recent CI seeds happened to iterate ObjectSpace in an order that paired matching MODULE+CLASS entries. Fix: in `collect_extractable_modules`, require that `mod.name` still resolves to a constant pointing at `mod` (via `Object.const_get(mod_name).equal?(mod)`). `const_defined?` short-circuits the lookup so unresolved paths do not trigger `const_missing` on the customer's `Object` class. Anonymous-but-named Classes leaked by `remove_const` are skipped before they can collide in the name-keyed `entries` hash. Verified locally on Ruby 3.2.3: - The new unit test `Extractor.extract_all class detached from its constant via remove_const extracts the currently-bound class only` passes (and would fail on master without this change). - The flaky scenario reproduced with seed 32729 on `spec/datadog/symbol_database/**/*_spec.rb,spec/datadog/core/**/*_spec.rb` no longer reports the symdb failure. - All 350 examples in `spec/datadog/symbol_database/**/*_spec.rb` pass. - `bundle exec steep check lib/datadog/symbol_database/extractor.rb` clean.

dd-octo-sts · 2026-06-08T22:16:09Z

Typing analysis

Note: Ignored files are excluded from the next sections.

`steep:ignore` comments

This PR introduces 12 steep:ignore comments, and clears 12 steep:ignore comments.

steep:ignore comments (+12-12)

❌ Introduced:

lib/datadog/symbol_database/extractor.rb:306
lib/datadog/symbol_database/extractor.rb:318
lib/datadog/symbol_database/extractor.rb:332
lib/datadog/symbol_database/extractor.rb:477
lib/datadog/symbol_database/extractor.rb:588
lib/datadog/symbol_database/extractor.rb:618
lib/datadog/symbol_database/extractor.rb:740
lib/datadog/symbol_database/extractor.rb:804
lib/datadog/symbol_database/extractor.rb:805
lib/datadog/symbol_database/extractor.rb:885
lib/datadog/symbol_database/extractor.rb:886
lib/datadog/symbol_database/extractor.rb:934

✅ Cleared:

lib/datadog/symbol_database/extractor.rb:254
lib/datadog/symbol_database/extractor.rb:266
lib/datadog/symbol_database/extractor.rb:280
lib/datadog/symbol_database/extractor.rb:425
lib/datadog/symbol_database/extractor.rb:536
lib/datadog/symbol_database/extractor.rb:566
lib/datadog/symbol_database/extractor.rb:678
lib/datadog/symbol_database/extractor.rb:742
lib/datadog/symbol_database/extractor.rb:743
lib/datadog/symbol_database/extractor.rb:823
lib/datadog/symbol_database/extractor.rb:824
lib/datadog/symbol_database/extractor.rb:872

Untyped methods

This PR introduces 7 partially typed methods, and clears 7 partially typed methods. It increases the percentage of typed methods from 65.14% to 65.16% (+0.02%).

Partially typed methods (+7-7)

❌ Introduced:

sig/datadog/symbol_database/extractor.rbs:41
└── def build_class_language_specifics: (Class klass) -> Hash[::Symbol, untyped]
sig/datadog/symbol_database/extractor.rbs:55
└── def collect_extractable_modules: () -> Hash[String, untyped]
sig/datadog/symbol_database/extractor.rbs:57
└── def group_methods_by_file: (Module mod) -> Hash[String, Array[untyped]]
sig/datadog/symbol_database/extractor.rbs:59
└── def build_file_trees: (Hash[String, untyped] entries) -> Hash[String, untyped]
sig/datadog/symbol_database/extractor.rbs:61
└── def place_in_tree: (untyped root, Array[String] name_parts, Module mod, String mod_name, Array[untyped] methods, String file_path) -> void
sig/datadog/symbol_database/extractor.rbs:65
└── def convert_trees_to_scopes: (Hash[String, untyped] file_trees) -> Array[Scope]
sig/datadog/symbol_database/extractor.rbs:67
└── def convert_node_to_scope: (untyped node) -> Scope

✅ Cleared:

sig/datadog/symbol_database/extractor.rbs:39
└── def build_class_language_specifics: (Class klass) -> Hash[::Symbol, untyped]
sig/datadog/symbol_database/extractor.rbs:53
└── def collect_extractable_modules: () -> Hash[String, untyped]
sig/datadog/symbol_database/extractor.rbs:55
└── def group_methods_by_file: (Module mod) -> Hash[String, Array[untyped]]
sig/datadog/symbol_database/extractor.rbs:57
└── def build_file_trees: (Hash[String, untyped] entries) -> Hash[String, untyped]
sig/datadog/symbol_database/extractor.rbs:59
└── def place_in_tree: (untyped root, Array[String] name_parts, Module mod, String mod_name, Array[untyped] methods, String file_path) -> void
sig/datadog/symbol_database/extractor.rbs:63
└── def convert_trees_to_scopes: (Hash[String, untyped] file_trees) -> Array[Scope]
sig/datadog/symbol_database/extractor.rbs:65
└── def convert_node_to_scope: (untyped node) -> Scope

If you believe a method or an attribute is rightfully untyped or partially typed, you can add # untyped:accept on the line before the definition to remove it from the stats.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a0a36edde2

ℹ️ 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".

testing/test-strategy.md lists 26 tests for the implicit-enablement work. The tracer-side tests through 13 had landed; the integration and guard tests below were still open. Each test is a unique implicit- enablement assertion; the testing.md text per test explains what the fix protects against if a future change breaks it. Tests added: - Test 8 — ComponentsState round-trip across Datadog.configure. New spec/datadog/core/configuration/components_state_spec.rb covers the value class. components_spec.rb gains #state (captures di_implicitly_enabled? from started?, false otherwise) and #startup! with old_state contexts (carry-over starts new component, no-carry stays stopped, env var still wins on initial startup). - Tests 14, 15 — APM_TRACING → handle_rc_enablement integration. New spec/datadog/di/integration/implicit_enablement_spec.rb drives Tracing::Remote.process_config end-to-end and asserts start/stop, idempotence, restart, code-tracking activation, receiver gating before and after enablement, and probe removal on RC disable. - Tests 21, 22 — line and method probe coverage across enablement timing. New spec/datadog/di/integration/probe_coverage_spec.rb plus four distinct fixture files (pre/post × line/method). Distinct class names per matrix cell — reusing a class name would force remove_const-then-redefine, which leaks stale Class objects with cached Module#name into ObjectSpace (the same CRuby behavior PR #5872 addresses for SymDB) and made the test order-dependent. - Test 23 — RC handler leaves DI settings untouched. Snapshot diff on settings.dynamic_instrumentation around handle_rc_enablement(true) and (false). Guards the "RC only enables/disables, never updates settings" decision from design/architecture.md. - Test 24 — code tracker registry survives Component#stop!. Loads a fixture under tracking, capture registry keys, stop!/start! the component, assert all keys still present. Without this, a probe whose target file was loaded before the stop!/start! cycle would silently never fire after restart. - Test 25 — DI.add_current_component invariant from build. Asserts Component.build registers the component in DI.@current_components so the code-tracker callback can locate it without going through Datadog.send(:components). Removal on shutdown! also asserted. - Test 26 — Component#build performs no I/O and spawns no threads. Companion to test 10's "RC handler returns promptly" — covers the initialize path that runs during Components#initialize and would delay application boot if it blocked. Bug surfaced and fixed: DI::Remote.explicitly_disabled? used options[:enabled].default_precedence?, which NoMethodError'd when the option hash hadn't been materialized — same root cause and same fix as the Component.explicitly_enabled? change in 03682a6. Switched to using_default?(:enabled). The integration spec exercises the path that hit this on the test 15 disable case. Verified: rake test:di:di_with_ext — 837 examples, 0 failures, 8 pending. probe_coverage_spec.rb run in isolation 5x — stable. bundle exec steep check lib/datadog/di/remote.rb — clean.

datadog-prod-us1-4 · 2026-06-08T22:33:55Z

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 7.69%
• Overall Coverage: 90.03% (-0.03%)

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: fe6b07d | Docs | Datadog PR Page | Give us feedback!}

pr-commenter · 2026-06-08T22:53:11Z

Benchmarks

Benchmark execution time: 2026-06-09 17:45:43

Comparing candidate commit 8f6a5d7 in PR branch symdb-fix-module-name-cache with baseline commit e014f83 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 45 metrics, 1 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

🟩 = significantly better candidate vs. baseline
🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

testing/test-strategy.md lists 26 tests for the implicit-enablement work. The tracer-side tests through 13 had landed; the integration and guard tests below were still open. Each test is a unique implicit- enablement assertion; the testing.md text per test explains what the fix protects against if a future change breaks it. Tests added: - Test 8 — ComponentsState round-trip across Datadog.configure. New spec/datadog/core/configuration/components_state_spec.rb covers the value class. components_spec.rb gains #state (captures di_implicitly_enabled? from started?, false otherwise) and #startup! with old_state contexts (carry-over starts new component, no-carry stays stopped, env var still wins on initial startup). - Tests 14, 15 — APM_TRACING → handle_rc_enablement integration. New spec/datadog/di/integration/implicit_enablement_spec.rb drives Tracing::Remote.process_config end-to-end and asserts start/stop, idempotence, restart, code-tracking activation, receiver gating before and after enablement, and probe removal on RC disable. - Tests 21, 22 — line and method probe coverage across enablement timing. New spec/datadog/di/integration/probe_coverage_spec.rb plus four distinct fixture files (pre/post × line/method). Distinct class names per matrix cell — reusing a class name would force remove_const-then-redefine, which leaks stale Class objects with cached Module#name into ObjectSpace (the same CRuby behavior PR #5872 addresses for SymDB) and made the test order-dependent. - Test 23 — RC handler leaves DI settings untouched. Snapshot diff on settings.dynamic_instrumentation around handle_rc_enablement(true) and (false). Guards the "RC only enables/disables, never updates settings" decision from design/architecture.md. - Test 24 — code tracker registry survives Component#stop!. Loads a fixture under tracking, capture registry keys, stop!/start! the component, assert all keys still present. Without this, a probe whose target file was loaded before the stop!/start! cycle would silently never fire after restart. - Test 25 — DI.add_current_component invariant from build. Asserts Component.build registers the component in DI.@current_components so the code-tracker callback can locate it without going through Datadog.send(:components). Removal on shutdown! also asserted. - Test 26 — Component#build performs no I/O and spawns no threads. Companion to test 10's "RC handler returns promptly" — covers the initialize path that runs during Components#initialize and would delay application boot if it blocked. Bug surfaced and fixed: DI::Remote.explicitly_disabled? used options[:enabled].default_precedence?, which NoMethodError'd when the option hash hadn't been materialized — same root cause and same fix as the Component.explicitly_enabled? change in 03682a6. Switched to using_default?(:enabled). The integration spec exercises the path that hit this on the test 15 disable case. Verified: rake test:di:di_with_ext — 837 examples, 0 failures, 8 pending. probe_coverage_spec.rb run in isolation 5x — stable. bundle exec steep check lib/datadog/di/remote.rb — clean.

…autoload Object.const_get(mod_name) on a name registered for autoload triggers the autoload — loading customer code as a side effect of symbol extraction and raising LoadError when the target file is missing. LoadError is ScriptError, not StandardError, so neither the local rescue (NameError, ArgumentError) nor the outer rescue in collect_extractable_modules (StandardError) catches it, and extract_all aborts instead of skipping the leaked class. Walk the namespace segment-by-segment using Module#autoload? at each step. A pending autoload at any segment is treated as "does not resolve" without calling const_get on the autoload target. Address review comment on PR #5872 from chatgpt-codex-connector[bot]. Co-Authored-By: Claude <[email protected]>

The rescue catches expected "no" outcomes for the stale-class filter: NameError when a segment doesn't resolve, ArgumentError from to_sym on a malformed name. These are exactly the cases this predicate exists to detect — not failures worth logging. Per this file's own rescue convention in the header comment: "Inner per-item rescues (bare `rescue` in const_get loops, method.name): Skip one constant or name lookup without aborting the enclosing collection. These are expected failures — no logging needed." Address review feedback on PR #5872. Co-Authored-By: Claude <[email protected]>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 29d205f6ff

ℹ️ 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".

Module#autoload? defaults to inherit=true, so when a subclass has a constant directly defined and an ancestor has a pending autoload at the same name, the prior code returned false at the autoload? check before the const_defined? check ran. The real subclass binding was silently dropped from the symbol database. Also: const_get(sym, false) does NOT skip a direct autoload — it triggers it just like the default form would. So "check const_defined? first, then const_get safely" is not safe when the const_defined? result is "defined as an autoload registration on this namespace." The correct order is: detect pending direct autoload first; only then does const_get(sym, false) on a direct binding become safe. Use Module#autoload?(sym, false) on Ruby 2.7+ to ask "is there an autoload registered directly on this namespace?". The inherit parameter was added in Ruby 2.7 (Ruby 2.7.0 NEWS). dd-trace-rb's minimum Ruby is 2.5.0 (lib/datadog/version.rb), so the fallback branch uses Module#autoload?(sym) on 2.5/2.6, with the documented limitation that a subclass binding whose name collides with an ancestor's pending autoload is conservatively treated as stale on those Rubies. Test added: subclass with constant of same name as ancestor pending autoload. Verified the ancestor's autoload remains pending (no require triggered) and the subclass binding is included in the extracted payload. Address review comment on PR #5872 from chatgpt-codex-connector[bot]. Co-Authored-By: Claude <[email protected]>

The "subclass with constant of same name as ancestor pending autoload" test verifies that resolves_to_same_module? distinguishes a real subclass binding from an ancestor's pending autoload. That distinction requires Module#autoload?(name, false) to query autoloads registered directly on a namespace without checking ancestors. The `inherit` parameter was added in Ruby 2.7 (Ruby 2.7.0 NEWS). On Ruby 2.5/2.6 the implementation falls back to Module#autoload?(name) which checks ancestors — so a subclass binding that collides with an ancestor's pending autoload is conservatively treated as stale and dropped. That fallback is documented in resolves_to_same_module?'s YARD comment and is unavoidable without the 2.7+ API. Skip the test on Ruby < 2.7 to align verification with the platform that supports the asserted behavior. The conservative-fallback behavior on 2.5/2.6 is already exercised by every leaked-class case in collect_extractable_modules. Co-Authored-By: Claude <[email protected]>

eregon · 2026-06-10T11:39:55Z

        entries = {}
        seen = 0

        ObjectSpace.each_object(Module) do |mod|


Maybe we could walk through Object.constants and recursively?
That would avoid that expensive check for resolves_to_same_module? by construction.
It's likely quite. a bit faster than ObjectSpace.each_object too

This seems feasible. I will investigate as a follow-up.

Copilot

Pull request overview

This PR hardens SymDB extraction (Extractor#extract_all) against leaked Class/Module objects whose cached Module#name no longer matches the current constant table (e.g., remove_const + redefine patterns), preventing stale bindings from being uploaded and avoiding downstream scope-tree inconsistencies.

Changes:

Add resolves_to_same_module? and use it during collect_extractable_modules to skip detached/stale Class/Module objects before building the name-keyed entries hash.
Add unit coverage for remove_const-detached classes and autoload-related edge cases (including “do not trigger autoload” behavior).
Update the RBS signature for the new private predicate.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
`lib/datadog/symbol_database/extractor.rb`	Adds constant-table resolution predicate and filters stale modules during ObjectSpace collection.
`spec/datadog/symbol_database/extractor_spec.rb`	Adds regression tests for leaked classes and autoload safety during extraction.
`sig/datadog/symbol_database/extractor.rbs`	Declares the new private helper method in the RBS surface.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+      def resolves_to_same_module?(mod_name, mod)
+        current = Object
+        mod_name.split('::').each do |seg|
+          sym = seg.to_sym
+          pending_autoload = if RUBY_VERSION >= '2.7'
+            current.autoload?(sym, false)
+          else
+            current.autoload?(sym)
+          end
+          return false if pending_autoload
+          return false unless current.const_defined?(sym, false)
+          current = current.const_get(sym, false)
+        end
+        current.equal?(mod)


…r_map on 2.5/2.6 Two fixes on the per-file streaming index path: - Port the resolves_to_same_module? guard from collect_extractable_modules (added in #5872) over to build_per_file_index. The new production entry point was missing this check, so the autoload-registered-after-remove_const and remove_const-detached extract_all specs regressed once master was merged. The check is the same one-liner between safe_mod_name and user_code_module? as in the legacy method. - Replace bare method_names.filter_map at build_file_scope with Core::Utils::EnumerableCompat.filter_map. Array#filter_map is 2.7+; the bare call would raise NoMethodError on every module with indexed methods on Ruby 2.5/2.6, get swallowed by the per-mod rescue, and silently drop every FILE scope from initial uploads on those Rubies. Matches the pattern already used at line 692 and 1052 of the same file.

testing/test-strategy.md lists 26 tests for the implicit-enablement work. The tracer-side tests through 13 had landed; the integration and guard tests below were still open. Each test is a unique implicit- enablement assertion; the testing.md text per test explains what the fix protects against if a future change breaks it. Tests added: - Test 8 — ComponentsState round-trip across Datadog.configure. New spec/datadog/core/configuration/components_state_spec.rb covers the value class. components_spec.rb gains #state (captures di_implicitly_enabled? from started?, false otherwise) and #startup! with old_state contexts (carry-over starts new component, no-carry stays stopped, env var still wins on initial startup). - Tests 14, 15 — APM_TRACING → handle_rc_enablement integration. New spec/datadog/di/integration/implicit_enablement_spec.rb drives Tracing::Remote.process_config end-to-end and asserts start/stop, idempotence, restart, code-tracking activation, receiver gating before and after enablement, and probe removal on RC disable. - Tests 21, 22 — line and method probe coverage across enablement timing. New spec/datadog/di/integration/probe_coverage_spec.rb plus four distinct fixture files (pre/post × line/method). Distinct class names per matrix cell — reusing a class name would force remove_const-then-redefine, which leaks stale Class objects with cached Module#name into ObjectSpace (the same CRuby behavior PR #5872 addresses for SymDB) and made the test order-dependent. - Test 23 — RC handler leaves DI settings untouched. Snapshot diff on settings.dynamic_instrumentation around handle_rc_enablement(true) and (false). Guards the "RC only enables/disables, never updates settings" decision from design/architecture.md. - Test 24 — code tracker registry survives Component#stop!. Loads a fixture under tracking, capture registry keys, stop!/start! the component, assert all keys still present. Without this, a probe whose target file was loaded before the stop!/start! cycle would silently never fire after restart. - Test 25 — DI.add_current_component invariant from build. Asserts Component.build registers the component in DI.@current_components so the code-tracker callback can locate it without going through Datadog.send(:components). Removal on shutdown! also asserted. - Test 26 — Component#build performs no I/O and spawns no threads. Companion to test 10's "RC handler returns promptly" — covers the initialize path that runs during Components#initialize and would delay application boot if it blocked. Bug surfaced and fixed: DI::Remote.explicitly_disabled? used options[:enabled].default_precedence?, which NoMethodError'd when the option hash hadn't been materialized — same root cause and same fix as the Component.explicitly_enabled? change in 03682a6. Switched to using_default?(:enabled). The integration spec exercises the path that hit this on the test 15 disable case. Verified: rake test:di:di_with_ext — 837 examples, 0 failures, 8 pending. probe_coverage_spec.rb run in isolation 5x — stable. bundle exec steep check lib/datadog/di/remote.rb — clean.

@eregon

Replaces ObjectSpace.each_object(Module) with a const-walk rooted at Object that follows Module#constants(false) + const_get(sym, false) with a per-segment autoload? guard. Per @eregon's suggestion on #5872: const-walk reachability is the leak filter by construction — anything reached IS the binding at that name — so resolves_to_same_module? (the post-hoc filter the legacy ObjectSpace path needs) is unnecessary here. Coverage equivalence vs ObjectSpace.each_object(Module): - Const-reachable modules: yielded by both. - Anonymous modules (no Module#name): not yielded; safe_mod_name filtered these out anyway. - Singleton classes: not yielded; legacy code skipped via MODULE_SINGLETON_CLASS_PRED. - "Leaked" classes whose const was remove_const-ed: not yielded; legacy code filtered via resolves_to_same_module?. Autoload safety: const_get(sym, false) still triggers autoload on a pending name. Each segment is guarded with autoload? first (with the same Ruby 2.7+ inherit-false / 2.5-2.6 inherited-included branch used in resolves_to_same_module?), so pending autoloads abort descent into that subtree without triggering customer code or raising LoadError. The legacy collect_extractable_modules still uses ObjectSpace.each_object — it is the path Component#extract_and_upload took prior to #5883's block-form migration and is retained as long as the non-block extract_all API exists. Verified: - bundle exec rspec spec/datadog/symbol_database/extractor_spec.rb (134 examples, 0 failures) — including the existing leak tests (`autoload registered after remove_const`, `class detached from its constant via remove_const`, `subclass with constant of same name as ancestor pending autoload`) which now exercise the const-walk path. - bundle exec rspec spec/datadog/symbol_database/ (362 examples, 0 failures). - bundle exec steep check lib/datadog/symbol_database/extractor.rb (clean). - bundle exec standardrb (clean on changed files). Co-Authored-By: Claude <[email protected]>

testing/test-strategy.md lists 26 tests for the implicit-enablement work. The tracer-side tests through 13 had landed; the integration and guard tests below were still open. Each test is a unique implicit- enablement assertion; the testing.md text per test explains what the fix protects against if a future change breaks it. Tests added: - Test 8 — ComponentsState round-trip across Datadog.configure. New spec/datadog/core/configuration/components_state_spec.rb covers the value class. components_spec.rb gains #state (captures di_implicitly_enabled? from started?, false otherwise) and #startup! with old_state contexts (carry-over starts new component, no-carry stays stopped, env var still wins on initial startup). - Tests 14, 15 — APM_TRACING → handle_rc_enablement integration. New spec/datadog/di/integration/implicit_enablement_spec.rb drives Tracing::Remote.process_config end-to-end and asserts start/stop, idempotence, restart, code-tracking activation, receiver gating before and after enablement, and probe removal on RC disable. - Tests 21, 22 — line and method probe coverage across enablement timing. New spec/datadog/di/integration/probe_coverage_spec.rb plus four distinct fixture files (pre/post × line/method). Distinct class names per matrix cell — reusing a class name would force remove_const-then-redefine, which leaks stale Class objects with cached Module#name into ObjectSpace (the same CRuby behavior PR #5872 addresses for SymDB) and made the test order-dependent. - Test 23 — RC handler leaves DI settings untouched. Snapshot diff on settings.dynamic_instrumentation around handle_rc_enablement(true) and (false). Guards the "RC only enables/disables, never updates settings" decision from design/architecture.md. - Test 24 — code tracker registry survives Component#stop!. Loads a fixture under tracking, capture registry keys, stop!/start! the component, assert all keys still present. Without this, a probe whose target file was loaded before the stop!/start! cycle would silently never fire after restart. - Test 25 — DI.add_current_component invariant from build. Asserts Component.build registers the component in DI.@current_components so the code-tracker callback can locate it without going through Datadog.send(:components). Removal on shutdown! also asserted. - Test 26 — Component#build performs no I/O and spawns no threads. Companion to test 10's "RC handler returns promptly" — covers the initialize path that runs during Components#initialize and would delay application boot if it blocked. Bug surfaced and fixed: DI::Remote.explicitly_disabled? used options[:enabled].default_precedence?, which NoMethodError'd when the option hash hadn't been materialized — same root cause and same fix as the Component.explicitly_enabled? change in 03682a6. Switched to using_default?(:enabled). The integration spec exercises the path that hit this on the test 15 disable case. Verified: rake test:di:di_with_ext — 837 examples, 0 failures, 8 pending. probe_coverage_spec.rb run in isolation 5x — stable. bundle exec steep check lib/datadog/di/remote.rb — clean.

p-datadog added the AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos label Jun 8, 2026

p-datadog requested a review from a team as a code owner June 8, 2026 22:15

p-datadog added the AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos label Jun 8, 2026

dd-octo-sts Bot added the debugger Live Debugger (+Dynamic Instrumentation, +Symbol Database) label Jun 8, 2026

chatgpt-codex-connector Bot reviewed Jun 8, 2026

View reviewed changes

Comment thread lib/datadog/symbol_database/extractor.rb Outdated

p-datadog marked this pull request as draft June 8, 2026 22:23

p-ddsign and others added 2 commits June 9, 2026 10:00

Merge branch 'master' into symdb-fix-module-name-cache

d49717e

p-datadog mentioned this pull request Jun 9, 2026

Clarify CLAUDE.md changelog rule: split internal vs product-facing telemetry #5878

Merged

p-datadog marked this pull request as ready for review June 9, 2026 15:33

chatgpt-codex-connector Bot reviewed Jun 9, 2026

View reviewed changes

Comment thread lib/datadog/symbol_database/extractor.rb Outdated

p-ddsign and others added 2 commits June 9, 2026 13:18

anmarchenko approved these changes Jun 10, 2026

View reviewed changes

Strech approved these changes Jun 10, 2026

View reviewed changes

Comment thread lib/datadog/symbol_database/extractor.rb

eregon reviewed Jun 10, 2026

View reviewed changes

p-datadog merged commit d055999 into master Jun 10, 2026
868 of 871 checks passed

p-datadog deleted the symdb-fix-module-name-cache branch June 10, 2026 14:04

dd-octo-sts Bot added this to the 2.36.0 milestone Jun 10, 2026

p-datadog requested a review from Copilot June 10, 2026 14:05

Copilot AI reviewed Jun 10, 2026

View reviewed changes

p-datadog mentioned this pull request Jun 10, 2026

SymDB: walk Object.constants in build_per_file_index #5889

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

SymDB: filter Class objects detached from their constant in extract_all#5872

SymDB: filter Class objects detached from their constant in extract_all#5872
p-datadog merged 6 commits into
masterfrom
symdb-fix-module-name-cache

p-datadog commented Jun 8, 2026 •

edited

Loading

Uh oh!

dd-octo-sts Bot commented Jun 8, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

datadog-prod-us1-4 Bot commented Jun 8, 2026 •

edited by datadog-datadog-prod-us1 Bot

Loading

Uh oh!

pr-commenter Bot commented Jun 8, 2026 •

edited

Loading

Explanation

More details about the CI and significant changes

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

eregon Jun 10, 2026

Uh oh!

p-datadog Jun 10, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

p-datadog commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dd-octo-sts Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Typing analysis

steep:ignore comments

Untyped methods

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

datadog-prod-us1-4 Bot commented Jun 8, 2026 • edited by datadog-datadog-prod-us1 Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pr-commenter Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Explanation

More details about the CI and significant changes

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

eregon Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

p-datadog Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

p-datadog commented Jun 8, 2026 •

edited

Loading

dd-octo-sts Bot commented Jun 8, 2026 •

edited

Loading

`steep:ignore` comments

datadog-prod-us1-4 Bot commented Jun 8, 2026 •

edited by datadog-datadog-prod-us1 Bot

Loading

pr-commenter Bot commented Jun 8, 2026 •

edited

Loading