From 46b996c24da17cdc7a16bc3037edab5c6132ccd0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 24 Jul 2024 10:29:18 +0100 Subject: [PATCH] settings: add support for observing settings after post-process hooks (#16339) (#16348) Because logging configuration occurs after loading the `logstash.yml` settings, deprecation logs from `LogStash::Settings::DeprecatedAlias#set` are effectively emitted to a null logger and lost. By re-emitting after the post-process hooks, we can ensure that they make their way to the deprecation log. This change adds support for any setting that responds to `Object#observe_post_process` to receive it after all post-processing hooks have been executed. Resolves: elastic/logstash#16332 (cherry picked from commit c633ad25682f3376adb0ba434d658b9eb820ac92) Co-authored-by: Ry Biesemeyer --- logstash-core/lib/logstash/settings.rb | 23 ++++++++++++++++--- .../setting_with_deprecated_alias_spec.rb | 21 +++++++++++++++++ logstash-core/spec/logstash/settings_spec.rb | 12 ++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/logstash-core/lib/logstash/settings.rb b/logstash-core/lib/logstash/settings.rb index 4e23e34c3a9..d6238fe1cb0 100644 --- a/logstash-core/lib/logstash/settings.rb +++ b/logstash-core/lib/logstash/settings.rb @@ -197,6 +197,13 @@ def post_process callback.call(self) end end + + # because we cannot rely on deprecation logger being wired up when the setters + # are initially used, we re-emit setter-related deprecations after all post-processing + # hooks have been activated (and therefore after logging has been configured) + @settings.each_value do |setting| + setting.observe_post_process if setting.respond_to?(:observe_post_process) + end end def on_post_process(&block) @@ -839,9 +846,7 @@ def initialize(canonical_proxy, alias_name) end def set(value) - deprecation_logger.deprecated(I18n.t("logstash.settings.deprecation.set", - :deprecated_alias => name, - :canonical_name => canonical_proxy.name)) + do_log_setter_deprecation super end @@ -856,6 +861,18 @@ def validate_value # bypass deprecation warning wrapped.validate_value if set? end + + def observe_post_process + do_log_setter_deprecation if set? + end + + private + + def do_log_setter_deprecation + deprecation_logger.deprecated(I18n.t("logstash.settings.deprecation.set", + :deprecated_alias => name, + :canonical_name => canonical_proxy.name)) + end end ## diff --git a/logstash-core/spec/logstash/settings/setting_with_deprecated_alias_spec.rb b/logstash-core/spec/logstash/settings/setting_with_deprecated_alias_spec.rb index 9ddd87a783c..481e387ba03 100644 --- a/logstash-core/spec/logstash/settings/setting_with_deprecated_alias_spec.rb +++ b/logstash-core/spec/logstash/settings/setting_with_deprecated_alias_spec.rb @@ -52,6 +52,13 @@ end include_examples '#validate_value success' + + context "#observe_post_process" do + it 'does not emit a deprecation warning' do + expect(LogStash::Settings.deprecation_logger).to_not receive(:deprecated).with(a_string_including(deprecated_setting_name)) + settings.get_setting(deprecated_setting_name).observe_post_process + end + end end context "when only the deprecated alias is set" do @@ -71,6 +78,13 @@ include_examples '#validate_value success' + context "#observe_post_process" do + it 're-emits the deprecation warning' do + expect(LogStash::Settings.deprecation_logger).to receive(:deprecated).with(a_string_including(deprecated_setting_name)) + settings.get_setting(deprecated_setting_name).observe_post_process + end + end + it 'validates deprecated alias' do expect { settings.get_setting(canonical_setting_name).deprecated_alias.validate_value }.to_not raise_error end @@ -107,6 +121,13 @@ end include_examples '#validate_value success' + + context "#observe_post_process" do + it 'does not emit a deprecation warning' do + expect(LogStash::Settings.deprecation_logger).to_not receive(:deprecated).with(a_string_including(deprecated_setting_name)) + settings.get_setting(deprecated_setting_name).observe_post_process + end + end end context "when both the canonical setting and deprecated alias are set" do diff --git a/logstash-core/spec/logstash/settings_spec.rb b/logstash-core/spec/logstash/settings_spec.rb index 241ac2272eb..3e0b425dd46 100644 --- a/logstash-core/spec/logstash/settings_spec.rb +++ b/logstash-core/spec/logstash/settings_spec.rb @@ -144,6 +144,18 @@ it "should preserve original settings" do expect(settings.get("foo")).to eq("bar") end + + context 'when a registered setting responds to `observe_post_process`' do + let(:observe_post_process_setting) do + LogStash::Setting::Boolean.new("this.that", true).tap { |s| allow(s).to receive(:observe_post_process) } + end + subject(:settings) do + described_class.new.tap { |s| s.register(observe_post_process_setting) } + end + it 'it sends `observe_post_process`' do + expect(observe_post_process_setting).to have_received(:observe_post_process) + end + end end context "transient settings" do