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 logger to axum router #773

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

wkazmierczak
Copy link
Collaborator

No description provided.

@wkazmierczak wkazmierczak marked this pull request as ready for review September 23, 2024 08:12
src/routes.rs Outdated
@@ -111,3 +117,42 @@ where
}
}
}

async fn log_request_response_body(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
async fn log_request_response_body(
async fn body_logger_middleware(

plus move everything to separate file

src/routes.rs Outdated
Ok(response)
}

async fn buffer_request_body(request: Request) -> Result<Request, Response> {
Copy link
Member

Choose a reason for hiding this comment

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

this should only happen if sth specific is set e.g. logger option to trace

in our logger settings we can write live_compositor::routes=debug to enable debugging in that one file, check if is there a way to provide a custom values that can enable specific log.

If above is not possible then lets enable that only when trace loggin is on

@wkozyra95 wkozyra95 linked an issue Sep 23, 2024 that may be closed by this pull request
@wkazmierczak wkazmierczak force-pushed the @wkazmierczak/add-logger-to-axum-router branch from 62014f9 to db650eb Compare September 23, 2024 09:08
async fn buffer_request_body(request: Request) -> Result<Request, Response> {
let (parts, body) = request.into_parts();

let bytes = body
Copy link
Member

Choose a reason for hiding this comment

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

if logging is enabled then do not read/restore that body

.map_err(|err| (StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response())?
.to_bytes();

if log_enabled!(target: "live_compositor::routes", Level::Debug) {
Copy link
Member

Choose a reason for hiding this comment

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

there is already file route that use sth that does not already exist e.g. live_compositor::log_request_body

@wkazmierczak wkazmierczak force-pushed the @wkazmierczak/add-logger-to-axum-router branch 2 times, most recently from 8159d9e to 0895584 Compare September 24, 2024 08:18
.map_err(|err| (StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response())?
.to_bytes();

trace!(target: "live_compositor::log_request_body", "Request body: {:?}", str::from_utf8(&bytes).unwrap());
Copy link
Member

Choose a reason for hiding this comment

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

  • print other parts of the response too, like route or method, for response status code
  • do not unwrap

Also, it would be great to try parsing requests as JSON values to remove the escaped characters

@wkazmierczak wkazmierczak force-pushed the @wkazmierczak/add-logger-to-axum-router branch from 0895584 to 506f19f Compare September 26, 2024 12:51
@wkazmierczak wkazmierczak force-pushed the @wkazmierczak/add-logger-to-axum-router branch from 506f19f to 60e5983 Compare September 26, 2024 12:53
@wkazmierczak wkazmierczak merged commit f487f83 into master Sep 30, 2024
3 checks passed
@wkazmierczak wkazmierczak deleted the @wkazmierczak/add-logger-to-axum-router branch September 30, 2024 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add logger to axum router
2 participants