dost4 opened a new pull request, #71950:
URL: https://github.com/apache/airflow/pull/71950
## What
`KubernetesPodOperator._write_logs` — the bulk log relay on the deferrable
resume path — emits every container line with a hardcoded `self.log.info(...)`.
It never consults `_parse_log_level`, and it silently ignores the operator's
`log_formatter` and `container_name_log_prefix_enabled` options.
The result is that on the deferrable path a container line reading `ERROR:
...` lands in the Airflow task log at INFO, so anything that filters task logs
by severity (alerting, log queries, log-tail extraction) cannot tell a
container error from routine output. The non-deferrable path already behaves
correctly via `PodManager._log_message`.
This PR lifts the emit logic out of `PodManager._log_message` into a
module-level `emit_pod_log_line()` in `utils/pod_manager.py` and has both paths
call it, so level forwarding, `log_formatter` and
`container_name_log_prefix_enabled` behave identically in deferrable and
non-deferrable mode.
## Why the gap exists
#64829 ("Fix KubernetesPodOperator to forward pod log levels to Airflow task
logs") changed `utils/pod_manager.py` and its unit tests only — per that PR's
file list, `operators/pod.py` was never touched. #67133 requests the same
behaviour for `EcsRunTaskOperator`, so forwarding container log levels appears
to be wanted consistently across providers.
## Reviewer notes
Three things worth an explicit look:
1. **User-visible message change.** Deferrable output was `[base] logs:
<line>`; it is now `[base] <line>`, matching the synchronous path and the
documented meaning of `container_name_log_prefix_enabled`. Consistency between
the two paths is the point of the change, but it will affect anyone
string-matching the old ` logs: ` infix. I am happy to preserve the old wording
when no formatter is set if reviewers prefer to avoid the break.
2. **One existing test was updated**, not to accommodate a regression but
because it asserted the old prefix:
`test_async_write_logs_should_execute_successfully` now expects `[base] ok`.
The assertion is still exact, not weakened to a substring match.
3. **Group markers.** `_log_message` routes `is_log_group_marker` lines to
`print()`; `_write_logs` previously did not. Sharing the helper gains that
handling on the deferrable path — intended as part of the parity fix.
A `SupportsLog` protocol is used for the helper's `logger` parameter because
the two call sites pass different structlog logger types
(`airflow._shared.logging.types.Logger` and
`airflow.sdk._shared.logging.types.Logger`), neither of which is a
`logging.Logger`. Passing the logger in rather than using `self.log` keeps each
caller's records under their own logger name.
## Tests
Added to `TestKubernetesPodOperatorAsync`, all exercising the deferrable
`_write_logs` path: level forwarding for `ERROR` / `WARNING` / `CRITICAL` /
unprefixed lines (asserting `record.levelno`, not just text), `log_formatter`
being honoured, `container_name_log_prefix_enabled=False`, and group markers
going to `print` rather than the logger.
`test_pod_manager.py` passes unchanged, which guards the synchronous path
against the extraction. Locally, the full `test_pod_manager.py` + `test_pod.py`
run is 398 passed / 4 failed, where the 4 failures are the pre-existing
time-dependent `since_time` tests that fail identically on an unmodified `main`.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Opus 5)
Generated-by: Claude Code (Opus 5) following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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]
