python-can icon indicating copy to clipboard operation
python-can copied to clipboard

TimedRotatingLogger

Open j-c-cook opened this issue 2 years ago • 1 comments

Is your feature request related to a problem? Please describe.

The existing SizedRotatingLogger can be used to frequently (or infrequently) write CAN traces to a file by specifying a maximum file size with the -s option. The frequency can become periodic if the number of CAN traces being sent on the bus are nearly consistent or the dominant message(s) are periodic. My specific application involves a remote logger running embedded Linux with a cellular connection that transports the rolled over logged files via secure shell protocol. I have been trying to balance the number of files produced vs. total file size. More importantly thus far, I must limit the number of file transfers from a specific device due to the firewall settings on the server where the files are being transported to. Therefore, my application would be well suited for a TimedRotatingLogger, where I can define the amount of time in between rollovers.

Describe the solution you'd like

The TimedRotatingLogger would rollover after the specified amount of time as long as there were CAN messages observed in that time. If no CAN messages were sent on the bus in that time, an empty trace should not be written.

Describe alternatives you've considered

At this point in time I plan to focus on the implementation of a TimedRotatingLogger, but I wonder if both options should be able to be given. For example, if a user supplies a time and file size constraint, the first to occur (timeout or size limit) would trigger the rollover.

Additional context

The idea of the TimedRotatingLogger was (to my knowledge) originally introduced in #1359 by @zariiii9003 in https://github.com/hardbyte/python-can/issues/1359#issuecomment-1207256273.

j-c-cook avatar Aug 07 '22 18:08 j-c-cook

If we implement something like this, the API should be close to the builtin TimedRotatingFileHandler. But that looks super complicated to be honest.

zariiii9003 avatar Aug 07 '22 22:08 zariiii9003

@zariiii9003 Will PR #1365 be able to resolve this?

j-c-cook avatar Dec 25 '22 22:12 j-c-cook

If we implement something like this, the API should be close to the builtin TimedRotatingFileHandler. But that looks super complicated to be honest.

I still think we should create a separate TimedRotatingLogger class which is more configurable and has a familiar API. That said, if @hardbyte or @felixdivo merge #1365, i'm absolutely okay with it.

zariiii9003 avatar Dec 25 '22 22:12 zariiii9003

Are you in favor of a single API that allows for both a max_bytes and max_seconds to be input? In the case of one API, the determining rollover factor can be the first condition to be met. I think incorporating the option to have both inputs into one object provides flexibility. I'm not currently sure how I could simply create the same behavior with two separate (TimedRotatingLogger and SizedRotatingLogger) API's defined. Therefore, two separate objects may result in reduced functionality compared to the current implementation.

The description above of either/or rollover conditions is different from what I initially described in this issue. The implementation in #1365 achieves an additional goal; both time and size can be input, which is different than inputting time or size.

j-c-cook avatar Dec 26 '22 16:12 j-c-cook

If we implement something like this, the API should be close to the builtin TimedRotatingFileHandler.

This is very reasonable. Can we pass a TimedRotatingFileHandler directly to the underlying file type handlers? That way, it would be trivial to imitate the API. But I suppose that would be tricky since we would have to re-init the writers on each rollover and I do not see how that is supported by the class.

felixdivo avatar Dec 28 '22 11:12 felixdivo