Moved log appender spy class into spec_helper, used the same tecnhique also in runner_spec, removing the programmatic configuration way

Author: andsel

logstash-core/spec/logstash/runner_spec.rb

@@ -263,41 +263,40 @@
         let(:runner_deprecation_logger_stub) { double("DeprecationLogger(Runner)").as_null_object }
         before(:each) { allow(runner).to receive(:deprecation_logger).and_return(runner_deprecation_logger_stub) }
 
-        def configure_log_spy
-          java_import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilderFactory
-          java_import org.apache.logging.log4j.Level
-          config_builder = ConfigurationBuilderFactory.newConfigurationBuilder
-          return config_builder
-                   .add(
-                     config_builder
-                       .newAppender("LOG_SPY", "List")
-                       .add(config_builder.newLayout("PatternLayout").addAttribute("pattern", "%-5p [%t]: %m%n"))
-                   )
-                   .add(
-                     config_builder
-                       .newRootLogger(Level::INFO)
-                       .add(config_builder.newAppenderRef("LOG_SPY")))
-                   .build(false)
-        end
-
         context "when deprecated :http.host is defined by the user" do
           let(:args) { ["--http.host", "localhost", "-e", pipeline_string]}
-          it "creates an Agent whose `api.http.host` uses the provided value and provides helpful deprecation message" do
-            java_import org.apache.logging.log4j.core.config.Configurator
-            java_import org.apache.logging.log4j.core.config.Configuration
+          let(:events) { [] }
 
+          before(:each) do
             java_import org.apache.logging.log4j.LogManager
-            java_import org.apache.logging.log4j.core.impl.Log4jContextFactory
-            # This is done because the LoggerExt calls setFactory LogstashLoggerContextFactory implements
-            # org.apache.logging.log4j.spi.LoggerContextFactory and doesn't extend org.apache.logging.log4j.core.impl.Log4jContextFactory
-            # which is the class expected by the following Configurator to use the programmatic configuration.
-            LogManager.setFactory(Log4jContextFactory.new)
-            log_ctx = Configurator.java_send(:initialize, [Configuration], configure_log_spy)
-            expect(log_ctx).not_to be nil
-            log_ctx.reconfigure(configure_log_spy) # force the programmatic configuration, without this it's not used
+            logger = LogManager.getLogger("org.logstash.settings.DeprecatedAlias")
+            deprecated_logger = LogManager.getLogger("org.logstash.deprecation.settings.DeprecatedAlias")
 
-            appender_spy = log_ctx.getConfiguration().getAppender("LOG_SPY")
+            @custom_appender = CustomAppender.new(events).tap {|appender| appender.start }
+
+            java_import org.apache.logging.log4j.Level
+            logger.addAppender(@custom_appender)
+            deprecated_logger.addAppender(@custom_appender)
+            # had to set level after appending as it was "error" for some reason
+            logger.setLevel(Level::INFO)
+            deprecated_logger.setLevel(Level::INFO)
+
+            expect(@custom_appender.started?).to be_truthy
+          end
 
+          after(:each) do
+            events.clear
+            java_import org.apache.logging.log4j.LogManager
+            logger = LogManager.getLogger("org.logstash.settings.DeprecatedAlias")
+            deprecated_logger = LogManager.getLogger("org.logstash.deprecation.settings.DeprecatedAlias")
+            # The Logger's AbstractConfiguration contains a cache of Appender, by class name. The cache is updated
+            # iff is absent, so to make subsequent add_appender effective we have cleanup on teardown, else the new
+            # appender instance is simply not used by the logger
+            logger.remove_appender(@custom_appender)
+            deprecated_logger.remove_appender(@custom_appender)
+          end
+
+          it "creates an Agent whose `api.http.host` uses the provided value and provides helpful deprecation message" do
             expect(runner_deprecation_logger_stub).to receive(:deprecated).with(a_string_including 'The flag ["--http.host"] has been deprecated')
             expect(LogStash::Agent).to receive(:new) do |settings|
               expect(settings.set?("api.http.host")).to be(true)
@@ -306,10 +305,8 @@ def configure_log_spy
 
             subject.run("bin/logstash", args)
 
