Add REST API metrics#64523
Conversation
4233a57 to
75bbd10
Compare
75bbd10 to
d18b47b
Compare
7fa1d0b to
33604c0
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds basic REST API request instrumentation to Airflow’s FastAPI/Starlette stack by emitting standardized metrics via Airflow’s existing Stats abstraction, enabling QPS/latency/error monitoring for /api/v2 and /ui traffic.
Changes:
- Added
api.requests,api.request.duration, andapi.request.errorsmetric definitions to the shared metrics template. - Implemented metric emission in
HttpAccessLogMiddlewarewith bounded tags (api_surface,method,route,status_code,status_family) and guarded failure handling. - Added unit tests validating metric emission behavior for API/UI paths, health checks, non-API paths, failures, and unmatched routes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| shared/observability/src/airflow_shared/observability/metrics/metrics_template.yaml | Registers new API request metrics in the shared metrics catalog. |
| airflow-core/src/airflow/api_fastapi/common/http_access_log.py | Emits API metrics from the access-log middleware after request completion. |
| airflow-core/tests/unit/api_fastapi/common/test_http_access_log.py | Adds unit tests asserting correct metric emission and tagging behavior. |
33604c0 to
af907c0
Compare
pierrejeambrun
left a comment
There was a problem hiding this comment.
Cool that looks promising.
A few comments before we can move forward.
Would love another pair of eyes. (@jedcunningham?)
af907c0 to
719146c
Compare
|
Also cardinality can be a problem and we need a futher understanding of the impact of such metrics. I believe last time I checked it was around 60k time series introduced, which seems too much. |
eb855b2 to
6ca8a2b
Compare
3552007 to
84fa5a1
Compare
84fa5a1 to
c997391
Compare
c997391 to
365fdf6
Compare
ashb
left a comment
There was a problem hiding this comment.
As well as the individaul review comments:
Was using a prometheus style /metricz endpoint considered? That will likely work much better for many deployments rather than needing a push-style metric that the Airflow stats mechanism requires.
365fdf6 to
4523b2d
Compare
3387960 to
1384714
Compare
dbab760 to
fbbee16
Compare
fbbee16 to
309e389
Compare
77145d4 to
82cc3d7
Compare
Remove the configurable path-to-surface mapping and emit metrics only for core API and UI routes. Use normalized route templates with method and status family tags to keep metric cardinality bounded.
82cc3d7 to
edb9715
Compare
|
ready for review :) |
What
adds REST API instrumentation using Airflow's existing metrics abstraction.
Changes included:
HttpAccessLogMiddleware.api.requests,api.request.duration, andapi.request.errorsfor/api/v2and/uirequests.api_surface,method,route,status_code, andstatus_family.closes: #56034
Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.Important
🛠️ Maintainer triage note for @henry3260 · by
@potiuk· 2026-06-22 06:31 UTCYour review threads from
@ashblook addressed — please confirm this PR is ready for maintainer review confirmation:The ball is in your court — you've been assigned to this PR. Reply
yes / ready(and mark the threads resolved) and a maintainer will pick it up from the queue.Automated triage — may be imperfect; a maintainer takes the next look.