You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "safaehar (via GitHub)" <gi...@apache.org> on 2026/06/15 18:49:00 UTC

[PR] Fix triggerer CrashLoopBackOff when json_logs is enabled [airflow]

safaehar opened a new pull request, #68584:
URL: https://github.com/apache/airflow/pull/68584

   ### What
   
   `TriggerRunnerSupervisor._process_log_messages_from_subprocess` primes itself by calling `airflow.sdk.log.configure_logging()` with **no arguments**. `json_output` defaults to `False`, so this call reconfigures structlog **globally** and installs the text `WriteLogger` factory — overwriting the bytes `BytesLogger` factory that startup set up when `[logging] json_logs = True`.
   
   The stdout/stderr forwarders (`_create_log_forwarder` → `forward_to_log`) are already wrapped with the JSON (bytes) processor chain but bind their underlying logger **lazily**. As soon as a trigger subprocess writes to stdout/stderr — e.g. an import-time `DeprecationWarning` from a provider trigger that imports a heavy client such as `KubernetesPodTrigger` (kubernetes) or `S3KeyTrigger` (boto3) — the lazy bind resolves against the now-text factory, and `WriteLogger.msg` does `message + "\n"` on bytes produced by the JSON renderer:
   
   ```
   TypeError: can't concat str to bytes
     File ".../structlog/_output.py", line 217, in msg
       self._write(message + "\n")
   ```
   
   This crashes the `TriggerRunnerSupervisor.run` loop and the triggerer enters CrashLoopBackOff. It only manifests when `json_logs` is enabled **and** the triggerer actually runs a trigger whose subprocess emits to stdout/stderr — idle triggerers and those running only quiet triggers (`DateTimeTrigger`, etc.) never reach `forward_to_log`, which is why the crash looks intermittent across deployments.
   
   ### Fix
   
   Pass `json_output` from the `[logging] json_logs` config (the same `conf.getboolean("logging", "json_logs", fallback=False)` used elsewhere, e.g. `airflow/logging_config.py`) so the global structlog factory stays consistent with the rest of the process. This matches the already-hardcoded `logging_processors(json_output=True)` a few lines below.
   
   ### Tests
   
   Adds a parametrized regression test asserting `_process_log_messages_from_subprocess` calls `configure_logging(json_output=<json_logs>)` for both `True` and `False`.
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   -->
   
   ---
   
   ^ Add meaningful description above. Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines) for more information.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix triggerer CrashLoopBackOff when json_logs is enabled [airflow]

Posted by "SameerMesiah97 (via GitHub)" <gi...@apache.org>.
SameerMesiah97 commented on code in PR #68584:
URL: https://github.com/apache/airflow/pull/68584#discussion_r3424432375


##########
airflow-core/tests/unit/jobs/test_triggerer_job.py:
##########
@@ -314,6 +314,31 @@ def fake_run_once(self):
     assert events == ["enter", "tick-1", "tick-2", "tick-3", "exit"]
 
 
+@pytest.mark.parametrize("json_logs", [True, False])
+def test_process_log_messages_configures_logging_matching_json_logs(supervisor_builder, mocker, json_logs):
+    """``_process_log_messages_from_subprocess`` must reconfigure structlog with the
+    ``logging.json_logs`` setting, not the ``configure_logging`` default of ``False``.
+
+    Priming this generator calls ``configure_logging()``, which reconfigures structlog
+    globally. With ``json_logs=True`` but ``json_output`` left to default ``False`` the
+    global logger factory becomes the text ``WriteLogger`` while the stdout/stderr
+    forwarders emit bytes from the JSON renderer, crashing the triggerer with
+    ``TypeError: can't concat str to bytes`` the first time a trigger subprocess writes
+    to stdout/stderr. See the regression where enabling json_logs CrashLoopBackOff-ed
+    triggerers running providers that warn at import time.

Review Comment:
   This docstring is too long. I would suggest something like this instead:
   
   ```
   """
   _process_log_messages_from_subprocess() must reconfigure logging using the
   configured ``logging.json_logs`` value rather than the default
   ``json_output=False``.
   
   This generator reconfigures structlog globally when first primed. Failing to
   propagate the configured JSON logging mode can leave the triggerer with an
   inconsistent logging configuration and break subprocess log forwarding.
   """
   ```



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix triggerer CrashLoopBackOff when json_logs is enabled [airflow]

Posted by "safaehar (via GitHub)" <gi...@apache.org>.
safaehar commented on code in PR #68584:
URL: https://github.com/apache/airflow/pull/68584#discussion_r3426817904


##########
airflow-core/tests/unit/jobs/test_triggerer_job.py:
##########
@@ -314,6 +314,31 @@ def fake_run_once(self):
     assert events == ["enter", "tick-1", "tick-2", "tick-3", "exit"]
 
 
+@pytest.mark.parametrize("json_logs", [True, False])
+def test_process_log_messages_configures_logging_matching_json_logs(supervisor_builder, mocker, json_logs):
+    """``_process_log_messages_from_subprocess`` must reconfigure structlog with the
+    ``logging.json_logs`` setting, not the ``configure_logging`` default of ``False``.
+
+    Priming this generator calls ``configure_logging()``, which reconfigures structlog
+    globally. With ``json_logs=True`` but ``json_output`` left to default ``False`` the
+    global logger factory becomes the text ``WriteLogger`` while the stdout/stderr
+    forwarders emit bytes from the JSON renderer, crashing the triggerer with
+    ``TypeError: can't concat str to bytes`` the first time a trigger subprocess writes
+    to stdout/stderr. See the regression where enabling json_logs CrashLoopBackOff-ed
+    triggerers running providers that warn at import time.

