diff --git a/benchmarks/profiler_sample_loop.rb b/benchmarks/profiler_sample_loop.rb index 2d722ded5c8..261ff1b96bb 100644 --- a/benchmarks/profiler_sample_loop.rb +++ b/benchmarks/profiler_sample_loop.rb @@ -23,10 +23,10 @@ def create_profiler end # Stop background threads - Datadog.profiler.shutdown! + Datadog.shutdown! # Call collection directly - @stack_collector = Datadog.profiler.collectors.first + @stack_collector = Datadog.send(:components).profiler.collectors.first @recorder = @stack_collector.recorder end diff --git a/benchmarks/profiler_submission.rb b/benchmarks/profiler_submission.rb index e9e6e7842d1..7e09aaa04c9 100644 --- a/benchmarks/profiler_submission.rb +++ b/benchmarks/profiler_submission.rb @@ -37,10 +37,10 @@ def create_profiler end # Stop background threads - Datadog.profiler.shutdown! + Datadog.shutdown! # Call exporter directly - @exporter = Datadog.profiler.scheduler.exporters.first + @exporter = Datadog.send(:components).profiler.scheduler.exporters.first @flush = Marshal.load( Zlib::GzipReader.new(File.open(ENV['FLUSH_DUMP_FILE'] || 'benchmarks/data/profiler-submission-marshal.gz')) ) diff --git a/docs/ProfilingDevelopment.md b/docs/ProfilingDevelopment.md index 49d2726a4e3..a9fda86912c 100644 --- a/docs/ProfilingDevelopment.md +++ b/docs/ProfilingDevelopment.md @@ -34,8 +34,8 @@ Components below live inside <../lib/datadog/profiling>: When started via `ddtracerb exec` (together with `DD_PROFILING_ENABLED=true`), initialization goes through the following flow: -1. <../lib/datadog/profiling/preload.rb> triggers the creation of the `Datadog.profiler` instance by calling the method -2. `Datadog.profiler` is handled by `Datadog::Configuration`, which triggers the configuration of `ddtrace` components +1. <../lib/datadog/profiling/preload.rb> triggers the creation of the profiler instance by calling the method `Datadog::Profiling.start_if_enabled` +2. The profiler instance is handled by `Datadog::Configuration`, which triggers the configuration of `ddtrace` components in `#build_components` 3. Inside `Datadog::Components`, the `build_profiler` method triggers the execution of the `Tasks::Setup` 4. The `Setup` task activates our extensions diff --git a/integration/apps/rack/app/acme.rb b/integration/apps/rack/app/acme.rb index 87e45650320..4ef92f89d4f 100644 --- a/integration/apps/rack/app/acme.rb +++ b/integration/apps/rack/app/acme.rb @@ -89,7 +89,7 @@ def check(request) def detailed_check(request) ['200', { 'Content-Type' => 'application/json'}, [JSON.pretty_generate( webserver_process: $PROGRAM_NAME, - profiler_available: !!Datadog.profiler, + profiler_available: Datadog::Profiling.start_if_enabled, # NOTE: Threads can't be named on Ruby 2.1 and 2.2 profiler_threads: ((Thread.list.map(&:name).select { |it| it && it.include?('Profiling') }) unless RUBY_VERSION < '2.3') )], "\n"] diff --git a/integration/apps/rack/app/resque_background_job.rb b/integration/apps/rack/app/resque_background_job.rb index 4cde247a236..6af61c61cb8 100644 --- a/integration/apps/rack/app/resque_background_job.rb +++ b/integration/apps/rack/app/resque_background_job.rb @@ -28,7 +28,7 @@ def self.perform(key, value) key: key, value: value, resque_process: $PROGRAM_NAME, - profiler_available: !!Datadog.profiler, + profiler_available: Datadog::Profiling.start_if_enabled, # NOTE: Threads can't be named on Ruby 2.1 and 2.2 profiler_threads: ((Thread.list.map(&:name).select { |it| it && it.include?('Profiling') }) unless RUBY_VERSION < '2.3') )) diff --git a/integration/apps/rack/app/sidekiq_background_job.rb b/integration/apps/rack/app/sidekiq_background_job.rb index 03c3a59903d..84a2699d182 100644 --- a/integration/apps/rack/app/sidekiq_background_job.rb +++ b/integration/apps/rack/app/sidekiq_background_job.rb @@ -26,7 +26,7 @@ def perform(key, value) key: key, value: value, sidekiq_process: $PROGRAM_NAME, - profiler_available: !!Datadog.profiler, + profiler_available: Datadog::Profiling.start_if_enabled, # NOTE: Threads can't be named on Ruby 2.1 and 2.2 profiler_threads: ((Thread.list.map(&:name).select { |it| it && it.include?('Profiling') }) unless RUBY_VERSION < '2.3') )) diff --git a/integration/apps/rails-five/app/controllers/health_controller.rb b/integration/apps/rails-five/app/controllers/health_controller.rb index 07d7c7dc869..2c55f15cc4b 100644 --- a/integration/apps/rails-five/app/controllers/health_controller.rb +++ b/integration/apps/rails-five/app/controllers/health_controller.rb @@ -12,7 +12,7 @@ def check def detailed_check render json: { webserver_process: $PROGRAM_NAME, - profiler_available: !!Datadog.profiler, + profiler_available: Datadog::Profiling.start_if_enabled, profiler_threads: Thread.list.map(&:name).select { |it| it && it.include?('Profiling') } } end diff --git a/lib/datadog/core/configuration.rb b/lib/datadog/core/configuration.rb index a5a676c9798..be1db7b69ca 100644 --- a/lib/datadog/core/configuration.rb +++ b/lib/datadog/core/configuration.rb @@ -153,8 +153,7 @@ def configuration_for(target, option = nil) def_delegators \ :components, - :health_metrics, - :profiler + :health_metrics def logger # avoid initializing components if they didn't already exist diff --git a/lib/datadog/profiling.rb b/lib/datadog/profiling.rb index ac3a3448a74..6b3c22911ef 100644 --- a/lib/datadog/profiling.rb +++ b/lib/datadog/profiling.rb @@ -82,6 +82,18 @@ def self.configuration ) end + # Starts the profiler, if the profiler is supported by in + # this runtime environment and if the profiler has been enabled + # in configuration. + # + # @return [Boolean] `true` if the profiler has successfully started, otherwise `false`. + # @public_api + def self.start_if_enabled + # Getting the profiler instance triggers start as a side-effect; + # otherwise we get nil + !!Datadog.send(:components).profiler + end + private_class_method def self.ruby_engine_unsupported? 'JRuby is not supported' if RUBY_ENGINE == 'jruby' end diff --git a/lib/datadog/profiling/preload.rb b/lib/datadog/profiling/preload.rb index 61cd81a7e6f..99dbbfefdf8 100644 --- a/lib/datadog/profiling/preload.rb +++ b/lib/datadog/profiling/preload.rb @@ -1,4 +1,4 @@ # typed: strict require 'ddtrace' -Datadog.profiler.start if Datadog.profiler +Datadog::Profiling.start_if_enabled diff --git a/lib/datadog/profiling/tasks/setup.rb b/lib/datadog/profiling/tasks/setup.rb index be8379028ce..72ac7802900 100644 --- a/lib/datadog/profiling/tasks/setup.rb +++ b/lib/datadog/profiling/tasks/setup.rb @@ -61,7 +61,7 @@ def setup_at_fork_hooks Thread.current.send(:update_native_ids) if Thread.current.respond_to?(:update_native_ids, true) # Restart profiler, if enabled - Datadog.profiler.start if Datadog.profiler + Datadog::Profiling.start_if_enabled rescue StandardError => e Datadog.logger.warn do "Error during post-fork hooks. Cause: #{e.message} Location: #{Array(e.backtrace).first}" diff --git a/lib/ddtrace/auto_instrument.rb b/lib/ddtrace/auto_instrument.rb index 09d1374dacd..3731c58c6c1 100644 --- a/lib/ddtrace/auto_instrument.rb +++ b/lib/ddtrace/auto_instrument.rb @@ -5,4 +5,4 @@ require 'ddtrace' Datadog.add_auto_instrument -Datadog.profiler.start if Datadog.profiler +Datadog::Profiling.start_if_enabled diff --git a/spec/datadog/core/configuration_spec.rb b/spec/datadog/core/configuration_spec.rb index 6a48e374233..ec963d8d981 100644 --- a/spec/datadog/core/configuration_spec.rb +++ b/spec/datadog/core/configuration_spec.rb @@ -325,7 +325,7 @@ it 'starts the profiler' do configure - expect(test_class.profiler).to have_received(:start) + expect(test_class.send(:components).profiler).to have_received(:start) end end end diff --git a/spec/datadog/profiling/preload_spec.rb b/spec/datadog/profiling/preload_spec.rb index d84448a1f43..9759bcb26c9 100644 --- a/spec/datadog/profiling/preload_spec.rb +++ b/spec/datadog/profiling/preload_spec.rb @@ -6,19 +6,8 @@ subject(:preload) { load 'datadog/profiling/preload.rb' } it 'starts the profiler' do - profiler = instance_double(Datadog::Profiling::Profiler) - - expect(Datadog).to receive(:profiler).and_return(profiler).at_least(:once) - expect(profiler).to receive(:start) + expect(Datadog::Profiling).to receive(:start_if_enabled) preload end - - context 'when the profiler is not available' do - it 'does not raise any error' do - expect(Datadog).to receive(:profiler).and_return(nil) - - preload - end - end end diff --git a/spec/datadog/profiling/tasks/setup_spec.rb b/spec/datadog/profiling/tasks/setup_spec.rb index 4e34b99b803..0c0b228162e 100644 --- a/spec/datadog/profiling/tasks/setup_spec.rb +++ b/spec/datadog/profiling/tasks/setup_spec.rb @@ -151,7 +151,7 @@ context 'when Process#at_fork is available' do before do allow(Process).to receive(:respond_to?).with(:at_fork).and_return(true) - allow(Datadog).to receive(:profiler) + allow(Datadog::Profiling).to receive(:start_if_enabled) without_partial_double_verification do allow(Process).to receive(:at_fork) @@ -173,17 +173,14 @@ end it 'sets up an at_fork hook that restarts the profiler' do - profiler = instance_double(Datadog::Profiling::Profiler) - - expect(Datadog).to receive(:profiler).and_return(profiler).at_least(:once) - expect(profiler).to receive(:start) + expect(Datadog::Profiling).to receive(:start_if_enabled) at_fork_hook.call end context 'when there is an issue starting the profiler' do before do - expect(Datadog).to receive(:profiler).and_raise('Dummy exception') + expect(Datadog::Profiling).to receive(:start_if_enabled).and_raise('Dummy exception') allow(Datadog.logger).to receive(:warn) # Silence logging during tests end diff --git a/spec/ddtrace/profiling_spec.rb b/spec/datadog/profiling_spec.rb similarity index 81% rename from spec/ddtrace/profiling_spec.rb rename to spec/datadog/profiling_spec.rb index 63e842eaecf..80c5427899a 100644 --- a/spec/ddtrace/profiling_spec.rb +++ b/spec/datadog/profiling_spec.rb @@ -5,6 +5,64 @@ RSpec.describe Datadog::Profiling do extend ConfigurationHelpers + describe '.configure' do + subject(:configure) { described_class.configure(&block) } + let(:block) { proc {} } + + it 'delegates to the global configuration' do + expect(Datadog).to receive(:internal_configure) { |&b| expect(b).to be_a_kind_of(Proc) } + configure + end + + context 'validation' do + it 'wraps the configuration object with a proxy' do + described_class.configure do |c| + expect(c).to be_a_kind_of(Datadog::Core::Configuration::ValidationProxy::Profiling) + end + end + + it 'allows profiling options' do + described_class.configure do |c| + expect(c).to respond_to(:profiling) + end + end + + it 'raises errors for non-profiling options' do + described_class.configure do |c| + expect(c).to_not respond_to(:tracer) + end + end + end + end + + describe '.configuration' do + subject(:configuration) { described_class.configuration } + it 'returns the global configuration' do + expect(configuration) + .to be_a_kind_of(Datadog::Core::Configuration::ValidationProxy::Profiling) + + expect(configuration.send(:settings)).to eq(Datadog.send(:internal_configuration)) + end + end + + describe '.start_if_enabled' do + subject(:start_if_enabled) { described_class.start_if_enabled } + + before do + allow(Datadog.send(:components)).to receive(:profiler).and_return(result) + end + + context 'with the profiler instance available' do + let(:result) { double } + it { is_expected.to be(true) } + end + + context 'with the profiler instance not available' do + let(:result) { nil } + it { is_expected.to be(false) } + end + end + describe '::supported?' do subject(:supported?) { described_class.supported? } diff --git a/spec/ddtrace/auto_instrument_spec.rb b/spec/ddtrace/auto_instrument_spec.rb index 5089084614b..f51d5dcbef4 100644 --- a/spec/ddtrace/auto_instrument_spec.rb +++ b/spec/ddtrace/auto_instrument_spec.rb @@ -112,22 +112,7 @@ def migrate_db end it 'starts the profiler' do - skip 'Profiling is not supported on JRuby' if PlatformHelpers.jruby? - skip 'Profiling is not supported on TruffleRuby' if PlatformHelpers.truffleruby? - - profiler = instance_double('Datadog::Profiling::Profiler') - - expect(Datadog).to receive(:profiler).and_return(profiler).at_least(:once) - expect(profiler).to receive(:start) - + expect(Datadog::Profiling).to receive(:start_if_enabled) auto_instrument end - - context 'when the profiler is not available' do - it 'does not raise any error' do - expect(Datadog).to receive(:profiler).and_return(nil) - - auto_instrument - end - end end