-
Notifications
You must be signed in to change notification settings - Fork 19
Feat metric logging #132
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
base: dev
Are you sure you want to change the base?
Feat metric logging #132
Changes from 6 commits
a224c01
12caa8d
30c61aa
bf8f963
173b8e6
e8c5175
29ac9bd
fc55a4b
04083b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,3 +17,4 @@ bruno.json | |
| .ruff_cache/ | ||
| *.csv | ||
| *.png | ||
| METRICS_IMPLEMENTATION.md | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,5 @@ | ||
| """PESUAuth API - A simple API to authenticate PESU credentials using PESU Academy.""" | ||
|
|
||
| from app.metrics import metrics | ||
|
|
||
| __all__ = ["metrics"] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,9 @@ | |
| from pydantic import ValidationError | ||
|
|
||
| from app.docs import authenticate_docs, health_docs, readme_docs | ||
| from app.docs.metrics import metrics_docs | ||
| from app.exceptions.base import PESUAcademyError | ||
| from app.metrics import metrics # Global metrics instance | ||
| from app.models import RequestModel, ResponseModel | ||
| from app.pesu import PESUAcademy | ||
|
|
||
|
|
@@ -29,8 +31,13 @@ async def _refresh_csrf_token_with_lock() -> None: | |
| """Refresh the CSRF token with a lock.""" | ||
| logging.debug("Refreshing unauthenticated CSRF token...") | ||
| async with CSRF_TOKEN_REFRESH_LOCK: | ||
| await pesu_academy.prefetch_client_with_csrf_token() | ||
| logging.info("Unauthenticated CSRF token refreshed successfully.") | ||
| try: | ||
| await pesu_academy.prefetch_client_with_csrf_token() | ||
| metrics.inc("csrf_token_refresh_success_total") | ||
| logging.info("Unauthenticated CSRF token refreshed successfully.") | ||
| except Exception: | ||
| metrics.inc("csrf_token_refresh_failure_total") | ||
| raise | ||
|
|
||
|
|
||
| async def _csrf_token_refresh_loop() -> None: | ||
|
|
@@ -40,6 +47,7 @@ async def _csrf_token_refresh_loop() -> None: | |
| logging.debug("Refreshing unauthenticated CSRF token...") | ||
| await _refresh_csrf_token_with_lock() | ||
| except Exception: | ||
| metrics.inc("csrf_token_refresh_failure_total") | ||
| logging.exception("Failed to refresh unauthenticated CSRF token in the background.") | ||
| await asyncio.sleep(CSRF_TOKEN_REFRESH_INTERVAL_SECONDS) | ||
|
|
||
|
|
@@ -100,6 +108,7 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: | |
| @app.exception_handler(RequestValidationError) | ||
| async def validation_exception_handler(request: Request, exc: RequestValidationError) -> JSONResponse: | ||
| """Handler for request validation errors.""" | ||
| metrics.inc("validation_error_total") | ||
| logging.exception("Request data could not be validated.") | ||
| errors = exc.errors() | ||
| message = "; ".join([f"{'.'.join(str(loc) for loc in e['loc'])}: {e['msg']}" for e in errors]) | ||
|
|
@@ -116,6 +125,19 @@ async def validation_exception_handler(request: Request, exc: RequestValidationE | |
| @app.exception_handler(PESUAcademyError) | ||
| async def pesu_exception_handler(request: Request, exc: PESUAcademyError) -> JSONResponse: | ||
| """Handler for PESUAcademy specific errors.""" | ||
| metrics.inc("pesu_academy_error_total") | ||
|
|
||
| # Track specific error types | ||
| exc_type = type(exc).__name__.lower() | ||
| if "csrf" in exc_type: | ||
| metrics.inc("csrf_token_error_total") | ||
| elif "profilefetch" in exc_type: | ||
| metrics.inc("profile_fetch_error_total") | ||
| elif "profileparse" in exc_type: | ||
| metrics.inc("profile_parse_error_total") | ||
| elif "authentication" in exc_type: | ||
| metrics.inc("auth_failure_total") | ||
|
|
||
|
Comment on lines
+159
to
+168
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a much cleaner solution. Look into a middleware layer. Here is some pseudo code to get you started: @app.middleware("http")
async def metrics_middleware(request: Request, call_next):
metrics.inc("requests_total")
start_time = time.time()
try:
response: Response = await call_next(request)
latency = time.time() - start_time
# Track successes vs failures
if 200 <= response.status_code < 300:
metrics.inc("requests_success")
else:
metrics.inc("requests_failed")
metrics.inc(f"requests_failed_status_{response.status_code}")
# Latency metrics
metrics.inc("request_latency_sum", latency)
# Also add route metrics: route = request.scope.get("route")
return response
except Exception as e:
latency = time.time() - start_time
metrics.inc("requests_failed")
metrics.inc(f"requests_failed_exception_{type(e).__name__}")
metrics.inc("request_latency_sum", latency)
raiseNote, you will need to accordingly increment other metrics like how many with and without profile data by parsing the request. |
||
| logging.exception(f"PESUAcademyError: {exc.message}") | ||
| return JSONResponse( | ||
| status_code=exc.status_code, | ||
|
|
@@ -130,6 +152,7 @@ async def pesu_exception_handler(request: Request, exc: PESUAcademyError) -> JSO | |
| @app.exception_handler(Exception) | ||
| async def unhandled_exception_handler(request: Request, exc: Exception) -> JSONResponse: | ||
| """Handler for unhandled exceptions.""" | ||
| metrics.inc("unhandled_exception_total") | ||
| logging.exception("Unhandled exception occurred.") | ||
| return JSONResponse( | ||
| status_code=500, | ||
|
|
@@ -160,6 +183,27 @@ async def health() -> JSONResponse: | |
| ) | ||
|
|
||
|
|
||
| @app.get( | ||
| "/metrics", | ||
| response_class=JSONResponse, | ||
| responses=metrics_docs.response_examples, | ||
| tags=["Monitoring"], | ||
| ) | ||
| async def get_metrics() -> JSONResponse: | ||
| """Get current application metrics.""" | ||
| logging.debug("Metrics requested.") | ||
| current_metrics = metrics.get() | ||
| return JSONResponse( | ||
| status_code=200, | ||
| content={ | ||
| "status": True, | ||
| "message": "Metrics retrieved successfully", | ||
| "timestamp": datetime.datetime.now(IST).isoformat(), | ||
| "metrics": current_metrics, | ||
| }, | ||
| ) | ||
|
|
||
|
|
||
| @app.get( | ||
| "/readme", | ||
| response_class=RedirectResponse, | ||
|
|
@@ -199,6 +243,7 @@ async def authenticate(payload: RequestModel, background_tasks: BackgroundTasks) | |
| # Authenticate the user | ||
| authentication_result = {"timestamp": current_time} | ||
| logging.info(f"Authenticating user={username} with PESU Academy...") | ||
|
|
||
| authentication_result.update( | ||
| await pesu_academy.authenticate( | ||
| username=username, | ||
|
|
@@ -207,6 +252,7 @@ async def authenticate(payload: RequestModel, background_tasks: BackgroundTasks) | |
| fields=fields, | ||
| ), | ||
| ) | ||
|
|
||
| # Prefetch a new client with an unauthenticated CSRF token for the next request | ||
| background_tasks.add_task(_refresh_csrf_token_with_lock) | ||
|
|
||
|
|
@@ -216,6 +262,10 @@ async def authenticate(payload: RequestModel, background_tasks: BackgroundTasks) | |
| logging.info(f"Returning auth result for user={username}: {authentication_result}") | ||
| authentication_result = authentication_result.model_dump(exclude_none=True) | ||
| authentication_result["timestamp"] = current_time.isoformat() | ||
|
|
||
| # Track successful authentication only after validation succeeds | ||
| metrics.inc("auth_success_total") | ||
|
|
||
| return JSONResponse( | ||
| status_code=200, | ||
| content=authentication_result, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| """Custom docs for the /metrics PESUAuth endpoint.""" | ||
|
|
||
| from app.docs.base import ApiDocs | ||
|
|
||
| metrics_docs = ApiDocs( | ||
| request_examples={}, # GET endpoint doesn't need request examples | ||
| response_examples={ | ||
| 200: { | ||
| "description": "Metrics retrieved successfully", | ||
| "content": { | ||
| "application/json": { | ||
| "examples": { | ||
| "metrics_response": { | ||
| "summary": "Current Metrics", | ||
| "description": ( | ||
| "All current application metrics including authentication counts and error rates" | ||
| ), | ||
| "value": { | ||
| "status": True, | ||
| "message": "Metrics retrieved successfully", | ||
| "timestamp": "2025-08-28T15:30:45.123456+05:30", | ||
| "metrics": { | ||
| "auth_success_total": 150, | ||
| "auth_failure_total": 12, | ||
|
Comment on lines
+23
to
+24
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also track how many auth requests are received, including a split for how many with and without profile data |
||
| "validation_error_total": 8, | ||
| "pesu_academy_error_total": 5, | ||
| "unhandled_exception_total": 0, | ||
| "csrf_token_error_total": 2, | ||
| "profile_fetch_error_total": 1, | ||
| "profile_parse_error_total": 0, | ||
| "csrf_token_refresh_success_total": 45, | ||
| "csrf_token_refresh_failure_total": 1, | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| } | ||
| }, | ||
|
Comment on lines
+8
to
+40
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Create a model for this. The response model will also need an update. |
||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| """Metrics collector for tracking authentication successes, failures, and error types.""" | ||
|
|
||
| import threading | ||
| from collections import defaultdict | ||
|
|
||
|
|
||
| class MetricsCollector: | ||
| """Thread-safe metrics collector for tracking application performance and usage.""" | ||
|
|
||
| def __init__(self) -> None: | ||
| """Initialize the metrics collector with thread safety.""" | ||
| self.lock = threading.Lock() | ||
| self.metrics = defaultdict(int) | ||
|
|
||
| def inc(self, key: str) -> None: | ||
| """Increment a metric counter by 1. | ||
|
|
||
| Args: | ||
| key (str): The metric key to increment. | ||
| """ | ||
| with self.lock: | ||
| self.metrics[key] += 1 | ||
|
|
||
| def get(self) -> dict[str, int]: | ||
| """Get a copy of all current metrics. | ||
|
|
||
| Returns: | ||
| dict[str, int]: Dictionary containing all metrics and their current values. | ||
| """ | ||
| with self.lock: | ||
| return dict(self.metrics) | ||
|
|
||
|
|
||
| # Global metrics instance | ||
| metrics = MetricsCollector() |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these dependencies necessary to be added ? |
|
snigenigmatic marked this conversation as resolved.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this; we should not want to initialize a global collector outside the entry point.