Skip to content

Commit

Permalink
Merge pull request #1034 from DataDog/fix/configure_eager_loading_laz…
Browse files Browse the repository at this point in the history
…y_vars

Fix #configure forcing all options to eager load
  • Loading branch information
delner authored and marcotc committed May 11, 2020
1 parent 4a05193 commit 95fefe2
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 24 deletions.
2 changes: 1 addition & 1 deletion lib/ddtrace/configuration/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def new_settings_class(&block)
# Instance methods for configuration
module InstanceMethods
def initialize(options = {})
configure(options)
configure(options) unless options.empty?
end

def configure(opts = {})
Expand Down
2 changes: 1 addition & 1 deletion lib/ddtrace/configuration/options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def option_defined?(name)
end

def options_hash
options.each_with_object({}) do |(key, _), hash|
self.class.options.merge(options).each_with_object({}) do |(key, _), hash|
hash[key] = get_option(key)
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/ddtrace/contrib/configuration/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class Settings

def configure(options = {})
self.class.options.dependency_order.each do |name|
self[name] = options.fetch(name, self[name])
self[name] = options[name] if options.key?(name)
end

yield(self) if block_given?
Expand Down
13 changes: 13 additions & 0 deletions lib/ddtrace/contrib/rails/configuration/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,19 @@ module Rails
module Configuration
# Custom settings for the Rails integration
class Settings < Contrib::Configuration::Settings
def initialize(options = {})
super(options)

# NOTE: Eager load these
# Rails integration is responsible for orchestrating other integrations.
# When using environment variables, settings will not be automatically
# filled because nothing explicitly calls them. They must though, so
# integrations like ActionPack can receive the value as it should.
# Trigger these manually to force an eager load and propagate them.
analytics_enabled
analytics_sample_rate
end

option :analytics_enabled do |o|
o.default { env_to_bool(Ext::ENV_ANALYTICS_ENABLED, nil) }
o.lazy
Expand Down
58 changes: 37 additions & 21 deletions spec/ddtrace/contrib/configuration/settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,47 @@

it { is_expected.to be_a_kind_of(Datadog::Configuration::Base) }

describe '#options' do
subject(:options) { settings.options }
describe '#service_name' do
subject(:service_name) { settings.service_name }
it { expect(settings.service_name).to be nil }
it { expect(settings[:service_name]).to be nil }
end

describe ':service_name' do
subject(:option) { options[:service_name] }
it { expect(options).to include(:service_name) }
it { expect(option.get).to be nil }
end
describe '#tracer' do
subject(:tracer) { settings.tracer }
it { expect(settings.tracer).to be Datadog.tracer }
it { expect(settings[:tracer]).to be Datadog.tracer }
end

describe ':tracer' do
subject(:option) { options[:tracer] }
it { expect(options).to include(:tracer) }
it { expect(option.get).to be Datadog.tracer }
end
describe '#analytics_enabled' do
subject(:analytics_enabled) { settings.analytics_enabled }
it { expect(settings.analytics_enabled).to be false }
it { expect(settings[:analytics_enabled]).to be false }
end

describe ':analytics_enabled' do
subject(:option) { options[:analytics_enabled] }
it { expect(options).to include(:analytics_enabled) }
it { expect(option.get).to be false }
end
describe '#analytics_sample_rate' do
subject(:analytics_sample_rate) { settings.analytics_sample_rate }
it { expect(settings.analytics_sample_rate).to eq 1.0 }
it { expect(settings[:analytics_sample_rate]).to eq 1.0 }
end

describe '#configure' do
subject(:configure) { settings.configure(options) }

context 'given an option' do
let(:options) { { service_name: service_name } }
let(:service_name) { double('service_name') }

before { allow(settings).to receive(:set_option).and_call_original }

it 'doesn\'t initialize other options' do
expect { configure }
.to change { settings.service_name }
.from(nil)
.to(service_name)

describe ':analytics_sample_rate' do
subject(:option) { options[:analytics_sample_rate] }
it { expect(options).to include(:analytics_sample_rate) }
it { expect(option.get).to eq 1.0 }
expect(settings).to_not have_received(:set_option).with(:tracer, any_args)
end
end
end
end
2 changes: 2 additions & 0 deletions spec/ddtrace/contrib/rails/analytics_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
around do |example|
# Reset before and after each example; don't allow global state to linger.
Datadog.registry[:rails].reset_configuration!
Datadog.registry[:action_pack].reset_configuration!
example.run
Datadog.registry[:rails].reset_configuration!
Datadog.registry[:action_pack].reset_configuration!
end

let(:span) { spans.first }
Expand Down

0 comments on commit 95fefe2

Please sign in to comment.