martomi / chiadog

A watch dog providing a peace in mind that your Chia farm is running smoothly 24/7.
MIT License
457 stars 121 forks source link

use pythonic way to read log #102

Closed kungfoome closed 3 years ago

kungfoome commented 3 years ago

I was trying to run this with WSL to see if that would work. It seems like there is an issue where you can't follow a file in WSL without the ---disable-inotify flag on tail. I tested this and added that option in the log consumer and it does work after that. I noticed there have been issues with Windows as well during log rotations. I'm not sure if switching over to a pythonic way of reading the debug file will solve this, but it seems like it would be better since you don't need differentiate between OSes either.

Not sure if remote would work with this either, so it would require a bit of testing.

https://github.com/martomi/chiadog/blob/da1a904466e6df25b930d58cefb70eca6dce4a3a/src/chia_log/log_consumer.py#L72-L80

Maybe something like: https://stackoverflow.com/questions/12523044/how-can-i-tail-a-log-file-in-python/53121178#53121178

Edit:

Initially something like this appears to work with little resources:

    def _consume_loop(self):
        expanded_user_log_path = self._log_path.expanduser()
        logging.info(f"Consuming log file from {expanded_user_log_path}")

        try:
            fp = open(expanded_user_log_path, "r")
            st_results = os.stat(expanded_user_log_path)
            st_size = st_results[6]
            fp.seek(st_size)

            while self._is_running:
                where = fp.tell()
                line = fp.readline()
                if not line:
                    time.sleep(1)
                    fp.seek(where)
                elif fp.tell() > os.path.getsize(expanded_user_log_path):
                    logging.info("Log rotated, opening latest log file")
                    fp.close()
                    fp = open(expanded_user_log_path, "r")
                    fp.seek(0,2)
                else:
                    self._notify_subscribers(line)
        except Exception as e:
            fp.close()
            logging.critical(f"Error consuming log file: {e}")

I can run it for a bit and see what happens during rotations as well. This seems to work the same for windows.

martomi commented 3 years ago

Thanks @kungfoome! I see you have some very specific suggestions that are more suitable for a PR. You can fork the repository and submit a PR towards the dev branch here directly from your fork. Then it'll be easier to review and test the changes.

@pieterhelsen Check this out, maybe a fix for the Windows issues with log rotations.

pieterhelsen commented 3 years ago

Definitely worth a try and would be nice if it worked cross-platform. My work on the log rotation issue is available here: https://github.com/martomi/chiadog/tree/windows-rotation

It's tested on Linux local, Linux remote and Windows local, but I can't test Windows remote without setting up a dummy node

pieterhelsen commented 3 years ago

@kungfoome I just implemented your solution and one other, but did not get either of them to work reliably on my test setup due to the fact that open() locks up the file and as such makes my dummy LogWriter crash when it tries to rotate.

Now, it is possible that Chia uses another method that handles these rotations better, but I am currently unable to test this on my machine. Were you able to try this out further?

martomi commented 3 years ago

Chia uses this:

https://github.com/Chia-Network/chia-blockchain/blob/1c808b6c2910ed32fdbfdfc576ba1bc5a5adeac9/setup.py#L15

You can see how they use it here:

https://github.com/Chia-Network/chia-blockchain/blob/59de4ffe9f98de62d281695d1f519d65ef2e2ece/chia/util/chia_logging.py#L6

pieterhelsen commented 3 years ago

I'll update my dummy logger to use this package and see if that alters behavior. From the description, it should!

kungfoome commented 3 years ago

Yeah sorry. Haven't had time to follow up. I get the same behavior, although I don't think it's really because of open. That shouldn't create a lock on the file, but I think it's more so that it continues to read from the file and it doesn't have time to rotate. I think if you were to read x amount of bytes and set a timeout period, it would probably be ok.

But im interested in seeing if that other library works as well, as that would be much easier than trying to implement something from scratch.

pieterhelsen commented 3 years ago

When using the Concurrent Log Handler it doesn't produce any errors, but reading the logfile in a pythonic still locks up the file, and prevents log rotation. (at least on Windows)

So it's a little more robust, but the issue persists.

martomi commented 3 years ago

164 added improvements, the discussion should continue in #72 if there are still issues