Skip to content
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

Tailer: add option to start tailing at specified seek offset #47

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

virtuald
Copy link
Contributor

There could be a better way to construct this, but seems good enough for now.

@virtuald
Copy link
Contributor Author

It would be nice to have a -n option similar to tail, but that sounds like a lot of work.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.005%) to 69.359% when pulling d6093c0 on virtuald:seekto into 691b510 on fstab:master.

@virtuald
Copy link
Contributor Author

I don't have any idea why the Windows integration test is failing, I think the behavior introduced by this patch should be the same, and the go tests pass. Do the integration tests depend on the go tests somehow?

@fstab fstab force-pushed the master branch 4 times, most recently from cf67c51 to 694910a Compare October 10, 2018 20:44
@fstab
Copy link
Owner

fstab commented Oct 11, 2018

The tests sometimes fail on travis-ci or appveyor. This has nothing to do with your pr. It seems to be a timing issue, but I can't reproduce it locally. Restarted the Windows build and it succeeded.

@fstab
Copy link
Owner

fstab commented Oct 11, 2018

I think it's a good idea to make the tailer useful as a library for other applications than grok_exporter. However, I would like to understand better how the seek offset would be used. Do you have a specific application in mind?

@virtuald
Copy link
Contributor Author

The usecase I have in mind is similar to tail -f -n 1000, which follows a file after outputting the last 1000 lines. Specifically, I'd like to read some portion of the end of the file, and continue getting its output as the file is written. This is particularly useful with a really large file.

Now, because my patch doesn't give an option to go back N lines, this implementation requires the caller to either know a particular offset they want to seek to, or make some kind of guess about how far from the end they want to see. However, either of these are better than reading the entire file from disk.

@fstab
Copy link
Owner

fstab commented Oct 12, 2018

As you said, the main usage would be to skip a number of lines (like tail -n).

It would be great to provide a tail -n API instead of a tail -c API so that the caller does not need to know the byte offset.

One way to implement that would be to add the following method to file.go and file_windows.go and call this instead of Seek() in openLogfile():

func (file *File) SeekLines(nLines int, whence int) {
	// ...
}

I think it would be worthwhile to add that.

@virtuald
Copy link
Contributor Author

Yes, it would be great to provide an API that does that. However, it's not something I need at the moment (as I'm storing offsets), and it seems like a lot of work.

In theory one could just translate the tail implementation into go... but that version of tail I linked above is GPL3 so that could be problematic. The BSD version is available at https://github.com/freebsd/freebsd/blob/master/usr.bin/tail/read.c#L137

Also, I think it would be useful to have an offset-based API in addition to a line-based API. I guess yet another function could be added for that?

@fstab
Copy link
Owner

fstab commented Oct 24, 2018

I can implement a SeekLines function. But first I would like to add support for tailing multiple log files, because this is currently the top issue with grok_exporter. I'll get back to this later...

@fstab fstab force-pushed the master branch 2 times, most recently from 50bbec7 to 801df77 Compare January 2, 2020 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants