tracing-journald: Write literal string values to journal #1714
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@Ralith Would you mind to take a look? I'm not entirely happy with this code, but I'm not sure how to make it better, short of a refactoring of all these |
Ralith
reviewed
Nov 10, 2021
swsnr
changed the title
Ralith
requested changes
Nov 11, 2021
|
@hawkw I've rebased onto master and address all remaining change requests; would you mind to take another look? 😊 |
hawkw
approved these changes
Nov 20, 2021
hawkw
pushed a commit
that referenced
this pull request
Dec 2, 2021
See #1710: Do not write strings in Debug representation. ## Motivation As discussed in #1710 writing strings literally makes tracing-journald behave like other journal clients, and allows 3rd party journal readers to extract the original value from the journal without having to "un"-parse the Debug representation of Rust strings. Fixes #1710.
Merged
hawkw
pushed a commit
that referenced
this pull request
Dec 3, 2021
See #1710: Do not write strings in Debug representation. ## Motivation As discussed in #1710 writing strings literally makes tracing-journald behave like other journal clients, and allows 3rd party journal readers to extract the original value from the journal without having to "un"-parse the Debug representation of Rust strings. Fixes #1710.
hawkw
pushed a commit
that referenced
this pull request
Dec 19, 2021
See #1710: Do not write strings in Debug representation. ## Motivation As discussed in #1710 writing strings literally makes tracing-journald behave like other journal clients, and allows 3rd party journal readers to extract the original value from the journal without having to "un"-parse the Debug representation of Rust strings. Fixes #1710.
hawkw
pushed a commit
that referenced
this pull request
Dec 19, 2021
See #1710: Do not write strings in Debug representation. ## Motivation As discussed in #1710 writing strings literally makes tracing-journald behave like other journal clients, and allows 3rd party journal readers to extract the original value from the journal without having to "un"-parse the Debug representation of Rust strings. Fixes #1710.
hawkw
pushed a commit
that referenced
this pull request
Dec 20, 2021
See #1710: Do not write strings in Debug representation. ## Motivation As discussed in #1710 writing strings literally makes tracing-journald behave like other journal clients, and allows 3rd party journal readers to extract the original value from the journal without having to "un"-parse the Debug representation of Rust strings. Fixes #1710.
hawkw
added a commit
that referenced
this pull request
Dec 30, 2021
# 0.2.1 (December 29, 2021) This release improves how `tracing-journald` communicates with `journald`, including the handling of large payloads. ### Added - Use an unconnected socket, so that logging can resume after a `journald` restart ([#1758]) ### Fixed - Fixed string values being written using `fmt::Debug` ([#1714]) - Fixed `EMSGSIZE` when log entries exceed a certain size ([#1744]) A huge thank-you to new contributor @lunaryorn, for contributing all of the changes in this release! [#1714]: #1714 [#1744]: #1744 [#1758]: #1758
hawkw
added a commit
that referenced
this pull request
Dec 30, 2021
# 0.2.1 (December 29, 2021) This release improves how `tracing-journald` communicates with `journald`, including the handling of large payloads. ### Added - Use an unconnected socket, so that logging can resume after a `journald` restart ([#1758]) ### Fixed - Fixed string values being written using `fmt::Debug` ([#1714]) - Fixed `EMSGSIZE` when log entries exceed a certain size ([#1744]) A huge thank-you to new contributor @lunaryorn, for contributing all of the changes in this release! [#1714]: #1714 [#1744]: #1744 [#1758]: #1758
Ralith
reviewed
Dec 29, 2022
Comment on lines
+334
to
+336
| /// `value` must not contain an internal newline. | ||
| /// | ||
| /// For a "newline-safe" variant, see `put_field_length_encoded`. |
There was a problem hiding this comment.
Revisiting these comments, is this correct? We're using length-prefixing, so per https://systemd.io/JOURNAL_NATIVE_PROTOCOL/ internal newlines in the value are fine. put_field_length_encoded uses exactly the same encoding, it just doesn't require the length to be known in advance.
There was a problem hiding this comment.
Perhaps not. It really looks as if both function are the same…but I only took a superficial look 🤔
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See #1710: Do not write strings in Debug representation.
Motivation
As discussed in #1710 writing strings literally makes tracing-journald behave like other journal clients, and allows 3rd party journal readers to extract the original value from the journal without having to "un"-parse the Debug representation of Rust strings.