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 some 🟥🟧🟨🟩🟦🟪 to the logs #478

Merged
merged 3 commits into from
Apr 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions goth/assertions/monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import sys
from typing import Callable, Generic, List, Optional, Sequence, Union

import colors

from goth.assertions import Assertion, AssertionFunction, E


Expand Down Expand Up @@ -257,15 +259,20 @@ async def _check_assertions(self, events_ended: bool) -> None:

await a.update_events(events_ended=events_ended)

if a.done:
self._logger.debug(
"Assertion '%s' finished after event %s", a.name, event_descr
)

Comment on lines +262 to +266
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previous assertion success/failure messages that included details of events that made the assertion succeed or fail were a bit unreadable, so I've decided to split them in two messages: one containing the event details and logged with DEBUG level, and another one, in color, only stating that the assertion succeeds or fails with a given result, logged as before (usually as INFO).

if a.accepted:
result = a.result()
msg = "Assertion '%s' succeeded after event: %s; result: %s"
self._logger.log(level, msg, a.name, event_descr, result)
msg = colors.green("Assertion '%s' succeeded; result: %s", style="bold")
self._logger.log(level, msg, a.name, result)

elif a.failed:
await self._report_failure(a, event_descr)
await self._report_failure(a)

async def _report_failure(self, a: Assertion, event_descr: str) -> None:
async def _report_failure(self, a: Assertion) -> None:
try:
a.result()
except Exception as exc:
Expand All @@ -276,10 +283,8 @@ async def _report_failure(self, a: Assertion, event_descr: str) -> None:
# functions are left.
for _ in (1, 2, 3):
tb = tb.tb_next if tb else tb
msg = "Assertion '%s' failed after event: %s; cause: %s"
self._logger.error(
msg, a.name, event_descr, exc, exc_info=(type(exc), exc, tb)
)
msg = colors.red("Assertion '%s' failed; cause: %s", style="bold")
self._logger.error(msg, a.name, exc, exc_info=(type(exc), exc, tb))

@property
def satisfied(self) -> Sequence[Assertion[E]]:
Expand Down
57 changes: 36 additions & 21 deletions goth/runner/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import time
from typing import Iterator, Optional, Union

import colors

import goth
import goth.api_monitor
from goth.assertions.monitor import EventMonitor
Expand All @@ -20,32 +22,37 @@
logger = logging.getLogger(__name__)


class UTCFormatter(logging.Formatter):
"""Custom `logging` formatter that uses `time.gmtime`."""
class CustomFileLogFormatter(logging.Formatter):
"""`Formatter` that uses `time.gmtime` for time and strips ANSI color codes."""

converter = time.gmtime

def format(self, record: logging.LogRecord) -> str:
"""Format the message and remove ANSI color codes from it."""
text = super().format(record)
return colors.strip_color(text)


LOGGING_CONFIG = {
"version": 1,
"formatters": {
"none": {"format": "%(message)s"},
"simple": {"format": "%(levelname)-8s [%(name)-30s] %(message)s"},
"date": {
"()": UTCFormatter,
"console": {"format": "%(levelname)-8s [%(name)-30s] %(message)s"},
"file": {
"()": CustomFileLogFormatter,
"format": "%(asctime)s %(levelname)-8s %(name)-30s %(message)s",
"datefmt": "%Y-%m-%d %H:%M:%S%z",
},
},
"handlers": {
"console": {
"class": "logging.StreamHandler",
"formatter": "simple",
"formatter": "console",
"level": "INFO",
},
"runner_file": {
"class": "logging.FileHandler",
"formatter": "date",
"formatter": "file",
"filename": "%(base_log_dir)s/runner.log",
"encoding": "utf-8",
"level": "DEBUG",
Expand Down Expand Up @@ -123,9 +130,9 @@ def configure_logging_for_test(test_log_dir: Path) -> None:
proxy_handler = None

try:
formatter = logging.Formatter(
fmt=LOGGING_CONFIG["formatters"]["date"]["format"],
datefmt=LOGGING_CONFIG["formatters"]["date"]["datefmt"],
formatter = CustomFileLogFormatter(
fmt=LOGGING_CONFIG["formatters"]["file"]["format"],
datefmt=LOGGING_CONFIG["formatters"]["file"]["datefmt"],
)

# TODO: ensure the new files created here do not conflict with probe logs
Expand All @@ -148,16 +155,25 @@ def configure_logging_for_test(test_log_dir: Path) -> None:
api_monitor_logger.handlers.remove(proxy_handler)


class MonitorHandler(logging.Handler):
"""A logging handler that passes messages from log records to an event monitor."""
class MonitoringFilter(logging.Filter):
Copy link
Contributor Author

@azawlocki azawlocki Apr 9, 2021

Choose a reason for hiding this comment

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

I switched from using custom logging.Handler to logging.Filter. Filters are run before handlers for a record, so this ensures that the record is modified by the filter (by adding color) before passing it to any handler. Moreover, after reading logging docs I think it's more idiomatic to modify the record in a filter than in a handler.

"""A logging `Filter` that feeds messages to the underlying event monitor.

After doing this it also adds some color to the messages for greater fun.
Copy link
Contributor

Choose a reason for hiding this comment

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

👯

"""

def __init__(self, monitor: EventMonitor[str]):
self._monitor = monitor
def __init__(self, monitor: EventMonitor[str], color: Optional[str] = None):
super().__init__()
self._monitor: EventMonitor[str] = monitor
self._color: Optional[str] = color

def handle(self, record: logging.LogRecord) -> None:
"""Add the `record`'s message to the associated event monitor."""
self._monitor.add_event_sync(record.getMessage())
def filter(self, record: logging.LogRecord) -> bool:
"""Pass the record's message to the monitor, add colors to the message."""
message = record.getMessage()
self._monitor.add_event_sync(message)
if self._color:
record.msg = colors.color(message, fg=self._color)
record.args = ()
return True


@contextlib.contextmanager
Expand All @@ -170,10 +186,9 @@ def monitored_logger(name: str, monitor: EventMonitor[str]) -> Iterator[logging.
"""

logger_ = logging.getLogger(name)
handler = MonitorHandler(monitor)
filter = MonitoringFilter(monitor, "cyan")
logger_.filters.insert(0, filter)
try:
logger_.addHandler(handler)
yield logger_
finally:
if handler in logger_.handlers:
logger_.removeHandler(handler)
logger.removeFilter(filter)
16 changes: 15 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ readme = "README.md"
[tool.poetry.dependencies]
python = "^3.8"
aiohttp = "3.7.3"
ansicolors = "^1.1.0"
docker-compose = "^1.29"
docker = "^5.0"
func_timeout = "4.3.5"
Expand Down