From 7f88f28b38f19134a300729809b128f4817bb54c Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 15 Mar 2014 23:19:25 -0700 Subject: [PATCH 1/5] Disable Logging panel when DEBUG is set to False to prevent memory leaks --- debug_toolbar/panels/logging.py | 16 +++++++++------- docs/changes.rst | 1 + 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/debug_toolbar/panels/logging.py b/debug_toolbar/panels/logging.py index 1ee19cefe..6eff75207 100644 --- a/debug_toolbar/panels/logging.py +++ b/debug_toolbar/panels/logging.py @@ -6,6 +6,7 @@ import threading except ImportError: threading = None +from django.conf import settings from django.utils.translation import ungettext, ugettext_lazy as _ from debug_toolbar.panels import Panel from debug_toolbar.utils import ThreadCollector @@ -45,13 +46,14 @@ def emit(self, record): self.collector.collect(record) -# We don't use enable/disable_instrumentation because logging is global. -# We can't add thread-local logging handlers. Hopefully logging is cheap. - -collector = LogCollector() -logging_handler = ThreadTrackingHandler(collector) -logging.root.setLevel(logging.NOTSET) -logging.root.addHandler(logging_handler) +# Check to make sure DEBUG is enabled to prevent silent memory leaks. +if settings.DEBUG: + # We don't use enable/disable_instrumentation because logging is global. + # We can't add thread-local logging handlers. Hopefully logging is cheap. + collector = LogCollector() + logging_handler = ThreadTrackingHandler(collector) + logging.root.setLevel(logging.NOTSET) + logging.root.addHandler(logging_handler) class LoggingPanel(Panel): diff --git a/docs/changes.rst b/docs/changes.rst index 2f9755687..e0463ea40 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -11,6 +11,7 @@ New features * The SQL panel colors queries depending on the stack level. * The Profiler panel allows configuring the maximum depth. +* The Logging panel will not attempt to collect data when DEBUG is False. Bugfixes ~~~~~~~~ From 5d5cdcc537c02bd3ab8b296c1a050aa7e56d264f Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 15 Mar 2014 23:27:59 -0700 Subject: [PATCH 2/5] Initialize LogCollector to ensure LoggingPanel can always run --- debug_toolbar/panels/logging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debug_toolbar/panels/logging.py b/debug_toolbar/panels/logging.py index 6eff75207..412b422c7 100644 --- a/debug_toolbar/panels/logging.py +++ b/debug_toolbar/panels/logging.py @@ -45,12 +45,12 @@ def emit(self, record): } self.collector.collect(record) +collector = LogCollector() # Check to make sure DEBUG is enabled to prevent silent memory leaks. if settings.DEBUG: # We don't use enable/disable_instrumentation because logging is global. # We can't add thread-local logging handlers. Hopefully logging is cheap. - collector = LogCollector() logging_handler = ThreadTrackingHandler(collector) logging.root.setLevel(logging.NOTSET) logging.root.addHandler(logging_handler) From c614d1b227a7f2baa5ebf447c79e0d3057f2b65b Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 16 Mar 2014 00:02:36 -0700 Subject: [PATCH 3/5] Fix tests by ensuring LogCollector is in the right state based on the DEBUG state --- debug_toolbar/panels/logging.py | 36 ++++++++++++++++++++++++++------- tests/panels/test_logging.py | 2 ++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/debug_toolbar/panels/logging.py b/debug_toolbar/panels/logging.py index 412b422c7..9e201eec5 100644 --- a/debug_toolbar/panels/logging.py +++ b/debug_toolbar/panels/logging.py @@ -46,14 +46,30 @@ def emit(self, record): self.collector.collect(record) collector = LogCollector() +collector_enabled = False -# Check to make sure DEBUG is enabled to prevent silent memory leaks. -if settings.DEBUG: - # We don't use enable/disable_instrumentation because logging is global. - # We can't add thread-local logging handlers. Hopefully logging is cheap. - logging_handler = ThreadTrackingHandler(collector) - logging.root.setLevel(logging.NOTSET) - logging.root.addHandler(logging_handler) + +def enable_collector(): + """ + Enable the LogCollector if it has not already been enabled and DEBUG is True. + """ + global collector_enabled + + if collector_enabled: + return + + # Check to make sure DEBUG is enabled to prevent silent memory leaks. + if settings.DEBUG: + # We don't use enable/disable_instrumentation because logging is global. + # We can't add thread-local logging handlers. Hopefully logging is cheap. + logging_handler = ThreadTrackingHandler(collector) + logging.root.setLevel(logging.NOTSET) + logging.root.addHandler(logging_handler) + collector_enabled = True + + +# Enable logging immediately if DEBUG is enabled +enable_collector() class LoggingPanel(Panel): @@ -75,9 +91,15 @@ def nav_subtitle(self): title = _("Log messages") def process_request(self, request): + # The state of DEBUG can be changed after the app is first initialized; ensure + # that the collector is in the right state. + enable_collector() collector.clear_collection() def process_response(self, request, response): + # The state of DEBUG can be changed after the app is first initialized; ensure + # that the collector is in the right state. + enable_collector() records = collector.get_collection() self._records[threading.currentThread()] = records collector.clear_collection() diff --git a/tests/panels/test_logging.py b/tests/panels/test_logging.py index 288efadf1..34c0f918d 100644 --- a/tests/panels/test_logging.py +++ b/tests/panels/test_logging.py @@ -4,10 +4,12 @@ from debug_toolbar.panels.logging import ( collector, MESSAGE_IF_STRING_REPRESENTATION_INVALID) +from django.test.utils import override_settings from ..base import BaseTestCase +@override_settings(DEBUG=True) class LoggingPanelTestCase(BaseTestCase): def setUp(self): From a9c62aa0b4aad2ae688ba286f17529ac1fdf2a74 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 16 Mar 2014 01:01:35 -0700 Subject: [PATCH 4/5] Refactor enable_collector() into a method for LogCollector that can be called explicitly to ensure logging is initiated. --- debug_toolbar/panels/logging.py | 50 +++++++++++++++------------------ tests/panels/test_logging.py | 3 ++ 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/debug_toolbar/panels/logging.py b/debug_toolbar/panels/logging.py index 9e201eec5..9b300fac6 100644 --- a/debug_toolbar/panels/logging.py +++ b/debug_toolbar/panels/logging.py @@ -16,6 +16,10 @@ class LogCollector(ThreadCollector): + def __init__(self): + ThreadCollector.__init__(self) + self.enabled = False + def collect(self, item, thread=None): # Avoid logging SQL queries since they are already in the SQL panel # TODO: Make this check whether SQL panel is enabled @@ -23,6 +27,22 @@ def collect(self, item, thread=None): return super(LogCollector, self).collect(item, thread) + def enable_logging(self): + """ + Enable logging if it has not already been enabled and DEBUG is True. + """ + if self.enabled: + return + + # Check to make sure DEBUG is enabled to prevent silent memory leaks. + if settings.DEBUG: + # We don't use enable/disable_instrumentation because logging is global. + # We can't add thread-local logging handlers. Hopefully logging is cheap. + logging_handler = ThreadTrackingHandler(collector) + logging.root.setLevel(logging.NOTSET) + logging.root.addHandler(logging_handler) + self.enabled = True + class ThreadTrackingHandler(logging.Handler): def __init__(self, collector): @@ -45,31 +65,9 @@ def emit(self, record): } self.collector.collect(record) -collector = LogCollector() -collector_enabled = False - - -def enable_collector(): - """ - Enable the LogCollector if it has not already been enabled and DEBUG is True. - """ - global collector_enabled - if collector_enabled: - return - - # Check to make sure DEBUG is enabled to prevent silent memory leaks. - if settings.DEBUG: - # We don't use enable/disable_instrumentation because logging is global. - # We can't add thread-local logging handlers. Hopefully logging is cheap. - logging_handler = ThreadTrackingHandler(collector) - logging.root.setLevel(logging.NOTSET) - logging.root.addHandler(logging_handler) - collector_enabled = True - - -# Enable logging immediately if DEBUG is enabled -enable_collector() +collector = LogCollector() +collector.enable_logging() class LoggingPanel(Panel): @@ -93,13 +91,9 @@ def nav_subtitle(self): def process_request(self, request): # The state of DEBUG can be changed after the app is first initialized; ensure # that the collector is in the right state. - enable_collector() collector.clear_collection() def process_response(self, request, response): - # The state of DEBUG can be changed after the app is first initialized; ensure - # that the collector is in the right state. - enable_collector() records = collector.get_collection() self._records[threading.currentThread()] = records collector.clear_collection() diff --git a/tests/panels/test_logging.py b/tests/panels/test_logging.py index 34c0f918d..1ddc0759d 100644 --- a/tests/panels/test_logging.py +++ b/tests/panels/test_logging.py @@ -16,6 +16,9 @@ def setUp(self): super(LoggingPanelTestCase, self).setUp() self.panel = self.toolbar.get_panel_by_id('LoggingPanel') self.logger = logging.getLogger(__name__) + # DEBUG may be set to False initially, preventing the default tracking + # from executing. Force an enable here to ensure that logging is activated. + collector.enable_logging() collector.clear_collection() def test_happy_case(self): From e0331dd5dc12b4a6fd2ceedae1c7b624c0b9042b Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 16 Mar 2014 01:03:36 -0700 Subject: [PATCH 5/5] Remove unneeded comments --- debug_toolbar/panels/logging.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/debug_toolbar/panels/logging.py b/debug_toolbar/panels/logging.py index 9b300fac6..c5b4165da 100644 --- a/debug_toolbar/panels/logging.py +++ b/debug_toolbar/panels/logging.py @@ -89,8 +89,6 @@ def nav_subtitle(self): title = _("Log messages") def process_request(self, request): - # The state of DEBUG can be changed after the app is first initialized; ensure - # that the collector is in the right state. collector.clear_collection() def process_response(self, request, response):