OpenCue icon indicating copy to clipboard operation
OpenCue copied to clipboard

Add support for logging to Loki (and others)

Open lithorus opened this issue 1 year ago • 1 comments

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)

lithorus avatar Jul 01 '24 21:07 lithorus

I should also mention that I've replaced the pipe_to_file with a MUCH simpler method that "should" be cross-platform.

lithorus avatar Jul 01 '24 21:07 lithorus

It should be noted that I cannot re-produce the error in the "Python Unit Tests (CY2023)" locally (in the docker container).

lithorus avatar Jul 08 '24 23:07 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.

lithorus avatar Jul 16 '24 21:07 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.

ramonfigueiredo avatar Jul 16 '24 23:07 ramonfigueiredo

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.

lithorus avatar Jul 17 '24 07:07 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/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.

Great, thanks @lithorus !

ramonfigueiredo avatar Jul 17 '24 17:07 ramonfigueiredo

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.

Great, thanks @lithorus !

Have added some more details on it now.

lithorus avatar Jul 17 '24 18:07 lithorus

@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.

lithorus avatar Jul 22 '24 20:07 lithorus

@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.

DiegoTavares avatar Jul 24 '24 20:07 DiegoTavares

Will re-do the implementation.

lithorus avatar Jul 31 '24 19:07 lithorus