Copilot commented on code in PR #64523:
URL: https://github.com/apache/airflow/pull/64523#discussion_r3025327578
##########
airflow-core/src/airflow/api_fastapi/common/http_access_log.py:
##########
@@ -23,13 +23,87 @@
from typing import TYPE_CHECKING
import structlog
+from starlette.routing import Match
+
+from airflow._shared.observability.metrics.stats import Stats
if TYPE_CHECKING:
from starlette.types import ASGIApp, Message, Receive, Scope, Send
logger = structlog.get_logger(logger_name="http.access")
_HEALTH_PATHS = frozenset(["/api/v2/monitor/health"])
+_API_PATH_PREFIX_TO_SURFACE = (
+ ("/api/v2", "public"),
+ ("/ui", "ui"),
+)
+
+
+def _get_api_surface(path: str) -> str | None:
+ for prefix, surface in _API_PATH_PREFIX_TO_SURFACE:
+ if path == prefix or path.startswith(f"{prefix}/"):
+ return surface
+ return None
+
+
+def _get_status_family(status_code: int) -> str:
+ if status_code < 100:
+ return "unknown"
+ return f"{status_code // 100}xx"
+
+
+def _get_route_tag(scope: Scope) -> str:
+ router = scope.get("router")
+ routes = getattr(router, "routes", None)
+ partial_route_path: str | None = None
+ if routes:
+ for route in routes:
+ match, _ = route.matches(scope)
+ route_path = getattr(route, "path", None)
+ if not isinstance(route_path, str) or not route_path:
+ continue
+ if match == Match.FULL:
+ return route_path
+ if match == Match.PARTIAL and partial_route_path is None:
+ partial_route_path = route_path
Review Comment:
`_get_route_tag()` relies on `scope.get("router").routes` and re-runs route
matching on every request. In Starlette/FastAPI the resolved route is typically
already available on `scope["route"]` (see e.g.
`airflow/api_fastapi/execution_api/security.py`), so this can both add
avoidable per-request overhead and fail to return the expected templated path
when `router` isn’t present in the scope. Prefer reading `route =
scope.get("route")` and returning `route.path` when available, and only fall
back to scanning/matching routes when `route` is missing (e.g. for 404/405
cases).
```suggestion
# Prefer the already-resolved route attached to the scope when available.
route = scope.get("route")
route_path = getattr(route, "path", None) if route is not None else None
if isinstance(route_path, str) and route_path:
return route_path
router = scope.get("router")
routes = getattr(router, "routes", None)
partial_route_path: str | None = None
if routes:
for candidate_route in routes:
match, _ = candidate_route.matches(scope)
candidate_path = getattr(candidate_route, "path", None)
if not isinstance(candidate_path, str) or not candidate_path:
continue
if match == Match.FULL:
return candidate_path
if match == Match.PARTIAL and partial_route_path is None:
partial_route_path = candidate_path
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
