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.

Sign up for GitHub

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

fix: truncate log item strings #1227

Merged
merged 5 commits into from
Sep 12, 2023
Merged

fix: truncate log item strings #1227

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ada09ae
feat: limit log line length to 2048 chars
jonaro00 Sep 12, 2023
d9c9b98
log line touchups
jonaro00 Sep 12, 2023
f7037c5
feedback: reduce iterations and allocations when truncating
jonaro00 Sep 12, 2023
79dc2ed
print at better time
jonaro00 Sep 12, 2023
38197ec
cleanup
jonaro00 Sep 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feedback: reduce iterations and allocations when truncating
  • Loading branch information
@jonaro00
jonaro00 committed Sep 12, 2023
commit f7037c57f644e319e02f6aad367d7e5f92f8d929
53 changes: 49 additions & 4 deletions common/src/log.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,9 @@ const LOG_LINE_MAX_CHARS: usize = 2048;

impl LogItem {
pub fn new(id: Uuid, internal_origin: Backend, line: impl Into<String>) -> Self {
oddgrd marked this conversation as resolved.
Show resolved Hide resolved
let mut line = line.into();
if line.chars().count() > LOG_LINE_MAX_CHARS {
line = line.chars().take(LOG_LINE_MAX_CHARS).collect()
}
let mut line: String = line.into();

Self::truncate_line(&mut line);

Self {
id,
Expand All @@ -70,6 +69,26 @@ impl LogItem {
line,
}
}

fn truncate_line(line: &mut String) {
// Check if it can be over the limit (assuming ascii only), no iteration
if line.len() > LOG_LINE_MAX_CHARS {
// Then, check if it actually is over the limit.
// Find the char boundrary of the last char, but iterate no more than
// the max number of chars allowed.
let x = line
.char_indices()
.enumerate()
.find(|(i, _)| *i == LOG_LINE_MAX_CHARS);
// If char iterator reached max iteration count
if let Some((_, (ci, _))) = x {
// Truncate to the char boundrary found
line.truncate(ci);
// New allocation unlikely since it keeps its capacity
write!(line, "... (truncated)").expect("write to string");
}
}
}
}

#[cfg(feature = "display")]
Expand Down Expand Up @@ -289,4 +308,30 @@ mod tests {
assert!(log_line.contains(&utc_dt));
});
}

#[test]
fn log_item_truncate() {
let mut l = "öl".repeat(100);
LogItem::truncate_line(&mut l);
assert_eq!(l.len(), 300);
assert_eq!(l.chars().count(), 200);

let mut l = "🍪".repeat(LOG_LINE_MAX_CHARS);
LogItem::truncate_line(&mut l);
assert_eq!(l.len(), 4 * (LOG_LINE_MAX_CHARS));
assert_eq!(l.chars().count(), LOG_LINE_MAX_CHARS);

// one cookie should be truncated, and the suffix message should be appended
// ✨ = 3b, 🍪 = 4b

let mut l = format!("A{}", "🍪".repeat(LOG_LINE_MAX_CHARS));
LogItem::truncate_line(&mut l);
assert_eq!(l.len(), 1 + 4 * (LOG_LINE_MAX_CHARS - 1) + 15);
assert_eq!(l.chars().count(), LOG_LINE_MAX_CHARS + 15);

let mut l = format!("✨{}", "🍪".repeat(LOG_LINE_MAX_CHARS));
LogItem::truncate_line(&mut l);
assert_eq!(l.len(), 3 + 4 * (LOG_LINE_MAX_CHARS - 1) + 15);
assert_eq!(l.chars().count(), LOG_LINE_MAX_CHARS + 15);
}
}