WIP: Rework logging #48

Draft
Levitating wants to merge 2 commits from Levitating/Cytoplasm:log into master
Contributor

This is a draft PR.

I would like to redesign the logging output somewhat.
I want to be able to log to stdout, syslog and a file at the same time if the user desires to.

This code still needs to be tested with a related patch to Telodendria.

In the future for Telodendria I think it would be best if setting a logfile and use of stdout can be configured over command args.
This is the most unix way and should play well with most inits.
Of course that's not necessary.

Let me know what you think

This is a draft PR. I would like to redesign the logging output somewhat. I want to be able to log to stdout, syslog and a file at the same time if the user desires to. This code still needs to be tested with a related patch to Telodendria. In the future for Telodendria I think it would be best if setting a logfile and use of stdout can be configured over command args. This is the most unix way and should play well with most inits. Of course that's not necessary. Let me know what you think
Levitating added 1 commit 2024-08-25 22:11:42 +00:00
lda requested changes 2024-08-25 23:15:00 +00:00
lda left a comment
Contributor

Aside from some few notes, I think this looks good enough.

Aside from some few notes, I think this looks good enough.
src/Log.c Outdated
@ -26,6 +26,7 @@
#include <Memory.h>
#include <Util.h>
#include <cstdarg>
Contributor

I think you meant stdarg.h, as those kind of includes are C++isms.

I think you meant stdarg.h, as those kind of includes are C++isms.
Author
Contributor

It was most likely automatically added by my IDE by accident.

It was most likely automatically added by my IDE by accident.
Levitating marked this conversation as resolved
src/Log.c Outdated
@ -222,42 +223,14 @@ LogConfigUnindent(LogConfig * config)
}
void
Contributor

I'd probably consider making LogPrint static here, as it's not even part of the public includes.

I'd probably consider making LogPrint static here, as it's not even part of the public includes.
Author
Contributor

Yes it should be static!

Yes it should be static!
Levitating marked this conversation as resolved
Levitating added 1 commit 2024-08-26 21:32:09 +00:00
Owner

I think this is a good idea.

I want to be able to log to stdout, syslog and a file at the same time if the user desires to.

My initial design didn't do this because that seems pretty redundant (some init systems will redirect stdout to the syslog automatically, IIRC). That being said, it doesn't hurt to have the option.

In the future for Telodendria I think it would be best if setting a logfile and use of stdout can be configured over command args.

That's not a bad idea. I agree that it is more unix-y to do it this way. We already have the option to override the log level with the verbose flag, so this wouldn't be a far step from that.

I think this is a good idea. > I want to be able to log to stdout, syslog and a file at the same time if the user desires to. My initial design didn't do this because that seems pretty redundant (some init systems will redirect stdout to the syslog automatically, IIRC). That being said, it doesn't hurt to have the option. > In the future for Telodendria I think it would be best if setting a logfile and use of stdout can be configured over command args. That's not a bad idea. I agree that it is more unix-y to do it this way. We already have the option to override the log level with the verbose flag, so this wouldn't be a far step from that.
Author
Contributor

That's not a bad idea. I agree that it is more unix-y to do it this way. We already have the option to override the log level with the verbose flag, so this wouldn't be a far step from that.

Actually OpenRC expects a ("well-behaved") daemon to be able to background itself (with a -d daemon flag), set its own PID and use the syslog.

https://github.com/OpenRC/openrc/blob/master/service-script-guide.md#dont-write-your-own-startstop-functions

> That's not a bad idea. I agree that it is more unix-y to do it this way. We already have the option to override the log level with the verbose flag, so this wouldn't be a far step from that. Actually OpenRC expects a ("well-behaved") daemon to be able to background itself (with a -d daemon flag), set its own PID and use the syslog. https://github.com/OpenRC/openrc/blob/master/service-script-guide.md#dont-write-your-own-startstop-functions
This pull request is marked as a work in progress.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u log:Levitating-log
git checkout Levitating-log

Merge

Merge the changes and update on Forgejo.
git checkout master
git merge --no-ff Levitating-log
git checkout master
git merge --ff-only Levitating-log
git checkout Levitating-log
git rebase master
git checkout master
git merge --no-ff Levitating-log
git checkout master
git merge --squash Levitating-log
git checkout master
git merge --ff-only Levitating-log
git checkout master
git merge Levitating-log
git push origin master
Sign in to join this conversation.
No reviewers
lda
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Telodendria/Cytoplasm#48
No description provided.