diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a715ff53f..8493b1b885 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,16 @@ Version adds Apache Kafka instrumentation for the rdkafka and ruby-kafka g [PR#2851](https://github.com/newrelic/newrelic-ruby-agent/pull/2851) +- **Bugfix: Corrected Boolean coercion for `newrelic.yml` configuration** + + Previously, any String assigned to New Relic configurations expecting a Boolean value were evaluated as `true`. This could lead to unexpected behavior. For example, setting `application_logging.enabled: 'false'` in `newrelic.yml` would incorrectly evaluate to `application_logging.enabled: true` due to the truthy nature of Strings. + + Now, the agent strictly interprets Boolean configuration values. It recognizes both actual Boolean values and certain Strings/Symbols: + - `'true'`, `'yes'`, or `'on'` (evaluates to `true`) + - `'false'`, `'no'`, or `'off'` (evaluates to `false`) + + Any other inputs will revert to the setting's default configuration value. [PR#2847](https://github.com/newrelic/newrelic-ruby-agent/pull/2847) + - **Bugfix: Jruby not saving configuration values correctly in configuration manager** Previously, a change made to fix a different JRuby bug caused the agent to not save configuration values correctly in the configuration manager when running on JRuby. This has been fixed. [PR#2848](https://github.com/newrelic/newrelic-ruby-agent/pull/2848) diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 0dfd2a07d1..246af67829 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -35,6 +35,15 @@ def self.===(o) end class DefaultSource + BOOLEAN_MAP = { + 'true' => true, + 'yes' => true, + 'on' => true, + 'false' => false, + 'no' => false, + 'off' => false + }.freeze + attr_reader :defaults extend Forwardable @@ -64,6 +73,12 @@ def self.allowlist_for(key) value_from_defaults(key, :allowlist) end + def self.boolean_for(key, value) + string_value = (value.respond_to?(:call) ? value.call : value).to_s + + BOOLEAN_MAP.fetch(string_value, nil) + end + def self.default_for(key) value_from_defaults(key, :default) end @@ -2268,7 +2283,7 @@ def self.notify :description => 'Enable or disable debugging version of JavaScript agent loader for browser monitoring instrumentation.' }, :'browser_monitoring.ssl_for_http' => { - :default => nil, + :default => false, :allow_nil => true, :public => false, :type => Boolean, diff --git a/lib/new_relic/agent/configuration/manager.rb b/lib/new_relic/agent/configuration/manager.rb index 9d56c2c006..f1c5b6bb24 100644 --- a/lib/new_relic/agent/configuration/manager.rb +++ b/lib/new_relic/agent/configuration/manager.rb @@ -142,6 +142,9 @@ def evaluate_and_apply_transformations(key, value) default = enforce_allowlist(key, evaluated) return default if default + boolean = enforce_boolean(key, value) + return boolean if [true, false].include?(boolean) + apply_transformations(key, evaluated) end @@ -167,6 +170,18 @@ def enforce_allowlist(key, value) default end + def enforce_boolean(key, value) + type = default_source.value_from_defaults(key, :type) + return unless type == Boolean + + bool_value = default_source.boolean_for(key, value) + return bool_value unless bool_value.nil? + + default = default_source.default_for(key) + NewRelic::Agent.logger.warn "Invalid value '#{value}' for #{key}, applying default value of '#{default}'" + default + end + def transform_from_default(key) default_source.transform_for(key) end diff --git a/lib/new_relic/agent/javascript_instrumentor.rb b/lib/new_relic/agent/javascript_instrumentor.rb index e753d2f25c..3e1742c5db 100644 --- a/lib/new_relic/agent/javascript_instrumentor.rb +++ b/lib/new_relic/agent/javascript_instrumentor.rb @@ -164,9 +164,8 @@ def data_for_js_agent(transaction) def add_ssl_for_http(data) ssl_for_http = NewRelic::Agent.config[:'browser_monitoring.ssl_for_http'] - unless ssl_for_http.nil? - data[SSL_FOR_HTTP_KEY] = ssl_for_http - end + + data[SSL_FOR_HTTP_KEY] = ssl_for_http if ssl_for_http end def add_attributes(data, txn) diff --git a/test/new_relic/agent/configuration/default_source_test.rb b/test/new_relic/agent/configuration/default_source_test.rb index 5dc8e1a1a8..dd68ce405d 100644 --- a/test/new_relic/agent/configuration/default_source_test.rb +++ b/test/new_relic/agent/configuration/default_source_test.rb @@ -140,12 +140,6 @@ def test_config_search_path_in_warbler end end - def test_application_logging_enabled_default - with_config(:'application_logging.enabled' => :foo) do - assert_equal :foo, NewRelic::Agent.config['application_logging.enabled'] - end - end - def test_agent_attribute_settings_convert_comma_delimited_strings_into_an_arrays types = %w[transaction_tracer. transaction_events. error_collector. browser_monitoring.] types << '' @@ -313,6 +307,73 @@ def test_automatic_custom_instrumentation_method_list_supports_a_comma_delmited_ end end + def test_boolean_configs_accepts_yes_on_and_true_as_strings + key = :'send_data_on_exit' + config_array = %w[yes on true] + + config_array.each do |value| + with_config(key => value) do + assert NewRelic::Agent.config[key], "The '#{value}' value failed to evaluate as truthy!" + end + end + end + + def test_boolean_configs_accepts_yes_on_and_true_as_symbols + key = :'send_data_on_exit' + config_array = %i[yes on true] + + config_array.each do |value| + with_config(key => value) do + assert NewRelic::Agent.config[key], "The '#{value}' value failed to evaluate as truthy!" + end + end + end + + def test_boolean_configs_accepts_no_off_and_false_as_strings + key = :'send_data_on_exit' + + %w[no off false].each do |value| + with_config(key => value) do + refute NewRelic::Agent.config[key], "The '#{value}' value failed to evaluate as falsey!" + end + end + end + + def test_boolean_configs_accepts_no_off_and_false_as_strings_as_symbols + key = :'send_data_on_exit' + + %i[no off false].each do |value| + with_config(key => value) do + refute NewRelic::Agent.config[key], "The '#{value}' value failed to evaluate as falsey!" + end + end + end + + def test_enforce_boolean_uses_defult_on_invalid_value + key = :'send_data_on_exit' # default value is `true` + + with_config(key => 'invalid_value') do + assert NewRelic::Agent.config[key] + end + end + + def test_enforce_boolean_logs_warning_on_invalid_value + key = :'send_data_on_exit' + default = ::NewRelic::Agent::Configuration::DefaultSource.default_for(key) + + with_config(key => 'yikes!') do + expects_logging(:warn, includes("Invalid value 'yikes!' for #{key}, applying default value of '#{default}'")) + end + end + + def test_boolean_config_evaluates_proc_configs + key = :agent_enabled # default value is a proc + + with_config(key => 'off') do + refute NewRelic::Agent.config[key] + end + end + def get_config_value_class(value) type = value.class