Merge pull request #2198 from vparfonov/log1555

LOG-1555: call strip() only in case the object has that method
openshift · Oct 30, 2021 · 74919e5 · 74919e5
2 parents 25fbe33 + c4cb30a
commit 74919e5
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 6 deletions.
diff --git a/fluentd/lib/filter_parse_json_field/filter_parse_json_field.gemspec b/fluentd/lib/filter_parse_json_field/filter_parse_json_field.gemspec
@@ -21,4 +21,5 @@ Gem::Specification.new do |gem|
   gem.add_development_dependency("rr", ["~> 1.0"])
   gem.add_development_dependency("test-unit", ["~> 3.2"])
   gem.add_development_dependency("test-unit-rr", ["~> 1.0"])
+  gem.add_development_dependency("serverengine", ["2.2.1"])
 end
diff --git a/fluentd/lib/filter_parse_json_field/lib/filter_parse_json_field.rb b/fluentd/lib/filter_parse_json_field/lib/filter_parse_json_field.rb
@@ -60,8 +60,10 @@ def filter_stream(tag, es)
     def do_merge_json_log(record)
       json_fields.each do |merge_json_log_key|
         if record.has_key?(merge_json_log_key)
-          value = (record[merge_json_log_key] || "").strip
-          if value.start_with?('{') && value.end_with?('}')
+          value = record[merge_json_log_key] || ''
+          value = value.strip if value.respond_to?(:strip)
+          if value.respond_to?(:start_with?) && value.respond_to?(:end_with?) &&
+            (value.start_with?('{') && value.end_with?('}'))
             begin
               record = JSON.parse(value).merge(record)
               unless @preserve_json_log
@@ -80,20 +82,21 @@ def do_merge_json_log(record)
     def do_replace_json_log(record)
       json_fields.each do |merge_json_log_key|
         if record.has_key?(merge_json_log_key)
-          value = (record[merge_json_log_key] || "").strip
-          if value.start_with?('{') && value.end_with?('}')
+          value = record[merge_json_log_key] || ''
+          value = value.strip if value.respond_to?(:strip)
+          if value.respond_to?(:start_with?) && value.respond_to?(:end_with?) &&
+             (value.start_with?('{') && value.end_with?('}'))
             begin
               parsed_value = JSON.parse(value)
               record[merge_json_log_key] = parsed_value
             rescue JSON::ParserError
               log.debug "parse_json_field could not parse field [#{merge_json_log_key}] as JSON: value [#{value}]"
             end
-          end
+            end
           break
         end
       end
       record
     end
-
   end
 end
diff --git a/fluentd/lib/filter_parse_json_field/test/filter_parse_json_field_test.rb b/fluentd/lib/filter_parse_json_field/test/filter_parse_json_field_test.rb
@@ -134,6 +134,33 @@ def emit_with_tag(tag, msg={}, conf='')
       ')
       assert_equal({'a'=>{'b'=>'c'}, 'd'=>['e', 'f'], 'g'=>97, 'h'=>{'i'=>'j'}}, rec['message'])
     end
+    test 'replace json field no raising NoMethodError for Number' do
+      json_string_val = 100
+      orig_a_value = 'orig a value'
+      assert_nothing_raised(NoMethodError) {
+        emit_with_tag('tag', {'message'=>json_string_val, 'a'=>orig_a_value},'
+          merge_json_log false
+          replace_json_log true
+          json_fields message
+        ')
+      }
+    end
+    test 'replace json field no raising NoMethodError for Hash' do
+      json_string_val = {
+        1 => ['a', 'b'],
+        2 => ['c'],
+        3 => ['d', 'e', 'f', 'g'],
+        4 => ['h']
+      }
+      orig_a_value = 'orig a value'
+      assert_nothing_raised(NoMethodError) {
+        emit_with_tag('tag', {'message'=>json_string_val, 'a'=>orig_a_value},'
+        merge_json_log false
+        replace_json_log true
+        json_fields message
+      ')
+      }
+    end
     test 'no fallback if parsing error in given field' do
       # test that - skip1 is skipped, skip2 is attempted to parse and fail
       # jsonfield is skipped - message is logged at debug level