out_file/out_secondary_file: Support ${chunk_id} placeholder. fix #1705 #1708
Conversation
lib/fluent/plugin/out_file.rb
Outdated
| @@ -178,7 +178,7 @@ def format(tag, time, record) | |||
| end | |||
|
|
|||
| def write(chunk) | |||
| path = extract_placeholders(@path_template, chunk.metadata) | |||
| path = extract_placeholders(@path_template.gsub(CHUNK_ID_PLACEHOLDER_PATTERN, dump_unique_id_hex(chunk.unique_id)), chunk.metadata) | |||
There was a problem hiding this comment.
It seems better if the replacements of CHUNK_ID_PLACEHOLDER_PATTERN is done by extract_placeholders instead of each plugins. For now, only these two plugins use this method, but it is useful if all plugins can use this feature automatically in the future.
There was a problem hiding this comment.
Yeah, it is one idea.
For it, we need to change extract_placeholders API.
- 2nd argument changed to chunk, not
chunk.metadata
For compatibility, we need to check 2nd argument type.
metadata = chunk.metadata if metadata.is_a?(Chunk)
- Add 3rd argument for chunk
def extract_placeholders(str, metadata, chunk = nil)
metadata is from chunk so this API is a little strange.
- Add 3rd argument for additional params
This avoids further API changes.
def extract_placeholders(str, metadata, extras = nil)
if extras && extras.has_key?(:chunk)
end
end
I think 1 is better because this doesn't break exsiting API and doesn't add more argument.
We can change Plugin API before stable announcement.
There was a problem hiding this comment.
I agree to change extract_placeholders API.
👍 > 1 and 3. Both of them look good to me.
|
@okkez @cosmo0920 How about above change? Do you have other opinion? |
|
Change |
${chunk_id}placeholder is useful for out_file like plugins.So I added
CHUNK_ID_PLACEHOLDER_PATTERNin Output to share.For example, out_webhdfs uses own
${chunk_id}value but it should use this predefined constant.