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.

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

Add feature for limit request to log in middleware logger using status code #3086

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions actix-web/CHANGES.md
Expand Up @@ -2,6 +2,9 @@

## Unreleased

### Added
- Add `Logger::statuses` to filter the range of statuses logged.

### Changed

- Updated `zstd` dependency to `0.13`.
Expand Down
1 change: 1 addition & 0 deletions actix-web/Cargo.toml
Expand Up @@ -123,6 +123,7 @@ tls-openssl = { package = "openssl", version = "0.10.55" }
tls-rustls = { package = "rustls", version = "0.21" }
tokio = { version = "1.24.2", features = ["rt-multi-thread", "macros"] }
zstd = "0.13"
capture-logger = "0.1"

[[test]]
name = "test_server"
Expand Down
97 changes: 93 additions & 4 deletions actix-web/src/middleware/logger.rs
Expand Up @@ -7,6 +7,7 @@ use std::{
fmt::{self, Display as _},
future::Future,
marker::PhantomData,
ops::{Bound, RangeBounds},
pin::Pin,
rc::Rc,
task::{Context, Poll},
Expand All @@ -23,7 +24,7 @@ use time::{format_description::well_known::Rfc3339, OffsetDateTime};

use crate::{
body::{BodySize, MessageBody},
http::header::HeaderName,
http::{header::HeaderName, StatusCode},
service::{ServiceRequest, ServiceResponse},
Error, Result,
};
Expand Down Expand Up @@ -89,6 +90,7 @@ struct Inner {
exclude: HashSet<String>,
exclude_regex: RegexSet,
log_target: Cow<'static, str>,
status_range: (Bound<StatusCode>, Bound<StatusCode>),
}

impl Logger {
Expand All @@ -99,6 +101,10 @@ impl Logger {
exclude: HashSet::new(),
exclude_regex: RegexSet::empty(),
log_target: Cow::Borrowed(module_path!()),
status_range: (
Bound::Included(StatusCode::from_u16(100).unwrap()),
Bound::Included(StatusCode::from_u16(999).unwrap()),
),
}))
}

Expand All @@ -121,6 +127,23 @@ impl Logger {
self
}

/// Set a range of status to include in the logging
///
/// # Examples
/// ```
/// use actix_web::{middleware::Logger, App, http::StatusCode};
///
/// // Log only the requests with status code higher or equal to BAD_REQUEST(400)
/// let app = App::new()
/// .wrap(Logger::default().statuses(StatusCode::BAD_REQUEST..));
///
/// ```
pub fn statuses<R: RangeBounds<StatusCode>>(mut self, status: R) -> Self {
let inner = Rc::get_mut(&mut self.0).unwrap();
inner.status_range = (status.start_bound().cloned(), status.end_bound().cloned());
self
}

/// Sets the logging target to `target`.
///
/// By default, the log target is `module_path!()` of the log call location. In our case, that
Expand Down Expand Up @@ -242,6 +265,10 @@ impl Default for Logger {
exclude: HashSet::new(),
exclude_regex: RegexSet::empty(),
log_target: Cow::Borrowed(module_path!()),
status_range: (
Bound::Included(StatusCode::from_u16(100).unwrap()),
Bound::Included(StatusCode::from_u16(999).unwrap()),
),
}))
}
}
Expand Down Expand Up @@ -306,6 +333,7 @@ where
LoggerResponse {
fut: self.service.call(req),
format: None,
status_range: self.inner.status_range,
time: OffsetDateTime::now_utc(),
log_target: Cow::Borrowed(""),
_phantom: PhantomData,
Expand All @@ -321,6 +349,7 @@ where
LoggerResponse {
fut: self.service.call(req),
format: Some(format),
status_range: self.inner.status_range,
time: now,
log_target: self.inner.log_target.clone(),
_phantom: PhantomData,
Expand All @@ -339,6 +368,7 @@ pin_project! {
fut: S::Future,
time: OffsetDateTime,
format: Option<Format>,
status_range:(Bound<StatusCode>,Bound<StatusCode>),
log_target: Cow<'static, str>,
_phantom: PhantomData<B>,
}
Expand All @@ -363,7 +393,13 @@ where
debug!("Error in response: {:?}", error);
}

let res = if let Some(ref mut format) = this.format {
let mut format = if this.status_range.contains(&res.status()) {
this.format.take()
} else {
None
};

let res = if let Some(ref mut format) = format {
// to avoid polluting all the Logger types with the body parameter we swap the body
// out temporarily since it's not usable in custom response functions anyway

Expand All @@ -384,7 +420,6 @@ where
};

let time = *this.time;
let format = this.format.take();
let log_target = this.log_target.clone();

Poll::Ready(Ok(res.map_body(move |_, body| StreamLog {
Expand Down Expand Up @@ -745,7 +780,13 @@ mod tests {
header::HeaderValue::from_static("ACTIX-WEB"),
))
.to_srv_request();
let _res = srv.call(req).await;
capture_logger::begin_capture();
// The log is executed on drop, so the result need to be dropped
let _ = srv.call(req).await;
let log = capture_logger::pop_captured().unwrap();
assert!(log.message().contains("ttt"));
assert!(log.message().contains("ACTIX-WEB"));
capture_logger::end_capture();
}

#[actix_rt::test]
Expand All @@ -771,6 +812,54 @@ mod tests {
let _res = srv.call(req).await.unwrap();
}

#[actix_rt::test]
async fn test_logger_status_range_include() {
let srv = |req: ServiceRequest| {
ok(req.into_response(HttpResponse::build(StatusCode::OK).finish()))
};
let logger = Logger::new("%{User-Agent}i test_included %s").statuses(StatusCode::OK..);

let srv = logger.new_transform(srv.into_service()).await.unwrap();

let req = TestRequest::default()
.insert_header((
header::USER_AGENT,
header::HeaderValue::from_static("ACTIX-WEB"),
))
.to_srv_request();
capture_logger::begin_capture();
// The log is executed on drop, so the result need to be dropped
let _ = srv.call(req).await;
let log = capture_logger::pop_captured().unwrap();
assert!(log.message().contains("200"));
assert!(log.message().contains("ACTIX-WEB"));
capture_logger::end_capture();
}

#[actix_rt::test]
async fn test_logger_status_range_exclude() {
let srv = |req: ServiceRequest| {
ok(req.into_response(HttpResponse::build(StatusCode::OK).finish()))
};
let logger =
Logger::new("%{User-Agent}i test_excluded %s").statuses(StatusCode::BAD_REQUEST..);

let srv = logger.new_transform(srv.into_service()).await.unwrap();

let req = TestRequest::default()
.insert_header((
header::USER_AGENT,
header::HeaderValue::from_static("ACTIX-WEB"),
))
.to_srv_request();
capture_logger::begin_capture();
// The log is executed on drop, so the result need to be dropped
let _ = srv.call(req).await;
let log = capture_logger::pop_captured();
assert!(log.is_none());
capture_logger::end_capture();
}

#[actix_rt::test]
async fn test_escape_percent() {
let mut format = Format::new("%%{r}a");
Expand Down