stream cvd logs#2798
Conversation
jemoreira
left a comment
There was a problem hiding this comment.
The code LGTM, but I have two main concerns with the feature itself:
streaming beyond stop
The launcher.log file (for example) is deleted and created anew on cvd start. If stop and start are called while launcher.log is being streamed, the stream will continue as if nothing had happened, it will not fail and will not include the contents of the new created file. This happens because the HO holds an fd to old launcher.log file, which is unlinked from the file system but not deleted until this fd is closed.
The ideal behavior for the log file streaming is for the stream to end when the device is stopped (or crashes or ..., basically anything that moves it to a non-active state).
Can't download a single file
While the bugreport endpoint allows downloading a snapshot of the logs, it doesn't allow downloading a single file. That's still a nice feature to have. The logs endpoint was designed for this specific feature but with this change it can't serve this purpose anymore as it will never stop streaming the file until the client disconnects and the client doesn't have a way to know when the end of the file was reached.
Is it possible to allow both behaviors selected by a URL parameter?
| } | ||
| } | ||
| // Live tailing loop | ||
| ticker := time.NewTicker(500 * time.Millisecond) |
There was a problem hiding this comment.
nit: The more elegant approach to this problem is to use something like fsnotify to ask the OS to notify us of changes to the underlying file instead of waking up every 500 milliseconds to try to read it. This is more relevant with rarely modified files like the kernel log.
8310d29 to
5c43076
Compare
Bug: b/529882198
Assisted-by: Jetski:Gemini-Next Bug: b/529882198
Bug: b/529882198
Remove time.NewTicker and use rather fsnotify to stream cvd logs. Assisted-by: Jetski:Gemini-Next Bug: b/529882198
Staticcheck error: "io/ioutil" has been deprecated since Go 1.19.
No description provided.