[DEBUG PR] Added debug logs to check why the spy doesn't work

logstash-core/lib/logstash/environment.rb

@@ -35,7 +35,7 @@ module Environment
 
   [
            Setting::Boolean.new("allow_superuser", true),
-            Setting::String.new("node.name", Socket.gethostname),
+            Setting::StringSetting.new("node.name", Socket.gethostname),
     Setting::NullableString.new("path.config", nil, false),
  Setting::WritableDirectory.new("path.data", ::File.join(LogStash::Environment::LOGSTASH_HOME, "data")),
     Setting::NullableString.new("config.string", nil, false),
@@ -50,10 +50,10 @@ module Environment
            Setting::Boolean.new("config.reload.automatic", false),
          Setting::TimeValue.new("config.reload.interval", "3s"), # in seconds
            Setting::Boolean.new("config.support_escapes", false),
-            Setting::String.new("config.field_reference.escape_style", "none", true, %w(none percent ampersand)),
-            Setting::String.new("event_api.tags.illegal", "rename", true, %w(rename warn)),
+            Setting::StringSetting.new("config.field_reference.escape_style", "none", true, %w(none percent ampersand)),
+            Setting::StringSetting.new("event_api.tags.illegal", "rename", true, %w(rename warn)),
            Setting::Boolean.new("metric.collect", true),
-            Setting::String.new("pipeline.id", "main"),
+            Setting::StringSetting.new("pipeline.id", "main"),
            Setting::Boolean.new("pipeline.system", false),
    Setting::PositiveInteger.new("pipeline.workers", LogStash::Config::CpuCoreStrategy.maximum),
    Setting::PositiveInteger.new("pipeline.batch.size", 125),
@@ -67,30 +67,30 @@ module Environment
                     Setting.new("path.plugins", Array, []),
     Setting::NullableString.new("interactive", nil, false),
            Setting::Boolean.new("config.debug", false),
-            Setting::String.new("log.level", "info", true, ["fatal", "error", "warn", "debug", "info", "trace"]),
+            Setting::StringSetting.new("log.level", "info", true, ["fatal", "error", "warn", "debug", "info", "trace"]),
            Setting::Boolean.new("version", false),
            Setting::Boolean.new("help", false),
            Setting::Boolean.new("enable-local-plugin-development", false),
-            Setting::String.new("log.format", "plain", true, ["json", "plain"]),
+            Setting::StringSetting.new("log.format", "plain", true, ["json", "plain"]),
            Setting::Boolean.new("log.format.json.fix_duplicate_message_fields", false),
            Setting::Boolean.new("api.enabled", true).with_deprecated_alias("http.enabled"),
-            Setting::String.new("api.http.host", "127.0.0.1").with_deprecated_alias("http.host"),
+            Setting::StringSetting.new("api.http.host", "127.0.0.1").with_deprecated_alias("http.host"),
          Setting::PortRange.new("api.http.port", 9600..9700).with_deprecated_alias("http.port"),
-            Setting::String.new("api.environment", "production").with_deprecated_alias("http.environment"),
-            Setting::String.new("api.auth.type", "none", true, %w(none basic)),
-            Setting::String.new("api.auth.basic.username", nil, false).nullable,
+            Setting::StringSetting.new("api.environment", "production").with_deprecated_alias("http.environment"),
+            Setting::StringSetting.new("api.auth.type", "none", true, %w(none basic)),
+            Setting::StringSetting.new("api.auth.basic.username", nil, false).nullable,
           Setting::Password.new("api.auth.basic.password", nil, false).nullable,
-            Setting::String.new("api.auth.basic.password_policy.mode", "WARN", true, %w[WARN ERROR]),
+            Setting::StringSetting.new("api.auth.basic.password_policy.mode", "WARN", true, %w[WARN ERROR]),
            Setting::Numeric.new("api.auth.basic.password_policy.length.minimum", 8),
-            Setting::String.new("api.auth.basic.password_policy.include.upper", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
-            Setting::String.new("api.auth.basic.password_policy.include.lower", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
-            Setting::String.new("api.auth.basic.password_policy.include.digit", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
-            Setting::String.new("api.auth.basic.password_policy.include.symbol", "OPTIONAL", true, %w[REQUIRED OPTIONAL]),
+            Setting::StringSetting.new("api.auth.basic.password_policy.include.upper", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
+            Setting::StringSetting.new("api.auth.basic.password_policy.include.lower", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
+            Setting::StringSetting.new("api.auth.basic.password_policy.include.digit", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
+            Setting::StringSetting.new("api.auth.basic.password_policy.include.symbol", "OPTIONAL", true, %w[REQUIRED OPTIONAL]),
            Setting::Boolean.new("api.ssl.enabled", false),
   Setting::ExistingFilePath.new("api.ssl.keystore.path", nil, false).nullable,
           Setting::Password.new("api.ssl.keystore.password", nil, false).nullable,
        Setting::StringArray.new("api.ssl.supported_protocols", nil, true, %w[TLSv1 TLSv1.1 TLSv1.2 TLSv1.3]),
-            Setting::String.new("queue.type", "memory", true, ["persisted", "memory"]),
+            Setting::StringSetting.new("queue.type", "memory", true, ["persisted", "memory"]),
            Setting::Boolean.new("queue.drain", false),
              Setting::Bytes.new("queue.page_capacity", "64mb"),
              Setting::Bytes.new("queue.max_bytes", "1024mb"),
@@ -102,16 +102,16 @@ module Environment
            Setting::Boolean.new("dead_letter_queue.enable", false),
              Setting::Bytes.new("dead_letter_queue.max_bytes", "1024mb"),
            Setting::Numeric.new("dead_letter_queue.flush_interval", 5000),
-            Setting::String.new("dead_letter_queue.storage_policy", "drop_newer", true, ["drop_newer", "drop_older"]),
+            Setting::StringSetting.new("dead_letter_queue.storage_policy", "drop_newer", true, ["drop_newer", "drop_older"]),
     Setting::NullableString.new("dead_letter_queue.retain.age"), # example 5d
          Setting::TimeValue.new("slowlog.threshold.warn", "-1"),
          Setting::TimeValue.new("slowlog.threshold.info", "-1"),
          Setting::TimeValue.new("slowlog.threshold.debug", "-1"),
          Setting::TimeValue.new("slowlog.threshold.trace", "-1"),
-            Setting::String.new("keystore.classname", "org.logstash.secret.store.backend.JavaKeyStore"),
-            Setting::String.new("keystore.file", ::File.join(::File.join(LogStash::Environment::LOGSTASH_HOME, "config"), "logstash.keystore"), false), # will be populated on
+            Setting::StringSetting.new("keystore.classname", "org.logstash.secret.store.backend.JavaKeyStore"),
+            Setting::StringSetting.new("keystore.file", ::File.join(::File.join(LogStash::Environment::LOGSTASH_HOME, "config"), "logstash.keystore"), false), # will be populated on
     Setting::NullableString.new("monitoring.cluster_uuid"),
-            Setting::String.new("pipeline.buffer.type", "direct", true, ["direct", "heap"])
+            Setting::StringSetting.new("pipeline.buffer.type", "direct", true, ["direct", "heap"])
   # post_process
   ].each {|setting| SETTINGS.register(setting) }
 

logstash-core/lib/logstash/settings.rb

@@ -524,6 +524,9 @@ def validate(value)
       end
     end
 
+
+    java_import org.logstash.settings.StringSetting
+
     class String < Setting
       def initialize(name, default = nil, strict = true, possible_strings = [])
         @possible_strings = possible_strings
@@ -824,6 +827,8 @@ def validate(value)
       end
     end
 
+    java_import org.logstash.settings.NullableSetting
+
     # @see Setting#nullable
     # @api internal
     class Nullable < SimpleDelegator

logstash-core/lib/logstash/util/settings_helper.rb

@@ -33,8 +33,8 @@ module LogStash::Util::SettingsHelper
   # The `path.settings` and `path.logs` can not be defined in "logstash/environment" since the `Environment::LOGSTASH_HOME` doesn't
   # exist unless launched via "bootstrap/environment"
   def self.pre_process
-    LogStash::SETTINGS.register(LogStash::Setting::String.new("path.settings", ::File.join(LogStash::Environment::LOGSTASH_HOME, "config")))
-    LogStash::SETTINGS.register(LogStash::Setting::String.new("path.logs", ::File.join(LogStash::Environment::LOGSTASH_HOME, "logs")))
+    LogStash::SETTINGS.register(LogStash::Setting::StringSetting.new("path.settings", ::File.join(LogStash::Environment::LOGSTASH_HOME, "config")))
+    LogStash::SETTINGS.register(LogStash::Setting::StringSetting.new("path.logs", ::File.join(LogStash::Environment::LOGSTASH_HOME, "logs")))
   end
 
   # Ensure that any settings are re-calculated after loading yaml

logstash-core/spec/logstash/queue_factory_spec.rb

@@ -23,15 +23,15 @@
   let(:settings_array) do
     [
       LogStash::Setting::WritableDirectory.new("path.queue", Stud::Temporary.pathname),
-      LogStash::Setting::String.new("queue.type", "memory", true, ["persisted", "memory"]),
+      LogStash::Setting::StringSetting.new("queue.type", "memory", true, ["persisted", "memory"]),
       LogStash::Setting::Bytes.new("queue.page_capacity", "8mb"),
       LogStash::Setting::Bytes.new("queue.max_bytes", "64mb"),
       LogStash::Setting::Numeric.new("queue.max_events", 0),
       LogStash::Setting::Numeric.new("queue.checkpoint.acks", 1024),
       LogStash::Setting::Numeric.new("queue.checkpoint.writes", 1024),
       LogStash::Setting::Numeric.new("queue.checkpoint.interval", 1000),
       LogStash::Setting::Boolean.new("queue.checkpoint.retry", false),
-      LogStash::Setting::String.new("pipeline.id", pipeline_id),
+      LogStash::Setting::StringSetting.new("pipeline.id", pipeline_id),
       LogStash::Setting::PositiveInteger.new("pipeline.batch.size", 125),
       LogStash::Setting::PositiveInteger.new("pipeline.workers", LogStash::Config::CpuCoreStrategy.maximum)
     ]

logstash-core/spec/logstash/runner_spec.rb

@@ -263,17 +263,73 @@
         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) }
 
+        let(:configuration_spy) { configure_log_spy }
+
+        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
-            expect(deprecation_logger_stub).to receive(:deprecated).with(a_string_including "`http.host` is a deprecated alias for `api.http.host`")
+            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
+
+            # logger = log_ctx.getLogger("test_dna")
+            # Disabling the following the test doesn't receive any message, enabling it receives these and the expected log lines, but fails other tests with log spy
+            # logger.info("DNADBG%% Rspec test direct log")
+
+            appender_spy = log_ctx.getConfiguration().getAppender("LOG_SPY")
+            # logger = log_ctx.getLogger("test_dna")
+            # expect(logger.info_enabled?).to be_truthy
+            # logger.info("DNADBG>> Rspec test direct log")
+            # # puts "DNADBG>> log spy in before #{log_spy}"
+            # # puts "DNADBG>> all appenders #{@log_ctx.getConfiguration().getAppenders}"
+            # expect(appender_spy.messages[0]).to include("Rspec test direct log")
+            # appender_spy.clear
+            # expect(appender_spy.messages).to be_empty
+
+
             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)
               expect(settings.get("api.http.host")).to eq("localhost")
+              # settings.get_setting('http.host').observe_post_process
             end
 
             subject.run("bin/logstash", args)
+            #appender_spy.stop
+            # sleep 1
+            # log_ctx.stop()
+
+            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
+            # LogManager.shutdown(log_ctx)
           end
         end
 

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

@@ -20,13 +20,13 @@
 
 describe LogStash::Setting::Nullable do
   let(:setting_name) { "this.that" }
-  let(:normal_setting) { LogStash::Setting::String.new(setting_name, nil, false, possible_strings) }
+  let(:normal_setting) { LogStash::Setting::StringSetting.new(setting_name, nil, false, possible_strings) }
   let(:possible_strings) { [] } # empty means any string passes
 
   subject(:nullable_setting) { normal_setting.nullable }
 
   it 'is a kind of Nullable' do
-    expect(nullable_setting).to be_a_kind_of(described_class)
+    expect(nullable_setting).to be_a_kind_of(LogStash::Setting::NullableSetting)
   end
 
   it "retains the wrapped setting's name" do
@@ -56,14 +56,14 @@
     context 'to an invalid wrong-type value' do
       let(:candidate_value) { 127 } # wrong type, expects String
       it 'is an invalid setting' do
-        expect { nullable_setting.validate_value }.to raise_error(ArgumentError, a_string_including("Setting \"#{setting_name}\" must be a "))
+        expect { nullable_setting.validate_value }.to raise_error(java.lang.ClassCastException, a_string_including("class java.lang.Long cannot be cast to class java.lang.String"))
       end
     end
     context 'to an invalid value not in the allow-list' do
       let(:possible_strings) { %w(this that)}
       let(:candidate_value) { "another" } # wrong type, expects String
       it 'is an invalid setting' do
-        expect { nullable_setting.validate_value }.to raise_error(ArgumentError, a_string_including("Invalid value"))
+        expect { nullable_setting.validate_value }.to raise_error(java.lang.IllegalArgumentException, a_string_including("Invalid value"))
       end
     end
     context 'to a valid value' do