-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Reduce tailwatcher argument #2832
Conversation
Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
TailWatcher is not related to this Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
and this log is no need to be put here Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
Signed-off-by: Yuta Iwama <ganmacs@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.
Looks reasonable for me.
And I added a nitpicks for logging messages in #update_watcher
.
Is it copy and paste mistake?
@@ -384,6 +398,9 @@ def close_watcher_handles | |||
|
|||
# refresh_watchers calls @tails.keys so we don't use stop_watcher -> start_watcher sequence for safety. | |||
def update_watcher(path, pe) | |||
log_msg = "detected rotation of #{path}" | |||
log_msg << "; waiting #{@rotate_wait} seconds" # wait rotate_time if previous file exists | |||
|
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.
Missing log.info
here?
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.
Good catch…thank you 🙏 9940869
Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
lib/fluent/plugin/in_tail.rb
Outdated
def on_change(prev, cur) | ||
@callback.call | ||
rescue | ||
# TODO log? |
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.
Can remove too old comment
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.
fixed 269ea68
lib/fluent/plugin/in_tail.rb
Outdated
@@ -384,6 +398,10 @@ def close_watcher_handles | |||
|
|||
# refresh_watchers calls @tails.keys so we don't use stop_watcher -> start_watcher sequence for safety. | |||
def update_watcher(path, pe) | |||
log_msg = "detected rotation of #{path}" | |||
log_msg << "; waiting #{@rotate_wait} seconds" # wait rotate_time if previous file exists | |||
log.info(log_msg) |
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.
Can combine 3 lines.
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.
fixed 857d9cb
if watcher_needs_update | ||
@update_watcher.call(@path, swap_state(@pe)) | ||
else | ||
@io_handler = IOHandler.new(self, &method(:wrap_receive_lines)) | ||
@log.info "detected rotation of #{@path}" |
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 this line needed?
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.
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, I see.
Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
Which issue(s) this PR fixes:
no
What this PR does / why we need it:
Reduce
TailWatcher
's arguments. it's hard to maintain now.Docs Changes:
no
Release Note:
no