-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add logic to support sending structured entries to the journald log #44
Conversation
A bunch of high-level things:
|
01e4ec6
to
d1ee2e2
Compare
) -> Result<()> | ||
where | ||
K: AsRef<str>, | ||
V: AsRef<str>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is OK, though... if we want to truly match the API semantics, the value could technically just be bytes. UTF-8 is recommended, but binary is fully supported. So this could even be AsRef<[u8]>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like both the C and the Go API handle strings, and even binary stuff like MESSAGE_ID
is hex-encoded. I think that handling strings here is ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The protocol itself allows for any arbitrary binary data. systemd-coredump
for example can be configured to store full coredump in the journal (which is why journal messages have a crazy limit like 768M). And in rpm-ostree, I almost made use of binary values as well in the journal when implementing coreos/rpm-ostree#1813.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(To be clear, this WFM as is too, just wanted to flag we weren't exposing the full capabilities of the API. We can always add a separate API later on if the need arises.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it is starting to get tricky. The C API has a further split between _send
and _sendv
for the string-vs-bytes topic.
I'd say we keep this signature for the moment and then iterate on a lower level helper on the side. I think in that case having split priority/message does not make sense either.
d1ee2e2
to
a70701a
Compare
a70701a
to
221b5a2
Compare
221b5a2
to
3366618
Compare
Travis reporting is still KO. Manually triggered a build, status is green: https://travis-ci.org/github/lucab/libsystemd-rs/builds/677601755. Merging. |
Fixes #43 and enable us to use that logic in the
afterburn
repo to solve(coreos/afterburn#395)