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

Use 'journalctl` for getting logs #98

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Use 'journalctl` for getting logs #98

wants to merge 1 commit into from

Conversation

miekg
Copy link
Collaborator

@miekg miekg commented Feb 4, 2021

This removes the CGO deps and makes for a completely static Go binary.
The downside is that is fork/execs a binary to get to the logs. It
supports the same options as the current impl. A thing that's different
is the time/date output which doesn't match up.

No real intentation to get this merged (current binary runs on RH8 which
I care about atm), but also don't want to loose the code as there were
some tricky bits about tailing the logs and not creating zombie
journalctls.

See #84

Signed-off-by: Miek Gieben [email protected]

This removes the CGO deps and makes for a completely static Go binary.
The downside is that is fork/execs a binary to get to the logs. It
supports the same options as the current impl. A thing that's different
is the time/date output which doesn't match up.

No real intentation to get this merged (current binary runs on RH8 which
I care about atm), but also don't want to loose the code as there were
some tricky bits about tailing the logs and not creating zombie
journalctls.

See #84

Signed-off-by: Miek Gieben <[email protected]>
@miekg miekg requested a review from pires as a code owner February 4, 2021 08:46
@pires
Copy link
Member

pires commented Feb 4, 2021

This discussion happened over slack but I'm sharing here for historic purposes.

I think we're in a bit of a mexican stand-off as I object to this. Wrapping the CLI was fine to get something started but it's not a clear contract, nor provides as much control, as the systemd D-BUS API, which, I think, we should adopt as the project matures. Yes, it introduces a dependency on CGO, because of go-systemd, but that shouldn't be a problem since this is a Linux-only project - it's not like systemd has been ported to other OSes, right? I understand some esoteric architectures may not be well supported, though, but I lack the data.

Also, FYI the code as is in main branch runs both on RHEL7 and 8, provided that systemd-devel is installed.

@pires
Copy link
Member

pires commented Feb 4, 2021

I am now thinking that maybe we could do with Go build tags, where the default builds with the code in main and a new tag that would be opt-in for the non-go implementation.

@miekg
Copy link
Collaborator Author

miekg commented Feb 5, 2021

let just leave this here and un-merged. Let put 'draft' on it, so accidental merge is harder to do

@miekg miekg marked this pull request as draft February 5, 2021 13:50
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.

2 participants