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

Add log config and custom API logger to log requests #6

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

kun98-liu
Copy link
Contributor

uvicorn log access of API like

api-server-1  | INFO:     192.168.65.1:41988 - "GET /swagger HTTP/1.1" 200 OK

but it's not helping, because it does not log the request body.

Now the log looks like:

api-server-1  | 2024-12-09 09:35:13,363 - INFO     - app.api_logger       - [request_id: 7608958e-36a6-428e-8427-83ced6b9d7c3, method:POST, path: http://0.0.0.0:8000/login/] Request body: {"e_mail":"123","password":"string"}.
api-server-1  | 2024-12-09 09:35:13,367 - INFO     - app.routers.login    - e_mail='123' password='string'
api-server-1  | 2024-12-09 09:35:13,367 - ERROR    - app.http_exc_handler - [request_id: 7608958e-36a6-428e-8427-83ced6b9d7c3, method:POST, path: http://0.0.0.0:8000/login/] Failed to handle request. status_code=404, detail=asdasdas

What can be improved in the future:

  • Instead of only outputting logs to console, we should also keep logs as files in disk and use logstash to send logs to es like real-world application does.
  • Response is not logged, but we may want to add it. APILoggerMiddleware has to build a dummy Response from the outgoing bytestreams, which is not good. So we may want to find another way to do so.

@RachelWang-98
Copy link
Contributor

very good!

Copy link
Contributor

@RachelWang-98 RachelWang-98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1

@RachelWang-98 RachelWang-98 merged commit 8c3af66 into kunlulukun:master Dec 10, 2024
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.

2 participants