Fix 'load core only and configure library with no settings' spec when run locally by eregon · Pull Request #5555 · DataDog/dd-trace-rb

eregon · 2026-04-02T15:46:25Z

What does this PR do?

Fix a spec failing when running the test suite locally:

Lines 76 to 96 in e191a14

    
           RSpec.describe 'load core only and configure library with no settings' do 
        
             let(:code) do 
        
               <<-E 
        
                 if defined?(Datadog) 
        
                   unless Datadog.constants == [:VERSION] 
        
                     exit 1 
        
                   end 
        
                 end 
        
                 require 'datadog/core' 
        
                 Datadog.configure do 
        
                 end 
        
               E 
        
             end 
        
             it 'configures successfully' do 
        
               rv = system("ruby -e #{Shellwords.shellescape(code)}") 
        
               expect(rv).to be true 
        
             end 
        
           end

Motivation:

Be green.

Change log entry

None.

Additional Notes:

How to test the change?

github-actions · 2026-04-02T15:46:36Z

Thank you for updating Change log entry section 👏

^{Visited at: 2026-04-02 15:49:37 UTC}

github-actions · 2026-04-02T15:46:52Z

Typing analysis

Note: Ignored files are excluded from the next sections.

`steep:ignore` comments

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

steep:ignore comments (+4-4)

❌ Introduced:

lib/datadog/core/telemetry/event/app_started.rb:141
lib/datadog/core/telemetry/event/app_started.rb:147
lib/datadog/core/telemetry/event/app_started.rb:158
lib/datadog/core/telemetry/event/app_started.rb:168

✅ Cleared:

lib/datadog/core/telemetry/event/app_started.rb:139
lib/datadog/core/telemetry/event/app_started.rb:145
lib/datadog/core/telemetry/event/app_started.rb:156
lib/datadog/core/telemetry/event/app_started.rb:166

eregon · 2026-04-02T15:47:28Z

The fix is suggested by Cursor, maybe there is a better fix?
Also how much is require 'datadog/core' really supported?

lib/datadog/core/telemetry/event/app_started.rb

ivoanjo · 2026-04-02T15:51:50Z

lib/datadog/core/telemetry/event/app_started.rb

+            # Profiling is not loaded when disabled (see Profiling::Component.build_profiler_component); skip error payload
+            if Datadog::Profiling.respond_to?(:unsupported_reason) && (unsupported_reason = Datadog::Profiling.unsupported_reason)


This doesn't look right? lib/datadog/profiling.rb is loaded even when disabled -- so if the module is loaded, the unsupported_reason method is expected to be here.

See

dd-trace-rb/lib/datadog/profiling/component.rb

Line 32 in 716872e

require_relative "../profiling"

which seems the likely cause.

I checked and I indeed get the NoMethodError when undoing this change.

Ah, you're right. It's because of the big comment above it -- we delay requiring profiling to break the potential require cycle.

Yeaaaah this whole thing needs a bit of a clean-up. Thanks for spotting it. I think this is indeed the correct interim fix for now.

eregon · 2026-04-02T15:54:58Z

lib/datadog/core/telemetry/event/app_started.rb

