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

Closed
wants to merge 1 commit into 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 ?

@robjtede robjtede added B-semver-patch A-web project: actix-web labels Jun 11, 2024
@robjtede
Copy link
Member

robjtede commented Jun 11, 2024

In middleware, any response (regardless of status code) that is sent is considered a success. Logging as INFO level is the policy of this textual logger. If you want to separately track errors, I encourage you to write a middleware to add in your stack.

Also, we're gradually transitioning to the tracing ecosystem and the whole Logger will need a re-think. Until then I'd like to keep this one relatively simple with obvious behavior.

Thank you for your ideas and contribution.

@robjtede robjtede closed this Jun 11, 2024
@fawni fawni deleted the feat/log-level branch June 11, 2024 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-web project: actix-web B-semver-patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants