-
-
Notifications
You must be signed in to change notification settings - Fork 165
dedup #193
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
dedup #193
Changes from all commits
03eef28
6914e98
37f449d
0615faf
d647ff2
9333cab
d67979c
c2cc0b9
de2d5db
28711ae
9433a77
f60e2ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,3 +4,4 @@ Cargo.lock | |
| .idea/ | ||
| *.iml | ||
| .vscode/ | ||
| log4rs.code-workspace | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "folders": [ | ||
| { | ||
| "path": "." | ||
| } | ||
| ], | ||
| "settings": {} | ||
| } | ||
| 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. | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)] | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(), | ||
| ) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tried that, format_args is amazingly hard to refactor
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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")] | ||
|
|
@@ -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 { | ||
|
|
@@ -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 _ = { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you dont need the |
||
| 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(()) | ||
|
|
@@ -66,6 +80,8 @@ impl FileAppender { | |
| FileAppenderBuilder { | ||
| encoder: None, | ||
| append: true, | ||
| #[cfg(feature = "dedup")] | ||
| dedup: false, | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -74,6 +90,8 @@ impl FileAppender { | |
| pub struct FileAppenderBuilder { | ||
| encoder: Option<Box<dyn Encode>>, | ||
| append: bool, | ||
| #[cfg(feature = "dedup")] | ||
| dedup: bool, | ||
| } | ||
|
|
||
| impl FileAppenderBuilder { | ||
|
|
@@ -91,6 +109,15 @@ impl FileAppenderBuilder { | |
| self | ||
| } | ||
|
|
||
| /// Determines if the appender will reject and count duplicate messages. | ||
pm100 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /// | ||
| /// 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(); | ||
|
|
@@ -104,9 +131,19 @@ impl FileAppenderBuilder { | |
| .create(true) | ||
| .open(&path)?; | ||
|
|
||
pm100 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| #[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())), | ||
|
|
@@ -150,6 +187,12 @@ impl Deserialize for FileAppenderDeserializer { | |
| if let Some(append) = config.append { | ||
| appender = appender.append(append); | ||
| } | ||
| #[cfg(feature = "dedup")] | ||
| let _ = { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can remove the |
||
| 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)?); | ||
| } | ||
|
|
||
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.
lets .gitignore this file, idk what that is
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.
I think you need to git rm --cached this file to remove it from the changeset