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

Conversation

azawlocki
Copy link
Contributor

Let's output some logs in color:
🟦 output of a command run with Probe.run_command_on_host()
🟩 assertion success messages
🟥 assertion failure messages

Colors are removed when logging to a file.

@azawlocki azawlocki changed the title Add some colors to the logs Add some 🟥🟧🟨🟩🟦🟪 to the logs Apr 9, 2021
Comment on lines +262 to +266
if a.done:
self._logger.debug(
"Assertion '%s' finished after event %s", a.name, event_descr
)

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

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

@azawlocki azawlocki requested a review from kmazurek April 9, 2021 12:24
Copy link
Contributor

@kmazurek kmazurek left a comment

Choose a reason for hiding this comment

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

Awesome! 🏳️‍🌈 I'd say we could use even more colours for some parts of our logs (e.g. making Running step ... log lines less contrasting could increase the overall readability). That said, I think it could be a good topic for a separate PR.

class MonitoringFilter(logging.Filter):
"""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.

👯

@azawlocki azawlocki merged commit 75d0384 into master Apr 9, 2021
@azawlocki azawlocki deleted the az/colorize-logs branch April 9, 2021 13:17
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.

2 participants