From a61c1e31daa13ac6a9c37d1e3a09d5246ec849f3 Mon Sep 17 00:00:00 2001 From: Jingguo Yao Date: Tue, 21 Dec 2021 16:55:33 +0800 Subject: [PATCH 1/4] Make out_file append option work without compression For Ruby 2.7.x and 3.0.x, out_file append option without compression does not work. The cause is the following Ruby language bug: https://bugs.ruby-lang.org/issues/18388. This commit is a workaround for this problem. See #3569. Signed-off-by: Jingguo Yao --- lib/fluent/plugin/out_file.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/fluent/plugin/out_file.rb b/lib/fluent/plugin/out_file.rb index fe7dca4797..ae983c936a 100644 --- a/lib/fluent/plugin/out_file.rb +++ b/lib/fluent/plugin/out_file.rb @@ -223,7 +223,12 @@ def write(chunk) def write_without_compression(path, chunk) File.open(path, "ab", @file_perm) do |f| - chunk.write_to(f) + if @append + content = chunk.read() + f.puts content + else + chunk.write_to(f) + end end end From 94df99ffc1bc8928e59bea7539b4b42482422a85 Mon Sep 17 00:00:00 2001 From: Takuro Ashie Date: Mon, 27 Dec 2021 22:47:16 +0900 Subject: [PATCH 2/4] Apply workaround for #3569 only on macOS Signed-off-by: Takuro Ashie --- lib/fluent/env.rb | 4 ++++ lib/fluent/plugin/out_file.rb | 9 ++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/fluent/env.rb b/lib/fluent/env.rb index 5ddf505bc2..e25711e77a 100644 --- a/lib/fluent/env.rb +++ b/lib/fluent/env.rb @@ -33,4 +33,8 @@ def self.windows? def self.linux? /linux/ === RUBY_PLATFORM end + + def self.macos? + /darwin/ =~ RUBY_PLATFORM + end end diff --git a/lib/fluent/plugin/out_file.rb b/lib/fluent/plugin/out_file.rb index ae983c936a..f09c149eb1 100644 --- a/lib/fluent/plugin/out_file.rb +++ b/lib/fluent/plugin/out_file.rb @@ -181,6 +181,13 @@ def configure(conf) @dir_perm = system_config.dir_permission || Fluent::DEFAULT_DIR_PERMISSION @file_perm = system_config.file_permission || Fluent::DEFAULT_FILE_PERMISSION @need_lock = system_config.workers > 1 + + # https://github.com/fluent/fluentd/issues/3569 + @need_ruby_on_macos_workaround = false + if @append && Fluent.macos? + condition = Gem::Dependency.new('', [">= 2.7.0", "< 3.1.0"]) + @need_ruby_on_macos_workaround = true if condition.match?('', RUBY_VERSION) + end end def multi_workers_ready? @@ -223,7 +230,7 @@ def write(chunk) def write_without_compression(path, chunk) File.open(path, "ab", @file_perm) do |f| - if @append + if @need_ruby_on_macos_workaround content = chunk.read() f.puts content else From a31d17f782785ff7ed2c6a0d04f53210f4507a91 Mon Sep 17 00:00:00 2001 From: Jingguo Yao Date: Sat, 4 Dec 2021 22:53:35 +0800 Subject: [PATCH 3/4] Add test case for append option without compression See #3569. Signed-off-by: Jingguo Yao --- test/plugin/test_out_file.rb | 80 ++++++++++++++++++++++-------------- 1 file changed, 50 insertions(+), 30 deletions(-) diff --git a/test/plugin/test_out_file.rb b/test/plugin/test_out_file.rb index ac897c41a3..aa23eb346e 100644 --- a/test/plugin/test_out_file.rb +++ b/test/plugin/test_out_file.rb @@ -376,6 +376,47 @@ def create_driver(conf = CONFIG, opts = {}) end end + def check_events_append(compression) + time = event_time("2011-01-02 13:14:15 UTC") + formatted_lines = %[2011-01-02T13:14:15Z\ttest\t{"a":1}#{@default_newline}] + %[2011-01-02T13:14:15Z\ttest\t{"a":2}#{@default_newline}] + + write_once = ->(){ + config = %[ + path #{TMP_DIR}/out_file_test + utc + append true + + timekey_use_utc true + + ] + if compression + config << " compress gz" + end + d = create_driver(config) + d.run(default_tag: 'test'){ + d.feed(time, {"a"=>1}) + d.feed(time, {"a"=>2}) + } + d.instance.last_written_path + } + + log_file_name = "out_file_test.20110102.log" + if compression + log_file_name << ".gz" + end + + 1.upto(3) do |i| + path = write_once.call + assert_equal "#{TMP_DIR}/#{log_file_name}", path + expect = formatted_lines * i + if compression + check_gzipped_result(path, expect) + else + check_result(path, expect) + end + end + end + def check_gzipped_result(path, expect) # Zlib::GzipReader has a bug of concatenated file: https://bugs.ruby-lang.org/issues/9790 # Following code from https://www.ruby-forum.com/topic/971591#979520 @@ -394,6 +435,11 @@ def check_gzipped_result(path, expect) assert_equal expect, result end + def check_result(path, expect) + result = File.read(path, mode: "rb") + assert_equal expect, result + end + sub_test_case 'write' do test 'basic case' do d = create_driver @@ -536,37 +582,11 @@ def parse_system(text) end test 'append' do - time = event_time("2011-01-02 13:14:15 UTC") - formatted_lines = %[2011-01-02T13:14:15Z\ttest\t{"a":1}#{@default_newline}] + %[2011-01-02T13:14:15Z\ttest\t{"a":2}#{@default_newline}] - - write_once = ->(){ - d = create_driver %[ - path #{TMP_DIR}/out_file_test - compress gz - utc - append true - - timekey_use_utc true - - ] - d.run(default_tag: 'test'){ - d.feed(time, {"a"=>1}) - d.feed(time, {"a"=>2}) - } - d.instance.last_written_path - } - - path = write_once.call - assert_equal "#{TMP_DIR}/out_file_test.20110102.log.gz", path - check_gzipped_result(path, formatted_lines) - - path = write_once.call - assert_equal "#{TMP_DIR}/out_file_test.20110102.log.gz", path - check_gzipped_result(path, formatted_lines * 2) + check_events_append(true) + end - path = write_once.call - assert_equal "#{TMP_DIR}/out_file_test.20110102.log.gz", path - check_gzipped_result(path, formatted_lines * 3) + test 'append without compression' do + check_events_append(false) end test 'append when JST' do From f17cb9e724e4fd9175b6cf089a9f6aab7ee87040 Mon Sep 17 00:00:00 2001 From: Takuro Ashie Date: Mon, 27 Dec 2021 22:20:25 +0900 Subject: [PATCH 4/4] test_out_file: Don't extract the content of "test append" as a method Use data driven test instead to make easy to read. Signed-off-by: Takuro Ashie --- test/plugin/test_out_file.rb | 88 +++++++++++++++++------------------- 1 file changed, 42 insertions(+), 46 deletions(-) diff --git a/test/plugin/test_out_file.rb b/test/plugin/test_out_file.rb index aa23eb346e..be593ab7b1 100644 --- a/test/plugin/test_out_file.rb +++ b/test/plugin/test_out_file.rb @@ -376,47 +376,6 @@ def create_driver(conf = CONFIG, opts = {}) end end - def check_events_append(compression) - time = event_time("2011-01-02 13:14:15 UTC") - formatted_lines = %[2011-01-02T13:14:15Z\ttest\t{"a":1}#{@default_newline}] + %[2011-01-02T13:14:15Z\ttest\t{"a":2}#{@default_newline}] - - write_once = ->(){ - config = %[ - path #{TMP_DIR}/out_file_test - utc - append true - - timekey_use_utc true - - ] - if compression - config << " compress gz" - end - d = create_driver(config) - d.run(default_tag: 'test'){ - d.feed(time, {"a"=>1}) - d.feed(time, {"a"=>2}) - } - d.instance.last_written_path - } - - log_file_name = "out_file_test.20110102.log" - if compression - log_file_name << ".gz" - end - - 1.upto(3) do |i| - path = write_once.call - assert_equal "#{TMP_DIR}/#{log_file_name}", path - expect = formatted_lines * i - if compression - check_gzipped_result(path, expect) - else - check_result(path, expect) - end - end - end - def check_gzipped_result(path, expect) # Zlib::GzipReader has a bug of concatenated file: https://bugs.ruby-lang.org/issues/9790 # Following code from https://www.ruby-forum.com/topic/971591#979520 @@ -581,12 +540,49 @@ def parse_system(text) assert_equal 3, Dir.glob("#{TMP_DIR}/out_file_test.*").size end - test 'append' do - check_events_append(true) - end + data( + "with compression" => true, + "without compression" => false, + ) + test 'append' do |compression| + time = event_time("2011-01-02 13:14:15 UTC") + formatted_lines = %[2011-01-02T13:14:15Z\ttest\t{"a":1}#{@default_newline}] + %[2011-01-02T13:14:15Z\ttest\t{"a":2}#{@default_newline}] + + write_once = ->(){ + config = %[ + path #{TMP_DIR}/out_file_test + utc + append true + + timekey_use_utc true + + ] + if compression + config << " compress gz" + end + d = create_driver(config) + d.run(default_tag: 'test'){ + d.feed(time, {"a"=>1}) + d.feed(time, {"a"=>2}) + } + d.instance.last_written_path + } - test 'append without compression' do - check_events_append(false) + log_file_name = "out_file_test.20110102.log" + if compression + log_file_name << ".gz" + end + + 1.upto(3) do |i| + path = write_once.call + assert_equal "#{TMP_DIR}/#{log_file_name}", path + expect = formatted_lines * i + if compression + check_gzipped_result(path, expect) + else + check_result(path, expect) + end + end end test 'append when JST' do