Skip to content

Make OpenSearch log handler respect the per-run log ID template#69897

Draft
jason810496 wants to merge 5 commits into
apache:mainfrom
jason810496:fix/logging/os-respect-log-template
Draft

Make OpenSearch log handler respect the per-run log ID template#69897
jason810496 wants to merge 5 commits into
apache:mainfrom
jason810496:fix/logging/os-respect-log-template

Conversation

@jason810496

@jason810496 jason810496 commented Jul 15, 2026

Copy link
Copy Markdown
Member

Why

The LogTemplate model pins the log ID template to each Dag run so task logs written before a config change stay readable, but the Airflow 3 paths of the OpenSearch handler render log_id from the current conf value on both the write (worker) and read (API server) sides — so changing [opensearch] log_id_template orphans every previously written log document.

What

  • Write side: RemoteLogIO.upload opts into the ti_context keyword and renders the log ID from the Dag-run-pinned template delivered via TIRunContext.log_id_template (Deliver the per-run log ID template to workers via TIRunContext #69688).
  • Read side (_read, get_external_log_url, RemoteLogIO.read): restore the per-run LogTemplate lookup with a conf fallback when the DB is unreachable or the row is missing — the same pattern the legacy OsTaskHandler already used; worker-side RuntimeTIs skip the DB lookup entirely.
  • _render_log_id now always supplies the full placeholder set (logical_date, execution_date, data_interval_start/end) because templates pinned by older rows use them and str.format raises KeyError on missing names.

Was generative AI tooling used to co-author this PR?

The LogTemplate model pins the log_id template to each Dag run so task
logs written before a config change stay readable, but the Airflow 3
paths of the Elasticsearch and OpenSearch handlers rendered log_id from
the current conf value on both the write (worker) and read (API server)
sides, so changing [elasticsearch/opensearch] log_id_template orphaned
all previously written logs.

Workers cannot query the metadata DB, so the pinned template is
delivered to the supervisor through a new optional TIRunContext field
populated by the ti_run Execution API endpoint (with a version
migration for older clients); the API-server read paths restore the
per-run lookup directly. Handlers that do not opt into the new
ti_context keyword keep their existing upload signature and behavior.
…r-side TIs

Worker-side RuntimeTIs have no get_dagrun and cannot reach the metadata
DB; the pinned template reaches them through TIRunContext instead.
Opening a session just to fail also trips the DB-access guard
(AirflowInternalRuntimeError is a BaseException, so the fallback except
clause cannot catch it) in non-DB test runs.
…n supervisor tests

The remote-logging supervisor tests drop airflow.sdk.log (and the
logging-config modules) from sys.modules to force a fresh import, but
the re-import rebinds the attribute on the parent package and
monkeypatch only restores the sys.modules entry. On Python < 3.12
mock.patch resolves dotted targets through getattr on the parent
package, so later tests on the same worker patched the stale module
object while the code under test imported the restored one, failing
test_upload_logs_forwards_ti_context and
TestUploadToRemoteTIContext::test_no_ti_context_keeps_handler_default
in CI.
Keep this branch scoped to the core plumbing (Execution API and Task SDK)
that delivers the per-run log ID template to workers. The Elasticsearch
and OpenSearch handler changes re-land in per-provider PRs stacked on
this branch, so each provider change can be reviewed and released on its
own cadence.
The LogTemplate model pins the log_id template to each Dag run so task
logs written before a config change stay readable, but the Airflow 3
paths of the OpenSearch handler rendered log_id from the current conf
value on both the write (worker) and read (API server) sides, so
changing [opensearch] log_id_template orphaned all previously written
logs.

The write side now uses the pinned template that the supervisor receives
through TIRunContext (added by the core change this branch is stacked
on), and the API-server read paths restore the per-run DB lookup with a
conf fallback.
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.

1 participant