Merge pull request #2546 from newrelic/adamantine_forge

Configuration: support allowlists

infinite_tracing/lib/infinite_tracing/config.rb

@@ -118,18 +118,7 @@ def compression_enabled?
       end
 
       def compression_level
-        @compression_level ||= begin
-          level = if COMPRESSION_LEVEL_LIST.include?(configured_compression_level)
-            configured_compression_level
-          else
-            NewRelic::Agent.logger.error("Invalid compression level '#{configured_compression_level}' specified! " \
-                                         "Must be one of #{COMPRESSION_LEVEL_LIST.join('|')}. Using default level " \
-                                         "of '#{COMPRESSION_LEVEL_DEFAULT}'")
-            COMPRESSION_LEVEL_DEFAULT
-          end
-          NewRelic::Agent.logger.debug("Infinite Tracer compression level set to #{level}")
-          level
-        end
+        @compression_level ||= configured_compression_level
       end
 
       def configured_compression_level

infinite_tracing/test/infinite_tracing/config_test.rb

@@ -124,20 +124,6 @@ def test_compression_enabled_returns_false
           end
         end
 
-        def test_invalid_compression_level
-          reset_compression_level
-          logger = MiniTest::Mock.new
-          logger.expect :error, nil, [/Invalid compression level/]
-          logger.expect :debug, nil, [/compression level set to/]
-
-          with_config({'infinite_tracing.compression_level': :bogus}) do
-            NewRelic::Agent.stub :logger, logger do
-              assert_equal Config::COMPRESSION_LEVEL_DEFAULT, Config.compression_level
-              logger.verify
-            end
-          end
-        end
-
         private
 
         def non_test_files

lib/new_relic/agent/configuration/default_source.rb

@@ -52,9 +52,24 @@ def default_values
           result
         end
 
+        def self.default_settings(key)
+          ::NewRelic::Agent::Configuration::DEFAULTS[key]
+        end
+
+        def self.value_from_defaults(key, subkey)
+          default_settings(key)&.send(:[], subkey)
+        end
+
+        def self.allowlist_for(key)
+          value_from_defaults(key, :allowlist)
+        end
+
+        def self.default_for(key)
+          value_from_defaults(key, :default)
+        end
+
         def self.transform_for(key)
-          default_settings = ::NewRelic::Agent::Configuration::DEFAULTS[key]
-          default_settings[:transform] if default_settings
+          value_from_defaults(key, :transform)
         end
 
         def self.config_search_paths # rubocop:disable Metrics/AbcSize
@@ -800,6 +815,7 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil)
           :public => true,
           :type => String,
           :allowed_from_server => false,
+          :allowlist => %w[debug info warn error fatal unknown DEBUG INFO WARN ERROR FATAL UNKNOWN],
           :description => <<~DESCRIPTION
             Sets the minimum level a log event must have to be forwarded to New Relic.
 
@@ -2263,6 +2279,7 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil)
           :public => true,
           :type => Symbol,
           :allowed_from_server => false,
+          :allowlist => %i[none low medium high],
           :external => :infinite_tracing,
           :description => <<~DESC
             Configure the compression level for data sent to the trace observer.

lib/new_relic/agent/configuration/manager.rb

@@ -138,24 +138,41 @@ def evaluate_procs(value)
         end
 
         def evaluate_and_apply_transformations(key, value)
-          apply_transformations(key, evaluate_procs(value))
+          evaluated = evaluate_procs(value)
+          default = enforce_allowlist(key, evaluated)
+          return default if default
+
+          apply_transformations(key, evaluated)
         end
 
         def apply_transformations(key, value)
           if transform = transform_from_default(key)
             begin
               transform.call(value)
             rescue => e
-              ::NewRelic::Agent.logger.error("Error applying transformation for #{key}, pre-transform value was: #{value}.", e)
+              NewRelic::Agent.logger.error("Error applying transformation for #{key}, pre-transform value was: #{value}.", e)
               raise e
             end
           else
             value
           end
         end
 
+        def enforce_allowlist(key, value)
+          return unless allowlist = default_source.allowlist_for(key)
+          return if allowlist.include?(value)
+
+          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)
-          ::NewRelic::Agent::Configuration::DefaultSource.transform_for(key)
+          default_source.transform_for(key)
+        end
+
+        def default_source
+          NewRelic::Agent::Configuration::DefaultSource
         end
 
         def register_callback(key, &proc)
@@ -214,7 +231,7 @@ def flattened
               begin
                 thawed_layer[k] = instance_eval(&v) if v.respond_to?(:call)
               rescue => e