-            expect(appender_spy.messages).not_to be_empty
-            expect(appender_spy.messages[0]).to match(/`http.host` is a deprecated alias for `api.http.host`/)
-
-            log_ctx.close
+            expect(events).not_to be_empty
+            expect(events[0]).to match(/`http.host` is a deprecated alias for `api.http.host`/)
           end
         end
 

logstash-core/spec/logstash/settings/setting_with_deprecated_alias_spec.rb

@@ -17,7 +17,6 @@
 
 require "spec_helper"
 require "logstash/settings"
-java_import org.apache.logging.log4j.core.appender.AbstractAppender
 
 describe LogStash::Setting::SettingWithDeprecatedAlias do
   let(:canonical_setting_name) { "canonical.setting" }
@@ -28,21 +27,6 @@
   let(:settings) { LogStash::Settings.new }
   let(:canonical_setting) { LogStash::Setting::StringSetting.new(canonical_setting_name, default_value, true) }
 
-  class CustomAppender < AbstractAppender
-
-    attr_reader :events_collector
-
-    def initialize(events)
-      super("CustomCaptorAppender", nil, nil, true, org.apache.logging.log4j.core.config.Property::EMPTY_ARRAY)
-      @events_collector = events
-    end
-
-    # override the append to catch all the calls and collect the events
-    def append(log_event)
-      @events_collector << log_event.message.formatted_message
-    end
-  end
-
   let(:events) { [] }
 
   before(:each) do
@@ -195,7 +179,6 @@ def append(log_event)
   context 'Settings#get on deprecated alias' do
     it 'produces a WARN-level message to the logger' do
       settings.get(deprecated_setting_name)
-      sleep 0.1
       expect(events[0]).to include("setting `#{canonical_setting_name}` has been queried by its deprecated alias `#{deprecated_setting_name}`")
     end
   end

spec/spec_helper.rb

@@ -91,42 +91,19 @@ def installed_plugins
   Gem::Specification.find_all.select { |spec| spec.metadata["logstash_plugin"] }.map { |plugin| plugin.name }
 end
 
+java_import org.apache.logging.log4j.core.appender.AbstractAppender
 
-def setup_logger_spy
-  java_import org.apache.logging.log4j.core.config.Configurator
-  java_import org.apache.logging.log4j.core.config.Configuration
-
-  java_import org.apache.logging.log4j.LogManager
-  java_import org.apache.logging.log4j.core.impl.Log4jContextFactory
-  # This is done because the LoggerExt calls setFactory LogstashLoggerContextFactory implements
-  # org.apache.logging.log4j.spi.LoggerContextFactory and doesn't extend org.apache.logging.log4j.core.impl.Log4jContextFactory
-  # which is the class expected by the following Configurator to use the programmatic configuration.
-  LogManager.setFactory(Log4jContextFactory.new)
-  log_ctx = Configurator.java_send(:initialize, [Configuration], configure_log_spy)
-  expect(log_ctx).not_to be nil
-  log_ctx.reconfigure(configure_log_spy) # force the programmatic configuration, without this it's not used
-
-  log_ctx
-end
+class CustomAppender < AbstractAppender
 
-def configure_log_spy
-  java_import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilderFactory
-  java_import org.apache.logging.log4j.Level
-  config_builder = ConfigurationBuilderFactory.newConfigurationBuilder
-  configuration = config_builder
-                        .add(
-                          config_builder
-                            .newAppender("LOG_SPY", "List")
-                            .add(config_builder.newLayout("PatternLayout").addAttribute("pattern", "%-5p [%t]: %m%n"))
-                        )
-                        .add(
-                          config_builder
-                            .newRootLogger(Level::INFO)
-                            .add(config_builder.newAppenderRef("LOG_SPY")))
-                        .build(false)
-  configuration
-end
+  attr_reader :events_collector
 
-def retrieve_logger_spy(log_ctx)
-  log_ctx.getConfiguration().getAppender("LOG_SPY")
-end
+  def initialize(events)
+    super("CustomCaptorAppender", nil, nil, true, org.apache.logging.log4j.core.config.Property::EMPTY_ARRAY)
+    @events_collector = events
+  end
+
+  # override the append to catch all the calls and collect the events
+  def append(log_event)
+    @events_collector << log_event.message.formatted_message
+  end
+end