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

feat(middleware): log content_length for 4xx #4655

Merged
merged 6 commits into from
May 16, 2024
Merged

Conversation

Narayanbhat166
Copy link
Member

@Narayanbhat166 Narayanbhat166 commented May 15, 2024

Type of Change

  • Enhancement

Description

In case of 4xx, we need to log the content length of the request from the content-length header. This PR adds that enhancement.

Motivation and Context

To provide better support for debugging issues related to missing fields.

How did you test it?

  • Create a 4xx error and check for the content length log.
image

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code

@Narayanbhat166 Narayanbhat166 self-assigned this May 15, 2024
@Narayanbhat166 Narayanbhat166 requested a review from a team as a code owner May 15, 2024 14:43
@Narayanbhat166 Narayanbhat166 added the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label May 15, 2024
@Narayanbhat166 Narayanbhat166 added this to the May 2024 Release milestone May 15, 2024
@Narayanbhat166 Narayanbhat166 requested review from a team as code owners May 16, 2024 07:58
SanchithHegde
SanchithHegde previously approved these changes May 16, 2024
Comment on lines +263 to +286

let content_length_header = new_req
.headers()
.get(headers::CONTENT_LENGTH)
.map(ToOwned::to_owned);
let response_fut = svc.call(new_req);
let response = response_fut.await?;
// Log the request_details when we receive 400 status from the application
if response.status() == 400 {
let request_id = request_id_fut.await?.as_hyphenated().to_string();
let content_length_header_string = content_length_header
.map(|content_length_header| {
content_length_header.to_str().map(ToOwned::to_owned)
})
.transpose()
.map_err(|error| {
logger::warn!("Could not convert content length to string {error:?}");
error
})
.ok()
.flatten();

logger::info!("Content length from header: {content_length_header_string:?}, Bytes length: {bytes_length}");

Copy link
Contributor

Choose a reason for hiding this comment

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

Have a look at pub fn get_header_value_by_key(key: String, headers: &HeaderMap)

@Gnanasundari24 Gnanasundari24 added this pull request to the merge queue May 16, 2024
Merged via the queue into main with commit 4b5b558 May 16, 2024
10 checks passed
@Gnanasundari24 Gnanasundari24 deleted the log_content_length branch May 16, 2024 10:45
@SanchithHegde SanchithHegde removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label May 19, 2024
kashif-m pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: hyperswitch-bot[bot] <148525504+hyperswitch-bot[bot]@users.noreply.github.com>
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.

None yet

5 participants