From 8b615eac1418de5e581789a6f34566de8c26cd2d Mon Sep 17 00:00:00 2001
From: Fujimoto Seiji <fujimoto@clear-code.com>
Date: Thu, 20 Oct 2022 13:27:09 +0900
Subject: [PATCH] Fix "unable to rotate log files on Windows" bug

Suppose we set up the log rotation on Windows as follows:

    <system>
      <log>
        rotate_age 5
      </log>
    </sytem>

Currently this does not work, because the supervisor and worker will
share the same log path, and they won't coordinate with each other.

This makes it sure that each process has a log file for themselves
on Windows.

Signed-off-by: Fujimoto Seiji <fujimoto@clear-code.com>
---
 lib/fluent/command/fluentd.rb | 11 ----------
 lib/fluent/supervisor.rb      | 41 +++++++++++++++++++++++++----------
 test/test_supervisor.rb       |  8 +++++++
 3 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/lib/fluent/command/fluentd.rb b/lib/fluent/command/fluentd.rb
index 088af53b8b..f9e2cd544e 100644
--- a/lib/fluent/command/fluentd.rb
+++ b/lib/fluent/command/fluentd.rb
@@ -345,17 +345,6 @@
 exit 0 if early_exit
 
 if opts[:supervise]
-  if Fluent.windows?
-    if opts[:log_path] && opts[:log_path] != "-"
-      if opts[:log_rotate_age] || opts[:log_rotate_size]
-        require 'pathname'
-
-        log_path = Pathname(opts[:log_path]).sub_ext("-supervisor#{Pathname(opts[:log_path]).extname}").to_s
-        opts[:log_path] = log_path
-      end
-    end
-  end
-
   supervisor = Fluent::Supervisor.new(opts)
   supervisor.configure(supervisor: true)
   supervisor.run_supervisor(dry_run: opts[:dry_run])
diff --git a/lib/fluent/supervisor.rb b/lib/fluent/supervisor.rb
index b702314702..b372f92249 100644
--- a/lib/fluent/supervisor.rb
+++ b/lib/fluent/supervisor.rb
@@ -525,10 +525,22 @@ def initialize(path, level, chuser, chgroup, opts, log_rotate_age: nil, log_rota
         @log_rotate_size = log_rotate_size
       end
 
-      def worker_id_suffixed_path(worker_id, path)
-        require 'pathname'
-
-        Pathname(path).sub_ext("-#{worker_id}#{Pathname(path).extname}").to_s
+      # Create a unique path for each process.
+      #
+      # >>> per_process_path(:worker, 1, "C:/tmp/test.log")
+      # C:/tmp/test-1.log
+      # >>> per_process_path(:supervisor, 0, "C:/tmp/test.log")
+      # C:/tmp/test-supervisor-0.log
+      def self.per_process_path(path, process_type, worker_id)
+        path = Pathname(path)
+        ext = path.extname
+
+        if process_type == :supervisor
+          suffix = "-#{process_type}-0#{ext}"  # "-0" for backword compatibility.
+        else
+          suffix = "-#{worker_id}#{ext}"
+        end
+        return path.sub_ext(suffix).to_s
       end
 
       def init(process_type, worker_id)
@@ -540,13 +552,19 @@ def init(process_type, worker_id)
             FileUtils.mkdir_p(File.dirname(@path))
           end
 
-          @logdev = if @log_rotate_age || @log_rotate_size
-                     Fluent::LogDeviceIO.new(Fluent.windows? ?
-                                               worker_id_suffixed_path(worker_id, @path) : @path,
-                                             shift_age: @log_rotate_age, shift_size: @log_rotate_size)
-                   else
-                     File.open(@path, "a")
-                   end
+          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)
+          else
+            @logdev = File.open(@path, "a")
+          end
+
           if @chuser || @chgroup
             chuid = @chuser ? ServerEngine::Privilege.get_etc_passwd(@chuser).uid : nil
             chgid = @chgroup ? ServerEngine::Privilege.get_etc_group(@chgroup).gid : nil
@@ -565,6 +583,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
       end
 
       def stdout?
diff --git a/test/test_supervisor.rb b/test/test_supervisor.rb
index 2022b132a8..71dedccef3 100644
--- a/test/test_supervisor.rb
+++ b/test/test_supervisor.rb
@@ -773,6 +773,14 @@ def server.config
     end
   end
 
+  def test_per_process_path
+    path = Fluent::Supervisor::LoggerInitializer.per_process_path("C:/tmp/test.log", :supervisor, 0)
+    assert_equal(path, "C:/tmp/test-supervisor-0.log")
+
+    path = Fluent::Supervisor::LoggerInitializer.per_process_path("C:/tmp/test.log", :worker, 1)
+    assert_equal(path, "C:/tmp/test-1.log")
+  end
+
   def create_debug_dummy_logger
     dl_opts = {}
     dl_opts[:log_level] = ServerEngine::DaemonLogger::DEBUG