From bd3980e6c4300e99d82a2717b582fc2afa5366a5 Mon Sep 17 00:00:00 2001 From: kares Date: Tue, 25 May 2021 13:05:04 +0200 Subject: [PATCH 01/10] Feat: introduce a target check helper for re-use across plugins - unifying the message --- .../ecs_compatibility_support/target_check.rb | 46 ++++++++++++++ .../target_check_spec.rb | 62 +++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb create mode 100644 spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb diff --git a/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb b/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb new file mode 100644 index 0000000..bd7a10a --- /dev/null +++ b/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb @@ -0,0 +1,46 @@ +# encoding: utf-8 + +require_relative '../ecs_compatibility_support' + +module LogStash + module PluginMixins + module ECSCompatibilitySupport + # A target option check that can be included into any `LogStash::Plugin`. + # + # @see ECSCompatibilitySupport() + module TargetCheck + ## + # @api internal (use: `LogStash::Plugin::include`) + # @param base [Class]: a class that inherits `LogStash::Plugin`, typically one + # descending from one of the four plugin base classes + # (e.g., `LogStash::Inputs::Base`) + # @return [void] + def self.included(base) + fail(ArgumentError, "`#{base}` must inherit LogStash::Plugin") unless base < LogStash::Plugin + fail(ArgumentError, "`#{base}` must include #{ECSCompatibilitySupport}") unless base.method_defined?(:ecs_compatibility) + end + + TARGET_NOT_SET_MESSAGE = ("ECS compatibility is enabled but no `target` option was specified, " + + "it is recommended to set the option to avoid potential schema conflicts (if your data is ECS compliant " + + "or non-conflicting feel free to ignore this message)").freeze + + private + + ## + # Logs an info message when ecs_compatibility is enabled but plugin has no `target` configuration specified. + # @note This method assumes a common plugin convention of using the target option. + # @return [nil] if ECS compatibility is disabled or no target option exists + # @return [true, false] + def check_target_set_in_ecs_mode + return if ecs_compatibility == :disabled || !respond_to?(:target) + if target.nil? + logger.info(TARGET_NOT_SET_MESSAGE) + return false + end + true + end + + end + end + end +end diff --git a/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb b/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb new file mode 100644 index 0000000..76aecf1 --- /dev/null +++ b/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb @@ -0,0 +1,62 @@ +# encoding: utf-8 + +require "logstash-core" + +require 'logstash/inputs/base' +require 'logstash/filters/base' +require 'logstash/codecs/base' +require 'logstash/outputs/base' + +require "logstash/plugin_mixins/ecs_compatibility_support/target_check" + +describe LogStash::PluginMixins::ECSCompatibilitySupport::TargetCheck do + + describe "check_target_set_in_ecs_mode" do + + context 'with a plugin' do + + subject(:plugin_class) do + Class.new(LogStash::Filters::Base) do + include LogStash::PluginMixins::ECSCompatibilitySupport + include LogStash::PluginMixins::ECSCompatibilitySupport::TargetCheck + + config :target, :validate => :string + + def register + check_target_set_in_ecs_mode + end + end + end + + it 'skips check when ECS disabled' do + plugin = plugin_class.new('ecs_compatibility' => 'disabled') + expect( plugin.register ).to be nil + end + + it 'warns when target is not set in ECS mode' do + plugin = plugin_class.new('ecs_compatibility' => 'v1') + allow( plugin.logger ).to receive(:info) + expect( plugin.register ).to be false + expect( plugin.logger ).to have_received(:info).with(/ECS compatibility is enabled but no `target` option was specified/) + end + + it 'does not warn when target is set' do + plugin = plugin_class.new('ecs_compatibility' => 'v1', 'target' => 'foo') + allow( plugin.logger ).to receive(:info) + expect( plugin.register ).to be true + expect( plugin.logger ).to_not have_received(:info) + end + + end + + it 'skips check when no target config' do + plugin_class = Class.new(LogStash::Filters::Base) do + include LogStash::PluginMixins::ECSCompatibilitySupport + include LogStash::PluginMixins::ECSCompatibilitySupport::TargetCheck + end + expect( plugin_class.new({}).send(:check_target_set_in_ecs_mode) ).to be nil + end + + end + +end \ No newline at end of file From d70a0e0d2765653a9b8f139a5069c0b5741e7b3a Mon Sep 17 00:00:00 2001 From: kares Date: Tue, 25 May 2021 13:34:46 +0200 Subject: [PATCH 02/10] bump version + changelog --- CHANGELOG.md | 3 +++ logstash-mixin-ecs_compatibility_support.gemspec | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 679a410..48624dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +# 1.3.0 + - Feat: introduce a target check helper [#6](https://github.com/logstash-plugins/logstash-mixin-ecs_compatibility_support/pull/6) + # 1.2.0 - Added support for resolution aliases, allowing a plugin that uses `ecs_select` to support multiple ECS versions with a single declaration. diff --git a/logstash-mixin-ecs_compatibility_support.gemspec b/logstash-mixin-ecs_compatibility_support.gemspec index eda4378..a89a640 100644 --- a/logstash-mixin-ecs_compatibility_support.gemspec +++ b/logstash-mixin-ecs_compatibility_support.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |s| s.name = 'logstash-mixin-ecs_compatibility_support' - s.version = '1.2.0' + s.version = '1.3.0' s.licenses = %w(Apache-2.0) s.summary = "Support for the ECS-Compatibility mode introduced in Logstash 7.x, for plugins wishing to use this API on older Logstashes" s.description = "This gem is meant to be a dependency of any Logstash plugin that wishes to use the ECS-Compatibility mode introduced in Logstash 7.x while maintaining backward-compatibility with earlier Logstash releases. When used on older Logstash versions, this adapter provides an implementation of ECS-Compatibility mode that can be controlled at the plugin instance level." From 14b524206d94a92df48f9e891a72af8152e535f3 Mon Sep 17 00:00:00 2001 From: kares Date: Wed, 26 May 2021 08:13:24 +0200 Subject: [PATCH 03/10] Refactor: use (another) module to decorate register --- .../ecs_compatibility_support/target_check.rb | 31 ++++++++++++------- .../target_check_spec.rb | 21 ++++++++----- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb b/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb index bd7a10a..bbd35c9 100644 --- a/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb +++ b/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb @@ -17,7 +17,8 @@ module TargetCheck # @return [void] def self.included(base) fail(ArgumentError, "`#{base}` must inherit LogStash::Plugin") unless base < LogStash::Plugin - fail(ArgumentError, "`#{base}` must include #{ECSCompatibilitySupport}") unless base.method_defined?(:ecs_compatibility) + fail(ArgumentError, "`#{base}` must include #{ECSCompatibilitySupport}") unless base < ECSCompatibilitySupport + base.prepend(RegisterDecorator) end TARGET_NOT_SET_MESSAGE = ("ECS compatibility is enabled but no `target` option was specified, " + @@ -27,19 +28,27 @@ def self.included(base) private ## - # Logs an info message when ecs_compatibility is enabled but plugin has no `target` configuration specified. - # @note This method assumes a common plugin convention of using the target option. - # @return [nil] if ECS compatibility is disabled or no target option exists + # Check whether `target` is specified. + # + # @return [nil] if ECS compatibility is disabled # @return [true, false] - def check_target_set_in_ecs_mode - return if ecs_compatibility == :disabled || !respond_to?(:target) - if target.nil? - logger.info(TARGET_NOT_SET_MESSAGE) - return false - end - true + def target_set? + return if ecs_compatibility == :disabled + ! target.nil? end + module RegisterDecorator + + ## + # Logs an info message when ecs_compatibility is enabled but plugin has no `target` configuration specified. + # @override + def register + super.tap do + logger.info(TARGET_NOT_SET_MESSAGE) if target_set? == false + end + end + + end end end end diff --git a/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb b/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb index 76aecf1..96fb3ad 100644 --- a/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb +++ b/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb @@ -22,39 +22,44 @@ config :target, :validate => :string - def register - check_target_set_in_ecs_mode - end + def register; 42 end + end end it 'skips check when ECS disabled' do plugin = plugin_class.new('ecs_compatibility' => 'disabled') - expect( plugin.register ).to be nil + allow( plugin.logger ).to receive(:info) + expect( plugin.register ).to eql 42 + expect( plugin.logger ).to_not have_received(:info) end it 'warns when target is not set in ECS mode' do plugin = plugin_class.new('ecs_compatibility' => 'v1') allow( plugin.logger ).to receive(:info) - expect( plugin.register ).to be false + expect( plugin.register ).to eql 42 expect( plugin.logger ).to have_received(:info).with(/ECS compatibility is enabled but no `target` option was specified/) end it 'does not warn when target is set' do plugin = plugin_class.new('ecs_compatibility' => 'v1', 'target' => 'foo') allow( plugin.logger ).to receive(:info) - expect( plugin.register ).to be true + expect( plugin.register ).to eql 42 expect( plugin.logger ).to_not have_received(:info) end end - it 'skips check when no target config' do + it 'fails check when no target config' do plugin_class = Class.new(LogStash::Filters::Base) do include LogStash::PluginMixins::ECSCompatibilitySupport include LogStash::PluginMixins::ECSCompatibilitySupport::TargetCheck + + def register; end + end - expect( plugin_class.new({}).send(:check_target_set_in_ecs_mode) ).to be nil + plugin = plugin_class.new('ecs_compatibility' => 'v1') + expect { plugin.register }.to raise_error NameError end end From 705c7d8669c2677ed9443acfd8177fb6e55e8f52 Mon Sep 17 00:00:00 2001 From: Karol Bucek Date: Mon, 31 May 2021 11:15:57 +0200 Subject: [PATCH 04/10] Update spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb Co-authored-by: Ry Biesemeyer --- .../ecs_compatibility_support/target_check_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb b/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb index 96fb3ad..497bca4 100644 --- a/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb +++ b/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb @@ -59,9 +59,9 @@ def register; end end plugin = plugin_class.new('ecs_compatibility' => 'v1') - expect { plugin.register }.to raise_error NameError + expect { plugin.register }.to raise_error NameError, /\btarget\b/ end end -end \ No newline at end of file +end From 96b496a6f7d884cd34341f898bbb93241fee373b Mon Sep 17 00:00:00 2001 From: kares Date: Mon, 31 May 2021 11:25:35 +0200 Subject: [PATCH 05/10] REVIEW: change target_set? to return true if set regardless of ECS compatibility setting --- .../ecs_compatibility_support/target_check.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb b/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb index bbd35c9..8606d54 100644 --- a/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb +++ b/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb @@ -30,11 +30,12 @@ def self.included(base) ## # Check whether `target` is specified. # - # @return [nil] if ECS compatibility is disabled + # @return [nil] if target is unspecified and ECS compatibility is disabled # @return [true, false] def target_set? - return if ecs_compatibility == :disabled - ! target.nil? + return true unless target.nil? + return nil if ecs_compatibility == :disabled + false # target isn't set end module RegisterDecorator From 2b03ee47bb554b063c2ca1a71faa9616a7f7f66e Mon Sep 17 00:00:00 2001 From: Karol Bucek Date: Mon, 7 Jun 2021 09:48:44 +0200 Subject: [PATCH 06/10] Update spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb Co-authored-by: Ry Biesemeyer --- .../ecs_compatibility_support/target_check_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb b/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb index 497bca4..3d5060f 100644 --- a/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb +++ b/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb @@ -45,7 +45,7 @@ def register; 42 end plugin = plugin_class.new('ecs_compatibility' => 'v1', 'target' => 'foo') allow( plugin.logger ).to receive(:info) expect( plugin.register ).to eql 42 - expect( plugin.logger ).to_not have_received(:info) + expect( plugin.logger ).to_not have_received(:info).with(a_string_including "`target` option") end end From 3794dab06c006eeac2100867e21d35798233a0d5 Mon Sep 17 00:00:00 2001 From: Karol Bucek Date: Mon, 7 Jun 2021 09:48:51 +0200 Subject: [PATCH 07/10] Update spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb Co-authored-by: Ry Biesemeyer --- .../ecs_compatibility_support/target_check_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb b/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb index 3d5060f..20128f4 100644 --- a/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb +++ b/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb @@ -31,7 +31,7 @@ def register; 42 end plugin = plugin_class.new('ecs_compatibility' => 'disabled') allow( plugin.logger ).to receive(:info) expect( plugin.register ).to eql 42 - expect( plugin.logger ).to_not have_received(:info) + expect( plugin.logger ).to_not have_received(:info).with(a_string_including "`target` option") end it 'warns when target is not set in ECS mode' do From 3c1f7782f57f442a9d922dfe3c4962c1b7673ef7 Mon Sep 17 00:00:00 2001 From: Karol Bucek Date: Mon, 7 Jun 2021 09:49:01 +0200 Subject: [PATCH 08/10] Update spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb Co-authored-by: Ry Biesemeyer --- .../ecs_compatibility_support/target_check_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb b/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb index 20128f4..85d2ba3 100644 --- a/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb +++ b/spec/logstash/plugin_mixins/ecs_compatibility_support/target_check_spec.rb @@ -38,7 +38,7 @@ def register; 42 end plugin = plugin_class.new('ecs_compatibility' => 'v1') allow( plugin.logger ).to receive(:info) expect( plugin.register ).to eql 42 - expect( plugin.logger ).to have_received(:info).with(/ECS compatibility is enabled but no `target` option was specified/) + expect( plugin.logger ).to have_received(:info).with(a_string_including "ECS compatibility is enabled but `target` option was not specified.") end it 'does not warn when target is set' do From 75a10835f9cb46d67706214fbac6fd2f43a30418 Mon Sep 17 00:00:00 2001 From: Karol Bucek Date: Mon, 7 Jun 2021 09:49:09 +0200 Subject: [PATCH 09/10] Update lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb Co-authored-by: Ry Biesemeyer --- .../plugin_mixins/ecs_compatibility_support/target_check.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb b/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb index 8606d54..fe93c86 100644 --- a/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb +++ b/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb @@ -50,6 +50,7 @@ def register end end + private_constant :RegisterDecorator end end end From 157d6537929532517571ea522ccb91abaf0ded13 Mon Sep 17 00:00:00 2001 From: Karol Bucek Date: Mon, 7 Jun 2021 09:49:31 +0200 Subject: [PATCH 10/10] Update lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb Co-authored-by: Ry Biesemeyer --- .../ecs_compatibility_support/target_check.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb b/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb index fe93c86..ea9c744 100644 --- a/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb +++ b/lib/logstash/plugin_mixins/ecs_compatibility_support/target_check.rb @@ -21,9 +21,10 @@ def self.included(base) base.prepend(RegisterDecorator) end - TARGET_NOT_SET_MESSAGE = ("ECS compatibility is enabled but no `target` option was specified, " + - "it is recommended to set the option to avoid potential schema conflicts (if your data is ECS compliant " + - "or non-conflicting feel free to ignore this message)").freeze + TARGET_NOT_SET_MESSAGE = ("ECS compatibility is enabled but `target` option was not specified. " + + "This may cause fields to be set at the top-level of the event where they are likely to clash with the Elastic Common Schema. " + + "It is recommended to set the `target` option to avoid potential schema conflicts (if your data is ECS compliant " + + "or non-conflicting, feel free to ignore this message)").freeze private