From 7b226f519a72b4a30411553d205d800dd6a26388 Mon Sep 17 00:00:00 2001 From: fallwith Date: Fri, 7 Apr 2023 15:12:03 -0700 Subject: [PATCH 1/7] auto convert :auto to :prepend/:chain in config For `:'instrumentation.*'` configuration parameters that are set to :auto (the default), the agent will automatically determine whether to use method prepending or chaining. The agent will now update its in-memory configuration to give each relevant parameter a value of either :prepend or :chain so that the result of the determination can be introspected. This is intended to help 3rd party libraries that wish to further enhance the agent's instrumentation capabilities by prepending or chaining additional logic. Environment variable, YAML file, and server-side configuration based values are not impacted. --- CHANGELOG.md | 4 +++ lib/new_relic/dependency_detection.rb | 11 ++++++- test/new_relic/dependency_detection_test.rb | 36 +++++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c304789df9..6be1ad5a32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## dev +- **Feature: The agent configuration will now reflect whether method prepending or chaining was used per instrumentation + + For `:'instrumentation.*'` configuration parameters that are set to :auto (the default), the agent will automatically determine whether to use method prepending or chaining. The agent will now update its in-memory configuration to give each relevant parameter a value of either :prepend or :chain so that the result of the determination can be introspected. This is intended to help 3rd party libraries that wish to further enhance the agent's instrumentation capabilities by prepending or chaining additional logic. Environment variable, YAML file, and server-side configuration based values are not impacted. [PR#1930](https://github.com/newrelic/newrelic-ruby-agent/pull/1930) + - **Feature: Deprecate memcached and memcache-client instrumentation** Instrumentation for the memcached and memcache-client libraries is deprecated and will be removed during the next major release. diff --git a/lib/new_relic/dependency_detection.rb b/lib/new_relic/dependency_detection.rb index 49a7ce2c41..eb86f2e0d4 100644 --- a/lib/new_relic/dependency_detection.rb +++ b/lib/new_relic/dependency_detection.rb @@ -179,6 +179,15 @@ def fetch_config_value(key) return valid_value end + # update any :auto config value to be either :prepend or :chain after auto + # determination has selected one of those to use + def update_config_value(use_prepend) + if config_key && auto_configured? + NewRelic::Agent.config.instance_variable_get(:@cache)[config_key] = use_prepend ? :prepend : :chain + end + use_prepend + end + def config_value return AUTO_CONFIG_VALUE unless config_key @@ -202,7 +211,7 @@ def conflicts_with_prepend(&block) end def use_prepend? - prepend_configured? || (auto_configured? && !prepend_conflicts?) + update_config_value(prepend_configured? || (auto_configured? && !prepend_conflicts?)) end def prepend_conflicts? diff --git a/test/new_relic/dependency_detection_test.rb b/test/new_relic/dependency_detection_test.rb index 9b0d062b5d..3403b31b1c 100644 --- a/test/new_relic/dependency_detection_test.rb +++ b/test/new_relic/dependency_detection_test.rb @@ -380,4 +380,40 @@ def test_log_and_instrument_uses_supportability_name_when_provided assert_log_contains(log, /#{supportability_name}/) assert_log_contains(log, /#{method}/) end + + # when an instrumentation value of :auto (the default) is present, the agent + # automatically determines whether to use :prepend or :chain. after the + # determination is made, the config value should be updated to be either + # :prepend or :chain so that the determined value can be introspected. + def test_prepend_or_chain_based_values_have_auto_converted_into_one_of_those + name = :bandu_gumbo + + dd = DependencyDetection.defer do + named(name) + executes { use_prepend? } + end + + with_config(:"instrumentation.#{name}" => 'auto') do + DependencyDetection.detect! + + assert_equal :prepend, dd.config_value + end + end + + # confirm that :auto becames :chain when :chain is automatically determined + def test_auto_is_replaced_by_chain_when_chain_is_used + name = :blank_and_jones + + dd = DependencyDetection.defer do + named(name) + conflicts_with_prepend { true } + executes { use_prepend? } + end + + with_config(:"instrumentation.#{name}" => 'auto') do + DependencyDetection.detect! + + assert_equal :chain, dd.config_value + end + end end From abd5c89193a6f69ff5a453ddea1c7e263ca43bbd Mon Sep 17 00:00:00 2001 From: James Bunch Date: Wed, 19 Apr 2023 15:56:48 -0700 Subject: [PATCH 2/7] Update CHANGELOG.md add missing ** closing style Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6be1ad5a32..76542d3e03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## dev -- **Feature: The agent configuration will now reflect whether method prepending or chaining was used per instrumentation +- **Feature: The agent configuration will now reflect whether method prepending or chaining was used per instrumentation** For `:'instrumentation.*'` configuration parameters that are set to :auto (the default), the agent will automatically determine whether to use method prepending or chaining. The agent will now update its in-memory configuration to give each relevant parameter a value of either :prepend or :chain so that the result of the determination can be introspected. This is intended to help 3rd party libraries that wish to further enhance the agent's instrumentation capabilities by prepending or chaining additional logic. Environment variable, YAML file, and server-side configuration based values are not impacted. [PR#1930](https://github.com/newrelic/newrelic-ruby-agent/pull/1930) From 7735f928121d32768ddf9ded4927ee4472aaa1b6 Mon Sep 17 00:00:00 2001 From: fallwith Date: Wed, 19 Apr 2023 17:39:06 -0700 Subject: [PATCH 3/7] config manager: don't clobber dependency detection when resetting the configuration manager's cache, do not allow already determined dependency detection params to be reverted back to either `nil` or `'auto'`. --- lib/new_relic/agent/configuration/manager.rb | 13 ++++++++++++ .../agent/configuration/manager_test.rb | 20 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/lib/new_relic/agent/configuration/manager.rb b/lib/new_relic/agent/configuration/manager.rb index 7254db4731..c21085d519 100644 --- a/lib/new_relic/agent/configuration/manager.rb +++ b/lib/new_relic/agent/configuration/manager.rb @@ -15,6 +15,8 @@ module NewRelic module Agent module Configuration class Manager + DEPENDENCY_DETECTION_VALUES = %i[prepend chain].freeze + # Defining these explicitly saves object allocations that we incur # if we use Forwardable and def_delegators. def [](key) @@ -357,7 +359,18 @@ def reset_to_defaults reset_cache end + # reset the configuration hash, but do not replace previously auto + # determined dependency detection values with nil or 'auto' def reset_cache + return new_cache unless @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' } + @cache + end + + def new_cache @cache = Hash.new { |hash, key| hash[key] = self.fetch(key) } end diff --git a/test/new_relic/agent/configuration/manager_test.rb b/test/new_relic/agent/configuration/manager_test.rb index c44f94e7a4..aa18c67456 100644 --- a/test/new_relic/agent/configuration/manager_test.rb +++ b/test/new_relic/agent/configuration/manager_test.rb @@ -491,6 +491,26 @@ def test_apply_transformations_reraises_errors end end + def test_auto_determined_values_stay_cached + name = :knockbreck_manse + + dd = DependencyDetection.defer do + named(name) + executes { use_prepend? } + end + + key = :"instrumentation.#{name}" + with_config(key => 'auto') do + DependencyDetection.detect! + + @manager.replace_or_add_config(ServerSource.new({})) + + assert_equal :prepend, @manager.instance_variable_get(:@cache)[key] + end + end + + private + def assert_parsed_labels(expected) result = @manager.parsed_labels From 0d6eb27d6e245ffba2d33a612259a3537cb2d212 Mon Sep 17 00:00:00 2001 From: fallwith Date: Wed, 19 Apr 2023 21:28:44 -0700 Subject: [PATCH 4/7] default source test: flush config cache on setup for each default source test, make sure the config cache is reset --- test/new_relic/agent/configuration/default_source_test.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/new_relic/agent/configuration/default_source_test.rb b/test/new_relic/agent/configuration/default_source_test.rb index 8ba3d4c5e6..bc6e0e8db5 100644 --- a/test/new_relic/agent/configuration/default_source_test.rb +++ b/test/new_relic/agent/configuration/default_source_test.rb @@ -10,6 +10,7 @@ class DefaultSourceTest < Minitest::Test def setup @default_source = DefaultSource.new @defaults = ::NewRelic::Agent::Configuration::DEFAULTS + NewRelic::Agent.config.send(:new_cache) end def test_default_values_have_a_public_setting From 24f816368d5cbef12ba3e1f9605a6f23afe8ea41 Mon Sep 17 00:00:00 2001 From: fallwith Date: Thu, 20 Apr 2023 17:50:19 -0700 Subject: [PATCH 5/7] manager: check for @cache (for the sake of older Rubies) don't attempt to reference `@cache`, even for an evaluation of truthiness, without making sure it is defined first --- lib/new_relic/agent/configuration/manager.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/new_relic/agent/configuration/manager.rb b/lib/new_relic/agent/configuration/manager.rb index c21085d519..3dc77b35ad 100644 --- a/lib/new_relic/agent/configuration/manager.rb +++ b/lib/new_relic/agent/configuration/manager.rb @@ -362,7 +362,7 @@ def reset_to_defaults # reset the configuration hash, but do not replace previously auto # determined dependency detection values with nil or 'auto' def reset_cache - return new_cache unless @cache + return new_cache unless defined?(@cache) && @cache preserved = @cache.select { |_k, v| DEPENDENCY_DETECTION_VALUES.include?(v) } new_cache From 5084f2fc9cad4bbe7c8676d643932d596f67bc3e Mon Sep 17 00:00:00 2001 From: fallwith Date: Fri, 21 Apr 2023 09:07:57 -0700 Subject: [PATCH 6/7] 'becames' -> 'becomes' typo fix 'becames' -> 'becomes' comment spelling fix --- test/new_relic/dependency_detection_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/new_relic/dependency_detection_test.rb b/test/new_relic/dependency_detection_test.rb index 3403b31b1c..341f4f2348 100644 --- a/test/new_relic/dependency_detection_test.rb +++ b/test/new_relic/dependency_detection_test.rb @@ -400,7 +400,7 @@ def test_prepend_or_chain_based_values_have_auto_converted_into_one_of_those end end - # confirm that :auto becames :chain when :chain is automatically determined + # confirm that :auto becomes :chain when :chain is automatically determined def test_auto_is_replaced_by_chain_when_chain_is_used name = :blank_and_jones From bf3b5ee288564b1c72f44cc139a5b742463a3a0a Mon Sep 17 00:00:00 2001 From: fallwith Date: Fri, 21 Apr 2023 12:27:30 -0700 Subject: [PATCH 7/7] PR 1930 CHANGELOG updates Update PR 1930's CHANGELOG entry to use the correct "module prepending" and "method chaining" verbiage. Also replace the potentially confusing "per" with "for". --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76542d3e03..144530e369 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,9 @@ ## dev -- **Feature: The agent configuration will now reflect whether method prepending or chaining was used per instrumentation** +- **Feature: The agent configuration will now reflect whether module prepending or method chaining was used for instrumentation** - For `:'instrumentation.*'` configuration parameters that are set to :auto (the default), the agent will automatically determine whether to use method prepending or chaining. The agent will now update its in-memory configuration to give each relevant parameter a value of either :prepend or :chain so that the result of the determination can be introspected. This is intended to help 3rd party libraries that wish to further enhance the agent's instrumentation capabilities by prepending or chaining additional logic. Environment variable, YAML file, and server-side configuration based values are not impacted. [PR#1930](https://github.com/newrelic/newrelic-ruby-agent/pull/1930) + For `:'instrumentation.*'` configuration parameters that are set to :auto (the default), the agent will automatically determine whether to use module prepending or method chaining. The agent will now update its in-memory configuration to give each relevant parameter a value of either :prepend or :chain so that the result of the determination can be introspected. This is intended to help 3rd party libraries that wish to further enhance the agent's instrumentation capabilities by prepending or chaining additional logic. Environment variable, YAML file, and server-side configuration based values are not impacted. [PR#1930](https://github.com/newrelic/newrelic-ruby-agent/pull/1930) - **Feature: Deprecate memcached and memcache-client instrumentation**