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

Error Logging #1925

Closed
alfechner opened this issue May 13, 2024 · 1 comment · Fixed by #1926
Closed

Error Logging #1925

alfechner opened this issue May 13, 2024 · 1 comment · Fixed by #1926

Comments

@alfechner
Copy link
Contributor

alfechner commented May 13, 2024

There are several places Connexion logs errors, for example:

Connexion does a great job of handling exceptions by retuning a problem details with the appropriate status code to the user.

Now it's pretty common for operation teams to monitor exceptions in the logs and trigger alarms.

And here comes the issue:

Operations want to be alerted about everything that results in 5xx status codes. That indicates that a situation couldn't be handled from a user perspective. Operations don't need to be alerted if the status code is 4xx. Because those cases are gracefully handled.

I'd propose to

  1. change ExceptionMiddleware to log to error if the resulting status code is 5xx and to warning for 4xx and
  2. remove the error log from connexion.validators.json since the BadRequestProblem raised is logged via the Exception middleware anyway.

I'm happy to contribute those changes. What do you think?

@Ruwann
Copy link
Member

Ruwann commented May 19, 2024

Thanks, @alfechner !

I can definitely understand where you're coming from. Based on the Python documention on logging, I agree that error logs for validation excpetions are indeed too severe as they are in most cases part of the normal flow of the application.

Note that you might be able to add a Filter for now to change the logging level for now (haven't tested it out myself): https://stackoverflow.com/questions/47412668/python-logging-change-warn-to-info

RobbeSneyders pushed a commit that referenced this issue May 27, 2024
Fixes #1925.

Changes proposed in this pull request:

 - Log handled errors to `warning` instead of `error`.
- Log validation errors to `info` because the intent of the log lines in
informational. The error is handled by raising a new error.

---------

Co-authored-by: Alex Fechner <[email protected]>
RobbeSneyders pushed a commit that referenced this issue May 27, 2024
Fixes #1925.

Changes proposed in this pull request:

 - Log handled errors to `warning` instead of `error`.
- Log validation errors to `info` because the intent of the log lines in
informational. The error is handled by raising a new error.

---------

Co-authored-by: Alex Fechner <[email protected]>
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 a pull request may close this issue.

2 participants