Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows: Fix a bug that the wrong log file is reopened with log rotate setting when flushing or graceful reloading #4054

Merged
merged 9 commits into from
Feb 16, 2023
+97 −27
Merged

Windows: Fix a bug that the wrong log file is reopened with log rotate setting when flushing or graceful reloading #4054

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7b285a0
Windows: Apply the modified log path for rotation to reopen
daipom Feb 13, 2023
560c1bb
Test: Make sure to close LogDevice
daipom Feb 14, 2023
abbcc64
Test: Reset tmp files at setup
daipom Feb 14, 2023
1305157
Test: Make sure LoggerInitializerTest doesn't change the global logger
daipom Feb 14, 2023
83041ee
Test: Add tests for rotate and reopening
daipom Feb 15, 2023
6107d00
Test: Refactor LoggerInitializerTest
daipom Feb 15, 2023
0d9581c
Test: Remove needless code
daipom Feb 15, 2023
ade4857
Test: Make sure not to leave tmp files
daipom Feb 15, 2023
60c94ad
Test: Refactor logic of recovering global logger
daipom Feb 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions lib/fluent/supervisor.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,12 +563,8 @@ def init(process_type, worker_id)
if @log_rotate_age || @log_rotate_size
# We need to prepare a unique path for each worker since
# Windows locks files.
if Fluent.windows?
path = LoggerInitializer.per_process_path(@path, process_type, worker_id)
else
path = @path
end
@logdev = Fluent::LogDeviceIO.new(path, shift_age: @log_rotate_age, shift_size: @log_rotate_size)
@path = LoggerInitializer.per_process_path(@path, process_type, worker_id) if Fluent.windows?
ashie marked this conversation as resolved.
Show resolved Hide resolved
@logdev = Fluent::LogDeviceIO.new(@path, shift_age: @log_rotate_age, shift_size: @log_rotate_size)
else
@logdev = File.open(@path, "a")
end
Expand All @@ -591,7 +587,7 @@ def init(process_type, worker_id)
$log = Fluent::Log.new(logger, @opts)
$log.enable_color(false) if @path
$log.enable_debug if @level <= Fluent::Log::LEVEL_DEBUG
$log.info "init #{process_type} logger", path: path, rotate_age: @log_rotate_age, rotate_size: @log_rotate_size
$log.info "init #{process_type} logger", path: @path, rotate_age: @log_rotate_age, rotate_size: @log_rotate_size
ashie marked this conversation as resolved.
Show resolved Hide resolved
end

def stdout?
Expand Down
11 changes: 5 additions & 6 deletions test/test_logger_initializer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,8 @@
class LoggerInitializerTest < ::Test::Unit::TestCase
TMP_DIR = File.expand_path(File.dirname(__FILE__) + "/tmp/logger_initializer#{ENV['TEST_ENV_NUMBER']}")

teardown do
begin
FileUtils.rm_rf(TMP_DIR)
rescue => _
end
setup do
FileUtils.rm_rf(TMP_DIR) rescue nil
ashie marked this conversation as resolved.
Show resolved Hide resolved
end

test 'when path is given' do
Expand All @@ -22,6 +19,7 @@ class LoggerInitializerTest < ::Test::Unit::TestCase
assert_nothing_raised do
logger.init(:supervisor, 0)
end
$log.out.close

assert_true File.exist?(TMP_DIR)
end
Expand All @@ -38,8 +36,9 @@ class LoggerInitializerTest < ::Test::Unit::TestCase
assert_nothing_raised do
logger.init(:supervisor, 0)
end

logger.apply_options(log_dir_perm: 0o777)
$log.out.close

assert_true File.exist?(TMP_DIR)
assert_equal 0o777, (File.stat(TMP_DIR).mode & 0xFFF)
end
Expand Down