From f44dfb0113c7f7bab9385f740cc19ac82bdc3b7f Mon Sep 17 00:00:00 2001
From: Christian Hubinger <christian@trigo.at>
Date: Mon, 10 Feb 2020 16:33:27 +0100
Subject: [PATCH 1/7] feat: basic file size based rotation implementation

---
 lib/logstash/outputs/file.rb | 54 +++++++++++++++++++++++++++++++
 spec/outputs/file_spec.rb    | 61 ++++++++++++++++++++++++++++++++++--
 2 files changed, 112 insertions(+), 3 deletions(-)

diff --git a/lib/logstash/outputs/file.rb b/lib/logstash/outputs/file.rb
index c644ad3..4996e35 100644
--- a/lib/logstash/outputs/file.rb
+++ b/lib/logstash/outputs/file.rb
@@ -74,6 +74,15 @@ class LogStash::Outputs::File < LogStash::Outputs::Base
   # recent event will appear in the file.
   config :write_behavior, :validate => [ "overwrite", "append" ], :default => "append"
 
+  # Size based file rotation
+  #
+  # Set the filesize in `bytes` after which the file is automatically rotated.
+  # The rotation automatically appends a number ending  .1, .2, .3 ... to the
+  # file name.
+  #
+  # If set to `file_rotation_size => 0` no rotation will be performed
+  config :file_rotation_size, :validate => :number, :default => 0
+
   default :codec, "json_lines"
 
   def register
@@ -85,6 +94,7 @@ def register
     @path = File.expand_path(path)
 
     validate_path
+    validate_file_rotation_size
 
     if path_with_field_ref?
       @file_root = extract_file_root
@@ -153,6 +163,13 @@ def validate_path
     end
   end
 
+  def validate_file_rotation_size
+    if (file_rotation_size < 0)
+      @logger.error("File: The file_rotation_size must not be a negative number", :file_rotation_size => @file_rotation_size)
+      raise LogStash::ConfigurationError.new("The file_rotation_size must not be a negative number.")
+    end
+  end
+
   def root_directory
     parts = @path.split(File::SEPARATOR).select { |item| !item.empty?  }
     if Gem.win_platform?
@@ -170,12 +187,47 @@ def inside_file_root?(log_path)
 
   def event_path(event)
     file_output_path = generate_filepath(event)
+
+    if (file_rotation_size > 0)
+      @io_mutex.synchronize do
+        # Check current size
+        if (File.exist?(file_output_path) && File.stat(file_output_path).size > file_rotation_size)
+          puts "Start file rotation..."
+          finished = false
+          cnt = 0
+          while File.exist?("#{file_output_path}.#{cnt}")
+            cnt += 1
+          end
+
+          # Flush file
+          if (@files.include?(file_output_path))
+            puts "Flush file: #{file_output_path}..."
+            @files[file_output_path].flush
+            @files[file_output_path].close
+            @files.delete(file_output_path)
+          end
+          puts "Having: #{cnt} rotations"
+          until cnt == 0
+            puts "Move file: #{file_output_path}.#{cnt -1 } =>  #{file_output_path}.#{cnt}"
+            File.rename("#{file_output_path}.#{cnt -1}", "#{file_output_path}.#{cnt}")
+            cnt -= 1
+          end
+          if (File.exist?("#{file_output_path}"))
+            puts "Final move file: #{file_output_path} =>  #{file_output_path}.0"
+            File.rename("#{file_output_path}", "#{file_output_path}.0")
+          end
+          puts "Finished rotation."
+        end
+      end
+    end
+
     if path_with_field_ref? && !inside_file_root?(file_output_path)
       @logger.warn("File: the event tried to write outside the files root, writing the event to the failure file",  :event => event, :filename => @failure_path)
       file_output_path = @failure_path
     elsif !@create_if_deleted && deleted?(file_output_path)
       file_output_path = @failure_path
     end
+
     @logger.debug("File, writing event to file.", :filename => file_output_path)
 
     file_output_path
@@ -202,6 +254,8 @@ def flush_pending_files
       @files.each do |path, fd|
         @logger.debug("Flushing file", :path => path, :fd => fd)
         fd.flush
+        fd.close
+        @files.delete(path)
       end
     end
   rescue => e
diff --git a/spec/outputs/file_spec.rb b/spec/outputs/file_spec.rb
index e3a2e77..f726f9a 100644
--- a/spec/outputs/file_spec.rb
+++ b/spec/outputs/file_spec.rb
@@ -83,6 +83,56 @@
     end
   end
 