@@ -52,7 +52,8 @@ def products(components)
            products = {
              appsec: {
                # TODO take appsec status out of component tree?


To address this at the same time maybe it should be components.appsec&.enabled? or so, but I'm not sure what kind of object is components.appsec when not nil.

Welcome to some of our legacy issues haha! The configuration system... is weird... and some things monkey patch themselves in there for reasons that made sense at the time when we wrote them but no longer make sense in 2026+

I tried and it doesn't work:

1) Datadog::Core::Telemetry::Event::AppStarted.payload with stable config with config id reports config id Failure/Error: enabled: !!components.appsec&.enabled?, NoMethodError: undefined method 'enabled?' for an instance of Datadog::AppSec::Component # ./lib/datadog/core/telemetry/event/app_started.rb:54:in 'Datadog::Core::Telemetry::Event::AppStarted#products' # ./lib/datadog/core/telemetry/event/app_started.rb:22:in 'Datadog::Core::Telemetry::Event::AppStarted#initialize' # ./spec/datadog/core/telemetry/event/app_started_spec.rb:7:in 'Class#new' # ./spec/datadog/core/telemetry/event/app_started_spec.rb:7:in 'block (2 levels) in <top (required)>' # ./spec/datadog/core/telemetry/event/app_started_spec.rb:256:in 'block (5 levels) in <top (required)>' # ./spec/spec_helper.rb:274:in 'block (2 levels) in <top (required)>' # ./spec/spec_helper.rb:154:in 'block (2 levels) in <top (required)>' # ./spec/support/execute_in_fork.rb:32:in 'ForkableExample#run' 2) Datadog::Core::Telemetry::Event::AppStarted.payload with stable config with config id without config id does not report config id Failure/Error: enabled: !!components.appsec&.enabled?, NoMethodError: undefined method 'enabled?' for an instance of Datadog::AppSec::Component # ./lib/datadog/core/telemetry/event/app_started.rb:54:in 'Datadog::Core::Telemetry::Event::AppStarted#products' # ./lib/datadog/core/telemetry/event/app_started.rb:22:in 'Datadog::Core::Telemetry::Event::AppStarted#initialize' # ./spec/datadog/core/telemetry/event/app_started_spec.rb:7:in 'Class#new' # ./spec/datadog/core/telemetry/event/app_started_spec.rb:7:in 'block (2 levels) in <top (required)>' # ./spec/datadog/core/telemetry/event/app_started_spec.rb:273:in 'block (6 levels) in <top (required)>' # ./spec/spec_helper.rb:274:in 'block (2 levels) in <top (required)>' # ./spec/spec_helper.rb:154:in 'block (2 levels) in <top (required)>' # ./spec/support/execute_in_fork.rb:32:in 'ForkableExample#run'

https://github.com/DataDog/dd-trace-rb/blob/master/lib/datadog/appsec/extensions.rb is where the appsec settings "show up" -- TBH we should just eagerly load all the settings instead of doing this but maybe not for this PR yet?

How about lib/datadog/appsec/component.rb defining an enabled? method when build_appsec_component doesn't return nil?

I guess that would work as well, but to me is slightly more weird than the settings.respond_to?(:appsec) as usually we don't ask the components about things being enabled -- that's usually the job of the settings.

(But no strong feelings on it, I think that one would work too for now)

FYI I worked on this in #5075, maybe you will find it helpful.

Overall I think our old "modularization" efforts mostly failed, so I'm of the opinion that in the future (not in this PR) we should just clean up the wish-it-was-modular code.

(Not for this PR :P) Once we're ready/need to be modular, then we can actually do the work end-to-end, I don't think we're saving ourselves any work from keeping the half-broken migration as-is.

I like the direction #5075 is going, this PR feels too hacky for me.
What's the reason it was closed?

datadog-official · 2026-04-02T16:07:03Z

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 95.35% (-0.02%)

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

🔗 Commit SHA: 711390a | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!}

… run locally

pr-commenter · 2026-04-03T13:16:25Z

Benchmarks

Benchmark execution time: 2026-04-03 13:16:22

Comparing candidate commit 711390a in PR branch bd/fix-load-core-only with baseline commit 716872e 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 ----------------------------------'

ivoanjo

👍 LGTM, thanks for catching this one!

ivoanjo · 2026-04-07T10:17:31Z

lib/datadog/core/telemetry/event/app_started.rb

+            # Profiling is not loaded when disabled (see Profiling::Component.build_profiler_component); skip error payload
+            if Datadog::Profiling.respond_to?(:unsupported_reason) && (unsupported_reason = Datadog::Profiling.unsupported_reason)


Ah, you're right. It's because of the big comment above it -- we delay requiring profiling to break the potential require cycle.

Yeaaaah this whole thing needs a bit of a clean-up. Thanks for spotting it. I think this is indeed the correct interim fix for now.

eregon requested a review from ivoanjo April 2, 2026 15:46

eregon requested a review from a team as a code owner April 2, 2026 15:46

github-actions bot added the core Involves Datadog core libraries label Apr 2, 2026

eregon commented Apr 2, 2026

View reviewed changes

lib/datadog/core/telemetry/event/app_started.rb Show resolved Hide resolved

ivoanjo reviewed Apr 2, 2026

View reviewed changes

eregon commented Apr 2, 2026

View reviewed changes

Fix 'load core only and configure library with no settings' spec when…

711390a

… run locally

eregon force-pushed the bd/fix-load-core-only branch from e83c1f1 to 711390a Compare April 3, 2026 12:48

ivoanjo approved these changes Apr 7, 2026

View reviewed changes

	RSpec.describe 'load core only and configure library with no settings' do
	let(:code) do
	<<-E
	if defined?(Datadog)
	unless Datadog.constants == [:VERSION]
	exit 1
	end
	end

	require 'datadog/core'

	Datadog.configure do
	end
	E
	end

	it 'configures successfully' do
	rv = system("ruby -e #{Shellwords.shellescape(code)}")
	expect(rv).to be true
	end
	end

		# Profiling is not loaded when disabled (see Profiling::Component.build_profiler_component); skip error payload
		if Datadog::Profiling.respond_to?(:unsupported_reason) && (unsupported_reason = Datadog::Profiling.unsupported_reason)

Conversation

eregon commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Typing analysis

steep:ignore comments

Uh oh!

eregon commented Apr 2, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eregon Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datadog-official bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pr-commenter bot commented Apr 3, 2026

Benchmarks

Explanation

More details about the CI and significant changes

Uh oh!

ivoanjo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eregon commented Apr 2, 2026 •

edited

Loading

github-actions bot commented Apr 2, 2026 •

edited

Loading

github-actions bot commented Apr 2, 2026 •

edited

Loading

`steep:ignore` comments

eregon Apr 2, 2026 •

edited

Loading

datadog-official bot commented Apr 2, 2026 •

edited

Loading