Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

instrumentation config: use :unsatisfied #2011

Merged
merged 1 commit into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions lib/new_relic/agent/configuration/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module NewRelic
module Agent
module Configuration
class Manager
DEPENDENCY_DETECTION_VALUES = %i[prepend chain].freeze
DEPENDENCY_DETECTION_VALUES = %i[prepend chain unsatisfied].freeze

# Defining these explicitly saves object allocations that we incur
# if we use Forwardable and def_delegators.
Expand Down Expand Up @@ -366,7 +366,8 @@ def reset_cache

preserved = @cache.select { |_k, v| DEPENDENCY_DETECTION_VALUES.include?(v) }
new_cache
preserved.each { |k, v| @cache[k] = v unless @cache[k] && @cache[k] != 'auto' }
preserved.each { |k, v| @cache[k] = v }

@cache
end

Expand Down
6 changes: 6 additions & 0 deletions lib/new_relic/dependency_detection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ def detect!
@items.each do |item|
if item.dependencies_satisfied?
item.execute
else
item.configure_as_unsatisfied unless item.disabled_configured?
end
end
end
Expand Down Expand Up @@ -62,6 +64,10 @@ def dependencies_satisfied?
!executed and check_dependencies
end

def configure_as_unsatisfied
NewRelic::Agent.config.instance_variable_get(:@cache)[config_key] = :unsatisfied
end

def source_location_for(klass, method_name)
Object.instance_method(:method).bind(klass.allocate).call(method_name).source_location.to_s
end
Expand Down
21 changes: 21 additions & 0 deletions test/new_relic/agent/configuration/manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,27 @@ def test_auto_determined_values_stay_cached
end
end

def test_unsatisfied_values_stay_cached
name = :tears_of_the_kingdom

dd = DependencyDetection.defer do
named(name)

# guarantee the instrumentation's dependencies are unsatisfied
depends_on { return false }
executes { use_prepend? }
end

key = :"instrumentation.#{name}"
with_config(key => 'prepend') do
DependencyDetection.detect!

@manager.replace_or_add_config(ServerSource.new({}))

assert_equal :unsatisfied, @manager.instance_variable_get(:@cache)[key]
end
end

private

def assert_parsed_labels(expected)
Expand Down
1 change: 1 addition & 0 deletions test/new_relic/agent/local_log_decorator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ def setup
:'instrumentation.logger' => 'auto'
}
NewRelic::Agent.config.add_config_for_testing(@enabled_config)
NewRelic::Agent.config.send(:new_cache)
end

def teardown
Expand Down
86 changes: 82 additions & 4 deletions test/new_relic/dependency_detection_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class DependencyDetectionTest < Minitest::Test
def setup
@original_items = DependencyDetection.instance_variable_get(:@items)
DependencyDetection.instance_variable_set(:@items, [])
NewRelic::Agent.config.send(:new_cache)
end

def teardown
Expand Down Expand Up @@ -132,7 +133,7 @@ def test_config_defaults_to_auto
assert_equal :auto, setting
end

def test_config_disabling
def test_config_disabling_with_disabled
executed = false

dd = DependencyDetection.defer do
Expand All @@ -149,6 +150,15 @@ def test_config_disabling
refute dd.allowed_by_config?
refute executed
end
end

def test_config_disabling_with_enabled
executed = false

dd = DependencyDetection.defer do
named(:testing)
executes { executed = true }
end

with_config(:'instrumentation.testing' => 'enabled') do
executed = false
Expand All @@ -159,8 +169,17 @@ def test_config_disabling
assert_predicate dd, :allowed_by_config?
assert executed
end
end

# TODO: MAJOR VERSION - Deprecated!
def test_config_disabling_with_disable_testing
executed = false

dd = DependencyDetection.defer do
named(:testing)
executes { executed = true }
end

# TODO: MAJOR VERSION - Deprecated!
with_config(:disable_testing => true) do
executed = false
DependencyDetection.detect!
Expand All @@ -172,7 +191,7 @@ def test_config_disabling
end
end

def test_config_enabling
def test_config_enabling_with_enabled
executed = false

dd = DependencyDetection.defer do
Expand All @@ -189,6 +208,15 @@ def test_config_enabling
assert executed
assert_predicate dd, :use_prepend?
end
end

def test_config_enabling_with_auto
executed = false

dd = DependencyDetection.defer do
named(:testing)
executes { executed = true }
end

with_config(:'instrumentation.testing' => 'auto') do
DependencyDetection.detect!
Expand All @@ -197,6 +225,15 @@ def test_config_enabling
refute dd.deprecated_disabled_configured?
assert_predicate dd, :use_prepend?
end
end

def test_config_enabling_with_prepend
executed = false

dd = DependencyDetection.defer do
named(:testing)
executes { executed = true }
end

with_config(:'instrumentation.testing' => 'prepend') do
DependencyDetection.detect!
Expand All @@ -205,6 +242,15 @@ def test_config_enabling
refute dd.deprecated_disabled_configured?
assert_predicate dd, :use_prepend?
end
end

def test_config_enabling_with_chain
executed = false

dd = DependencyDetection.defer do
named(:testing)
executes { executed = true }
end

with_config(:'instrumentation.testing' => 'chain') do
DependencyDetection.detect!
Expand All @@ -215,7 +261,7 @@ def test_config_enabling
end
end

def test_config_prepend
def test_config_prepend_with_default_config
dd = DependencyDetection.defer do
named(:testing)
executes { true }
Expand All @@ -227,13 +273,27 @@ def test_config_prepend
assert_equal :auto, dd.config_value
assert_predicate dd, :use_prepend?
end
end

def test_config_prepend_with_prepend
dd = DependencyDetection.defer do
named(:testing)
executes { true }
end

with_config(:'instrumentation.testing' => 'prepend') do
DependencyDetection.detect!

assert_equal :prepend, dd.config_value
assert_predicate dd, :use_prepend?
end
end

def test_config_prepend_with_disabled
dd = DependencyDetection.defer do
named(:testing)
executes { true }
end

with_config(:'instrumentation.testing' => 'disabled') do
DependencyDetection.detect!
Expand Down Expand Up @@ -416,4 +476,22 @@ def test_auto_is_replaced_by_chain_when_chain_is_used
assert_equal :chain, dd.config_value
end
end

# confirm that :auto/:chain/:prepend becomes :unsatisfied when the detection
# logic finds that the library is either missing or known to not be supported
def test_prepend_is_replaced_by_unsatisfied_when_appropriate
name = :calm_calm_belong

dd = DependencyDetection.defer do
named(name)
depends_on { return false } # unsatisfied
executes { use_prepend? }
end

with_config(:"instrumentation.#{name}" => 'prepend') do
DependencyDetection.detect!

assert_equal :unsatisfied, dd.config_value
end
end
end