-
Notifications
You must be signed in to change notification settings - Fork 3
Add TaskLogger dependency
#25
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
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import abc | ||
| import inspect | ||
| import logging | ||
| from datetime import timedelta | ||
| from typing import Any, Awaitable, Callable, Counter, cast | ||
|
|
||
|
|
@@ -55,6 +56,26 @@ def TaskKey() -> str: | |
| return cast(str, _TaskKey()) | ||
|
|
||
|
|
||
| class _TaskLogger(Dependency): | ||
| def __call__( | ||
| self, docket: Docket, worker: Worker, execution: Execution | ||
| ) -> logging.LoggerAdapter: | ||
| logger = logging.getLogger(f"docket.task.{execution.key}") | ||
|
|
||
| extra = { | ||
| "task.key": execution.key, | ||
| "task.attempt": execution.attempt, | ||
|
||
| "worker.name": worker.name, | ||
| "docket.name": docket.name, | ||
| } | ||
|
|
||
| return logging.LoggerAdapter(logger, extra) | ||
|
|
||
|
|
||
| def TaskLogger() -> logging.LoggerAdapter[logging.Logger]: | ||
| return cast(logging.LoggerAdapter[logging.Logger], _TaskLogger()) | ||
|
|
||
|
|
||
| class Retry(Dependency): | ||
| single: bool = True | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,9 @@ | |
| as possible to aid with understanding docket. | ||
| """ | ||
|
|
||
| import logging | ||
| from datetime import datetime, timedelta | ||
| from logging import LoggerAdapter | ||
| from typing import Callable | ||
| from unittest.mock import AsyncMock | ||
| from uuid import uuid4 | ||
|
|
@@ -20,6 +22,7 @@ | |
| Execution, | ||
| Retry, | ||
| TaskKey, | ||
| TaskLogger, | ||
| Worker, | ||
| ) | ||
|
|
||
|
|
@@ -390,3 +393,32 @@ async def the_task(a: str, b: str, this_key: str = TaskKey()): | |
| await worker.run_until_current() | ||
|
|
||
| assert called | ||
|
|
||
|
|
||
| async def test_logging_inside_of_task( | ||
| docket: Docket, | ||
| worker: Worker, | ||
| now: Callable[[], datetime], | ||
| caplog: pytest.LogCaptureFixture, | ||
| ): | ||
| """docket should support providing a logger with task context""" | ||
| called = False | ||
|
|
||
| async def the_task(a: str, b: str, logger: LoggerAdapter = TaskLogger()): | ||
| assert a == "a" | ||
| assert b == "c" | ||
|
|
||
| logger.info("Task is running") | ||
|
|
||
| nonlocal called | ||
| called = True | ||
|
|
||
| await docket.add(the_task, key="my-cool-task:123")("a", b="c") | ||
|
|
||
| with caplog.at_level(logging.INFO): | ||
| await worker.run_until_current() | ||
|
|
||
| assert called | ||
| assert "Task is running" in caplog.text | ||
| assert "docket.task.my-cool-task:123" in caplog.text | ||
| print(caplog.text) | ||
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could make this configurable but I kind of feel like we always want the logger to be the task identifier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this should be
"docket.task.{execution.function.__name__}"? The key is unique to each time a task runs, so if we made that the logger name, it might be pretty hard to filter it in logging tools? These also form like a hierarchical tree inside of theloggingmodule so it might get bogged down if there are tons of unique ones (not totally sure about that)