Add csv logging from daemon #432
Conversation
JakeRoggenbuck
changed the title
|
|
JakeRoggenbuck
requested review from
Camerooooon and
Shuzhengz
and removed request for
Camerooooon
src/daemon.rs
Outdated
|
|
||
| if let Some(name) = &self.settings.csv_file { | ||
| if !Path::new(name).exists() { | ||
| File::create(name).expect("CSV file could not be created."); | ||
| } | ||
| } |
There was a problem hiding this comment.
Maybe we should have an option to just override it on startup so u dont have to go in and delete the previous one every time
There was a problem hiding this comment.
New folders for each day? We could create a new CSV file hourly. We don't want massive individual files.
There was a problem hiding this comment.
yep sounds good, we can also just use the timestamp as the file name
There was a problem hiding this comment.
Good point about the size limit. It just writes out the current cpu objects with the to_csv to format them as strings then write it to a file. It does not store the whole thing in memory. I think a size limit would be very good so it does not break a hard drive.
There was a problem hiding this comment.
The file was getting multiple kilobytes in a few seconds
There was a problem hiding this comment.
yea, also the size limits are acutally all in mebibytes instead of megabites
|
The only thing I have not written yet is the "die after 5mb" for log file size |
|
@Shuzhengz Should I create the file size check in this pr or not? |
|
yeah i think it should be included in this pr, but we can also just make a new issue and deal with it later |
|
Yea, it's a very good thing to consider. I'll also check the permissions needed to create the csv file. I'll probably do it in this PR. |
|
Looks like i need all change requesters to review. Probably a good thing |
|
Cool, it has one review and I fixed all the requested changes, so i'm gonna merge it. |
fixes #359