From e90959104303434276a40cb055b8cd073624f384 Mon Sep 17 00:00:00 2001 From: Robert Iwatt Date: Sun, 3 Apr 2022 23:28:06 -0700 Subject: [PATCH 1/3] add regression test --- tests/panels/test_logging.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/panels/test_logging.py b/tests/panels/test_logging.py index 87f152ae3..70c2c7bd5 100644 --- a/tests/panels/test_logging.py +++ b/tests/panels/test_logging.py @@ -1,4 +1,5 @@ import logging +from unittest.mock import patch from debug_toolbar.panels.logging import ( MESSAGE_IF_STRING_REPRESENTATION_INVALID, @@ -86,3 +87,10 @@ def view(request): self.assertEqual( MESSAGE_IF_STRING_REPRESENTATION_INVALID, records[0]["message"] ) + + @patch("sys.stderr") + def test_fallback_logging(self, mock_stderr): + # make sure the log reaches stderr even though logging set up + # its own handler during its import + self.logger.warning("hello") + mock_stderr.write.assert_called_once_with("hello\n") From 79d86d487ac7f61ec1518f2efe82f7a0bcac5745 Mon Sep 17 00:00:00 2001 From: Robert Iwatt Date: Mon, 4 Apr 2022 01:02:11 -0700 Subject: [PATCH 2/3] change settings --- tests/settings.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/settings.py b/tests/settings.py index d24108247..da5067fbf 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -11,7 +11,10 @@ INTERNAL_IPS = ["127.0.0.1"] -LOGGING_CONFIG = None # avoids spurious output in tests +LOGGING = { # avoids spurious output in tests + "version": 1, + "disable_existing_loggers": True, +} # Application definition From f4703438133576c942ab9687714712f23f8972a7 Mon Sep 17 00:00:00 2001 From: Robert Iwatt Date: Thu, 31 Mar 2022 19:10:32 -0700 Subject: [PATCH 3/3] Preserve logs that LoggingPanel would previously overwrite LoggingPanel upon its import, changes the Python interpreter's root logger to log into a handler that collects its records for the LoggingPanel's display. However for users that don't explicitly configure the root logger this looks like the Debug Toolbar suppresses their logs that previously went to standard error. With this commit, the logs will still keep going to standard error even if the LoggingPanel is installed, but only if the root logger has not been explicitly configured with a handler yet. This behavior will make it so that installing DDT won't break logging neither for users that already have logging customizations nor for users that have an out-of-the-box django default logging setup. Also: change LOGGING config in tests/settings.py because now DDT's own test suite would show more logs without an explicit config. --- debug_toolbar/panels/logging.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/debug_toolbar/panels/logging.py b/debug_toolbar/panels/logging.py index cb0445108..ee484a41e 100644 --- a/debug_toolbar/panels/logging.py +++ b/debug_toolbar/panels/logging.py @@ -45,6 +45,24 @@ def emit(self, record): self.collector.collect(record) +# Preserve Python's fallback log mechanism before adding ThreadTrackingHandler. + +# If the root logger has no handlers attached then everything that reaches it goes +# to the "handler of last resort". +# So a Django app that doesn't explicitly configure the root logger actually logs +# through logging.lastResort. +# However, logging.lastResort is not used after ThreadTrackingHandler gets added to +# the root logger below. This means that users who have LoggingPanel enabled might +# find their logs are gone from their app as soon as they install DDT. +# Explicitly adding logging.lastResort to logging.root's handler sidesteps this +# potential confusion. +# Note that if root has already been configured, or logging.lastResort has been +# removed, then the configuration is unchanged, so users who configured their +# logging aren't exposed to the opposite confusion of seeing extra log lines from +# their app. +if not logging.root.hasHandlers() and logging.lastResort is not None: + logging.root.addHandler(logging.lastResort) + # We don't use enable/disable_instrumentation because logging is global. # We can't add thread-local logging handlers. Hopefully logging is cheap.