add input file tail#645
Conversation
Signed-off-by: Jem Davies <jemsot@gmail.com>
Signed-off-by: Jem Davies <jemsot@gmail.com>
Signed-off-by: Jem Davies <jemsot@gmail.com>
Signed-off-by: Jem Davies <jemsot@gmail.com>
Replace errChan draining with doneChan synchronization to ensure watch() goroutine exits before closing file handle. Also fix file handle leaks in newTail() error paths.
jem-davies
force-pushed
the
add-input-file-tail
branch
from
ed975dc to
bbd8915
Compare
Signed-off-by: Jem Davies <jemsot@gmail.com>
|
|
||
| if !os.SameFile(t.fileInfo, tempInfo) { | ||
| if t.logger != nil { | ||
| t.logger.Info(fmt.Sprintf("Handling rotation for %v", t.path)) |
There was a problem hiding this comment.
Can we downgrade these rotation logs to the debug level?
|
|
||
| pos, err := t.file.Seek(0, io.SeekCurrent) | ||
| if err != nil { | ||
| return fmt.Errorf("seek file: %w", err) |
There was a problem hiding this comment.
Perhaps some of these errors should have more info added:
| return fmt.Errorf("seek file: %w", err) | |
| return fmt.Errorf("failed to set seek on %s: %w", t.file.Name(). , err) |
| line, err := t.reader.ReadBytes('\n') | ||
| line = bytes.TrimRight(line, "\r\n") |
There was a problem hiding this comment.
Any particular reason why we're exclusively using \n as a delimiter here?
There was a problem hiding this comment.
Yes could have a config option for that
There was a problem hiding this comment.
I have updated the component description to say "Reads lines from a file continuously with handling of log rotation." -- we could add it as a config option I suppose - but then would be interested too see why / what else people would want to use as a delimiter.
If there is an issue raised it should be ok to add a new config option with a default of "\n" for backwards comp.
So thinking to leave this as-is
| file *os.File | ||
| fileInfo os.FileInfo |
There was a problem hiding this comment.
Thoughts on using the *service.FS from the mgr *service.Resources instead for file reading?
i.e
mgr.FS().OpenFile()This way we're relying on our FS fixtures instead of the os package
jem-davies
force-pushed
the
add-input-file-tail
branch
from
e7ae788 to
3bbbf5e
Compare
|
@gregfurman - thanks for review 🙏 - I have either altered the code / responded to each comment 😄 |
Signed-off-by: Jem Davies <jemsot@gmail.com>
jem-davies
force-pushed
the
add-input-file-tail
branch
from
3bbbf5e to
f813892
Compare
gregfurman
left a comment
gregfurman
left a comment
There was a problem hiding this comment.
LGTM! Thanks for addressing ym feedback 
2 participants
Some context: I had decided to implement this as a simpler alternative to #514.