output: Replace ${chunk_id} before logging warning.

The updated implementation makes sure to replace instances of ${chunk_id} prior to looking for and warning on unreplaced keys in the pattern. Prior to this, a warning line was printed for every chunk that was flushed even though nothing was actually wrong (replacement happened right after). Signed-off-by: Brian Atkinson <[email protected]>
fluent · Sep 25, 2020 · 37a1c0e · 37a1c0e
1 parent adca029
commit 37a1c0e
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 21 deletions.
diff --git a/lib/fluent/plugin/output.rb b/lib/fluent/plugin/output.rb
@@ -721,16 +721,15 @@ def extract_placeholders(str, chunk)
                      # For existing plugins. Old plugin passes Chunk.metadata instead of Chunk
                      chunk
                    end
-        if metadata.empty?
-          str.sub(CHUNK_ID_PLACEHOLDER_PATTERN) {
-            if chunk_passed
-              dump_unique_id_hex(chunk.unique_id)
-            else
-              log.warn "${chunk_id} is not allowed in this plugin. Pass Chunk instead of metadata in extract_placeholders's 2nd argument"
-            end
-          }
-        else
-          rvalue = str.dup
+        rvalue = str.dup
+        rvalue = rvalue.sub(CHUNK_ID_PLACEHOLDER_PATTERN) {
+          if chunk_passed
+            dump_unique_id_hex(chunk.unique_id)
+          else
+            log.warn "${chunk_id} is not allowed in this plugin. Pass Chunk instead of metadata in extract_placeholders's 2nd argument"
+          end
+        }
+        if not metadata.empty?
           # strftime formatting
           if @chunk_key_time # this section MUST be earlier than rest to use raw 'str'
             @output_time_formatter_cache[str] ||= Fluent::Timezone.formatter(@timekey_zone, str)
@@ -768,19 +767,13 @@ def extract_placeholders(str, chunk)
               end
             end
           end
+        end
 
-          if rvalue =~ CHUNK_KEY_PLACEHOLDER_PATTERN
-            log.warn "chunk key placeholder '#{$1}' not replaced. template:#{str}"
-          end
-
-          rvalue.sub(CHUNK_ID_PLACEHOLDER_PATTERN) {
-            if chunk_passed
-              dump_unique_id_hex(chunk.unique_id)
-            else
-              log.warn "${chunk_id} is not allowed in this plugin. Pass Chunk instead of metadata in extract_placeholders's 2nd argument"
-            end
-          }
+        if rvalue =~ CHUNK_KEY_PLACEHOLDER_PATTERN
+          log.warn "chunk key placeholder '#{$1}' not replaced. template:#{str}"
         end
+
+        rvalue
       end
 
       def emit_events(tag, es)

diff --git a/test/plugin/test_output.rb b/test/plugin/test_output.rb
@@ -378,6 +378,18 @@ def waiting(seconds)
       assert { logs.any? { |log| log.include?("${chunk_id} is not allowed in this plugin") } }
     end
 
+    test '#extract_placeholders does not log for ${chunk_id} placeholder' do
+      @i.configure(config_element('ROOT', '', {}, [config_element('buffer', '')]))
+      tmpl = "/mypath/${chunk_id}/tail"
+      t = event_time('2016-04-11 20:30:00 +0900')
+      v = {key1: "value1", key2: "value2"}
+      c = create_chunk(timekey: t, tag: 'fluentd.test.output', variables: v)
+      @i.log.out.logs.clear
+      @i.extract_placeholders(tmpl, c)
+      logs = @i.log.out.logs
+      assert { logs.none? { |log| log.include?("${chunk_id}") } }
+    end
+
     test '#extract_placeholders logs warn message with not replaced key' do
       @i.configure(config_element('ROOT', '', {}, [config_element('buffer', '')]))
       tmpl = "/mypath/${key1}/test"