Skip to content

Commit

Permalink
feat: remove some apis from DefaultLoggingLayer
Browse files Browse the repository at this point in the history
  • Loading branch information
@evenyag
evenyag committed Aug 8, 2024
1 parent a44c5c3 commit 48fbec1
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 112 deletions.
128 changes: 20 additions & 108 deletions core/src/layers/logging.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ use crate::*;
/// ```no_run
/// use crate::layers::LoggingInterceptor;
/// use crate::layers::LoggingLayer;
/// use crate::raw::Operation;
/// use crate::services;
/// use crate::Error;
/// use crate::Operator;
Expand All @@ -98,8 +99,9 @@ use crate::*;
/// fn log(
/// &self,
/// scheme: Scheme,
/// operation: &'static str,
/// operation: Operation,
/// path: &str,
/// context: &str,
/// message: &str,
/// err: Option<&Error>,
/// ) {
Expand Down Expand Up @@ -128,7 +130,7 @@ impl<I> Clone for LoggingLayer<I> {
impl Default for LoggingLayer {
fn default() -> Self {
Self {
notify: Arc::new(DefaultLoggingInterceptor::default()),
notify: Arc::new(DefaultLoggingInterceptor),
}
}
}
Expand Down Expand Up @@ -192,7 +194,6 @@ impl<I: LoggingInterceptor> LoggingContext<I> {
}
}

// TODO(yingwen): Update example.
/// LoggingInterceptor is used to intercept the log.
pub trait LoggingInterceptor: Debug + Send + Sync + 'static {
/// Everytime there is a log, this function will be called.
Expand Down Expand Up @@ -223,24 +224,9 @@ pub trait LoggingInterceptor: Debug + Send + Sync + 'static {
);
}

// TODO(yingwen): Remove fields.
/// The DefaultLoggingInterceptor will log the message by the standard logging macro.
#[derive(Debug)]
pub struct DefaultLoggingInterceptor {
error_level: Option<Level>,
failure_level: Option<Level>,
backtrace_output: bool,
}

impl Default for DefaultLoggingInterceptor {
fn default() -> Self {
Self {
error_level: Some(Level::Warn),
failure_level: Some(Level::Error),
backtrace_output: false,
}
}
}
pub struct DefaultLoggingInterceptor;

impl LoggingInterceptor for DefaultLoggingInterceptor {
fn log(
Expand All @@ -267,84 +253,22 @@ impl LoggingInterceptor for DefaultLoggingInterceptor {
return;
};

if let Some(lvl) = self.error_level(err) {
if self.print_backtrace(err) {
log!(
target: LOGGING_TARGET,
lvl,
"service={} operation={} path={} {} -> {} {:?}",
scheme,
operation,
path,
context,
message,
err,
)
} else {
log!(
target: LOGGING_TARGET,
lvl,
"service={} operation={} path={} {} -> {} {}",
scheme,
operation,
path,
context,
message,
err,
)
}
}
let lvl = self.error_level(err);
log!(
target: LOGGING_TARGET,
lvl,
"service={} operation={} path={} {} -> {} {}",
scheme,
operation,
path,
context,
message,
err,
);
}
}

