Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 17.7k
Send important executor logs to task logs#40468
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
76ba744
Send important executor logs to task logs
vincbeck de2af6c
Add unit test
vincbeck 907083b
Create session only when ti is `TaskInstanceKey`
vincbeck 6eb4286
Fix tests
vincbeck 34e00ce
Fix tests + adjustments
vincbeck 44fe1fc
Merge branch 'main' into vincbeck/task_log_executor
vincbeck 4fcdefc
Update error message
vincbeck ae80de4
Fix test
vincbeck 2abc248
Fix test
vincbeck 20e89e4
Remove `TaskInstanceKey` type from `_render_filename_db_access` method
vincbeck 595cbd6
Fix ruff
vincbeck bb7f1b6
Use correct `create_session`
vincbeck 9aad244
Update error message
vincbeck 083d15b
Update error message
vincbeck b3e6963
Move `ensure_ti` and makes it back private
vincbeck 260a1d5
Fix test
vincbeck 6b5061d
Reduce number of calls to `task_context_logger`
vincbeck b553197
Revert error message
vincbeck af1ca5b
Create method `send_message_to_task_logs` in `base_executor`
vincbeck 540e941
Handle case when `send_message_to_task_logs` does not exist in `base_…
vincbeck a56386c
Merge branch 'main' into vincbeck/task_log_executor
vincbeck File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
37 changes: 22 additions & 15 deletions
37 airflow/providers/amazon/aws/executors/ecs/ecs_executor.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -23,6 +23,7 @@ | ||
| from __future__ import annotations | ||
| import logging | ||
| import time | ||
| from collections import defaultdict, deque | ||
| from copy import deepcopy | ||
| @@ -347,7 +348,7 @@ def attempt_task_runs(self): | ||
| queue = ecs_task.queue | ||
| exec_config = ecs_task.executor_config | ||
| attempt_number = ecs_task.attempt_number | ||
| _failure_reasons = [] | ||
| failure_reasons = [] | ||
| if timezone.utcnow() < ecs_task.next_attempt_time: | ||
| self.pending_tasks.append(ecs_task) | ||
| continue | ||
| @@ -361,23 +362,21 @@ def attempt_task_runs(self): | ||
| if error_code in INVALID_CREDENTIALS_EXCEPTIONS: | ||
| self.pending_tasks.append(ecs_task) | ||
| raise | ||
| _failure_reasons.append(str(e)) | ||
| failure_reasons.append(str(e)) | ||
| except Exception as e: | ||
| # Failed to even get a response back from the Boto3 API or something else went | ||
| # wrong. For any possible failure we want to add the exception reasons to the | ||
| # failure list so that it is logged to the user and most importantly the task is | ||
| # added back to the pending list to be retried later. | ||
| _failure_reasons.append(str(e)) | ||
| failure_reasons.append(str(e)) | ||
| else: | ||
| # We got a response back, check if there were failures. If so, add them to the | ||
| # failures list so that it is logged to the user and most importantly the task | ||
| # is added back to the pending list to be retried later. | ||
| if run_task_response["failures"]: | ||
| _failure_reasons.extend([f["reason"] for f in run_task_response["failures"]]) | ||
| failure_reasons.extend([f["reason"] for f in run_task_response["failures"]]) | ||
| if _failure_reasons: | ||
| for reason in _failure_reasons: | ||
| failure_reasons[reason] += 1 | ||
| if failure_reasons: | ||
| # Make sure the number of attempts does not exceed MAX_RUN_TASK_ATTEMPTS | ||
| if int(attempt_number) < int(self.__class__.MAX_RUN_TASK_ATTEMPTS): | ||
| ecs_task.attempt_number += 1 | ||
| @@ -386,14 +385,19 @@ def attempt_task_runs(self): | ||
| ) | ||
| self.pending_tasks.append(ecs_task) | ||
| else: | ||
| self.log.error( | ||
vincbeck marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| "ECS task %s has failed a maximum of %s times. Marking as failed", | ||
| self.send_message_to_task_logs( | ||
| logging.ERROR, | ||
| "ECS task %s has failed a maximum of %s times. Marking as failed. Reasons: %s", | ||
| task_key, | ||
| attempt_number, | ||
| ", ".join(failure_reasons), | ||
| ti=task_key, | ||
| ) | ||
| self.fail(task_key) | ||
| elif not run_task_response["tasks"]: | ||
| self.log.error("ECS RunTask Response: %s", run_task_response) | ||
| self.send_message_to_task_logs( | ||
| logging.ERROR, "ECS RunTask Response: %s", run_task_response, ti=task_key | ||
| ) | ||
| raise EcsExecutorException( | ||
| "No failures and no ECS tasks provided in response. This should never happen." | ||
| ) | ||
| @@ -407,11 +411,6 @@ def attempt_task_runs(self): | ||
| # executor feature). | ||
| # TODO: remove when min airflow version >= 2.9.2 | ||
| pass | ||
| if failure_reasons: | ||
| self.log.error( | ||
| "Pending ECS tasks failed to launch for the following reasons: %s. Retrying later.", | ||
| dict(failure_reasons), | ||
| ) | ||
| def _run_task( | ||
| self, task_id: TaskInstanceKey, cmd: CommandType, queue: str, exec_config: ExecutorConfigType | ||
| @@ -543,3 +542,11 @@ def try_adopt_task_instances(self, tis: Sequence[TaskInstance]) -> Sequence[Task | ||
| not_adopted_tis = [ti for ti in tis if ti not in adopted_tis] | ||
| return not_adopted_tis | ||
| def send_message_to_task_logs(self, level: int, msg: str, *args, ti: TaskInstance | TaskInstanceKey): | ||
| # TODO: remove this method when min_airflow_version is set to higher than 2.10.0 | ||
| try: | ||
| super().send_message_to_task_logs(level, msg, *args, ti=ti) | ||
| except AttributeError: | ||
| # ``send_message_to_task_logs`` is added in 2.10.0 | ||
| self.log.error(msg, *args) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -24,6 +24,9 @@ | ||
| from typing import TYPE_CHECKING | ||
| from airflow.configuration import conf | ||
| from airflow.models.taskinstancekey import TaskInstanceKey | ||
| from airflow.utils.log.file_task_handler import _ensure_ti | ||
| from airflow.utils.session import create_session | ||
| if TYPE_CHECKING: | ||
| from airflow.models.taskinstance import TaskInstance | ||
| @@ -57,7 +60,7 @@ def __init__(self, component_name: str, call_site_logger: Logger | None = None): | ||
| def _should_enable(self) -> bool: | ||
| if not conf.getboolean("logging", "enable_task_context_logger"): | ||
| return False | ||
| if not getattr(self.task_handler, "supports_task_context_logging", False): | ||
| if not self.task_handler: | ||
dstandish marked this conversation as resolved.
Outdated
Uh oh!There was an error while loading. Please reload this page. | ||
| logger.warning("Task handler does not support task context logging") | ||
| return False | ||
| logger.info("Task context logging is enabled") | ||
| @@ -78,13 +81,13 @@ def _get_task_handler() -> FileTaskHandler | None: | ||
| assert isinstance(h, FileTaskHandler) | ||
| return h | ||
| def _log(self, level: int, msg: str, *args, ti: TaskInstance): | ||
| def _log(self, level: int, msg: str, *args, ti: TaskInstance | TaskInstanceKey): | ||
| """ | ||
| Emit a log message to the task instance logs. | ||
| :param level: the log level | ||
| :param msg: the message to relay to task context log | ||
| :param ti: the task instance | ||
| :param ti: the task instance or the task instance key | ||
| """ | ||
| if self.call_site_logger and self.call_site_logger.isEnabledFor(level=level): | ||
| with suppress(Exception): | ||
| @@ -98,6 +101,9 @@ def _log(self, level: int, msg: str, *args, ti: TaskInstance): | ||
| task_handler = copy(self.task_handler) | ||
| try: | ||
| if isinstance(ti, TaskInstanceKey): | ||
| with create_session() as session: | ||
| ti = _ensure_ti(ti, session) | ||
| task_handler.set_context(ti, identifier=self.component_name) | ||
| if hasattr(task_handler, "mark_end_on_close"): | ||
| task_handler.mark_end_on_close = False | ||
| @@ -109,7 +115,7 @@ def _log(self, level: int, msg: str, *args, ti: TaskInstance): | ||
| finally: | ||
| task_handler.close() | ||
| def critical(self, msg: str, *args, ti: TaskInstance): | ||
| def critical(self, msg: str, *args, ti: TaskInstance | TaskInstanceKey): | ||
| """ | ||
| Emit a log message with level CRITICAL to the task instance logs. | ||
| @@ -118,7 +124,7 @@ def critical(self, msg: str, *args, ti: TaskInstance): | ||
| """ | ||
| self._log(logging.CRITICAL, msg, *args, ti=ti) | ||
| def fatal(self, msg: str, *args, ti: TaskInstance): | ||
| def fatal(self, msg: str, *args, ti: TaskInstance | TaskInstanceKey): | ||
| """ | ||
| Emit a log message with level FATAL to the task instance logs. | ||
| @@ -127,7 +133,7 @@ def fatal(self, msg: str, *args, ti: TaskInstance): | ||
| """ | ||
| self._log(logging.FATAL, msg, *args, ti=ti) | ||
| def error(self, msg: str, *args, ti: TaskInstance): | ||
| def error(self, msg: str, *args, ti: TaskInstance | TaskInstanceKey): | ||
| """ | ||
| Emit a log message with level ERROR to the task instance logs. | ||
| @@ -136,7 +142,7 @@ def error(self, msg: str, *args, ti: TaskInstance): | ||
| """ | ||
| self._log(logging.ERROR, msg, *args, ti=ti) | ||
| def warn(self, msg: str, *args, ti: TaskInstance): | ||
| def warn(self, msg: str, *args, ti: TaskInstance | TaskInstanceKey): | ||
| """ | ||
| Emit a log message with level WARN to the task instance logs. | ||
| @@ -145,7 +151,7 @@ def warn(self, msg: str, *args, ti: TaskInstance): | ||
| """ | ||
| self._log(logging.WARNING, msg, *args, ti=ti) | ||
| def warning(self, msg: str, *args, ti: TaskInstance): | ||
| def warning(self, msg: str, *args, ti: TaskInstance | TaskInstanceKey): | ||
| """ | ||
| Emit a log message with level WARNING to the task instance logs. | ||
| @@ -154,7 +160,7 @@ def warning(self, msg: str, *args, ti: TaskInstance): | ||
| """ | ||
| self._log(logging.WARNING, msg, *args, ti=ti) | ||
| def info(self, msg: str, *args, ti: TaskInstance): | ||
| def info(self, msg: str, *args, ti: TaskInstance | TaskInstanceKey): | ||
| """ | ||
| Emit a log message with level INFO to the task instance logs. | ||
| @@ -163,7 +169,7 @@ def info(self, msg: str, *args, ti: TaskInstance): | ||
| """ | ||
| self._log(logging.INFO, msg, *args, ti=ti) | ||
| def debug(self, msg: str, *args, ti: TaskInstance): | ||
| def debug(self, msg: str, *args, ti: TaskInstance | TaskInstanceKey): | ||
| """ | ||
| Emit a log message with level DEBUG to the task instance logs. | ||
| @@ -172,7 +178,7 @@ def debug(self, msg: str, *args, ti: TaskInstance): | ||
| """ | ||
| self._log(logging.DEBUG, msg, *args, ti=ti) | ||
| def notset(self, msg: str, *args, ti: TaskInstance): | ||
| def notset(self, msg: str, *args, ti: TaskInstance | TaskInstanceKey): | ||
| """ | ||
| Emit a log message with level NOTSET to the task instance logs. | ||
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.