Skip to content

Surface worker DAG parse duration in task log#66138

Merged
kaxil merged 2 commits into
apache:mainfrom
astronomer:kaxilnaik/pm-2780-break-out-dag-parse-time-in-queued-to-running-path
May 1, 2026
Merged

Surface worker DAG parse duration in task log#66138
kaxil merged 2 commits into
apache:mainfrom
astronomer:kaxilnaik/pm-2780-break-out-dag-parse-time-in-queued-to-running-path

Conversation

@kaxil

@kaxil kaxil commented Apr 30, 2026

Copy link
Copy Markdown
Member

Customers and support engineers cannot tell whether a task's apparent slow startup (opaque gap between start_date and the first task log line) is due to their DAG file complexity or to Airflow-side bundle operations (git fetch, etc.).

Background

The worker's DAG parse happens after the TI state transitions to RUNNING:

  1. Supervisor calls client.task_instances.start(...) which hits the ti_run endpoint -- this sets TI.state=RUNNING and writes TI.start_date.
  2. Supervisor then sends StartupDetails to the worker subprocess.
  3. Worker calls parse() -- bundle fetch + DAG file import happen here.
  4. Worker runs the task.

So the existing queued_duration metric captures queued_dttm → RUNNING (pre-parse: pod boot, worker fork, supervisor handshake, API roundtrip). The parse phase itself is invisible in the UI today and shows up only as a gap between start_date and the first task log line.

What this PR does

Times the two phases inside parse() and emits a single structured log event just before return:

log.info(
    "Worker startup parse complete",
    bundle_name=...,
    bundle_version=...,
    dag_file=...,
    dag_id=...,
    bundle_prepare_ms=...,   # bundle fetch/verify (Airflow overhead)
    dag_file_parse_ms=...,   # DAG file import (user code)
)

The event lands in the customer's task log file (structlog kwargs, JSON-serialized by the supervisor's log channel). Support engineers can now answer "is it them or us?" from the first log line.

parse()'s public signature is unchanged (RuntimeTaskInstance); no test or pytest-fixture updates needed. Single file, ~18 lines added.

Design rationale

Why a log line and not a Stats.timing metric? Stats goes to StatsD/Prometheus dashboards, which most customers don't own. The task log file is what customers read when debugging slow tasks -- that's where the answer needs to be. Adding Stats.timing also requires registering metrics in shared/observability/src/airflow_shared/observability/metrics/metrics_template.yaml and following the _duration naming convention -- deferred as a separate PR if ops wants dashboard integration.

Why split bundle_prepare vs dag_file_parse? Different failure modes, different owners. bundle_prepare_ms is bimodal by nature: near-zero for warm GitDagBundle (cached worktree), non-trivial on first task after a bundle version change. dag_file_parse_ms attributes to user DAG code (heavy imports, top-level time.sleep, etc.).

Why emit from inside parse() instead of from startup()? Keeping parse()'s signature stable avoids breaking downstream callers (pytest fixtures, tests, any unknown use). Under run_as_user impersonation, parse() runs twice (pre-sudo and post-sudo), so this event fires twice for those tasks. The two entries expose impersonation re-parse overhead, which is informative rather than a defect.

Gotchas / known limitations

  • Pre-parse gap is not measured. Worker process fork, Python import, supervisor handshake, and StartupDetails fetch can dominate on cold K8sExecutor pods. queued_duration already measures that full pre-RUNNING gap end-to-end; this PR adds the post-RUNNING phase.
  • Warm GitDagBundle shows near-zero bundle_prepare_ms. Expected -- the cached worktree check short-circuits. Only cold pulls (first task after a version change) emit non-trivial values, and that latency is real.
  • run_as_user impersonation emits the event twice per task. Once in the pre-sudo process, once in the sudo'd process. Customers reading the task log see both -- the second one is the measurement that matters for their task's real startup.

kaxil added 2 commits April 18, 2026 19:37
Customers see an opaque 'queued -> running -> first log line' delay and
cannot tell whether slow task startup is due to their DAG file complexity
or Airflow-side bundle operations (e.g. git fetch).

The worker's parse() call happens AFTER the TI state transitions to
RUNNING (supervisor.py:1031 -> api_fastapi/.../task_instances.py:108 sets
state=RUNNING and writes start_date before StartupDetails is sent to the
worker). Today that post-RUNNING parse phase is invisible in the UI and
appears as a gap between start_date and the first task log line.

This change times the two phases inside parse() and emits a single
structured "Worker startup parse complete" log event in startup() after
the run_as_user re-exec check (so it fires exactly once per task, never
twice on impersonated runs):

- bundle_prepare_ms: bundle fetch/verify (Airflow overhead; near-zero for
  warm GitDagBundle, non-trivial on first task after a version change)
- dag_file_parse_ms: DAG file import (user code)

The log line surfaces directly in the customer's task log file. Support
engineers can now answer "is it them or us?" from the first log line.

This is a partial closure of PM-2780. Full closure requires persisting
parse duration on the TaskInstance model and surfacing it in the UI
(separate, larger PR). Emitting as Stats.timing metrics was considered
and deferred -- worker log is the customer-facing surface; Stats goes to
ops dashboards and requires metrics registry + naming convention work
(shared/observability/.../metrics_template.yaml).
Reverts the parse() signature change and the test/fixture adaptations
from the prior commit. parse() again returns just RuntimeTaskInstance.
The "Worker startup parse complete" log event is emitted from inside
parse() right before return, keeping the public surface of the function
stable for any downstream callers (pytest fixtures, etc.).

Under `run_as_user` impersonation, parse() runs twice (pre-sudo and
post-sudo), so this event fires twice for those tasks. The two entries
expose impersonation re-parse overhead, which is informative (tasks
with run_as_user pay for parse twice) rather than a defect.
@kaxil kaxil merged commit ce276e7 into apache:main May 1, 2026
211 of 212 checks passed
@kaxil kaxil deleted the kaxilnaik/pm-2780-break-out-dag-parse-time-in-queued-to-running-path branch May 1, 2026 11:18
@vatsrahul1001 vatsrahul1001 added this to the Airflow 3.3.0 milestone Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants