Skip to content
Closed

dedup #193

Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ Cargo.lock
.idea/
*.iml
.vscode/
log4rs.code-workspace
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ json_format = ["serde_json"]
toml_format = ["toml"]
xml_format = ["serde-xml-rs"]

console_appender = ["console_writer", "simple_writer", "pattern_encoder"]
console_appender = ["parking_lot","console_writer", "simple_writer", "pattern_encoder"]
file_appender = ["parking_lot", "simple_writer", "pattern_encoder"]
rolling_file_appender = ["parking_lot", "simple_writer", "pattern_encoder"]
compound_policy = []
Expand All @@ -32,7 +32,7 @@ console_writer = ["ansi_writer", "libc", "winapi"]
simple_writer = []
threshold_filter = []
background_rotation = []

dedup=[]
all_components = [
"console_appender",
"file_appender",
Expand Down
8 changes: 8 additions & 0 deletions log4rs.code-workspace
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"folders": [
{
"path": "."
}
],
"settings": {}
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets .gitignore this file, idk what that is

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to git rm --cached this file to remove it from the changeset

62 changes: 53 additions & 9 deletions src/append/console.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,8 @@
//!
//! Requires the `console_appender` feature.

use log::Record;
#[cfg(feature = "file")]
use serde_derive::Deserialize;
use std::{
error::Error,
fmt,
io::{self, Write},
};

#[cfg(feature = "dedup")]
use crate::append::dedup::*;
#[cfg(feature = "file")]
use crate::encode::EncoderConfig;
#[cfg(feature = "file")]
Expand All @@ -28,14 +21,26 @@ use crate::{
},
priv_io::{StdWriter, StdWriterLock},
};
use log::Record;

#[cfg(feature = "dedup")]
use parking_lot::Mutex;
#[cfg(feature = "file")]
use serde_derive::Deserialize;
use std::{
error::Error,
fmt,
io::{self, Write},
};
/// The console appender's configuration.
#[cfg(feature = "file")]
#[derive(Deserialize)]
#[serde(deny_unknown_fields)]
pub struct ConsoleAppenderConfig {
target: Option<ConfigTarget>,
encoder: Option<EncoderConfig>,
#[cfg(feature = "dedup")]
dedup: Option<bool>,
}

#[cfg(feature = "file")]
Expand Down Expand Up @@ -112,6 +117,8 @@ impl<'a> encode::Write for WriterLock<'a> {
pub struct ConsoleAppender {
writer: Writer,
encoder: Box<dyn Encode>,
#[cfg(feature = "dedup")]
deduper: Option<Mutex<DeDuper>>,
}

impl fmt::Debug for ConsoleAppender {
Expand All @@ -125,6 +132,14 @@ impl fmt::Debug for ConsoleAppender {
impl Append for ConsoleAppender {
fn append(&self, record: &Record) -> Result<(), Box<dyn Error + Sync + Send>> {
let mut writer = self.writer.lock();
#[cfg(feature = "dedup")]
let _ = {
if let Some(dd) = &self.deduper {
if dd.lock().dedup(&mut writer, &*self.encoder, record)? == DedupResult::Skip {
return Ok(());
}
}
};
self.encoder.encode(&mut writer, record)?;
writer.flush()?;
Ok(())
Expand All @@ -139,14 +154,19 @@ impl ConsoleAppender {
ConsoleAppenderBuilder {
encoder: None,
target: Target::Stdout,
#[cfg(feature = "dedup")]
dedup: false,
}
}
}

/// A builder for `ConsoleAppender`s.
pub struct ConsoleAppenderBuilder {
encoder: Option<Box<dyn Encode>>,

target: Target,
#[cfg(feature = "dedup")]
dedup: bool,
}

impl ConsoleAppenderBuilder {
Expand All @@ -163,6 +183,14 @@ impl ConsoleAppenderBuilder {
self.target = target;
self
}
/// Determines if the appender will reject and count duplicate messages.
///
/// Defaults to `false`.
#[cfg(feature = "dedup")]
pub fn dedup(mut self, dedup: bool) -> ConsoleAppenderBuilder {
self.dedup = dedup;
self
}

/// Consumes the `ConsoleAppenderBuilder`, producing a `ConsoleAppender`.
pub fn build(self) -> ConsoleAppender {
Expand All @@ -176,12 +204,22 @@ impl ConsoleAppenderBuilder {
None => Writer::Raw(StdWriter::stdout()),
},
};
#[cfg(feature = "dedup")]
let deduper = {
if self.dedup {
Some(Mutex::new(DeDuper::default()))
} else {
None
}
};

ConsoleAppender {
writer,
encoder: self
.encoder
.unwrap_or_else(|| Box::new(PatternEncoder::default())),
#[cfg(feature = "dedup")]
deduper,
}
}
}
Expand Down Expand Up @@ -233,6 +271,12 @@ impl Deserialize for ConsoleAppenderDeserializer {
if let Some(encoder) = config.encoder {
appender = appender.encoder(deserializers.deserialize(&encoder.kind, encoder.config)?);
}
#[cfg(feature = "dedup")]
let _ = {
if let Some(dedup) = config.dedup {
appender = appender.dedup(dedup);
}
};
Ok(Box::new(appender.build()))
}
}
94 changes: 94 additions & 0 deletions src/append/dedup.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
//! The dedup common handler
//!
use crate::append::Error;
use crate::encode::{Encode, Write};
use log::Record;

const REPEAT_COUNT: i32 = 1000;
/// dedup object to be used by deduping appender.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newline above please

/// internals are private to dedup
#[derive(Default)]
pub struct DeDuper {
count: i32,
last: String,
}
#[derive(PartialEq)]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newline above, comments first, then derives then struct decl please

/// Used by an appender that uses dedup.
/// Indicates whether or not the current message should be output.
///
/// sample use from console appender
/// if let Some(dd) = &self.deduper {
/// if dd.lock().dedup(&mut *file, &*self.encoder, record)? == DedupResult::Skip {
/// return Ok(());
/// }
/// ... output the message
pub enum DedupResult {
/// skip
Skip,
/// write
Write,
}
impl DeDuper {
// emits the extra line saying 'last line repeated n times'
fn write(
w: &mut dyn Write,
encoder: &dyn Encode,
record: &Record,
n: i32,
) -> Result<(), Box<dyn Error + Sync + Send>> {
if n == 1 {
encoder.encode(
w,
&Record::builder()
.args(format_args!("last message repeated, suppressing dups"))
.level(record.level())
.target(record.target())
.module_path_static(None)
.file_static(None)
.line(None)
.build(),
)
} else {
encoder.encode(
w,
&Record::builder()
.args(format_args!("last message repeated {} times", n))
.level(record.level())
.target(record.target())
.module_path_static(None)
.file_static(None)
.line(None)
.build(),
)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of calling these encode fns in the if else branches can we just return the msg and call the encode after in one place? Seems like a lot of repeated lines

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tried that, format_args is amazingly hard to refactor

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking something like this

let args = if n == 1 {
  format_args!("last message repeated, suppressing dups")
} else {
  format_args!("last message repeated {} times", n)
};
encoder.encode(
    w,
    &Record::builder()
        .args(args)
        .level(record.level())
        .target(record.target())
        .module_path_static(None)
        .file_static(None)
        .line(None)
        .build(),
)

}
}

/// Appender calls this.
/// If it returns Skip then appender should not write
/// If it returns Write then the appender should write as per normal
pub fn dedup(
&mut self,
w: &mut dyn Write,
encoder: &dyn Encode,
record: &Record,
) -> Result<DedupResult, Box<dyn Error + Sync + Send>> {
let msg = format!("{}", *record.args());
if msg == self.last {
self.count += 1;

// every now and then keep saying we saw lots of dups
if self.count % REPEAT_COUNT == 0 || self.count == 1 {
Self::write(w, encoder, record, self.count)?;
}
Ok(DedupResult::Skip)
} else {
self.last = msg;
let svct = self.count;
self.count = 0;
if svct > 1 {
Self::write(w, encoder, record, svct)?;
}
Ok(DedupResult::Write)
}
}
}
43 changes: 43 additions & 0 deletions src/append/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ use std::{
path::{Path, PathBuf},
};

#[cfg(feature = "dedup")]
use crate::append::dedup::*;
#[cfg(feature = "file")]
use crate::encode::EncoderConfig;
#[cfg(feature = "file")]
Expand All @@ -31,13 +33,17 @@ pub struct FileAppenderConfig {
path: String,
encoder: Option<EncoderConfig>,
append: Option<bool>,
#[cfg(feature = "dedup")]
dedup: Option<bool>,
}

/// An appender which logs to a file.
pub struct FileAppender {
path: PathBuf,
file: Mutex<SimpleWriter<BufWriter<File>>>,
encoder: Box<dyn Encode>,
#[cfg(feature = "dedup")]
deduper: Option<Mutex<DeDuper>>,
}

impl fmt::Debug for FileAppender {
Expand All @@ -52,6 +58,14 @@ impl fmt::Debug for FileAppender {
impl Append for FileAppender {
fn append(&self, record: &Record) -> Result<(), Box<dyn Error + Sync + Send>> {
let mut file = self.file.lock();
#[cfg(feature = "dedup")]
let _ = {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you dont need the let _ =

if let Some(dd) = &self.deduper {
if dd.lock().dedup(&mut *file, &*self.encoder, record)? == DedupResult::Skip {
return Ok(());
}
}
};
self.encoder.encode(&mut *file, record)?;
file.flush()?;
Ok(())
Expand All @@ -66,6 +80,8 @@ impl FileAppender {
FileAppenderBuilder {
encoder: None,
append: true,
#[cfg(feature = "dedup")]
dedup: false,
}
}
}
Expand All @@ -74,6 +90,8 @@ impl FileAppender {
pub struct FileAppenderBuilder {
encoder: Option<Box<dyn Encode>>,
append: bool,
#[cfg(feature = "dedup")]
dedup: bool,
}

impl FileAppenderBuilder {
Expand All @@ -91,6 +109,15 @@ impl FileAppenderBuilder {
self
}

/// Determines if the appender will reject and count duplicate messages.
///
/// Defaults to `false`.
#[cfg(feature = "dedup")]
pub fn dedup(mut self, dedup: bool) -> FileAppenderBuilder {
self.dedup = dedup;
self
}

/// Consumes the `FileAppenderBuilder`, producing a `FileAppender`.
pub fn build<P: AsRef<Path>>(self, path: P) -> io::Result<FileAppender> {
let path = path.as_ref().to_owned();
Expand All @@ -104,9 +131,19 @@ impl FileAppenderBuilder {
.create(true)
.open(&path)?;

#[cfg(feature = "dedup")]
let deduper = {
if self.dedup {
Some(Mutex::new(DeDuper::default()))
} else {
None
}
};
Ok(FileAppender {
path,
file: Mutex::new(SimpleWriter(BufWriter::with_capacity(1024, file))),
#[cfg(feature = "dedup")]
deduper,
encoder: self
.encoder
.unwrap_or_else(|| Box::new(PatternEncoder::default())),
Expand Down Expand Up @@ -150,6 +187,12 @@ impl Deserialize for FileAppenderDeserializer {
if let Some(append) = config.append {
appender = appender.append(append);
}
#[cfg(feature = "dedup")]
let _ = {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can remove the let _ =

if let Some(dedup) = config.dedup {
appender = appender.dedup(dedup);
}
};
if let Some(encoder) = config.encoder {
appender = appender.encoder(deserializers.deserialize(&encoder.kind, encoder.config)?);
}
Expand Down
2 changes: 2 additions & 0 deletions src/append/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use crate::filter::FilterConfig;

#[cfg(feature = "console_appender")]
pub mod console;
#[cfg(feature = "dedup")]
pub mod dedup;
#[cfg(feature = "file_appender")]
pub mod file;
#[cfg(feature = "rolling_file_appender")]
Expand Down