-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
in_forward: Add tag and add_tag_prefix parameters #2396
Conversation
Signed-off-by: Masahiro Nakagawa <repeatedly@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Port based tag handling
I think label is a good fit for these cases. Will you please tell me why not to use label?
@@ -293,6 +298,9 @@ def on_message(msg, chunk_size, conn) | |||
log.warn "Input chunk size is larger than 'chunk_size_warn_limit':", tag: tag, host: conn.remote_host, limit: @chunk_size_warn_limit, size: chunk_size | |||
end | |||
|
|||
tag = @tag.dup if @tag | |||
tag = "#{@add_tag_prefix}.#{tag}" if @add_tag_prefix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it ok without checking tag is an empty string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good catch. I will add empty check on configure
.
Yeah, label is one good approach but label itself doesn't change tag. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this patch is also good for label combo, e.g. some output plugins use tag placeholder.
make sense to me to add this feature.
Signed-off-by: Masahiro Nakagawa <repeatedly@gmail.com>
Signed-off-by: Masahiro Nakagawa repeatedly@gmail.com
Which issue(s) this PR fixes:
none
What this PR does / why we need it:
I sometimes receive this request from users for aggregator.
There are several reasons:
We can simulate this by adding new field for cluster detection in forwarder side and check it in aggregator side. But it makes configuration complicated.
We can rewrite tag with
route
or similar plugins but it has performance penalty for large traffic.The implementation is very simple and it resolves above cases. So supporting these parameters is reasonable.
Docs Changes:
Will send a patch to fluentd-docs after merged.
Release Note:
Same as title.