From 1d496bd39078a91a271df1370bd0ff3da60bf4be Mon Sep 17 00:00:00 2001
From: Shingo Nakayama <nkymsng@akane.waseda.jp>
Date: Thu, 23 May 2024 12:24:18 +0900
Subject: [PATCH 1/2] out_file:add warn message for symlink_path-setting

Signed-off-by: Shingo Nakayama <nkymsng@akane.waseda.jp>
---
 lib/fluent/plugin/out_file.rb |  9 +++++++++
 test/plugin/test_out_file.rb  | 21 ++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/lib/fluent/plugin/out_file.rb b/lib/fluent/plugin/out_file.rb
index 7de4b90979..fffdb4e190 100644
--- a/lib/fluent/plugin/out_file.rb
+++ b/lib/fluent/plugin/out_file.rb
@@ -172,6 +172,15 @@ def configure(conf)
           log.warn "symlink_path is unavailable on Windows platform. disabled."
           @symlink_path = nil
         else
+          #add check if symlink_path has a tag placeholder.
+            placeholder_validators(:symlink_path, @symlink_path).reject{|v| v.type == :time }.each do |v|
+            begin
+              v.validate!
+            end
+          rescue Fluent::ConfigError => e
+            log.warn "#{e}. This means multiple chunks are competing for a single symlink_path, so some logs may not be taken from the symlink."
+          end
+          
           @buffer.extend SymlinkBufferMixin
           @buffer.symlink_path = @symlink_path
           @buffer.output_plugin_for_symlink = self
diff --git a/test/plugin/test_out_file.rb b/test/plugin/test_out_file.rb
index 5727bc1e69..d3d72569d1 100644
--- a/test/plugin/test_out_file.rb
+++ b/test/plugin/test_out_file.rb
@@ -130,7 +130,7 @@ def create_driver(conf = CONFIG, opts = {})
           'path' => "#{TMP_DIR}/${tag}/${type}/conf_test.%Y%m%d.%H%M.log",
           'add_path_suffix' => 'false',
           'append' => "true",
-          'symlink_path' => "#{TMP_DIR}/conf_test.current.log",
+          'symlink_path' => "#{TMP_DIR}/${tag}/conf_test.current.log",
           'compress' => 'gzip',
           'recompress' => 'true',
         }, [
@@ -183,6 +183,25 @@ def create_driver(conf = CONFIG, opts = {})
         Fluent::Test::Driver::Output.new(Fluent::Plugin::NullOutput).configure(conf)
       end
     end
+
+    test 'symlink path has not tag placeholder or key placeholder' do
+      conf = config_element('match', '**', {
+          'path' => "#{TMP_DIR}/${tag}/${key1}/${key2}/conf_test.%Y%m%d.%H%M.log",
+          'symlink_path' => "#{TMP_DIR}/conf_test.current.log",
+        }, [
+          config_element('buffer', 'time,tag,key1,key2', {
+              '@type' => 'file',
+              'timekey' => '1d',
+              'path' => "#{TMP_DIR}/buf_conf_test",
+          }),
+      ])
+      assert_nothing_raised do
+        d = create_driver(conf)
+        assert do
+          d.logs.count { |log| log.include?("symlink_path:") } == 2
+        end
+      end
+    end
   end
 
   sub_test_case 'fully configured output' do

From 1a64c6ed42cd90b79f0af8188c6641b6a2ad552c Mon Sep 17 00:00:00 2001
From: Shingo Nakayama <152375941+Shingo-Nakayama@users.noreply.github.com>
Date: Fri, 24 May 2024 15:38:54 +0900
Subject: [PATCH 2/2] Apply suggestions from code review

Signed-off-by: Shingo Nakayama <nkymsng@akane.waseda.jp>
Co-authored-by: Daijiro Fukuda <fukuda@clear-code.com>
---
 lib/fluent/plugin/out_file.rb | 9 ++++-----
 test/plugin/test_out_file.rb  | 5 +++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/fluent/plugin/out_file.rb b/lib/fluent/plugin/out_file.rb
index fffdb4e190..19c0afa8ef 100644
--- a/lib/fluent/plugin/out_file.rb
+++ b/lib/fluent/plugin/out_file.rb
@@ -172,15 +172,14 @@ def configure(conf)
           log.warn "symlink_path is unavailable on Windows platform. disabled."
           @symlink_path = nil
         else
-          #add check if symlink_path has a tag placeholder.
-            placeholder_validators(:symlink_path, @symlink_path).reject{|v| v.type == :time }.each do |v|
+          placeholder_validators(:symlink_path, @symlink_path).reject{ |v| v.type == :time }.each do |v|
             begin
               v.validate!
+            rescue Fluent::ConfigError => e
+              log.warn "#{e}. This means multiple chunks are competing for a single symlink_path, so some logs may not be taken from the symlink."
             end
-          rescue Fluent::ConfigError => e
-            log.warn "#{e}. This means multiple chunks are competing for a single symlink_path, so some logs may not be taken from the symlink."
           end
-          
+
           @buffer.extend SymlinkBufferMixin
           @buffer.symlink_path = @symlink_path
           @buffer.output_plugin_for_symlink = self
diff --git a/test/plugin/test_out_file.rb b/test/plugin/test_out_file.rb
index d3d72569d1..8b6ab161d0 100644
--- a/test/plugin/test_out_file.rb
+++ b/test/plugin/test_out_file.rb
@@ -184,7 +184,8 @@ def create_driver(conf = CONFIG, opts = {})
       end
     end
 
-    test 'symlink path has not tag placeholder or key placeholder' do
+    test 'warning for symlink_path not including correct placeholders corresponding to chunk keys' do
+      omit "Windows doesn't support symlink" if Fluent.windows?
       conf = config_element('match', '**', {
           'path' => "#{TMP_DIR}/${tag}/${key1}/${key2}/conf_test.%Y%m%d.%H%M.log",
           'symlink_path' => "#{TMP_DIR}/conf_test.current.log",
@@ -198,7 +199,7 @@ def create_driver(conf = CONFIG, opts = {})
       assert_nothing_raised do
         d = create_driver(conf)
         assert do
-          d.logs.count { |log| log.include?("symlink_path:") } == 2
+          d.logs.count { |log| log.include?("multiple chunks are competing for a single symlink_path") } == 2
         end
       end
     end