From 7c9055c9cd5e5f988693c5ca89fef3fda9d96056 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Tue, 16 Nov 2021 06:38:12 -0500 Subject: [PATCH 1/6] logging: add config for trace loggers and logging directory - allow configuring loggers and their specified levels Some loggers can get spammy when debugging, this allows turning them down, or setting other loggers to lower levels than the root To use, set a MODMAIL_{level}_LOGGERS environment variable, delimiting the loggers with `,` Valid levels are all of the valid levels for logging, as follows trace, debug, info, notice, warning, error, critical - add support for configuring the file logging directory The directory for logging files was fully dependent on the current working directory This caused my environment to be littered with logging directories The solution to this was to continue to use the current working directory, unless the parent directory of the bot is also a parent of the cwd, in which case the bot parent directory is used for creating the logging directory. In addition, `MODMAIL_LOGGING_DIRECTORY` has been added as an override environment variable. Setting it will use that directory for logging. --- modmail/__init__.py | 25 +++++------- modmail/log.py | 98 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 15 deletions(-) diff --git a/modmail/__init__.py b/modmail/__init__.py index 9377ffd2..c5109633 100644 --- a/modmail/__init__.py +++ b/modmail/__init__.py @@ -1,30 +1,22 @@ import logging import logging.handlers -from pathlib import Path import coloredlogs -from modmail.log import ModmailLogger - - -logging.TRACE = 5 -logging.NOTICE = 25 -logging.addLevelName(logging.TRACE, "TRACE") -logging.addLevelName(logging.NOTICE, "NOTICE") +from modmail import log LOG_FILE_SIZE = 8 * (2 ** 10) ** 2 # 8MB, discord upload limit -# this logging level is set to logging.TRACE because if it is not set to the lowest level, -# the child level will be limited to the lowest level this is set to. -ROOT_LOG_LEVEL = logging.TRACE + +ROOT_LOG_LEVEL = log.get_logging_level() FMT = "%(asctime)s %(levelname)10s %(name)15s - [%(lineno)5d]: %(message)s" DATEFMT = "%Y/%m/%d %H:%M:%S" -logging.setLoggerClass(ModmailLogger) +logging.setLoggerClass(log.ModmailLogger) -# Set up file logging -log_file = Path("logs", "bot.log") +# Set up file logging relative to the current path +log_file = log.get_log_dir() / "bot.log" log_file.parent.mkdir(parents=True, exist_ok=True) # file handler @@ -49,7 +41,7 @@ coloredlogs.install(level=logging.TRACE, fmt=FMT, datefmt=DATEFMT) # Create root logger -root: ModmailLogger = logging.getLogger() +root: log.ModmailLogger = logging.getLogger() root.setLevel(ROOT_LOG_LEVEL) root.addHandler(file_handler) @@ -58,3 +50,6 @@ logging.getLogger("websockets").setLevel(logging.ERROR) # Set asyncio logging back to the default of INFO even if asyncio's debug mode is enabled. logging.getLogger("asyncio").setLevel(logging.INFO) + +# set up trace loggers +log.set_logger_levels() diff --git a/modmail/log.py b/modmail/log.py index 65afd23f..a2ee2586 100644 --- a/modmail/log.py +++ b/modmail/log.py @@ -1,7 +1,105 @@ import logging +import pathlib from typing import Any +__all__ = [ + "DEFAULT", + "get_logging_level", + "set_logger_levels", + "ModmailLogger", +] + +logging.TRACE = 5 +logging.NOTICE = 25 +logging.addLevelName(logging.TRACE, "TRACE") +logging.addLevelName(logging.NOTICE, "NOTICE") + +DEFAULT = logging.INFO + + +def _get_env() -> dict: + import os + + try: + from dotenv import dotenv_values + except ModuleNotFoundError: + dotenv_values = lambda *args, **kwargs: dict() # noqa: E731 + + return {**dotenv_values(), **os.environ} + + +def get_logging_level() -> None: + """Get the configured logging level, defaulting to logging.INFO.""" + key = "MODMAIL_LOG_LEVEL" + + level = _get_env().get(key, DEFAULT) + + try: + level = int(level) + except TypeError: + level = DEFAULT + except ValueError: + level = level.upper() + if hasattr(logging, level) and isinstance(getattr(logging, level), int): + return getattr(logging, level) + print( + f"Environment variable {key} must be able to be converted into an integer.\n" + f"To resolve this issue, set {key} to an integer value, or remove it from the environment.\n" + "It is also possible that it is sourced from an .env file." + ) + exit(1) + + return level + + +def set_logger_levels() -> None: + """ + Set all loggers to the provided environment variables. + + eg MODMAIL_TRACE_LOGGERS will be split by `,` and each logger will be set to the trace level + This is applied for every logging level. + """ + env_vars = _get_env() + fmt_key = "MODMAIL_{level}_LOGGERS" + + for level in ["trace", "debug", "info", "notice", "warning", "error", "critical"]: + level = level.upper() + key = fmt_key.format(level=level) + loggers: str = env_vars.get(key, None) + if loggers is None: + continue + + for logger in loggers.split(","): + logging.getLogger(logger.strip()).setLevel(level) + + +def get_log_dir() -> pathlib.Path: + """ + Return a directory to be used for logging. + + The log directory is made in the current directory + unless the current directory shares a parent directory with the bot. + + This is ignored if a environment variable provides the logging directory. + """ + env_vars = _get_env() + key = "MODMAIL_LOGGING_DIRECTORY" + if env_vars.get(key, None) is not None: + return pathlib.Path(env_vars[key]).expanduser() + + import modmail + + path = pathlib.Path(modmail.__file__).parent.parent + cwd = pathlib.Path.cwd() + try: + cwd.relative_to(path) + except ValueError: + return cwd / "logs" + else: + return path / "logs" + + class ModmailLogger(logging.Logger): """Custom logging class implementation.""" From 3fb03de6c1b9630c20194791f51278308265371f Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Tue, 16 Nov 2021 06:57:01 -0500 Subject: [PATCH 2/6] dispatcher: import ModmailLogger from the correct module --- modmail/dispatcher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index 5de7d721..929aedc8 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -4,7 +4,7 @@ import logging from typing import Callable, Coroutine, Dict, List, Optional, Tuple, Union -from modmail import ModmailLogger +from modmail.log import ModmailLogger from modmail.utils.general import module_function_disidenticality From 8b33dd9519c9b6cac897b0b2699f458b02ffb57a Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Tue, 16 Nov 2021 06:51:33 -0500 Subject: [PATCH 3/6] logging: add changelog entry --- docs/changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog.md b/docs/changelog.md index 11a81a73..77b92cec 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Embedified the meta commands so they have a nicer UI (#78) +- Improved the logging system to allow trace logging and a specific logging directory to be configured. (#118) ## [0.2.0] - 2021-09-29 From a6bd862c9ef8576119265facb308ba9b72e71b39 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Tue, 16 Nov 2021 07:08:36 -0500 Subject: [PATCH 4/6] deps: add python-dotenv as a top level dependency --- poetry.lock | 8 ++++---- pyproject.toml | 1 + requirements.txt | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/poetry.lock b/poetry.lock index 71294634..6daeefb0 100644 --- a/poetry.lock +++ b/poetry.lock @@ -988,7 +988,7 @@ six = ">=1.5" [[package]] name = "python-dotenv" -version = "0.19.0" +version = "0.19.2" description = "Read key-value pairs from a .env file and set them as environment variables" category = "main" optional = false @@ -1206,7 +1206,7 @@ testing = ["pytest (>=4.6)", "pytest-checkdocs (>=2.4)", "pytest-flake8", "pytes [metadata] lock-version = "1.1" python-versions = "^3.8" -content-hash = "b85bebf7a796b0b37e647fd7289589ea03a147c155839dfd1bcd561ead1fde92" +content-hash = "38bbc2b454a3dce67011a9bc253fb334abb91d437f8607e28e17acf45a9a32a1" [metadata.files] aiodns = [ @@ -1912,8 +1912,8 @@ python-dateutil = [ {file = "python_dateutil-2.8.2-py2.py3-none-any.whl", hash = "sha256:961d03dc3453ebbc59dbdea9e4e11c5651520a876d0f4db161e8674aae935da9"}, ] python-dotenv = [ - {file = "python-dotenv-0.19.0.tar.gz", hash = "sha256:f521bc2ac9a8e03c736f62911605c5d83970021e3fa95b37d769e2bbbe9b6172"}, - {file = "python_dotenv-0.19.0-py2.py3-none-any.whl", hash = "sha256:aae25dc1ebe97c420f50b81fb0e5c949659af713f31fdb63c749ca68748f34b1"}, + {file = "python-dotenv-0.19.2.tar.gz", hash = "sha256:a5de49a31e953b45ff2d2fd434bbc2670e8db5273606c1e737cc6b93eff3655f"}, + {file = "python_dotenv-0.19.2-py2.py3-none-any.whl", hash = "sha256:32b2bdc1873fd3a3c346da1c6db83d0053c3c62f28f1f38516070c4c8971b1d3"}, ] pyyaml = [ {file = "PyYAML-5.4.1-cp27-cp27m-macosx_10_9_x86_64.whl", hash = "sha256:3b2b1824fe7112845700f815ff6a489360226a5609b96ec2190a45e62a9fc922"}, diff --git a/pyproject.toml b/pyproject.toml index 245c8fad..f88017d8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,6 +19,7 @@ arrow = "^1.1.1" colorama = "^0.4.3" coloredlogs = "^15.0" "discord.py" = { url = "https://github.com/Rapptz/discord.py/archive/master.zip" } +python-dotenv = "^0.19.2" pydantic = { version = "^1.8.2", extras = ["dotenv"] } toml = "^0.10.2" diff --git a/requirements.txt b/requirements.txt index d0251ef6..cc52862f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -21,7 +21,7 @@ pycparser==2.20 ; python_version >= "2.7" and python_version != "3.0" and python pydantic==1.8.2 ; python_full_version >= "3.6.1" pyreadline3==3.3 ; sys_platform == "win32" python-dateutil==2.8.2 ; python_version != "3.0" -python-dotenv==0.19.0 ; python_version >= "3.5" +python-dotenv==0.19.2 ; python_version >= "3.5" six==1.16.0 ; python_version >= "2.7" and python_version != "3.0" and python_version != "3.1" and python_version != "3.2" toml==0.10.2 ; python_version >= "2.6" and python_version != "3.0" and python_version != "3.1" and python_version != "3.2" typing-extensions==3.10.0.2 From 9c40668053aabe9c74c79a1653000fb7efdfd604 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Tue, 16 Nov 2021 07:11:52 -0500 Subject: [PATCH 5/6] fix: skip trace logging if the log level isn't enabled --- tests/modmail/{test_logs.py => test_log.py} | 3 +++ 1 file changed, 3 insertions(+) rename tests/modmail/{test_logs.py => test_log.py} (91%) diff --git a/tests/modmail/test_logs.py b/tests/modmail/test_log.py similarity index 91% rename from tests/modmail/test_logs.py rename to tests/modmail/test_log.py index 97fffaf1..5a128bf3 100644 --- a/tests/modmail/test_logs.py +++ b/tests/modmail/test_log.py @@ -47,6 +47,9 @@ def test_notice_level(log: ModmailLogger) -> None: @pytest.mark.dependency(depends=["create_logger"]) def test_trace_level(log: ModmailLogger) -> None: """Test trace logging level prints a trace response.""" + if not log.isEnabledFor(logging.TRACE): + pytest.skip("Skipping because logging isn't enabled for the necessary level") + trace_test_phrase = "Getting in the weeds" stdout = io.StringIO() From c1022b4819dbe48e4b0ba67069c27b2b6939411b Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Thu, 2 Dec 2021 22:00:10 -0500 Subject: [PATCH 6/6] fix: fully resolve the provided logging dir --- modmail/log.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/modmail/log.py b/modmail/log.py index a2ee2586..c8403962 100644 --- a/modmail/log.py +++ b/modmail/log.py @@ -1,6 +1,7 @@ +import functools import logging import pathlib -from typing import Any +from typing import Any, Dict __all__ = [ @@ -18,7 +19,8 @@ DEFAULT = logging.INFO -def _get_env() -> dict: +@functools.lru_cache(maxsize=32) +def _get_env() -> Dict[str, str]: import os try: @@ -57,11 +59,11 @@ def set_logger_levels() -> None: """ Set all loggers to the provided environment variables. - eg MODMAIL_TRACE_LOGGERS will be split by `,` and each logger will be set to the trace level + eg MODMAIL_LOGGERS_TRACE will be split by `,` and each logger will be set to the trace level This is applied for every logging level. """ env_vars = _get_env() - fmt_key = "MODMAIL_{level}_LOGGERS" + fmt_key = "MODMAIL_LOGGERS_{level}" for level in ["trace", "debug", "info", "notice", "warning", "error", "critical"]: level = level.upper() @@ -85,19 +87,24 @@ def get_log_dir() -> pathlib.Path: """ env_vars = _get_env() key = "MODMAIL_LOGGING_DIRECTORY" - if env_vars.get(key, None) is not None: - return pathlib.Path(env_vars[key]).expanduser() + if log_dir := env_vars.get(key, None): + # return the log dir if its absolute, otherwise use the root/cwd trick + path = pathlib.Path(log_dir).expanduser() + if path.is_absolute(): + return path - import modmail + log_dir = log_dir or "logs" - path = pathlib.Path(modmail.__file__).parent.parent + # Get the directory above the bot module directory + path = pathlib.Path(__file__).parents[1] cwd = pathlib.Path.cwd() try: cwd.relative_to(path) except ValueError: - return cwd / "logs" + log_path = path / log_dir else: - return path / "logs" + log_path = cwd / log_dir + return log_path.resolve() class ModmailLogger(logging.Logger):