Skip to content

Commit

Permalink
Hide public profiler instance
Browse files Browse the repository at this point in the history
  • Loading branch information
marcotc committed Jan 27, 2022
1 parent a948598 commit 1fb25dd
Show file tree
Hide file tree
Showing 10 changed files with 80 additions and 40 deletions.
3 changes: 1 addition & 2 deletions lib/datadog/core/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions lib/datadog/profiling.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/datadog/profiling/preload.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# typed: strict
require 'ddtrace'

Datadog.profiler.start if Datadog.profiler
Datadog::Profiling.start_if_enabled
2 changes: 1 addition & 1 deletion lib/datadog/profiling/tasks/setup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down
2 changes: 1 addition & 1 deletion lib/ddtrace/auto_instrument.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
require 'ddtrace'

Datadog.add_auto_instrument
Datadog.profiler.start if Datadog.profiler
Datadog::Profiling.start_if_enabled
2 changes: 1 addition & 1 deletion spec/datadog/core/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 1 addition & 12 deletions spec/datadog/profiling/preload_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 3 additions & 6 deletions spec/datadog/profiling/tasks/setup_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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

Expand Down
58 changes: 58 additions & 0 deletions spec/ddtrace/profiling_spec.rb → spec/datadog/profiling_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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? }

Expand Down
17 changes: 1 addition & 16 deletions spec/ddtrace/auto_instrument_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 1fb25dd

Please sign in to comment.