Add support for logging to Loki (and others)
Summarize your change. Have introduces a new module and class for RQD which deals with writing the process log file.
This is to abstract file writing which gives the ability to support non-file based logging (eg. loki, elasticsearch)
I have not been able to test the code changes on Windows and Mac
(Still work-in-progress and based on the PySide6 PR)
I should also mention that I've replaced the pipe_to_file with a MUCH simpler method that "should" be cross-platform.
It should be noted that I cannot re-produce the error in the "Python Unit Tests (CY2023)" locally (in the docker container).
I'm afraid I had to undo the changes from #1335 since this changes how things are logged. Hopefully it should be easier to re-implement again.
I'm afraid I had to undo the changes from #1335 since this changes how things are logged. Hopefully it should be easier to re-implement again.
Hi @lithorus
Ok, I see.
How does your solution with Loki handle non-ASCII characters in the logs?
What happens with RQD when logs contain characters like these: 영화, café?
Does RQD crash with non-ASCII characters in the logs?
Additionally, I think it would be beneficial to provide a logging solution using Loki that can be enabled or disabled. This way, we can revert to the previous default logging methods if Loki logs are disabled.
So for me, this current solution should be kept if the Loki logs are disabled.
I'm afraid I had to undo the changes from #1335 since this changes how things are logged. Hopefully it should be easier to re-implement again.
Hi @lithorus
Ok, I see.
How does your solution with Loki handle non-ASCII characters in the logs?
What happens with RQD when logs contain characters like these:
영화, café?Does RQD crash with non-ASCII characters in the logs?
Additionally, I think it would be beneficial to provide a logging solution using Loki that can be enabled or disabled. This way, we can revert to the previous default logging methods if Loki logs are disabled.
So for me, this current solution should be kept if the Loki logs are disabled.
You're right, it would crash if it got non-ascii characters. I tested it out with cat /usr/bin/ps in the render command and it would crash. I've now implemented a generic way to convert (although I settled for utf-8 instead).
I will wrote the details on how Loki is enabled/disabled in the summary of this PR.
I'm afraid I had to undo the changes from #1335 since this changes how things are logged. Hopefully it should be easier to re-implement again.
Hi @lithorus Ok, I see. How does your solution with Loki handle non-ASCII characters in the logs? What happens with RQD when logs contain characters like these:
영화, café? Does RQD crash with non-ASCII characters in the logs? Additionally, I think it would be beneficial to provide a logging solution using Loki that can be enabled or disabled. This way, we can revert to the previous default logging methods if Loki logs are disabled. So for me, this current solution should be kept if the Loki logs are disabled.You're right, it would crash if it got non-ascii characters. I tested it out with
cat /usr/bin/psin the render command and it would crash. I've now implemented a generic way to convert (although I settled for utf-8 instead).I will wrote the details on how Loki is enabled/disabled in the summary of this PR.
Great, thanks @lithorus !
I'm afraid I had to undo the changes from #1335 since this changes how things are logged. Hopefully it should be easier to re-implement again.
Hi @lithorus Ok, I see. How does your solution with Loki handle non-ASCII characters in the logs? What happens with RQD when logs contain characters like these:
영화, café? Does RQD crash with non-ASCII characters in the logs? Additionally, I think it would be beneficial to provide a logging solution using Loki that can be enabled or disabled. This way, we can revert to the previous default logging methods if Loki logs are disabled. So for me, this current solution should be kept if the Loki logs are disabled.You're right, it would crash if it got non-ascii characters. I tested it out with
cat /usr/bin/psin the render command and it would crash. I've now implemented a generic way to convert (although I settled for utf-8 instead). I will wrote the details on how Loki is enabled/disabled in the summary of this PR.Great, thanks @lithorus !
Have added some more details on it now.
@DiegoTavares I've created another PR with only the log abstraction and without the Loki part. There was still missing a few bits and pieces on that part before it was ready for production and would like to explore a few ideas on how to make it more modular.
@DiegoTavares I've created another PR with only the log abstraction and without the Loki part. There was still missing a few bits and pieces on that part before it was ready for production and would like to explore a few ideas on how to make it more modular.
Great, I have it on my list to be reviewed soon.
As this change adds a new feature, it will require changes to the documentation on opencue.io's Other Guides section. This website is maintained in this repo.
Will re-do the implementation.