Skip to content

Commit

Permalink
settings: add support for observing settings after post-process hooks (
Browse files Browse the repository at this point in the history
…#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: #16332
(cherry picked from commit c633ad2)

Co-authored-by: Ry Biesemeyer <[email protected]>
  • Loading branch information
github-actions[bot] and yaauie authored Jul 24, 2024
1 parent b6bf12d commit 46b996c
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 3 deletions.
23 changes: 20 additions & 3 deletions logstash-core/lib/logstash/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand All @@ -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

##
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions logstash-core/spec/logstash/settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 46b996c

Please sign in to comment.