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

logger: log datetime when on debug mode #3325

Merged
merged 1 commit into from
Feb 14, 2020
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
18 changes: 17 additions & 1 deletion dvc/logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,28 @@ class ColorFormatter(logging.Formatter):
def format(self, record):
msg = record.msg.format(*record.args) if record.args else record.msg
exception, stack_trace = self._parse_exc(record)
return ("{prefix}{description}{stack_trace}").format(
return ("{asctime}{prefix}{description}{stack_trace}").format(
asctime=self.formatTime(record, self.datefmt),
prefix=self._prefix(record),
description=self._description(msg, exception),
stack_trace=stack_trace,
)

def formatTime(self, record, datefmt=None):
# only show if current level is set to DEBUG
# also, skip INFO as it is used for UI
if (
self._current_level() != logging.DEBUG
or record.levelno == logging.INFO
):
return ""

return "{color}{date}{nc} ".format(
color=colorama.Fore.GREEN,
date=super().formatTime(record, datefmt),
Copy link
Member Author

Choose a reason for hiding this comment

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

The default output looks like 2020-02-14 17:13:44,627. I did not really tried fighting with the format and went with default one. Plus, this supports setting datefmt from dictConfig.

Copy link
Member Author

Choose a reason for hiding this comment

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

If we need a different format later on, we can just change in the dictConfig anyway. πŸ™‚

Copy link
Contributor

@efiop efiop Feb 14, 2020

Choose a reason for hiding this comment

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

Yeah, a bit long for my taste. We could shorten stuff like year (cut it off or shorten to "20"), but it won't make a big deal. I'm ok with leaving it as is. πŸ‘

Also, really like the implementation πŸ˜‰

nc=colorama.Fore.RESET,
)

def _prefix(self, record):
if record.levelname == "INFO":
return ""
Expand Down
61 changes: 53 additions & 8 deletions tests/unit/test_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,40 @@
import colorama

import dvc.logger
import time
from datetime import datetime
import pytest

from dvc.exceptions import DvcException


logger = logging.getLogger("dvc")
formatter = dvc.logger.ColorFormatter()
colors = {
"blue": colorama.Fore.BLUE,
"green": colorama.Fore.GREEN,
"red": colorama.Fore.RED,
"yellow": colorama.Fore.YELLOW,
"nc": colorama.Fore.RESET,
}


@pytest.fixture()
def dt(mocker):
Copy link
Contributor

Choose a reason for hiding this comment

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

πŸ‘

with mocker.patch(
"time.time", return_value=time.mktime(datetime(2020, 2, 2).timetuple())
):
yield "2020-02-02 00:00:00,000"


class TestColorFormatter:
def test_debug(self, caplog):
def test_debug(self, caplog, dt):
with caplog.at_level(logging.DEBUG, logger="dvc"):
logger.debug("message")

expected = "{blue}DEBUG{nc}: message".format(**colors)
expected = "{green}{datetime}{nc} {blue}DEBUG{nc}: message".format(
**colors, datetime=dt
)

assert expected == formatter.format(caplog.records[0])

Expand Down Expand Up @@ -81,7 +96,7 @@ def test_exception_with_description_and_message(self, caplog):

assert expected == formatter.format(caplog.records[0])

def test_exception_under_verbose(self, caplog):
def test_exception_under_verbose(self, caplog, dt):
with caplog.at_level(logging.DEBUG, logger="dvc"):
try:
raise Exception("description")
Expand All @@ -90,17 +105,21 @@ def test_exception_under_verbose(self, caplog):
logger.exception("")

expected = (
"{green}{datetime}{nc} "
"{red}ERROR{nc}: description\n"
"{red}{line}{nc}\n"
"{stack_trace}"
"{red}{line}{nc}".format(
line="-" * 60, stack_trace=stack_trace, **colors
line="-" * 60,
stack_trace=stack_trace,
**colors,
datetime=dt
)
)

assert expected == formatter.format(caplog.records[0])

def test_tb_only(self, caplog):
def test_tb_only(self, caplog, dt):
with caplog.at_level(logging.DEBUG, logger="dvc"):
try:
raise Exception("description")
Expand All @@ -109,17 +128,21 @@ def test_tb_only(self, caplog):
logger.exception("something", extra={"tb_only": True})

expected = (
"{green}{datetime}{nc} "
"{red}ERROR{nc}: something\n"
"{red}{line}{nc}\n"
"{stack_trace}"
"{red}{line}{nc}".format(
line="-" * 60, stack_trace=stack_trace, **colors
line="-" * 60,
stack_trace=stack_trace,
**colors,
datetime=dt
)
)

assert expected == formatter.format(caplog.records[0])

def test_nested_exceptions(self, caplog):
def test_nested_exceptions(self, caplog, dt):
with caplog.at_level(logging.DEBUG, logger="dvc"):
try:
raise Exception("first")
Expand All @@ -131,11 +154,15 @@ def test_nested_exceptions(self, caplog):
logger.exception("message")

expected = (
"{green}{datetime}{nc} "
"{red}ERROR{nc}: message - second: first\n"
"{red}{line}{nc}\n"
"{stack_trace}"
"{red}{line}{nc}".format(
line="-" * 60, stack_trace=stack_trace, **colors
line="-" * 60,
stack_trace=stack_trace,
**colors,
datetime=dt
)
)
assert expected == formatter.format(caplog.records[0])
Expand Down Expand Up @@ -179,3 +206,21 @@ def test_handlers():
assert out.level == logging.INFO
assert deb.level == logging.DEBUG
assert err.level == logging.WARNING


def test_logging_debug_with_datetime(caplog, dt):
with caplog.at_level(logging.DEBUG, logger="dvc"):
logger.warning("WARNING")
logger.debug("DEBUG")
logger.error("ERROR")

for record in caplog.records:
assert dt in formatter.format(record)
assert record.levelname == record.message


def test_info_with_debug_loglevel_shows_no_datetime(caplog, dt):
with caplog.at_level(logging.DEBUG, logger="dvc"):
logger.info("message")

assert "message" == formatter.format(caplog.records[0])