Skip to content
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

add log collection #306

Closed
wants to merge 2 commits into from
Closed

add log collection #306

wants to merge 2 commits into from

Conversation

zmarffy
Copy link
Contributor

@zmarffy zmarffy commented Mar 23, 2024

Implements a log collection system based on async task name.

@codecov-commenter
Copy link

codecov-commenter commented Mar 23, 2024

Codecov Report

Attention: Patch coverage is 90.14085% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 73.61%. Comparing base (ed37be4) to head (dd0d011).

Files Patch % Lines
taskiq/cli/worker/log_collector.py 90.47% 4 Missing ⚠️
taskiq/result/v1.py 0.00% 3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #306      +/-   ##
===========================================
- Coverage    78.00%   73.61%   -4.39%     
===========================================
  Files           61       61              
  Lines         1791     1823      +32     
===========================================
- Hits          1397     1342      -55     
- Misses         394      481      +87     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -294,7 +313,7 @@ async def run_task( # noqa: C901, PLR0912, PLR0915
# Assemble result.
result: "TaskiqResult[Any]" = TaskiqResult(
is_err=found_exception is not None,
log=None,
log=log,
Copy link
Contributor

Choose a reason for hiding this comment

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

The log here seems to be just the normal log in the task function, do we need to collect the exceptions generated by NoResultError and BaseException as well?

@zmarffy
Copy link
Contributor Author

zmarffy commented Mar 25, 2024

I have realized that this is not useful. It will only collect logs from the taskiq.tasklogger logger, not any modules that it calls. Celery essentially does the same thing with its get_task_logger function. However, Celery also implements something you can use to set up loggers to get imported modules to log with a Celery task ID: after_setup_logger.connect in conjunction with get_current_task. Maybe we should look into implementing something like this for Taskiq. I have no clue how that would be possible in an async environment, but it would unlock some good potential.

@zmarffy zmarffy closed this Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants