Skip to content

Fix missing HTTP access logs when api-server omits the core app - #72026

Open
manish1337 wants to merge 4 commits into
apache:mainfrom
manish1337:fix-execution-api-access-logs
Open

Fix missing HTTP access logs when api-server omits the core app#72026
manish1337 wants to merge 4 commits into
apache:mainfrom
manish1337:fix-execution-api-access-logs

Conversation

@manish1337

Copy link
Copy Markdown
Contributor

An api-server started without the core app — for example --apps execution, the shape used to run
the Task Execution API as its own deployment — serves requests normally but emits no access log
lines at all.

Three separate places disable the built-in access loggers, all resting on the premise that
HttpAccessLogMiddleware handles access logging:

  • AirflowUvicornWorker.CONFIG_KWARGSaccess_log: False
  • the uvicorn kwargs built in api_server_command
  • uvicorn.access / gunicorn.access muted (handlers: [], propagate: False) in logging_config

That premise only held while the middleware was installed inside the "core" in apps_list branch of
create_app, so an --apps selection without core had no access-log producer at all — and because
the loggers are muted at the logging-config level too, no configuration could bring the records back.

This installs the middleware for every apps selection, which makes the premise true again rather
than adding a fourth conditional. The alternative — enabling uvicorn's own access log when core is
absent — needs three coordinated conditionals across three files and would give execution-only
deployments a different log format from every other deployment.

For anyone running the Task Execution API as its own deployment, this restores per-request telemetry
(latency, status codes, endpoint counts) for the component every running task depends on for
heartbeats, state transitions and XComs.

Verification

Started a real api-server and issued a marked request, before and after the change:

--apps uvicorn gunicorn
execution no record → logged no record → logged
core logged logged
all logged logged
  • Middleware order for core/all is unchanged: [HttpAccessLogMiddleware, GZipMiddleware, JWTRefreshMiddleware].
  • The generated OpenAPI specs are byte-identical with and without the change.
  • The added test fails on [execution] without the fix and passes with it.
  • airflow-core/tests/unit/api_fastapi (3750), providers/fab (536) and providers/common/compat
    (197) pass. The test_hitl.py and example-Dag failures seen locally also fail on unmodified
    main, so they are unrelated.

Note for reviewers: #64523 (open) also edits init_middlewares to add a metrics middleware, so the
two may conflict textually.


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 5)

Generated-by: Claude Code (Opus 5) following the guidelines

An api-server started without the core app -- for example `--apps execution`,
the shape used to run the Task Execution API as its own deployment -- served
requests but recorded none of them. That leaves the component every running task
depends on for heartbeats, state transitions and XComs with no per-request
telemetry, which is the primary signal for diagnosing heartbeat stalls.

Both server backends disable their own access logger, and the built-in access
loggers are muted in the logging config, all on the premise that
HttpAccessLogMiddleware handles access logging. That premise only held while the
middleware was installed by the core app branch, so no configuration could
recover the records. Installing it for every apps selection makes the premise
true again instead of adding a fourth conditional that would give execution-only
deployments a different log format from every other deployment.

@ColtenOuO ColtenOuO left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM overall.

Just a small suggestion: If you want to update your branch with the latest main, consider using rebase instead of merge.

This helps keep the commit history cleaner and easier to follow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants