Telodendria/Cytoplasm
Telodendria/Cytoplasm
3
3
Fork 4
Code Issues 13 Pull requests 2 Projects Releases 2 Packages Activity Actions

WIP: Rework logging #48

Draft
Levitating wants to merge 2 commits from Levitating/Cytoplasm:log into master
pull from: Levitating/Cytoplasm:log
merge into: Telodendria:master
Conversation 2 Commits 2 Files changed 2 +62 -64
Levitating commented 2024-08-25 22:11:42 +00:00
Contributor
Copy link

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
Copy link

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>
lda commented 2024-08-25 23:08:13 +00:00
Contributor
Copy link

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.
Levitating commented 2024-08-26 17:45:34 +00:00
Author
Contributor
Copy link

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
lda commented 2024-08-25 23:13:44 +00:00
Contributor
Copy link

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.
Levitating commented 2024-08-26 17:45:04 +00:00
Author
Contributor
Copy link

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
jordan commented 2024-08-27 13:42:29 +00:00
Owner
Copy link

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.
Levitating commented 2024-08-27 17:01:05 +00:00
Author
Contributor
Copy link

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.
Reviewers
No reviewers
lda
Labels
Clear labels   
bug

Something is not working

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
question

More information is needed

  
wontfix

This won't be fixed

No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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.
Delete branch "Levitating/Cytoplasm:log"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?