-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add feature for limit request to log in middleware logger using status code #3086
base: master
Are you sure you want to change the base?
Conversation
Hi, I did add a simple dev-dependency to capture logs and check them in the tests, so I could add some tests that check the full flow, I updated also some existing tests to check that the output is what is expected. Now this PR should be complete of all the requirements. Regards |
The only problem I see with this is the issue of disjoint ranges. I can see it being reasonable to want everything except redirects being logged; in which case it's either:
It's tempting to go with the set subtraction technique since more common patterns can be expressed with just one builder method. Despite being terse, it might be confusing, too. So, the API could be |
Hi,
This is yet a preliminary change to just collect feedback:
So this PR add a new feature on the logger middleware, to limit the logging to a range of status code, so that are logged only some specific status in the range, a simple example that will log only the 400+ request/responses is:
Do you like this API is the naming right or you prefer a different one ?
Do you like this? shall I continue 😄 ?
PR Checklist
Could not really find a simple way to test this without an application, I didn't see any similar tests for other similar features where could I find/plug a test case for this case ?
work in progress
will do