Implement rotating file logger

[zariiii9003]

See #874

Any API or testing suggestions are very welcome.

[codecov]

Codecov Report

Merging #881 into develop will increase coverage by 0.37%.
The diff coverage is 90.58%.

@@             Coverage Diff             @@
##           develop     #881      +/-   ##
===========================================
+ Coverage    70.71%   71.08%   +0.37%     
===========================================
  Files           71       71              
  Lines         6915     6976      +61     
===========================================
+ Hits          4890     4959      +69     
+ Misses        2025     2017       -8     

[christiansandberg]

I remember we discussed this topic once and the challenge is that there are an infinite number of log rotation strategies so if we want to add support for one, we have to be prepared to include support for a large number of strategies, like size based, date and time based, or some external trigger like current test case or something. There will also be a need to add support to the logger CLI.

I'm not totally against it, it just needs to be carefully designed so it won't come back and bite us in the ass later on. :)

[zariiii9003]

Here is an alternative. This one is similar to the python builtins logger. I copied some code from here. I thought a familiar API might be beneficial.

What do you think?

class BaseRotatingCanLogger(Listener, ABC):
    supported_writers = {
        ".asc": ASCWriter,
        ".blf": BLFWriter,
        ".csv": CSVWriter,
        ".log": CanutilsLogWriter,
        ".txt": Printer,
    }
    namer = None
    rotator = None
    writer: FileIOMessageWriter

    def __init__(self, *args, **kwargs):
        self.writer_args = args
        self.writer_kwargs = kwargs

        self.rollover_count = 0

    def rotation_filename(self, default_name):
        if not callable(self.namer):
            result = default_name
        else:
            result = self.namer(default_name)
        return result

    def rotate(self, source: StringPathLike, dest: StringPathLike):
        if not callable(self.rotator):
            if os.path.exists(source):
                os.rename(source, dest)
        else:
            self.rotator(source, dest)

    def on_message_received(self, msg: Message):
        if self.should_rollover(msg):
            self.do_rollover()
            self.rollover_count += 1

        self.writer.on_message_received(msg)

    def get_new_writer(self, filename: StringPathLike) -> FileIOMessageWriter:
        suffix = pathlib.Path(filename).suffix.lower()
        try:
            writer_class = self.supported_writers[suffix]
        except KeyError:
            raise ValueError(
                f'Log format with suffix"{suffix}" is '
                f"not supported by {self.__class__.__name__}."
            )
        return writer_class(filename, *self.writer_args, **self.writer_kwargs)

    def on_error(self, exc: Exception):
        ...

    @abstractmethod
    def should_rollover(self, msg: Message) -> bool:
        ...

    @abstractmethod
    def do_rollover(self):
        ...


class SizedRotatingCanLogger(BaseRotatingCanLogger):
    def __init__(self, filename: StringPathLike, max_bytes: int = 0, *args, **kwargs):
        super(SizedRotatingCanLogger, self).__init__(*args, **kwargs)

        self.base_filename = os.path.abspath(filename)
        self.max_bytes = max_bytes

        self.writer = self.get_new_writer(self.base_filename)

    def should_rollover(self, msg: Message) -> bool:
        if self.writer.file.tell() >= self.max_bytes:
            return True

        return False

    def do_rollover(self):
        if self.writer:
            self.writer.stop()

        sfn = self.base_filename
        dfn = self.rotation_filename(self._default_name())
        self.rotate(sfn, dfn)

        self.writer = self.get_new_writer(self.base_filename)

    def _default_name(self) -> StringPathLike:
        path = pathlib.Path(self.base_filename)
        new_name = (
            path.stem
            + datetime.now().strftime("_%Y-%m-%d_%H-%M-%S")
            + f"_#{self.rollover_count:03}"
            + path.suffix
        )
        return str(path.parent / new_name)

[christiansandberg]

I like that!

[christiansandberg]

I like that!

[felixdivo]

Hey @zariiii9003, that's a great implementation! Nice code, documentation and tests.

The only thing that I might discuss is the naming of the *RotatingCanLogger. Do we need the "Can" in there, as the plain Logger also does not contain it? I'd leave it out.

Oh, and thanks for showing me the ellipsis object of Python ^^. It's quite rare (and I think usually pass is used instead) but we can keep it.

[zariiii9003]

Thank you for your review and your kind words @felixdivo
I updated the class names. Having CAN in the class names indeed seems a bit redundant in a can library.

[felixdivo]

Looks good 👍

[felixdivo]

@christiansandberg Do you also approve these changes? If yes, we can merge.