xqms / rosmon

ROS node launcher & monitoring daemon
Other
180 stars 47 forks source link

core: logger: Use append mode for log file #154

Closed amilcarlucas closed 1 year ago

amilcarlucas commented 3 years ago

…utputted logfile

This needs some testing yet.

I am using this together with this logrotate file:

copytruncate
nocompress
notifempty
olddir
/tmp/ros-*.log {
    sharedscripts
    postrotate
        pkill -SIGUSR1 "rosmon$"
    endscript
}
xqms commented 3 years ago

Hey, thanks for the PR!

I think from reading the linked stackoverflow posts that it should be enough to open the file with O_APPEND - maybe we should just switch to that instead of trying to detect that a truncation has happened.

Also, accessing m_file->_fileno is really unportable, we should at least use fileno(m_file).

xqms commented 3 years ago

The cleanest solution is probably to support the SIGHUP signal, which is usually used to signal log-generating processes that they should re-open the log file. That way, no data can be lost.

An alternative would be to integrate with the systemd journal framework and just use that for logging - it can handle log rotation / deletion natively.

amilcarlucas commented 3 years ago

Thanks, I just changed the patch

xqms commented 3 years ago

I removed the commented code and squashed the commits. Almost ready for merging, but somehow the CI does not work... I will need to investigate this tomorrow.

amilcarlucas commented 3 years ago

I tested the changes and they do not work, the log still does not get properly truncated :(

amilcarlucas commented 3 years ago

This s a new attempt, still untested.

amilcarlucas commented 3 years ago

It works fine :) But it is more complex than the append solution. I need to test the append solution again.

xqms commented 1 year ago

Since #172 has landed, your issue should be addressed by switching to --log=syslog. Therefore I'll close this.