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: change log level based on response status code #3107

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fawni
Copy link

@fawni fawni commented Aug 20, 2023

PR Type

Feature

PR Checklist

  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • A changelog entry has been made for the appropriate packages.
  • Format code with the latest stable rustfmt.
  • (Team) Label with affected crates and semver status.

Overview

Changes the log level to Warn on client errors and redirection messages, and to Error on server errors.

The current behavior is to log everything on an Info level.

This helps logging crates customize the output. Here's an example using the femme crate:

Before:

20_08_2023-17_16_WindowsTerminal_0vZ7vIB

After:

20_08_2023-17_17_WindowsTerminal_dYRJ1Pq

It would also be colored red on a 5xx status code.

I do not know how to test or document those changes so I apologize if I should have done that.

@tglman
Copy link
Contributor

tglman commented Sep 15, 2023

Hi @fawni,

I was thinking to do a similar thing, but instead of directly set a log level for a response type (which still do make sense), I was thinking to set the log level as parameter, and than filter the range of status to log, for the range filtering I did already a PR here : #3086

so let the user choose a what level to log request, with multiple loggers, something like:

 Logger::new("....").statuses(StatusCode::OK..StatusCode::BAD_REQUEST).level(Level::Info)
 Logger::new("....").statuses(StatusCode::BAD_REQUEST..).level(Level::Warn)

WDYT ?

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

2 participants