Skip to content

Add logger middleware#327

Merged
suhasunni merged 18 commits intomainfrom
suhas/add-logger-middleware
Mar 20, 2025
Merged

Add logger middleware#327
suhasunni merged 18 commits intomainfrom
suhas/add-logger-middleware

Conversation

@suhasunni
Copy link
Contributor

@suhasunni suhasunni commented Mar 9, 2025

Purpose

Closes #369

New Changes

  • Any incoming requests and their responses can now be logged via the logger-middleware
  • The logged information includes: Time of request, request method, request url, request size, request params, response status (error severity if applicable), response body, response size, and process time.
  • Endpoints that we don't want logged can now be passed to the middleware via backend_setup.py
  • Added loguru to requirements.txt

Testing

  • I didn't want to pollute the code with the fake endpoint, so here is the code if you want to test this. Add this code to gs/backend/api/v1/mcc/endpoints/commands.py.
from fastapi import APIRouter, HTTPException

commands_router = APIRouter(tags=["MCC", "Commands"])


@commands_router.get("/double")
def double(data: str):
    return {"data": f"{data} {data}"}

running the endpoint /api/v1/mcc/commands/double?data=hello world! should return {"data": "hello world! hello world!"}

Normal:
log_success

Error:
log_error

Critical Error:
log_critical

Outstanding Changes

  • Implement logging the userid. The current logic checks the request headers for "user_id", and sets it to anonymous if not found.

@github-actions
Copy link

github-actions bot commented Mar 9, 2025

Pull reviewers stats

Stats of the last 120 days for UWOrbital:

User Total reviews Time to review Total comments
kepler452b123 20 2d 4h 15m 87
Yarik-Popov 11 1d 5h 54m 105

⚡️ Pull request stats

@Yarik-Popov Yarik-Popov changed the title Suhas/add logger middleware Add logger middleware Mar 9, 2025
Copy link
Contributor

@Yarik-Popov Yarik-Popov left a comment

Choose a reason for hiding this comment

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

Overall quite good. Some changes are needed

@Yarik-Popov
Copy link
Contributor

Thanks for adding screenshots. Can you also include the endpoint code so i can play around with it?

@Yarik-Popov Yarik-Popov linked an issue Mar 13, 2025 that may be closed by this pull request
9 tasks
Copy link
Contributor

@Yarik-Popov Yarik-Popov left a comment

Choose a reason for hiding this comment

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

Overall quite good. Some minor changes are left.

One thing you should add is an request id that's automatically generated for each request that is a uuid so we can easily associate request and response pairs. Print this id in both the request/response logging

@Yarik-Popov Yarik-Popov added the project: backend Backend tasks label Mar 19, 2025
Copy link
Contributor

@Yarik-Popov Yarik-Popov left a comment

Choose a reason for hiding this comment

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

LGTM, the effort of unit testing this is quite large for a very small benefit. We will only need this for debugging and monitoring purposes

@suhasunni suhasunni merged commit e63f037 into main Mar 20, 2025
48 checks passed
@suhasunni suhasunni deleted the suhas/add-logger-middleware branch March 20, 2025 00:49
@Yarik-Popov Yarik-Popov added this to the 25W milestone Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

project: backend Backend tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add logger middleware

2 participants