Improve log messages for data stream support checks (#1109)

This commit changed the data-stream log messages providing more context on configuration issues and incompatible settings.

The check_data_stream_config! method was refactored to reduce its cyclomatic complexity. It also changed the data_stream_default log levels from debug to info as those messages are helpful in understanding the auto-configuration mechanism and troubleshooting possibly incompatible values.

Prior to this commit, a few data stream logs were being printed twice due to a race condition on the data_stream_config? method.

Author: edmocosta

CHANGELOG.md

@@ -1,3 +1,8 @@
+## 11.12.3
+- Changed the log messages for data stream checks [#1109](https://github.com/logstash-plugins/logstash-output-elasticsearch/pull/1109)
+    - Added more details about incompatible data streams supplied configurations
+    - Changed the data stream auto-configuration log levels from `debug` to `info`
+
 ## 11.12.2
  - [Doc] Fixes the broken apache http client link [#1101](https://github.com/logstash-plugins/logstash-output-elasticsearch/pull/1101)
 

lib/logstash/outputs/elasticsearch.rb

@@ -300,6 +300,10 @@ def register
     # to build_client down to the Pool class.
     @client = build_client(LicenseChecker.new(@logger))
 
+    # Avoids race conditions in the @data_stream_config initialization (invoking check_data_stream_config! twice).
+    # It's being concurrently invoked by this register method and by the finish_register on the @after_successful_connection_thread
+    data_stream_enabled = data_stream_config?
+
     @after_successful_connection_thread = after_successful_connection do
       begin
         finish_register
@@ -324,7 +328,7 @@ def register
       raise LogStash::ConfigurationError, "DLQ feature (dlq_custom_codes) is configured while DLQ is not enabled" unless dlq_custom_codes.empty?
     end
 
-    if data_stream_config?
+    if data_stream_enabled
       @event_mapper = -> (e) { data_stream_event_action_tuple(e) }
       @event_target = -> (e) { data_stream_name(e) }
       @index = "#{data_stream_type}-#{data_stream_dataset}-#{data_stream_namespace}".freeze # default name

lib/logstash/outputs/elasticsearch/data_stream_support.rb

@@ -28,7 +28,6 @@ def self.included(base)
       base.extend(Validator)
     end
 
-    # @note assumes to be running AFTER {after_successful_connection} completed, due ES version checks
     def data_stream_config?
       @data_stream_config.nil? ? @data_stream_config = check_data_stream_config! : @data_stream_config
     end
@@ -45,53 +44,64 @@ def data_stream_name(event)
       "#{type}-#{dataset}-#{namespace}"
     end
 
-    DATA_STREAMS_REQUIRES_ECS_LS_VERSION = '8.0.0'
+    DATA_STREAMS_AND_ECS_ENABLED_BY_DEFAULT_LS_VERSION = '8.0.0'
 
     # @param params the user configuration for the ES output
     # @note LS initialized configuration (with filled defaults) won't detect as data-stream
     # compatible, only explicit (`original_params`) config should be tested.
-    # @return [TrueClass|FalseClass] whether given configuration is data-stream compatible
+    # @return [Boolean] whether given configuration is data-stream compatible
     def check_data_stream_config!(params = original_params)
-      data_stream_params = params.select { |name, _| name.start_with?('data_stream_') } # exclude data_stream =>
-      invalid_data_stream_params = invalid_data_stream_params(params)
-
       case data_stream_explicit_value
       when false
-        if data_stream_params.any?
-          @logger.error "Ambiguous configuration; data stream settings must not be present when data streams is disabled (caused by: `data_stream => false`)", data_stream_params
-          raise LogStash::ConfigurationError, "Ambiguous configuration, please remove data stream specific settings: #{data_stream_params.keys}"
-        end
+        check_disabled_data_stream_config!(params)
         return false
       when true
-        if invalid_data_stream_params.any?
-          @logger.error "Invalid data stream configuration, following parameters are not supported:", invalid_data_stream_params
-          raise LogStash::ConfigurationError, "Invalid data stream configuration: #{invalid_data_stream_params.keys}"
-        end
-        if ecs_compatibility == :disabled
-          if ::Gem::Version.create(LOGSTASH_VERSION) < ::Gem::Version.create(DATA_STREAMS_REQUIRES_ECS_LS_VERSION)
-            @deprecation_logger.deprecated "In a future release of Logstash, the Elasticsearch output plugin's `data_stream => true` will require the plugin to be run in ECS compatibility mode. " + ENABLING_ECS_GUIDANCE
-          else
-            @logger.error "Invalid data stream configuration; `ecs_compatibility` must not be `disabled`. " + ENABLING_ECS_GUIDANCE
-            raise LogStash::ConfigurationError, "Invalid data stream configuration: `ecs_compatibility => disabled`"
-          end
-        end
+        check_enabled_data_stream_config!(params)
         return true
-      else
-        use_data_stream = data_stream_default(data_stream_params, invalid_data_stream_params)
-        if use_data_stream
-          @logger.info("Config is compliant with data streams. `data_stream => auto` resolved to `true`")
-        elsif data_stream_params.any?
-          # DS (auto) disabled but there's still some data-stream parameters (and no `data_stream => false`)
-          @logger.error "Ambiguous configuration; data stream settings are present, but data streams are not enabled", data_stream_params
-          raise LogStash::ConfigurationError, "Ambiguous configuration, please set data_stream => true " +
-              "or remove data stream specific settings: #{data_stream_params.keys}"
-        else
-          @logger.info("Config is not compliant with data streams. `data_stream => auto` resolved to `false`")
+      else # data_stream => auto or not set
+        use_data_stream = data_stream_default(params)
+
+        check_disabled_data_stream_config!(params) unless use_data_stream
+
+        @logger.info("Data streams auto configuration (`data_stream => auto` or unset) resolved to `#{use_data_stream}`")
+        return use_data_stream
+      end
+    end
+
+    def check_enabled_data_stream_config!(params)
+      invalid_data_stream_params = invalid_data_stream_params(params)
+
+      if invalid_data_stream_params.any?
+        @logger.error "Invalid data stream configuration, the following parameters are not supported:", invalid_data_stream_params
+        raise LogStash::ConfigurationError, "Invalid data stream configuration: #{invalid_data_stream_params.keys}"
+      end
+
+      if ecs_compatibility == :disabled
+        if ecs_compatibility_required?
+          @logger.error "Invalid data stream configuration; `ecs_compatibility` must not be `disabled`. " + ENABLING_ECS_GUIDANCE
+          raise LogStash::ConfigurationError, "Invalid data stream configuration: `ecs_compatibility => disabled`"
         end
-        use_data_stream
+
+        @deprecation_logger.deprecated "In a future release of Logstash, the Elasticsearch output plugin's `data_stream => true` will require the plugin to be run in ECS compatibility mode. " + ENABLING_ECS_GUIDANCE
       end
     end
 
+    def check_disabled_data_stream_config!(params)
+      data_stream_params = data_stream_params(params)
+
+      if data_stream_params.any?
+        @logger.error "Ambiguous configuration; data stream settings must not be present when data streams are disabled (caused by `data_stream => false`, `data_stream => auto` or unset resolved to false). " \
+                      "You can either manually set `data_stream => true` or remove the following specific data stream settings: ", data_stream_params
+
+        raise LogStash::ConfigurationError,
+              "Ambiguous configuration; data stream settings must not be present when data streams are disabled: #{data_stream_params.keys}"
+      end
+    end
+
+    def data_stream_params(params)
+      params.select { |name, _| name.start_with?('data_stream_') }
+    end
+
     def data_stream_explicit_value
       case @data_stream
       when 'true'
@@ -131,6 +141,7 @@ def inherited_internal_config_param?(name)
 
     DATA_STREAMS_ORIGIN_ES_VERSION = '7.9.0'
 
+    # @note assumes to be running AFTER {after_successful_connection} completed, due ES version checks
     # @return [Gem::Version] if ES supports DS nil (or raise) otherwise
     def assert_es_version_supports_data_streams
       fail 'no last_es_version' unless last_es_version # assert - should not happen
@@ -144,38 +155,41 @@ def assert_es_version_supports_data_streams
       es_version # return truthy
     end
 
-    DATA_STREAMS_ENABLED_BY_DEFAULT_LS_VERSION = '8.0.0'
-
     # when data_stream => is either 'auto' or not set
-    # @param data_stream_params [#any?]
-    # @param invalid_data_stream_config [#any?#inspect]
-    def data_stream_default(data_stream_params, invalid_data_stream_config)
+    def data_stream_default(params)
       if ecs_compatibility == :disabled
-        @logger.debug("Not eligible for data streams because ecs_compatibility is not enabled. " + ENABLING_ECS_GUIDANCE)
+        @logger.info("Not eligible for data streams because ecs_compatibility is not enabled. " + ENABLING_ECS_GUIDANCE)
         return false
       end
 
-      ds_default = ::Gem::Version.create(LOGSTASH_VERSION) >= ::Gem::Version.create(DATA_STREAMS_ENABLED_BY_DEFAULT_LS_VERSION)
+      invalid_data_stream_params = invalid_data_stream_params(params)
 
-      if ds_default # LS 8.0
-        if invalid_data_stream_config.any?
-          @logger.debug("Not eligible for data streams because config contains one or more settings that are not compatible with data streams: #{invalid_data_stream_config.inspect}")
+      if data_stream_and_ecs_enabled_by_default?
+        if invalid_data_stream_params.any?
+          @logger.info("Not eligible for data streams because config contains one or more settings that are not compatible with data streams: #{invalid_data_stream_params.inspect}")
           return false
         end
 
-        @logger.debug 'Configuration is data stream compliant'
         return true
       end
 
       # LS 7.x
-      if !invalid_data_stream_config.any? && !data_stream_params.any?
+      if !invalid_data_stream_params.any? && !data_stream_params(params).any?
         @logger.warn "Configuration is data stream compliant but due backwards compatibility Logstash 7.x will not assume " +
                      "writing to a data-stream, default behavior will change on Logstash 8.0 " +
                      "(set `data_stream => true/false` to disable this warning)"
       end
       false
     end
 
+    def ecs_compatibility_required?
+      data_stream_and_ecs_enabled_by_default?
+    end
+
+    def data_stream_and_ecs_enabled_by_default?
+      ::Gem::Version.create(LOGSTASH_VERSION) >= ::Gem::Version.create(DATA_STREAMS_AND_ECS_ENABLED_BY_DEFAULT_LS_VERSION)
+    end
+
     # an {event_action_tuple} replacement when a data-stream configuration is detected
     def data_stream_event_action_tuple(event)
       event_data = event.to_hash

logstash-output-elasticsearch.gemspec

@@ -1,6 +1,6 @@
 Gem::Specification.new do |s|
   s.name            = 'logstash-output-elasticsearch'
-  s.version         = '11.12.2'
+  s.version         = '11.12.3'
   s.licenses        = ['apache-2.0']
   s.summary         = "Stores logs in Elasticsearch"
   s.description     = "This gem is a Logstash plugin required to be installed on top of the Logstash core pipeline using $LS_HOME/bin/logstash-plugin install gemname. This gem is not a stand-alone program"

spec/unit/outputs/elasticsearch/data_stream_support_spec.rb

@@ -84,7 +84,8 @@
         context "on LS #{ls_version_desc}" do
           around(:each) { |example| change_constant(:LOGSTASH_VERSION, ls_version, &example) }
           it "does not use data-streams" do
-            expect( subject.logger ).to receive(:debug).with(a_string_including "ecs_compatibility is not enabled")
+            expect( subject.logger ).to receive(:info).with(a_string_including "ecs_compatibility is not enabled")
+            expect( subject.logger ).to receive(:info).with(a_string_including "Data streams auto configuration (`data_stream => auto` or unset) resolved to `false`")
             expect( subject.data_stream_config? ).to be false
           end
         end
@@ -161,7 +162,7 @@
 
     it "does not default to data-streams" do
       expect( subject.logger ).to receive(:error) do |msg|
-        expect(msg).to include "Ambiguous configuration; data stream settings are present, but data streams are not enabled"
+        expect(msg).to include "Ambiguous configuration; data stream settings must not be present when data streams are disabled"
       end
       change_constant :LOGSTASH_VERSION, '7.10.2' do
         expect { subject.data_stream_config? }.to raise_error(LogStash::ConfigurationError, /Ambiguous configuration/i)
@@ -173,8 +174,8 @@
       let(:options) { super().merge('data_stream' => 'false') }
 
       it "raises a configuration error (due ds specific settings)" do
-        expect( subject.logger ).to receive(:error).with(/Ambiguous configuration; data stream settings must not be present when data streams is disabled/,
-                                                         {"data_stream_auto_routing"=>"false", "data_stream_dataset"=>"test"})
+        expect( subject.logger ).to receive(:error).with(/Ambiguous configuration; data stream settings must not be present when data streams are disabled/,
+                                                          {"data_stream_auto_routing"=>"false", "data_stream_dataset"=>"test"})
         change_constant :LOGSTASH_VERSION, '7.10.2' do
           expect { subject.data_stream_config? }.to raise_error(LogStash::ConfigurationError, /Ambiguous configuration/i)
         end