Skip to content

Conversation

@HsiuChuanHsu
Copy link
Contributor

@HsiuChuanHsu HsiuChuanHsu commented Jul 21, 2025

Description

Addresses the issue where KubernetesPodOperator (KPO) logs always include a container name prefix, which can consume unnecessary horizontal space and feel redundant. This PR introduces two new optional parameters to KubernetesPodOperator:

  • log_prefix: A boolean to enable/disable the container name prefix in logs (default: True).
  • log_formatter: A callable to fully customize log output format, overriding log_prefix if provided.

This allows users to either disable the prefix or define a custom log format, improving log readability and flexibility.

Changes

  • Added log_prefix: bool = True and log_formatter: Callable[[str, str], str] | None = None to KubernetesPodOperator constructor.
  • Updated PodManager methods (fetch_container_logs, fetch_requested_init_container_logs, fetch_requested_container_logs) to accept and use these new parameters.
  • Modified log handling in PodManager.fetch_container_logs to:
    • Apply log_formatter if provided.
    • Use log_prefix to include/exclude container name prefix when log_formatter is not set.
  • Added pytest unit tests to verify:
    • Default behavior with log_prefix=True.
    • Behavior with log_prefix=False (no container name prefix).
    • Custom log formatting with log_formatter.

Related Issue

#53054


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! LGTM overall but the unit still needs fix.

@HsiuChuanHsu HsiuChuanHsu force-pushed the feature/kpo-log-prefix branch from 3d0d615 to af7174e Compare July 22, 2025 15:46
Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

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

Thanks for the update, only a small nit and this PR is good to be merged.

After #53475 has merged, we don't need pytest.mark.asyncio any more.
It would be nice to remove the mark and rebase again, thanks!

@HsiuChuanHsu HsiuChuanHsu force-pushed the feature/kpo-log-prefix branch from 4d639e0 to 0ce8fd1 Compare July 27, 2025 15:00
@NilsJPWerner
Copy link
Contributor

@HsiuChuanHsu I think this is ready to merge

@HsiuChuanHsu
Copy link
Contributor Author

@NilsJPWerner Yes, this PR is nearly complete, but I don't have the right to merge the code. Still wait for approval to merge.

Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for the fix!

HsiuChuanHsu and others added 6 commits August 20, 2025 21:20
- Introduce  parameter to toggle container name prefix in logs
- Add  parameter for custom log message formatting
- Update PodManager to support log_prefix and log_formatter in log fetching
- Add test_log_prefix_enabled to verify default log_prefix=True behavior
- Add test_log_prefix_disabled to confirm log_prefix=False removes container prefix
- Add test_custom_log_formatter to validate custom log_formatter functionality
…ter options

Consolidated log prefix and formatter tests into a single parameterized test_log_output_configurations, covering:
log_prefix=True: Includes container name prefix ([base]).
log_prefix=False: Excludes container name prefix.
Custom log formatter: Applies user-defined formatting (e.g., CUSTOM[base]: message)
Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com>
    - Replace caplog with targeted mocking of PodManager logger
…g in KubernetesPodOperator

- Rename log_prefix to  container_name_log_prefix_enabled parameter to control container name prefix in logs

- Extract duplicate log formatting logic into _log_message private method in PodManager

- Update docstrings with detailed parameter descriptions
@HsiuChuanHsu HsiuChuanHsu force-pushed the feature/kpo-log-prefix branch from 16ee837 to 4277d18 Compare August 20, 2025 14:53
@jason810496 jason810496 requested review from jedcunningham and removed request for jedcunningham August 22, 2025 14:58
@jason810496 jason810496 merged commit d9f5d47 into apache:main Aug 24, 2025
84 checks passed
@HsiuChuanHsu
Copy link
Contributor Author

Thanks for the review!! 🤩

mangal-vairalkar pushed a commit to mangal-vairalkar/airflow that referenced this pull request Aug 30, 2025
…pache#53598)

* Add custom log formatting and prefix control to KubernetesPodOperator

- Introduce  parameter to toggle container name prefix in logs
- Add  parameter for custom log message formatting
- Update PodManager to support log_prefix and log_formatter in log fetching

* Add tests for log prefix and custom formatter in KubernetesPodOperator

- Add test_log_prefix_enabled to verify default log_prefix=True behavior
- Add test_log_prefix_disabled to confirm log_prefix=False removes container prefix
- Add test_custom_log_formatter to validate custom log_formatter functionality

* Enhance KubernetesPodOperator logging with flexible prefix and formatter options

Consolidated log prefix and formatter tests into a single parameterized test_log_output_configurations, covering:
log_prefix=True: Includes container name prefix ([base]).
log_prefix=False: Excludes container name prefix.
Custom log formatter: Applies user-defined formatting (e.g., CUSTOM[base]: message)

* Remove @pytest.mark.asyncio

Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com>

* Modify test_log_output_configurations to improve log message capturing:
    - Replace caplog with targeted mocking of PodManager logger

* Add custom log formatting support to KubernetesPodOperator tests

* feat(kubernetes): Add log formatting options and refactor log handling in KubernetesPodOperator

- Rename log_prefix to  container_name_log_prefix_enabled parameter to control container name prefix in logs

- Extract duplicate log formatting logic into _log_message private method in PodManager

- Update docstrings with detailed parameter descriptions

---------

Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com>
nothingmin pushed a commit to nothingmin/airflow that referenced this pull request Sep 2, 2025
…pache#53598)

* Add custom log formatting and prefix control to KubernetesPodOperator

- Introduce  parameter to toggle container name prefix in logs
- Add  parameter for custom log message formatting
- Update PodManager to support log_prefix and log_formatter in log fetching

* Add tests for log prefix and custom formatter in KubernetesPodOperator

- Add test_log_prefix_enabled to verify default log_prefix=True behavior
- Add test_log_prefix_disabled to confirm log_prefix=False removes container prefix
- Add test_custom_log_formatter to validate custom log_formatter functionality

* Enhance KubernetesPodOperator logging with flexible prefix and formatter options

Consolidated log prefix and formatter tests into a single parameterized test_log_output_configurations, covering:
log_prefix=True: Includes container name prefix ([base]).
log_prefix=False: Excludes container name prefix.
Custom log formatter: Applies user-defined formatting (e.g., CUSTOM[base]: message)

* Remove @pytest.mark.asyncio

Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com>

* Modify test_log_output_configurations to improve log message capturing:
    - Replace caplog with targeted mocking of PodManager logger

* Add custom log formatting support to KubernetesPodOperator tests

* feat(kubernetes): Add log formatting options and refactor log handling in KubernetesPodOperator

- Rename log_prefix to  container_name_log_prefix_enabled parameter to control container name prefix in logs

- Extract duplicate log formatting logic into _log_message private method in PodManager

- Update docstrings with detailed parameter descriptions

---------

Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com>
@HsiuChuanHsu HsiuChuanHsu deleted the feature/kpo-log-prefix branch September 9, 2025 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants