Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

logging: add support for writing to journald #34

Merged
merged 1 commit into from
Nov 21, 2019
Merged

Conversation

jabedude
Copy link
Contributor

@jabedude jabedude commented Nov 18, 2019

@lucab could you please review my initial pass at adding journald logging support.

  1. I only have the core logic of forwarding messages to journald in place

  2. I am not able to pass journald a memfd and create a log entry. I've done a bit of research and only found some issues in nix with passing multiple fds, but that shouldn't matter in our case.

Close: #32

src/journal.rs Outdated Show resolved Hide resolved
@jabedude
Copy link
Contributor Author

Got the memfd message working. It needed to have been sealed before being passed to journald.

src/journal.rs Outdated Show resolved Hide resolved
src/journal.rs Outdated Show resolved Hide resolved
src/journal.rs Outdated Show resolved Hide resolved
src/journal.rs Outdated Show resolved Hide resolved
src/journal.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/journal.rs Outdated Show resolved Hide resolved
src/journal.rs Outdated Show resolved Hide resolved
@lucab
Copy link
Owner

lucab commented Nov 19, 2019

@jabedude nice! I did a first review pass and run the tests, I got the correct output in the journal. I think sealing is not strictly required but it plays better with the receiving logic in systemd-journald, so I'm ok with doing that.

@jabedude jabedude force-pushed the journal branch 4 times, most recently from 46e172b to 16e5ca2 Compare November 19, 2019 16:19
@jabedude jabedude marked this pull request as ready for review November 19, 2019 18:07
@jabedude jabedude changed the title [WIP] - journald logging support journald logging support Nov 19, 2019
@jabedude
Copy link
Contributor Author

@lucab thanks! I integrated your comments and made some small tweaks. I think the PR is ready for review!

@lucab lucab changed the title journald logging support logging: add support for writing to journald Nov 19, 2019
Copy link
Owner

@lucab lucab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left a few minor comments, but I plan to have a deeper pass tomorrow. Can you please squash into a single commit in the meanwhile?

src/logging.rs Outdated Show resolved Hide resolved
src/logging.rs Outdated Show resolved Hide resolved
src/logging.rs Show resolved Hide resolved
@jabedude jabedude force-pushed the journal branch 2 times, most recently from d0e2476 to d21e607 Compare November 19, 2019 19:07
@jabedude
Copy link
Contributor Author

Sure thing

src/logging.rs Outdated Show resolved Hide resolved
src/logging.rs Outdated Show resolved Hide resolved
src/logging.rs Outdated Show resolved Hide resolved
@lucab
Copy link
Owner

lucab commented Nov 21, 2019

@jabedude thanks for all your work on this! This PR looks fine to me now. Is there anything else you planned to add here, or can I go ahead and merge?

@jabedude
Copy link
Contributor Author

My pleasure! No, I think this is good to merge as is. I think we might want to add methods for consumers to log with arbitrary fields and data later though, but I need to look closer at sd_journal_sendv to decide on the API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

journald logging support tracking issue
2 participants