You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/07/15 18:45:15 UTC

[GitHub] [airflow] pmalafosse opened a new issue #17038: ECSOperator returns last logs when ECS task fails

pmalafosse opened a new issue #17038:
URL: https://github.com/apache/airflow/issues/17038


   **Description**
   
   Currently when the ECSOperator fails because the ECS task is not in 'success' state it returns a generic message like that in Airflow alerts that doesn't have much value when we want to debug things quickly.
   
   `This task is not in success state {<huge JSON from AWS containing all the ECS task details>}`
   
   
   **Use case / motivation**
   
   The idea would be to return instead the last lines of logs from Cloudwatch (that are printed above in Airflow logs) so when we receive the alert we know what failed in the ECS task instead of having to go to Airflow logs to find it. This feature would involve changes there I think:
   - https://github.com/apache/airflow/blob/2ce6e8de53adc98dd3ae80fa7c74b38eb352bc3a/airflow/providers/amazon/aws/operators/ecs.py#L354
   - https://github.com/apache/airflow/blob/2ce6e8de53adc98dd3ae80fa7c74b38eb352bc3a/airflow/providers/amazon/aws/operators/ecs.py#L375
   
   


-- 
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



[GitHub] [airflow] uranusjr commented on issue #17038: ECSOperator returns last logs when ECS task fails

Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #17038:
URL: https://github.com/apache/airflow/issues/17038#issuecomment-881074172


   Could you describe what exactly you would like to see (and probably how that can be implemented)? The value of `self._last_log_message()`?


-- 
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



[GitHub] [airflow] uranusjr commented on issue #17038: ECSOperator returns last logs when ECS task fails

Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #17038:
URL: https://github.com/apache/airflow/issues/17038#issuecomment-881048668


   Would it be better if we include the `task` object in the exception? If we’re to return anything more specific, we can subclass `AirflowException`, which allows putting a lot more context on the exception object.


-- 
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



[GitHub] [airflow] potiuk commented on issue #17038: ECSOperator returns last logs when ECS task fails

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #17038:
URL: https://github.com/apache/airflow/issues/17038#issuecomment-882785093


   Assigned you @pmalafosse 


-- 
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



[GitHub] [airflow] pmalafosse edited a comment on issue #17038: ECSOperator returns last logs when ECS task fails

Posted by GitBox <gi...@apache.org>.
pmalafosse edited a comment on issue #17038:
URL: https://github.com/apache/airflow/issues/17038#issuecomment-881080730


   Yes I was thinking of doing something like that as I'm not sure there is a consistent way to identify the rows corresponding to the Exception message from the code running in ECS from Cloudwatch logs:
   
   something like `raise AirflowException(self._last_log_messages(5))` that would return the last 5 lines from Cloudwatch for example (_last_log_message() would be equal to _last_log_messages(1))


-- 
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



[GitHub] [airflow] pmalafosse commented on issue #17038: ECSOperator returns last logs when ECS task fails

Posted by GitBox <gi...@apache.org>.
pmalafosse commented on issue #17038:
URL: https://github.com/apache/airflow/issues/17038#issuecomment-881071040


   Sorry I don’t understand what you mean. Now`task` object is in the exception but I would like instead to see, in Airflow alerts, the exception from the code that ran in ECS, not the exception defined in the operator that just says “ECS task failed” with a big JSON that I think is irrelevant noise.


-- 
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



[GitHub] [airflow] uranusjr commented on issue #17038: ECSOperator returns last logs when ECS task fails

Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #17038:
URL: https://github.com/apache/airflow/issues/17038#issuecomment-882260418


   We can probably use `collections.deque` for better performance, but aside from that, sounds good to me!
   
   p.s. I don’t think `IndexError` can ever be raised?


-- 
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



[GitHub] [airflow] potiuk commented on issue #17038: ECSOperator returns last logs when ECS task fails

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #17038:
URL: https://github.com/apache/airflow/issues/17038#issuecomment-882785093


   Assigned you @pmalafosse 


-- 
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



[GitHub] [airflow] kaxil closed issue #17038: ECSOperator returns last logs when ECS task fails

Posted by GitBox <gi...@apache.org>.
kaxil closed issue #17038:
URL: https://github.com/apache/airflow/issues/17038


   


-- 
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



[GitHub] [airflow] pmalafosse commented on issue #17038: ECSOperator returns last logs when ECS task fails

Posted by GitBox <gi...@apache.org>.
pmalafosse commented on issue #17038:
URL: https://github.com/apache/airflow/issues/17038#issuecomment-881308665


   I tried locally and my proposal would be like that:
   
   ```
       def _last_log_messages(self, number_messages):
           try:
               logs = [log["message"] for log in self._cloudwatch_log_events()]
               return "\n".join(logs[-number_messages:])
           except IndexError:
               return None
   
       def _last_log_message(self):
           return self._last_log_messages(1)
   
   ...
   
   raise AirflowException(f"This task is not in success state - last logs from Cloudwatch: \n{self._last_log_messages(10)}")
   ```


-- 
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



[GitHub] [airflow] potiuk commented on issue #17038: ECSOperator returns last logs when ECS task fails

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #17038:
URL: https://github.com/apache/airflow/issues/17038#issuecomment-882785093


   Assigned you @pmalafosse 


-- 
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



[GitHub] [airflow] uranusjr edited a comment on issue #17038: ECSOperator returns last logs when ECS task fails

Posted by GitBox <gi...@apache.org>.
uranusjr edited a comment on issue #17038:
URL: https://github.com/apache/airflow/issues/17038#issuecomment-882260418


   We can probably use `collections.deque` for better performance, but aside from that, sounds good to me! Would you be interested in opening a pull request for this?
   
   p.s. I don’t think `IndexError` can ever be raised?


-- 
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



[GitHub] [airflow] pmalafosse commented on issue #17038: ECSOperator returns last logs when ECS task fails

Posted by GitBox <gi...@apache.org>.
pmalafosse commented on issue #17038:
URL: https://github.com/apache/airflow/issues/17038#issuecomment-881080730


   Yes I was thinking of doing something like that as I'm not sure there is a consistent way to identify the rows corresponding to the Exception message from the code running in ECS from Cloudwatch logs:
   
   something like `raise AirflowException(self._last_log_messages(5)})` that would return the last 5 lines from Cloudwatch for example (_last_log_message() would be equal to _last_log_messages(1))


-- 
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