-                ::NewRelic::Agent.logger.debug("#{e.class.name} : #{e.message} - when accessing config key #{k}")
+                NewRelic::Agent.logger.debug("#{e.class.name} : #{e.message} - when accessing config key #{k}")
                 thawed_layer[k] = nil
               end
               thawed_layer.delete(:config)
@@ -383,7 +400,7 @@ def log_config(direction, source)
           # is expensive enough that we don't want to do it unless we're
           # actually going to be logging the message based on our current log
           # level, so use a `do` block.
-          ::NewRelic::Agent.logger.debug do
+          NewRelic::Agent.logger.debug do
             hash = flattened.delete_if { |k, _h| DEFAULTS.fetch(k, {}).fetch(:exclude_from_reported_settings, false) }
             "Updating config (#{direction}) from #{source.class}. Results: #{hash.inspect}"
           end

lib/new_relic/agent/log_event_aggregator.rb

@@ -247,21 +247,6 @@ def truncate_message(message)
         message.byteslice(0...MAX_BYTES)
       end
 
-      def minimum_log_level
-        if Logger::Severity.constants.include?(configured_log_level_constant)
-          configured_log_level_constant
-        else
-          NewRelic::Agent.logger.log_once(
-            :error,
-            'Invalid application_logging.forwarding.log_level ' \
-            "'#{NewRelic::Agent.config[LOG_LEVEL_KEY]}' specified! " \
-            "Must be one of #{Logger::Severity.constants.join('|')}. " \
-            "Using default level of 'debug'"
-          )
-          :DEBUG
-        end
-      end
-
       def configured_log_level_constant
         format_log_level_constant(NewRelic::Agent.config[LOG_LEVEL_KEY])
       end
@@ -275,7 +260,7 @@ def severity_too_low?(severity)
         # always record custom log levels
         return false unless Logger::Severity.constants.include?(severity_constant)
 
-        Logger::Severity.const_get(severity_constant) < Logger::Severity.const_get(minimum_log_level)
+        Logger::Severity.const_get(severity_constant) < Logger::Severity.const_get(configured_log_level_constant)
       end
     end
   end

test/new_relic/agent/configuration/default_source_test.rb

@@ -273,6 +273,24 @@ def test_convert_to_hash_raises_error_with_wrong_data_type
       assert_raises(ArgumentError) { DefaultSource.convert_to_hash(value) }
     end
 
+    def test_allowlist_permits_valid_values
+      valid_value = 'info'
+      key = :'application_logging.forwarding.log_level'
+
+      with_config(key => valid_value) do
+        assert_equal valid_value, NewRelic::Agent.config[key]
+      end
+    end
+
+    def test_allowlist_blocks_invalid_values_and_uses_a_default
+      key = :'application_logging.forwarding.log_level'
+      default = ::NewRelic::Agent::Configuration::DefaultSource.default_for(key)
+
+      with_config(key => 'bogus') do
+        assert_equal default, NewRelic::Agent.config[key]
+      end
+    end
+
     def get_config_value_class(value)
       type = value.class
 

test/new_relic/agent/log_event_aggregator_test.rb

@@ -455,20 +455,11 @@ def test_does_not_record_if_message_empty_string
     end
 
     def test_sets_minimum_log_level_to_debug_when_not_within_default_severities
+      # NOTE: a default string value is applied by the allowlist in the
+      #       DefaultSource hash which is then converted to an upcased symbol
+      #       via `configured_log_level_constant`.
       with_config(LogEventAggregator::LOG_LEVEL_KEY => 'milkshake') do
-        assert_equal :DEBUG, @aggregator.send(:minimum_log_level)
-      end
-    end
-
-    def test_logs_error_when_log_level_not_within_default_severities
-      logger = MiniTest::Mock.new
-      logger.expect :log_once, nil, [:error, /Invalid application_logging.forwarding.log_level/]
-
-      with_config(LogEventAggregator::LOG_LEVEL_KEY => 'milkshake') do
-        NewRelic::Agent.stub :logger, logger do
-          @aggregator.send(:minimum_log_level)
-          logger.verify
-        end
+        assert_equal :DEBUG, @aggregator.send(:configured_log_level_constant)
       end
     end
 
@@ -480,7 +471,7 @@ def test_sets_log_level_constant_to_symbolized_capitalized_level
 
     def test_sets_minimum_log_level_when_config_capitalized
       with_config(LogEventAggregator::LOG_LEVEL_KEY => 'INFO') do
-        assert_equal(:INFO, @aggregator.send(:minimum_log_level))
+        assert_equal(:INFO, @aggregator.send(:configured_log_level_constant))
       end
     end