Skip to content

Commit

Permalink
appender: replace chrono with time (tokio-rs#1652)
Browse files Browse the repository at this point in the history
This PR continues the work started in
tokio-rs#1646 to replace `chrono` with
`time`. I'll refer to @hawkw's motivation:

> Currently, `tracing-subscriber` supports the `chrono` crate for
> timestamp formatting, via a default-on feature flag. When this code
> was initially added to `tracing-subscriber`, the `time` crate did not
> have support for the timestamp formatting options we needed.
>
> Unfortunately, the `chrono` crate's maintenance status is now in
> question (see tokio-rs#1598). Furthermore, `chrono` depends on version 0.1 of
> the `time` crate, which contains a security vulnerability
> (https://rustsec.org/advisories/RUSTSEC-2020-0071.html). This
> vulnerability is fixed in more recent releases of `time`, but `chrono`
> still uses v0.1.

I've replaced chrono with time 0.3. Unfortunately, some of chrono's
builders for DateTimes are not present in `time`, which required the
usage of `macro_rules!` macros to construct some of the hard-coded
times. I also took the opportunity to tidy some of the tests and change
the internal representation of `Rotation::NEVER` from year 9,999 to an
`Option::None`.

This branch changes `tracing-appender`'s MSRV from Rust 1.42 to Rust
1.51, the MSRV for the `time` crate when certain required feature flags
are enabled. This does *not* effect the MSRV for other crates in this
repository.
  • Loading branch information
davidbarsky authored and kaffarell committed May 22, 2024
1 parent 67a560b commit 81d4639
Showing 1 changed file with 105 additions and 0 deletions.
105 changes: 105 additions & 0 deletions tracing-appender/src/inner.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
use std::io::{BufWriter, Write};
use std::{fs, io};

use crate::rolling::Rotation;
use std::fmt::Debug;
use std::fs::{File, OpenOptions};
use std::path::Path;
use time::OffsetDateTime;

#[derive(Debug)]
pub(crate) struct InnerAppender {
log_directory: String,
log_filename_prefix: String,
writer: BufWriter<File>,
next_date: Option<OffsetDateTime>,
rotation: Rotation,
}

impl io::Write for InnerAppender {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
let now = OffsetDateTime::now_utc();
self.write_timestamped(buf, now)
}

fn flush(&mut self) -> io::Result<()> {
self.writer.flush()
}
}

impl InnerAppender {
pub(crate) fn new(
log_directory: &Path,
log_filename_prefix: &Path,
rotation: Rotation,
now: OffsetDateTime,
) -> io::Result<Self> {
let log_directory = log_directory.to_str().unwrap();
let log_filename_prefix = log_filename_prefix.to_str().unwrap();

let filename = rotation.join_date(log_filename_prefix, &now);
let next_date = rotation.next_date(&now);

Ok(InnerAppender {
log_directory: log_directory.to_string(),
log_filename_prefix: log_filename_prefix.to_string(),
writer: create_writer(log_directory, &filename)?,
next_date,
rotation,
})
}

fn write_timestamped(&mut self, buf: &[u8], date: OffsetDateTime) -> io::Result<usize> {
// Even if refresh_writer fails, we still have the original writer. Ignore errors
// and proceed with the write.
let buf_len = buf.len();
self.refresh_writer(date);
self.writer.write_all(buf).map(|_| buf_len)
}

fn refresh_writer(&mut self, now: OffsetDateTime) {
if self.should_rollover(now) {
let filename = self.rotation.join_date(&self.log_filename_prefix, &now);

self.next_date = self.rotation.next_date(&now);

match create_writer(&self.log_directory, &filename) {
Ok(writer) => {
if let Err(err) = self.writer.flush() {
eprintln!("Couldn't flush previous writer: {}", err);
}
self.writer = writer
}
Err(err) => eprintln!("Couldn't create writer for logs: {}", err),
}
}
}

fn should_rollover(&self, date: OffsetDateTime) -> bool {
// the `None` case means that the `InnerAppender` *never* rorates log files.
match self.next_date {
None => false,
Some(next_date) => date >= next_date,
}
}
}

fn create_writer(directory: &str, filename: &str) -> io::Result<BufWriter<File>> {
let file_path = Path::new(directory).join(filename);
Ok(BufWriter::new(open_file_create_parent_dirs(&file_path)?))
}

fn open_file_create_parent_dirs(path: &Path) -> io::Result<File> {
let mut open_options = OpenOptions::new();
open_options.append(true).create(true);

let new_file = open_options.open(path);
if new_file.is_err() {
if let Some(parent) = path.parent() {
fs::create_dir_all(parent)?;
return open_options.open(path);
}
}

new_file
}

0 comments on commit 81d4639

Please sign in to comment.