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.

Sign up for GitHub

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

Jump to bottom

TimedRotatingLogger #1364

Open
j-c-cook opened this issue Aug 7, 2022 · 5 comments · May be fixed by #1365
Open

TimedRotatingLogger #1364

j-c-cook opened this issue Aug 7, 2022 · 5 comments · May be fixed by #1365
Labels
enhancement

Comments

@j-c-cook
Copy link
Contributor

j-c-cook commented Aug 7, 2022

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 #1359 (comment).
@j-c-cook j-c-cook linked a pull request Aug 7, 2022 that will close this issue
Open
4 tasks
@zariiii9003
Copy link
Collaborator

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

@j-c-cook
Copy link
Contributor Author

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

@zariiii9003
Copy link
Collaborator

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.

@j-c-cook
Copy link
Contributor Author

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.

@felixdivo
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
3 participants
@felixdivo @j-c-cook @zariiii9003