impl DefaultLoggingInterceptor {
/// Setting the log level while expected error happened.
///
/// For example: accessor returns NotFound.
///
/// `None` means disable the log for error.
pub fn with_error_level(mut self, level: Option<&str>) -> Result<Self> {
if let Some(level_str) = level {
let level = level_str.parse().map_err(|_| {
Error::new(ErrorKind::ConfigInvalid, "invalid log level")
.with_context("level", level_str)
})?;
self.error_level = Some(level);
} else {
self.error_level = None;
}
Ok(self)
}

/// Setting the log level while unexpected failure happened.
///
/// For example: accessor returns Unexpected network error.
///
/// `None` means disable the log for failure.
pub fn with_failure_level(mut self, level: Option<&str>) -> Result<Self> {
if let Some(level_str) = level {
let level = level_str.parse().map_err(|_| {
Error::new(ErrorKind::ConfigInvalid, "invalid log level")
.with_context("level", level_str)
})?;
self.failure_level = Some(level);
} else {
self.failure_level = None;
}
Ok(self)
}

/// Setting whether to output backtrace while unexpected failure happened.
///
/// # Notes
///
/// - When the error is an expected error, backtrace will not be output.
/// - backtrace output is disable by default.
pub fn with_backtrace_output(mut self, enable: bool) -> Self {
self.backtrace_output = enable;
self
}

fn operation_level(&self, operation: Operation) -> Level {
match operation {
Operation::ReaderRead
Expand All @@ -356,24 +280,12 @@ impl DefaultLoggingInterceptor {
}

#[inline]
fn error_level(&self, err: &Error) -> Option<Level> {
fn error_level(&self, err: &Error) -> Level {
if err.kind() == ErrorKind::Unexpected {
self.failure_level
Level::Error
} else {
self.error_level
}
}

/// Returns true if the error is unexpected and we need to
/// print the backtrace.
#[inline]
fn print_backtrace(&self, err: &Error) -> bool {
// Don't print backtrace if it's not unexpected error.
if err.kind() != ErrorKind::Unexpected {
return false;
Level::Warn
}

self.backtrace_output
}
}

Expand Down
1 change: 0 additions & 1 deletion core/src/layers/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ mod immutable_index;
pub use immutable_index::ImmutableIndexLayer;

mod logging;
pub use logging::DefaultLoggingInterceptor;
pub use logging::LoggingInterceptor;
pub use logging::LoggingLayer;

Expand Down
73 changes: 70 additions & 3 deletions core/src/raw/tests/utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@ use std::collections::HashMap;
use std::env;
use std::str::FromStr;

use log::{log, Level};
use once_cell::sync::Lazy;

use crate::layers::LoggingInterceptor;
use crate::raw::Operation;
use crate::*;

/// TEST_RUNTIME is the runtime used for running tests.
Expand Down Expand Up @@ -73,9 +76,7 @@ pub fn init_test_service() -> Result<Option<Operator>> {
let op = { op.layer(layers::ChaosLayer::new(0.1)) };

let mut op = op
.layer(layers::LoggingLayer::new(
layers::DefaultLoggingInterceptor::default().with_backtrace_output(true),
))
.layer(layers::LoggingLayer::new(BacktraceLoggingInterceptor))
.layer(layers::TimeoutLayer::new())
.layer(layers::RetryLayer::new().with_max_times(4));

Expand All @@ -90,3 +91,69 @@ pub fn init_test_service() -> Result<Option<Operator>> {

Ok(Some(op))
}

/// A logging interceptor that logs the backtrace.
#[derive(Debug, Clone)]
struct BacktraceLoggingInterceptor;

impl LoggingInterceptor for BacktraceLoggingInterceptor {
fn log(
&self,
scheme: Scheme,
operation: raw::Operation,
path: &str,
context: &str,
message: &str,
err: Option<&Error>,
) {
let Some(err) = err else {
let lvl = self.operation_level(operation);
log!(
target: "opendal::services",
lvl,
"service={} operation={} path={} {} -> {}",
scheme,
operation,
path,
context,
message,
);
return;
};

let err_msg = if err.kind() != ErrorKind::Unexpected {
format!("{err}")
} else {
format!("{err:?}")
};
let lvl = if err.kind() == ErrorKind::Unexpected {
Level::Error
} else {
Level::Warn
};

log!(
target: "opendal::services",
lvl,
"service={} operation={} path={} {} -> {} {}",
scheme,
operation,
path,
context,
message,
err_msg,
);
}
}

impl BacktraceLoggingInterceptor {
fn operation_level(&self, operation: Operation) -> Level {
match operation {
Operation::ReaderRead
| Operation::BlockingReaderRead
| Operation::WriterWrite
| Operation::BlockingWriterWrite => Level::Trace,
_ => Level::Debug,
}
}
}

0 comments on commit 48fbec1

Please sign in to comment.