Review Comment:
   thanks for the review, adressed



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix triggerer CrashLoopBackOff when json_logs is enabled [airflow]

Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala commented on code in PR #68584:
URL: https://github.com/apache/airflow/pull/68584#discussion_r3615069817


##########
airflow-core/newsfragments/68584.bugfix.rst:
##########


Review Comment:
   no need for a newsfragment, it is only needed when there is a significant change



##########
airflow-core/src/airflow/jobs/triggerer_job_runner.py:
##########
@@ -941,7 +941,13 @@ def _process_log_messages_from_subprocess(self) -> Generator[None, bytes | bytea
 
         from airflow.sdk.log import configure_logging
 
-        configure_logging()
+        # Must match the json_logs setting: calling configure_logging() here reconfigures
+        # structlog globally. Defaulting json_output to False would install the text
+        # WriteLogger factory, while the stdout/stderr forwarders (forward_to_log) emit
+        # bytes from the JSON renderer -- crashing the triggerer with
+        # "TypeError: can't concat str to bytes" the first time a trigger subprocess
+        # writes to stdout/stderr.

Review Comment:
   I believe the comment is useless (mostly auto-added by AI agent), it could be removed



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix triggerer CrashLoopBackOff when json_logs is enabled [airflow]

Posted by "vatsrahul1001 (via GitHub)" <gi...@apache.org>.
vatsrahul1001 merged PR #68584:
URL: https://github.com/apache/airflow/pull/68584


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix triggerer CrashLoopBackOff when json_logs is enabled [airflow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #68584:
URL: https://github.com/apache/airflow/pull/68584#issuecomment-5115272970

   ### Backport successfully created: v3-3-test
   
   Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
   the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.
   
   In matter of doubt please ask in [#release-management](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
   
   <table>
           <tr>
           <th>Status</th>
           <th>Branch</th>
           <th>Result</th>
       </tr>
       <tr>
           <td>✅</td>
           <td>v3-3-test</td>
           <td><a href="https://github.com/apache/airflow/pull/70669"><img src="https://img.shields.io/badge/PR-70669-blue" alt="PR Link"></a></td>
       </tr>
   </table>


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix triggerer CrashLoopBackOff when json_logs is enabled [airflow]

Posted by "safaehar (via GitHub)" <gi...@apache.org>.
safaehar commented on code in PR #68584:
URL: https://github.com/apache/airflow/pull/68584#discussion_r3615509462


##########
airflow-core/newsfragments/68584.bugfix.rst:
##########


Review Comment:
   adressed



##########
airflow-core/src/airflow/jobs/triggerer_job_runner.py:
##########
@@ -941,7 +941,13 @@ def _process_log_messages_from_subprocess(self) -> Generator[None, bytes | bytea
 
         from airflow.sdk.log import configure_logging
 
-        configure_logging()
+        # Must match the json_logs setting: calling configure_logging() here reconfigures
+        # structlog globally. Defaulting json_output to False would install the text
+        # WriteLogger factory, while the stdout/stderr forwarders (forward_to_log) emit
+        # bytes from the JSON renderer -- crashing the triggerer with
+        # "TypeError: can't concat str to bytes" the first time a trigger subprocess
+        # writes to stdout/stderr.

Review Comment:
   adressed



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org