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 <[email protected]>

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])

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?

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