editoast: make authorizer's logs less noisy #8834
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
So far, each request on an endpoint show the returned value of the
authorizer.
```
2024-09-11T05:01:31.226339Z INFO editoast_authz::authorizer: return: true
at editoast_authz/src/authorizer.rs
in editoast_authz::authorizer::check_roles with user: superuser (Super User), user_roles: {}, required_roles: {RollingStockCollectionRead}
in tower_http::trace::make_span::request with method: GET, uri: /light_rolling_stock, version: HTTP/1.1
```
It makes the logs on `editoast` pretty noisy. The `ret` on the
`tracing::instrument` produces a log event (without `ret` we would still
have the span). Ideally, we would like the information of the span to
live for the entire duration of the request/response cycle.
However, since the authorizer is not a middleware (feature is on the way
I believe), we cannot make the span lives for the entire request/response
cycle, it only lives for the very beginning of the endpoint. This means
that useful contextual information like `user`, `user_roles` or
`required_roles` won't be visible through the logs produced in the
endpoint.
In the meantime, leaving `ret`, but making the default level to `DEBUG`
seems an OK improvement.
Signed-off-by: Jean SIMARD <[email protected]>
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #8834 +/- ##
============================================
- Coverage 37.07% 37.04% -0.04%
+ Complexity 2212 2209 -3
============================================
Files 1255 1255
Lines 114111 114134 +23
Branches 3189 3182 -7
============================================
- Hits 42307 42279 -28
- Misses 69907 69963 +56
+ Partials 1897 1892 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So far, each request on an endpoint show the returned value of the authorizer.
It makes the logs on
editoastpretty noisy. Thereton thetracing::instrumentproduces a log event (withoutretwe would still have the span). Ideally, we would like the information of the span to live for the entire duration of the request/response cycle.However, since the authorizer is not a middleware (feature is on the way I believe), we cannot make the span lives for the entire request/response cycle, it only lives for the very beginning of the endpoint. This means that useful contextual information like
user,user_rolesorrequired_roleswon't be visible through the logs produced in the endpoint.In the meantime, leaving
ret, but making the default level toDEBUGseems an OK improvement.