+  describe "handle 'file_rotation_size setting'" do
+    tmp_file = Tempfile.new('logstash-spec-output-file')
+    event_count = 3000000
+    file_rotation_size = 1024*1024
+
+    config <<-CONFIG
+      input {
+        generator {
+          message => "hello world"
+          count => #{event_count}
+          type => "generator"
+        }
+      }
+      output {
+        file {
+          path => "#{tmp_file.path}"
+          file_rotation_size => #{file_rotation_size}
+        }
+      }
+    CONFIG
+
+    agent do
+      line_num = 0
+      # Now check all events for order and correctness.
+      puts "AAA: #{tmp_file.path}"
+
+      max_tolarated_size = (file_rotation_size + (file_rotation_size * 0.05))
+      cnt = 0
+
+      puts "Check: #{File.stat("#{tmp_file.path}").size} < #{max_tolarated_size}"
+      insist { File.stat("#{tmp_file.path}").size } < max_tolarated_size
+
+      while File.exists?("#{tmp_file.path}.#{cnt}") do
+        actual_size = File.stat("#{tmp_file.path}.#{cnt}").size
+        puts "Actual size: #{actual_size} Max: #{max_tolarated_size} Configured: #{file_rotation_size} Ratio: #{(actual_size.to_f/file_rotation_size.to_f)}  "
+        insist { actual_size } < max_tolarated_size
+        cnt += 1
+      end
+      # events = tmp_file.map {|line| LogStash::Event.new(LogStash::Json.load(line))}
+      # sorted = events.sort_by {|e| e.get('sequence')}
+      # sorted.each do |event|
+        # insist {event.get("message")} == "hello world"
+        # insist {event.get("sequence")} == line_num
+        # line_num += 1
+      # end
+
+      # insist {line_num} == event_count
+    end # agent
+  end
+
   describe "#register" do
     let(:path) { '/%{name}' }
     let(:output) { LogStash::Outputs::File.new({ "path" => path }) }
@@ -110,6 +160,11 @@
       output = LogStash::Outputs::File.new({ "path" => path })
       expect { output.register }.not_to raise_error
     end
+
+    it 'does not allow negative "file_rotation_size"' do
+      output = LogStash::Outputs::File.new({ "path" => '/tmp/%{name}', "file_rotation_size" => -1 })
+      expect { output.register }.to raise_error(LogStash::ConfigurationError)
+    end
   end
 
   describe "receiving events" do
@@ -117,7 +172,7 @@
     context "when write_behavior => 'overwrite'" do
       let(:tmp) { Stud::Temporary.pathname }
       let(:config) {
-        { 
+        {
           "write_behavior" => "overwrite",
           "path" => tmp,
           "codec" => LogStash::Codecs::JSONLines.new,
@@ -127,7 +182,7 @@
       let(:output) { LogStash::Outputs::File.new(config) }
 
       let(:count) { Flores::Random.integer(1..10) }
-      let(:events) do 
+      let(:events) do
         Flores::Random.iterations(1..10).collect do |i|
           LogStash::Event.new("value" => i)
         end
@@ -179,7 +234,7 @@
           event = LogStash::Event.new("event_id" => i+10)
           output.multi_receive([event])
         end
-        
+
         expect(FileTest.size(temp_file.path)).to be > 0
       end
 

From 6e16f787dd50ab33f322acef6f2a35f3d269f3c2 Mon Sep 17 00:00:00 2001
From: Christian Hubinger <christian@trigo.at>
Date: Tue, 11 Feb 2020 17:44:46 +0100
Subject: [PATCH 2/7] fix: check for missing events & cleanup

---
 lib/logstash/outputs/file.rb |  34 +++++++-----
 spec/outputs/file_spec.rb    | 104 +++++++++++++++++++++++++----------
 2 files changed, 93 insertions(+), 45 deletions(-)

diff --git a/lib/logstash/outputs/file.rb b/lib/logstash/outputs/file.rb
index 4996e35..b31ff3f 100644
--- a/lib/logstash/outputs/file.rb
+++ b/lib/logstash/outputs/file.rb
@@ -188,12 +188,26 @@ def inside_file_root?(log_path)
   def event_path(event)
     file_output_path = generate_filepath(event)
 
+    rotate_log_file(file_output_path)
+
+    if path_with_field_ref? && !inside_file_root?(file_output_path)
+      @logger.warn("File: the event tried to write outside the files root, writing the event to the failure file",  :event => event, :filename => @failure_path)
+      file_output_path = @failure_path
+    elsif !@create_if_deleted && deleted?(file_output_path)
+      file_output_path = @failure_path
+    end
+
+    @logger.debug("File, writing event to file.", :filename => file_output_path)
+
+    file_output_path
+  end
+
+  def rotate_log_file(file_output_path)
     if (file_rotation_size > 0)
       @io_mutex.synchronize do
         # Check current size
         if (File.exist?(file_output_path) && File.stat(file_output_path).size > file_rotation_size)
           puts "Start file rotation..."
-          finished = false
           cnt = 0
           while File.exist?("#{file_output_path}.#{cnt}")
             cnt += 1
@@ -208,31 +222,21 @@ def event_path(event)
           end
           puts "Having: #{cnt} rotations"
           until cnt == 0
-            puts "Move file: #{file_output_path}.#{cnt -1 } =>  #{file_output_path}.#{cnt}"
-            File.rename("#{file_output_path}.#{cnt -1}", "#{file_output_path}.#{cnt}")
+            puts "Move file: #{file_output_path}.#{cnt - 1} => #{file_output_path}.#{cnt}"
+            File.rename("#{file_output_path}.#{cnt - 1}", "#{file_output_path}.#{cnt}")
             cnt -= 1
           end
           if (File.exist?("#{file_output_path}"))
-            puts "Final move file: #{file_output_path} =>  #{file_output_path}.0"
+            puts "Final file: #{file_output_path} =>  #{file_output_path}.0"
             File.rename("#{file_output_path}", "#{file_output_path}.0")
           end
           puts "Finished rotation."
         end
       end
     end
-
-    if path_with_field_ref? && !inside_file_root?(file_output_path)
-      @logger.warn("File: the event tried to write outside the files root, writing the event to the failure file",  :event => event, :filename => @failure_path)
-      file_output_path = @failure_path
-    elsif !@create_if_deleted && deleted?(file_output_path)
-      file_output_path = @failure_path
-    end
-
-    @logger.debug("File, writing event to file.", :filename => file_output_path)
-
-    file_output_path
   end
 
+
   def generate_filepath(event)
     event.sprintf(@path)
   end
diff --git a/spec/outputs/file_spec.rb b/spec/outputs/file_spec.rb
index f726f9a..c2ad437 100644
--- a/spec/outputs/file_spec.rb
+++ b/spec/outputs/file_spec.rb
@@ -83,12 +83,13 @@
     end
   end
 
-  describe "handle 'file_rotation_size setting'" do
-    tmp_file = Tempfile.new('logstash-spec-output-file')
-    event_count = 3000000
-    file_rotation_size = 1024*1024
+  describe "handle 'file_rotation_size' setting" do
+    describe "create files of configured maximum size (5% tolerance)" do
+      tmp_file = Tempfile.new('logstash-spec-output-file')
+      event_count = 300000
+      file_rotation_size = 1024*1024
 
-    config <<-CONFIG
+      config <<-CONFIG
       input {
         generator {
           message => "hello world"
@@ -102,35 +103,78 @@
           file_rotation_size => #{file_rotation_size}
         }
       }
-    CONFIG
+      CONFIG
 
-    agent do
-      line_num = 0
-      # Now check all events for order and correctness.
-      puts "AAA: #{tmp_file.path}"
+      agent do
+        line_num = 0
+        # Check that files are within 5% size tolerance
+        max_tolarated_size = (file_rotation_size + (file_rotation_size * 0.05))
+        cnt = 0
+
+        puts "Check: #{File.stat("#{tmp_file.path}").size} < #{max_tolarated_size}"
+        insist { File.stat("#{tmp_file.path}").size } < max_tolarated_size
+
+        while File.exists?("#{tmp_file.path}.#{cnt}") do
+          actual_size = File.stat("#{tmp_file.path}.#{cnt}").size
+          puts "Actual size: #{actual_size} Max: #{max_tolarated_size} Configured: #{file_rotation_size} ratio: #{((((actual_size.to_f/file_rotation_size.to_f))*100)-100).round(3)}%"
+          insist { actual_size } < max_tolarated_size
+          cnt += 1
+        end
 
-      max_tolarated_size = (file_rotation_size + (file_rotation_size * 0.05))
-      cnt = 0
+        Dir.glob("#{tmp_file.path}.*").each do |file|
+          File.delete(file)
+        end
+      end # agent
+    end
 
-      puts "Check: #{File.stat("#{tmp_file.path}").size} < #{max_tolarated_size}"
-      insist { File.stat("#{tmp_file.path}").size } < max_tolarated_size
+    describe "no events are lost during file rotation" do
+      tmp_file = Tempfile.new('logstash-spec-output-file')
+      event_count = 300000
+      file_rotation_size = 1024*1024
 
-      while File.exists?("#{tmp_file.path}.#{cnt}") do
-        actual_size = File.stat("#{tmp_file.path}.#{cnt}").size
-        puts "Actual size: #{actual_size} Max: #{max_tolarated_size} Configured: #{file_rotation_size} Ratio: #{(actual_size.to_f/file_rotation_size.to_f)}  "
-        insist { actual_size } < max_tolarated_size
-        cnt += 1
-      end
-      # events = tmp_file.map {|line| LogStash::Event.new(LogStash::Json.load(line))}
-      # sorted = events.sort_by {|e| e.get('sequence')}
-      # sorted.each do |event|
-        # insist {event.get("message")} == "hello world"
-        # insist {event.get("sequence")} == line_num
-        # line_num += 1
-      # end
-
-      # insist {line_num} == event_count
-    end # agent
+      config <<-CONFIG
+      input {
+        generator {
+          message => "hello world"
+          count => #{event_count}
+          type => "generator"
+        }
+      }
+      output {
+        file {
+          path => "#{tmp_file.path}"
+          file_rotation_size => #{file_rotation_size}
+        }
+      }
+      CONFIG
+
+      agent do
+        line_num = 0
+        max_tolarated_size = (file_rotation_size + (file_rotation_size * 0.05))
+        cnt = 0
+        line_num = 0
+
+        # Check that all events are logged
+        events = File.open("#{tmp_file.path}").map {|line| LogStash::Event.new(LogStash::Json.load(line))}
+        sorted = events.sort_by {|e| e.get('sequence')}
+        sorted.each do |event|
+          line_num += 1
+        end
+        while File.exists?("#{tmp_file.path}.#{cnt}") do
+          events = File.open("#{tmp_file.path}.#{cnt}").map {|line| LogStash::Event.new(LogStash::Json.load(line))}
+          sorted = events.sort_by {|e| e.get('sequence')}
+          sorted.each do |event|
+            line_num += 1
+          end
+          cnt += 1
+        end
+
+        insist {line_num} == event_count
+        Dir.glob("#{tmp_file.path}.*").each do |file|
+          File.delete(file)
+        end
+      end # agent
+    end
   end
 
   describe "#register" do

From e565907ba7cceaf6deca31a9fbcd8b2c6cd640bb Mon Sep 17 00:00:00 2001
From: Christian Hubinger <christian@trigo.at>
Date: Mon, 17 Feb 2020 13:38:13 +0100
Subject: [PATCH 3/7] docs & cleaup

---
 docs/index.asciidoc          |  2 +
 lib/logstash/outputs/file.rb | 88 +++++++++++++++++++++++-------------
 logstash-output-file.gemspec |  2 +-
 spec/outputs/file_spec.rb    | 48 ++++++++++++++++++--
 4 files changed, 103 insertions(+), 37 deletions(-)

diff --git a/docs/index.asciidoc b/docs/index.asciidoc
index 0fa418c..5c65a27 100644
--- a/docs/index.asciidoc
+++ b/docs/index.asciidoc
@@ -50,6 +50,8 @@ This plugin supports the following configuration options plus the <<plugins-{typ
 | <<plugins-{type}s-{plugin}-gzip>> |<<boolean,boolean>>|No
 | <<plugins-{type}s-{plugin}-path>> |<<string,string>>|Yes
 | <<plugins-{type}s-{plugin}-write_behavior>> |<<string,string>>|No
+| <<plugins-{type}s-{plugin}-write_behavior>> |<<string,string>>|No
+| <<plugins-{type}s-{plugin}-write_behavior>> |<<string,string>>|No
 |=======================================================================
 
 Also see <<plugins-{type}s-{plugin}-common-options>> for a list of options supported by all
diff --git a/lib/logstash/outputs/file.rb b/lib/logstash/outputs/file.rb
index b31ff3f..e8768a5 100644
--- a/lib/logstash/outputs/file.rb
+++ b/lib/logstash/outputs/file.rb
@@ -77,12 +77,23 @@ class LogStash::Outputs::File < LogStash::Outputs::Base
   # Size based file rotation
   #
   # Set the filesize in `bytes` after which the file is automatically rotated.
-  # The rotation automatically appends a number ending  .1, .2, .3 ... to the
+  # The rotation automatically appends a number ending  .0, .1, .2, .3 ... to the
   # file name.
   #
   # If set to `file_rotation_size => 0` no rotation will be performed
   config :file_rotation_size, :validate => :number, :default => 0
 
+  # Max number of rotations to keep
+  #
+  # Set the maximum number of rotation for each logfile to keep. The deletion
+  # of out-dated filers is performed after each rotation.
+  # Example: `"max_file_rotations" => 3 will allow up to `4` files
+  # `/path/to/logfile`, `/path/to/logfile.0`,`/path/to/logfile.1`,`/path/to/logfile.2`,
+  #
+  # If set to `max_file_rotations => 0` no cleanup will be performed
+  # If `file_rotation_size => 0` this setting will be ignored
+  config :max_file_rotations, :validate => :number, :default => 0
+
   default :codec, "json_lines"
 
   def register
@@ -94,7 +105,7 @@ def register
     @path = File.expand_path(path)
 
     validate_path
-    validate_file_rotation_size
+    validate_file_rotation_settings
 
     if path_with_field_ref?
       @file_root = extract_file_root
@@ -163,11 +174,15 @@ def validate_path
     end
   end
 
-  def validate_file_rotation_size
+  def validate_file_rotation_settings
     if (file_rotation_size < 0)
       @logger.error("File: The file_rotation_size must not be a negative number", :file_rotation_size => @file_rotation_size)
       raise LogStash::ConfigurationError.new("The file_rotation_size must not be a negative number.")
     end
+    if (max_file_rotations < 0)
+      @logger.error("File: The max_file_rotations must not be a negative number", :max_file_rotations => @max_file_rotations)
+      raise LogStash::ConfigurationError.new("Setting  max_file_rotations must not be a negative number.")
+    end
   end
 
   def root_directory
@@ -202,37 +217,46 @@ def event_path(event)
     file_output_path
   end
 
+  def cleanup_rotated_files(file_output_path)
+    return unless max_file_rotations > 0
+
+    if File.exist?("#{file_output_path}.#{max_file_rotations}")
+      File.unlink("#{file_output_path}.#{max_file_rotations}")
+      @logger.info("Deleted rotated file: #{file_output_path}.#{max_file_rotations}")
+    end
+  end
+
   def rotate_log_file(file_output_path)
-    if (file_rotation_size > 0)
-      @io_mutex.synchronize do
-        # Check current size
-        if (File.exist?(file_output_path) && File.stat(file_output_path).size > file_rotation_size)
-          puts "Start file rotation..."
-          cnt = 0
-          while File.exist?("#{file_output_path}.#{cnt}")
-            cnt += 1
-          end
-
-          # Flush file
-          if (@files.include?(file_output_path))
-            puts "Flush file: #{file_output_path}..."
-            @files[file_output_path].flush
-            @files[file_output_path].close
-            @files.delete(file_output_path)
-          end
-          puts "Having: #{cnt} rotations"
-          until cnt == 0
-            puts "Move file: #{file_output_path}.#{cnt - 1} => #{file_output_path}.#{cnt}"
-            File.rename("#{file_output_path}.#{cnt - 1}", "#{file_output_path}.#{cnt}")
-            cnt -= 1
-          end
-          if (File.exist?("#{file_output_path}"))
-            puts "Final file: #{file_output_path} =>  #{file_output_path}.0"
-            File.rename("#{file_output_path}", "#{file_output_path}.0")
-          end
-          puts "Finished rotation."
-        end
+    return unless file_rotation_size > 0
+    @io_mutex.synchronize do
+      # Check current size
+      return unless (File.exist?(file_output_path) && File.stat(file_output_path).size > file_rotation_size)
+
+      cnt = 0
+      while File.exist?("#{file_output_path}.#{cnt}")
+        cnt += 1
+      end
+
+      # Flush file
+      if (@files.include?(file_output_path))
+        @logger.debug("Flush and close file: #{file_output_path}")
+        @files[file_output_path].flush
+        @files[file_output_path].close
+        @files.delete(file_output_path)
       end
+
+      until cnt == 0
+        @logger.debug("Move file: #{file_output_path}.#{cnt - 1} => #{file_output_path}.#{cnt}")
+        File.rename("#{file_output_path}.#{cnt - 1}", "#{file_output_path}.#{cnt}")
+        cnt -= 1
+      end
+      if (File.exist?("#{file_output_path}"))
+        @logger.debug("Move file: #{file_output_path} =>  #{file_output_path}.0")
+        File.rename("#{file_output_path}", "#{file_output_path}.0")
+      end
+
+      cleanup_rotated_files(file_output_path)
+      @logger.info("Finished file rotation")
     end
   end
 
diff --git a/logstash-output-file.gemspec b/logstash-output-file.gemspec
index 6e705a4..bf494a5 100644
--- a/logstash-output-file.gemspec
+++ b/logstash-output-file.gemspec
@@ -1,7 +1,7 @@
 Gem::Specification.new do |s|
 
   s.name            = 'logstash-output-file'
-  s.version         = '4.2.6'
+  s.version         = '4.3.0'
   s.licenses        = ['Apache License (2.0)']
   s.summary         = "Writes events to files on disk"
   s.description     = "This gem is a Logstash plugin required to be installed on top of the Logstash core pipeline using $LS_HOME/bin/logstash-plugin install gemname. This gem is not a stand-alone program"
diff --git a/spec/outputs/file_spec.rb b/spec/outputs/file_spec.rb
index c2ad437..02c4533 100644
--- a/spec/outputs/file_spec.rb
+++ b/spec/outputs/file_spec.rb
@@ -83,7 +83,7 @@
     end
   end
 
-  describe "handle 'file_rotation_size' setting" do
+  describe "Size based file rotation" do
     describe "create files of configured maximum size (5% tolerance)" do
       tmp_file = Tempfile.new('logstash-spec-output-file')
       event_count = 300000
@@ -111,12 +111,12 @@
         max_tolarated_size = (file_rotation_size + (file_rotation_size * 0.05))
         cnt = 0
 
-        puts "Check: #{File.stat("#{tmp_file.path}").size} < #{max_tolarated_size}"
+        # puts "Check: #{File.stat("#{tmp_file.path}").size} < #{max_tolarated_size}"
         insist { File.stat("#{tmp_file.path}").size } < max_tolarated_size
 
         while File.exists?("#{tmp_file.path}.#{cnt}") do
           actual_size = File.stat("#{tmp_file.path}.#{cnt}").size
-          puts "Actual size: #{actual_size} Max: #{max_tolarated_size} Configured: #{file_rotation_size} ratio: #{((((actual_size.to_f/file_rotation_size.to_f))*100)-100).round(3)}%"
+          # puts "Actual size: #{actual_size} Max: #{max_tolarated_size} Configured: #{file_rotation_size} ratio: #{((((actual_size.to_f/file_rotation_size.to_f))*100)-100).round(3)}%"
           insist { actual_size } < max_tolarated_size
           cnt += 1
         end
@@ -175,6 +175,42 @@
         end
       end # agent
     end
+
+    describe "handles `max_file_rotations` correctly" do
+      tmp_file = Tempfile.new('logstash-spec-output-file')
+      event_count = 50000
+      file_rotation_size = 1024*1024
+      max_file_rotations = 2
+
+      config <<-CONFIG
+      input {
+        generator {
+          message => "hello world"
+          count => #{event_count}
+          type => "generator"
+        }
+      }
+      output {
+        file {
+          path => "#{tmp_file.path}"
+          file_rotation_size => #{file_rotation_size}
+          max_file_rotations => #{max_file_rotations}
+        }
+      }
+      CONFIG
+
+      agent do
+        insist { File.exists?("#{tmp_file.path}") } == true
+        insist { File.exists?("#{tmp_file.path}.0") } == true
+        insist { File.exists?("#{tmp_file.path}.1") } == true
+        insist { File.exists?("#{tmp_file.path}.2") } == false
+        insist { File.exists?("#{tmp_file.path}.3") } == false
+
+        Dir.glob("#{tmp_file.path}.*").each do |file|
+          File.delete(file)
+        end
+      end # agent
+    end
   end
 
   describe "#register" do
@@ -209,10 +245,14 @@
       output = LogStash::Outputs::File.new({ "path" => '/tmp/%{name}', "file_rotation_size" => -1 })
       expect { output.register }.to raise_error(LogStash::ConfigurationError)
     end
+
+    it 'does not allow negative "max_file_rotations"' do
+      output = LogStash::Outputs::File.new({ "path" => '/tmp/%{name}', "max_file_rotations" => -1 })
+      expect { output.register }.to raise_error(LogStash::ConfigurationError)
+    end
   end
 
   describe "receiving events" do
-
     context "when write_behavior => 'overwrite'" do
       let(:tmp) { Stud::Temporary.pathname }
       let(:config) {

From c71515e401b06fd95dfd1eb47a96da4e37acd58b Mon Sep 17 00:00:00 2001
From: Christian Hubinger <christian@trigo.at>
Date: Tue, 18 Feb 2020 10:11:05 +0100
Subject: [PATCH 4/7] fix: remove useless change

---
 lib/logstash/outputs/file.rb | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/logstash/outputs/file.rb b/lib/logstash/outputs/file.rb
index e8768a5..bd1e032 100644
--- a/lib/logstash/outputs/file.rb
+++ b/lib/logstash/outputs/file.rb
@@ -282,8 +282,6 @@ def flush_pending_files
       @files.each do |path, fd|
         @logger.debug("Flushing file", :path => path, :fd => fd)
         fd.flush
-        fd.close
-        @files.delete(path)
       end
     end
   rescue => e

From e9c1c54852ed97eea3085a7d9ea935548065ab3b Mon Sep 17 00:00:00 2001
From: Christian Hubinger <christian@trigo.at>
Date: Mon, 24 Feb 2020 09:06:00 +0100
Subject: [PATCH 5/7] feat: handle .0.gq .1.gz etc. comresseed log segments

---
 lib/logstash/outputs/file.rb | 28 ++++++++++++++++++++++------
 spec/outputs/file_spec.rb    |  1 +
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/logstash/outputs/file.rb b/lib/logstash/outputs/file.rb
index bd1e032..6d442dc 100644
--- a/lib/logstash/outputs/file.rb
+++ b/lib/logstash/outputs/file.rb
@@ -80,6 +80,12 @@ class LogStash::Outputs::File < LogStash::Outputs::Base
   # The rotation automatically appends a number ending  .0, .1, .2, .3 ... to the
   # file name.
   #
+  # The current rotation number is evaluated dyamically by scanning the directory,
+  # use either `max_file_rotation` or a date based file name pattern to avoid
+  # performance issues due to large amount of files to be moved.
+  # Files ending with .0.gz .1.gz ... will be deteced automatially to intigrate with
+  # log compression performed by other tools
+  #
   # If set to `file_rotation_size => 0` no rotation will be performed
   config :file_rotation_size, :validate => :number, :default => 0
 
@@ -220,9 +226,14 @@ def event_path(event)
   def cleanup_rotated_files(file_output_path)
     return unless max_file_rotations > 0
 
-    if File.exist?("#{file_output_path}.#{max_file_rotations}")
-      File.unlink("#{file_output_path}.#{max_file_rotations}")
-      @logger.info("Deleted rotated file: #{file_output_path}.#{max_file_rotations}")
+    fileName ="#{file_output_path}.#{max_file_rotations}"
+    if File.exist?(fileName)
+      File.unlink(fileName)
+      @logger.info("Deleted rotated file: #{fileName}")
+    elsif
+      File.exist?("#{fileName}.gz")
+      File.unlink("#{fileName}.gz")
+      @logger.info("Deleted rotated file: #{fileName}.gz")
     end
   end
 
@@ -233,7 +244,7 @@ def rotate_log_file(file_output_path)
       return unless (File.exist?(file_output_path) && File.stat(file_output_path).size > file_rotation_size)
 
       cnt = 0
-      while File.exist?("#{file_output_path}.#{cnt}")
+      while File.exist?("#{file_output_path}.#{cnt}") or File.exist?("#{file_output_path}.#{cnt}.gz")
         cnt += 1
       end
 
@@ -246,8 +257,13 @@ def rotate_log_file(file_output_path)
       end
 
       until cnt == 0
-        @logger.debug("Move file: #{file_output_path}.#{cnt - 1} => #{file_output_path}.#{cnt}")
-        File.rename("#{file_output_path}.#{cnt - 1}", "#{file_output_path}.#{cnt}")
+        if File.exist?("#{file_output_path}.#{cnt - 1}")
+          @logger.debug("Move file: #{file_output_path}.#{cnt - 1} => #{file_output_path}.#{cnt}")
+          File.rename("#{file_output_path}.#{cnt - 1}", "#{file_output_path}.#{cnt}")
+        elsif File.exist?("#{file_output_path}.#{cnt - 1}.gz")
+          @logger.debug("Move file: #{file_output_path}.#{cnt - 1}.gz => #{file_output_path}.#{cnt}.gz")
+          File.rename("#{file_output_path}.#{cnt - 1}.gz", "#{file_output_path}.#{cnt}.gz")
+        end
         cnt -= 1
       end
       if (File.exist?("#{file_output_path}"))
diff --git a/spec/outputs/file_spec.rb b/spec/outputs/file_spec.rb
index 02c4533..a212a6e 100644
--- a/spec/outputs/file_spec.rb
+++ b/spec/outputs/file_spec.rb
@@ -11,6 +11,7 @@
 require "uri"
 require "fileutils"
 require "flores/random"
+require "insist"
 
 describe LogStash::Outputs::File do
   describe "ship lots of events to a file" do

From ddc0822875c5bb1295e840338189a805a69479c0 Mon Sep 17 00:00:00 2001
From: Christian Hubinger <christian@trigo.at>
Date: Fri, 6 Mar 2020 10:00:07 +0100
Subject: [PATCH 6/7] fix: remove unkononw require statement

---
 spec/outputs/file_spec.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/spec/outputs/file_spec.rb b/spec/outputs/file_spec.rb
index a212a6e..02c4533 100644
--- a/spec/outputs/file_spec.rb
+++ b/spec/outputs/file_spec.rb
@@ -11,7 +11,6 @@
 require "uri"
 require "fileutils"
 require "flores/random"
-require "insist"
 
 describe LogStash::Outputs::File do
   describe "ship lots of events to a file" do

From 47a2d4de655d0677a743ff8108182388ef681a93 Mon Sep 17 00:00:00 2001
From: Christian Hubinger <christian@trigo.at>
Date: Mon, 4 May 2020 16:11:08 +0200
Subject: [PATCH 7/7] feat: implement "keep_file_extension"

---
 lib/logstash/outputs/file.rb |  54 ++++++++++++----
 spec/outputs/file_spec.rb    | 115 ++++++++++++++++++++++++++++++++++-
 2 files changed, 155 insertions(+), 14 deletions(-)

diff --git a/lib/logstash/outputs/file.rb b/lib/logstash/outputs/file.rb
index 6d442dc..438ff0e 100644
--- a/lib/logstash/outputs/file.rb
+++ b/lib/logstash/outputs/file.rb
@@ -92,14 +92,22 @@ class LogStash::Outputs::File < LogStash::Outputs::Base
   # Max number of rotations to keep
   #
   # Set the maximum number of rotation for each logfile to keep. The deletion
-  # of out-dated filers is performed after each rotation.
-  # Example: `"max_file_rotations" => 3 will allow up to `4` files
+  # of out-dated files is performed after each rotation.
+  # Example: `"max_file_rotations" => 3` will allow up to `4` files
   # `/path/to/logfile`, `/path/to/logfile.0`,`/path/to/logfile.1`,`/path/to/logfile.2`,
   #
   # If set to `max_file_rotations => 0` no cleanup will be performed
   # If `file_rotation_size => 0` this setting will be ignored
   config :max_file_rotations, :validate => :number, :default => 0
 
+  # Keep file extension with log rotation
+  #
+  # Set whether the file extension, segment of the filename after the last `.`, should
+  # be preserved when rotating logfiles
+  # Example: `"keep_file_extension" => true` will preserve the extension
+  # `/path/to/logfile.log`, `/path/to/logfile.0.log`,`/path/to/logfile.1.log`,`/path/to/logfile.2.log ...`,
+  config :keep_file_extension, :validate => :boolean, :default => false
+
   default :codec, "json_lines"
 
   def register
@@ -226,7 +234,7 @@ def event_path(event)
   def cleanup_rotated_files(file_output_path)
     return unless max_file_rotations > 0
 
-    fileName ="#{file_output_path}.#{max_file_rotations}"
+    fileName = get_rotated_output_file_name(file_output_path, max_file_rotations, false)
     if File.exist?(fileName)
       File.unlink(fileName)
       @logger.info("Deleted rotated file: #{fileName}")
@@ -237,6 +245,22 @@ def cleanup_rotated_files(file_output_path)
     end
   end
 
+  def get_rotated_output_file_name(filename, rotation, compressed)
+    newname = filename
+    if (keep_file_extension)
+      newname = "#{File.dirname(filename)}/#{File.basename(filename, ".*")}.#{rotation}#{File.extname(filename)}"
+    else
+      newname = "#{filename}.#{rotation}"
+    end
+    if compressed
+      newname = "#{newname}.gz"
+    else
+      newname = "#{newname}"
+    end
+    return newname
+  end
+
+
   def rotate_log_file(file_output_path)
     return unless file_rotation_size > 0
     @io_mutex.synchronize do
@@ -244,7 +268,7 @@ def rotate_log_file(file_output_path)
       return unless (File.exist?(file_output_path) && File.stat(file_output_path).size > file_rotation_size)
 
       cnt = 0
-      while File.exist?("#{file_output_path}.#{cnt}") or File.exist?("#{file_output_path}.#{cnt}.gz")
+      while File.exist?(get_rotated_output_file_name(file_output_path, cnt, false)) or File.exist?(get_rotated_output_file_name(file_output_path, cnt, true))
         cnt += 1
       end
 
@@ -257,18 +281,22 @@ def rotate_log_file(file_output_path)
       end
 
       until cnt == 0
-        if File.exist?("#{file_output_path}.#{cnt - 1}")
-          @logger.debug("Move file: #{file_output_path}.#{cnt - 1} => #{file_output_path}.#{cnt}")
-          File.rename("#{file_output_path}.#{cnt - 1}", "#{file_output_path}.#{cnt}")
-        elsif File.exist?("#{file_output_path}.#{cnt - 1}.gz")
-          @logger.debug("Move file: #{file_output_path}.#{cnt - 1}.gz => #{file_output_path}.#{cnt}.gz")
-          File.rename("#{file_output_path}.#{cnt - 1}.gz", "#{file_output_path}.#{cnt}.gz")
+        if File.exist?(get_rotated_output_file_name(file_output_path, cnt - 1, false))
+          @logger.debug("Move file: #{get_rotated_output_file_name(file_output_path, cnt - 1, false)} => #{get_rotated_output_file_name(file_output_path, cnt , false)}")
+          File.rename(
+            get_rotated_output_file_name(file_output_path, cnt - 1, false),
+            get_rotated_output_file_name(file_output_path, cnt, false))
+        elsif File.exist?(get_rotated_output_file_name(file_output_path, cnt - 1, true))
+          @logger.debug("Move file: #{get_rotated_output_file_name(file_output_path, cnt - 1, true)} => #{get_rotated_output_file_name(file_output_path, cnt, true)}")
+          File.rename(
+            get_rotated_output_file_name(file_output_path, cnt - 1, true),
+            get_rotated_output_file_name(file_output_path, cnt, true))
         end
         cnt -= 1
       end
-      if (File.exist?("#{file_output_path}"))
-        @logger.debug("Move file: #{file_output_path} =>  #{file_output_path}.0")
-        File.rename("#{file_output_path}", "#{file_output_path}.0")
+      if (File.exist?(file_output_path))
+        @logger.debug("Move file: #{file_output_path} => #{get_rotated_output_file_name(file_output_path, 0, false)}")
+        File.rename(file_output_path, get_rotated_output_file_name(file_output_path, 0, false))
       end
 
       cleanup_rotated_files(file_output_path)
diff --git a/spec/outputs/file_spec.rb b/spec/outputs/file_spec.rb
index 02c4533..3a18cca 100644
--- a/spec/outputs/file_spec.rb
+++ b/spec/outputs/file_spec.rb
@@ -211,8 +211,121 @@
         end
       end # agent
     end
-  end
 
+    describe "handles `keep_file_extension` correctly" do
+      tmp_file = Tempfile.new('logstash-spec-output-file')
+      event_count = 50000
+      max_file_rotations = 2
+      file_rotation_size = 1024*1024
+
+      config <<-CONFIG
+      input {
+        generator {
+          message => "hello world"
+          count => #{event_count}
+          type => "generator"
+        }
+      }
+      output {
+        file {
+          path => "#{tmp_file.path}.log"
+          file_rotation_size => #{file_rotation_size}
+          max_file_rotations => #{max_file_rotations}
+          keep_file_extension => true
+        }
+      }
+      CONFIG
+
+      agent do
+        puts "#{tmp_file.path}.log"
+        insist { File.exists?("#{tmp_file.path}.log") } == true
+        insist { File.exists?("#{tmp_file.path}.0.log") } == true
+        insist { File.exists?("#{tmp_file.path}.1.log") } == true
+        insist { File.exists?("#{tmp_file.path}.2.log") } == false
+        insist { File.exists?("#{tmp_file.path}.3.log") } == false
+
+        Dir.glob("#{tmp_file.path}.*").each do |file|
+          File.delete(file)
+        end
+      end # agent
+    end
+
+    describe "handles `keep_file_extension` without extension correctly" do
+      tmp_file = Tempfile.new('logstash-spec-output-file')
+      event_count = 50000
+      max_file_rotations = 2
+      file_rotation_size = 1024*1024
+
+      config <<-CONFIG
+      input {
+        generator {
+          message => "hello world"
+          count => #{event_count}
+          type => "generator"
+        }
+      }
+      output {
+        file {
+          path => "#{tmp_file.path}"
+          file_rotation_size => #{file_rotation_size}
+          max_file_rotations => #{max_file_rotations}
+          keep_file_extension => true
+        }
+      }
+      CONFIG
+
+      agent do
+        puts "#{tmp_file.path}.log"
+        insist { File.exists?("#{tmp_file.path}") } == true
+        insist { File.exists?("#{tmp_file.path}.0") } == true
+        insist { File.exists?("#{tmp_file.path}.1") } == true
+        insist { File.exists?("#{tmp_file.path}.2") } == false
+        insist { File.exists?("#{tmp_file.path}.3") } == false
+
+        Dir.glob("#{tmp_file.path}.*").each do |file|
+          File.delete(file)
+        end
+      end # agent
+    end
+
+    describe "handles `keep_file_extension` with `.` in filename correctly" do
+      tmp_file = Tempfile.new('logstash-spec-output-file')
+      event_count = 50000
+      max_file_rotations = 2
+      file_rotation_size = 1024*1024
+
+      config <<-CONFIG
+      input {
+        generator {
+          message => "hello world"
+          count => #{event_count}
+          type => "generator"
+        }
+      }
+      output {
+        file {
+          path => "#{tmp_file.path}.afterdot.log"
+          file_rotation_size => #{file_rotation_size}
+          max_file_rotations => #{max_file_rotations}
+          keep_file_extension => true
+        }
+      }
+      CONFIG
+
+      agent do
+        puts "#{tmp_file.path}.log"
+        insist { File.exists?("#{tmp_file.path}.afterdot.log") } == true
+        insist { File.exists?("#{tmp_file.path}.afterdot.0.log") } == true
+        insist { File.exists?("#{tmp_file.path}.afterdot.1.log") } == true
+        insist { File.exists?("#{tmp_file.path}.afterdot.2.log") } == false
+        insist { File.exists?("#{tmp_file.path}.afterdot.3.log") } == false
+
+        Dir.glob("#{tmp_file.path}.*").each do |file|
+          File.delete(file)
+        end
+      end # agent
+    end
+  end
   describe "#register" do
     let(:path) { '/%{name}' }
     let(:output) { LogStash::Outputs::File.new({ "path" => path }) }