Merge pull request #453 from aaugustin/disable-instrumentation-for-disabled-panels

Disable instrumentation for disabled panels

Author: aaugustin

debug_toolbar/middleware.py

@@ -54,10 +54,6 @@ class DebugToolbarMiddleware(object):
     """
     debug_toolbars = {}
 
-    @classmethod
-    def get_current(cls):
-        return cls.debug_toolbars.get(threading.current_thread().ident)
-
     def __init__(self):
         self._urlconfs = {}
 
@@ -92,10 +88,10 @@ def process_request(self, request):
 
             toolbar = DebugToolbar(request)
             for panel in toolbar.panels:
-                panel.disabled = panel.dom_id() in request.COOKIES
-                panel.enabled = not panel.disabled
-                if panel.disabled:
+                panel.enabled = panel.dom_id() not in request.COOKIES
+                if not panel.enabled:
                     continue
+                panel.enable_instrumentation()
                 panel.process_request(request)
             self.__class__.debug_toolbars[threading.current_thread().ident] = toolbar
 
@@ -106,7 +102,7 @@ def process_view(self, request, view_func, view_args, view_kwargs):
             return
         result = None
         for panel in toolbar.panels:
-            if panel.disabled:
+            if not panel.enabled:
                 continue
             response = panel.process_view(request, view_func, view_args, view_kwargs)
             if response:
@@ -131,12 +127,13 @@ def process_response(self, request, response):
                     {'redirect_to': redirect_to}
                 )
                 response.cookies = cookies
+        for panel in toolbar.panels:
+            if not panel.enabled:
+                continue
+            panel.process_response(request, response)
+            panel.disable_instrumentation()
         if ('gzip' not in response.get('Content-Encoding', '') and
                 response.get('Content-Type', '').split(';')[0] in _HTML_TYPES):
-            for panel in toolbar.panels:
-                if panel.disabled:
-                    continue
-                panel.process_response(request, response)
             response.content = replace_insensitive(
                 force_text(response.content, encoding=settings.DEFAULT_CHARSET),
                 self.tag,

debug_toolbar/panels/__init__.py

@@ -10,38 +10,56 @@ class DebugPanel(object):
     """
     # name = 'Base'
     # template = 'debug_toolbar/panels/base.html'
-    has_content = False  # If content returns something, set to True in subclass
+
+    # If content returns something, set to True in subclass
+    has_content = False
+
+    # This can be set to False in instances if the panel is disabled.
+    enabled = True
 
     # We'll maintain a local context instance so we can expose our template
     # context variables to panels which need them:
     context = {}
 
     # Panel methods
+
     def __init__(self, toolbar, context={}):
         self.toolbar = toolbar
         self.context.update(context)
         self.slug = slugify(self.name)
 
+    def content(self):
+        if self.has_content:
+            context = self.context.copy()
+            context.update(self.get_stats())
+            return render_to_string(self.template, context)
+
     def dom_id(self):
         return 'djDebug%sPanel' % (self.name.replace(' ', ''))
 
+    # Titles and subtitles
+
     def nav_title(self):
-        """Title showing in toolbar"""
+        """Title showing in sidebar"""
         raise NotImplementedError
 
     def nav_subtitle(self):
-        """Subtitle showing until title in toolbar"""
+        """Subtitle showing under title in sidebar"""
         return ''
 
     def title(self):
         """Title showing in panel"""
         raise NotImplementedError
 
-    def content(self):
-        if self.has_content:
-            context = self.context.copy()
-            context.update(self.get_stats())
-            return render_to_string(self.template, context)
+    # Enable and disable (expensive) instrumentation
+
+    def enable_instrumentation(self):
+        pass
+
+    def disable_instrumentation(self):
+        pass
+
+    # Store and retrieve stats (shared between panels)
 
     def record_stats(self, stats):
         panel_stats = self.toolbar.stats.get(self.slug)

debug_toolbar/panels/cache.py

@@ -6,7 +6,7 @@
 
 from django.conf import settings
 from django.core import cache
-from django.core.cache import get_cache as base_get_cache
+from django.core.cache import cache as original_cache, get_cache as original_get_cache
 from django.core.cache.backends.base import BaseCache
 from django.dispatch import Signal
 from django.template import Node
@@ -123,6 +123,10 @@ def decr_version(self, *args, **kwargs):
         return self.cache.decr_version(*args, **kwargs)
 
 
+def get_cache(*args, **kwargs):
+    return CacheStatTracker(original_get_cache(*args, **kwargs))
+
+
 class CacheDebugPanel(DebugPanel):
     """
     Panel that displays the cache statistics.
@@ -195,6 +199,16 @@ def title(self):
                          'Cache calls from %(count)d backends',
                          count) % dict(count=count)
 
+    def enable_instrumentation(self):
+        # This isn't thread-safe because cache connections aren't thread-local
+        # in Django, unlike database connections.
+        cache.cache = CacheStatTracker(cache.cache)
+        cache.get_cache = get_cache
+
+    def disable_instrumentation(self):
+        cache.cache = original_cache
+        cache.get_cache = original_get_cache
+
     def process_response(self, request, response):
         self.record_stats({
             'total_calls': len(self.calls),
@@ -204,12 +218,3 @@ def process_response(self, request, response):
             'misses': self.misses,
             'counts': self.counts,
         })
-
-
-def get_cache_debug(*args, **kwargs):
-    base_cache = base_get_cache(*args, **kwargs)
-    return CacheStatTracker(base_cache)
-
-
-cache.cache = CacheStatTracker(cache.cache)
-cache.get_cache = get_cache_debug

debug_toolbar/panels/logger.py

@@ -73,6 +73,9 @@ def emit(self, record):
 logging.root.setLevel(logging.NOTSET)
 logging.root.addHandler(logging_handler)  # register with logging
 
+# We don't use enable/disable_instrumentation because we can't make these
+# functions thread-safe and (hopefully) logging isn't too expensive.
+
 try:
     import logbook
     logbook_supported = True

debug_toolbar/panels/sql.py

@@ -4,28 +4,13 @@
 from copy import copy
 
 from django.db import connections
-from django.db.backends import BaseDatabaseWrapper
 from django.utils.translation import ugettext_lazy as _, ungettext_lazy as __
 
 from debug_toolbar.forms import SQLSelectForm
-from debug_toolbar.middleware import DebugToolbarMiddleware
 from debug_toolbar.panels import DebugPanel
 from debug_toolbar.utils import render_stacktrace
 from debug_toolbar.utils.sql import reformat_sql
 from debug_toolbar.utils.tracking.db import CursorWrapper
-from debug_toolbar.utils.tracking import replace_method
-
-
-@replace_method(BaseDatabaseWrapper, 'cursor')
-def cursor(original, self):
-    result = original(self)
-
-    djdt = DebugToolbarMiddleware.get_current()
-    if not djdt:
-        return result
-    logger = djdt.get_panel(SQLDebugPanel)
-
-    return CursorWrapper(result, self, logger=logger)
 
 
 def get_isolation_level_display(engine, level):
@@ -131,6 +116,16 @@ def title(self):
                   'SQL Queries from %(count)d connections',
                   count) % dict(count=count)
 
+    def enable_instrumentation(self):
+        # This is thread-safe because database connections are thread-local.
+        for connection in connections.all():
+            old_cursor = connection.cursor
+            connection.cursor = lambda: CursorWrapper(old_cursor(), connection, self)
+
+    def disable_instrumentation(self):
+        for connection in connections.all():
+            del connection.cursor
+
     def process_response(self, request, response):
         if self._queries:
             width_ratio_tally = 0

debug_toolbar/panels/template.py

@@ -18,9 +18,9 @@
 # Code taken and adapted from Simon Willison and Django Snippets:
 # http://www.djangosnippets.org/snippets/766/
 
-# Monkeypatch instrumented test renderer from django.test.utils - we could use
-# django.test.utils.setup_test_environment for this but that would also set up
-# e-mail interception, which we don't want
+# Monkey-patch to enable the template_rendered signal. The receiver returns
+# immediately when the panel is disabled to keep the overhead small.
+
 from django.test.utils import instrumented_test_render
 from django.template import Template
 
@@ -56,13 +56,17 @@ def __init__(self, *args, **kwargs):
         template_rendered.connect(self._store_template_info)
 
     def _store_template_info(self, sender, **kwargs):
-        t = kwargs['template']
-        if t.name and t.name.startswith('debug_toolbar/'):
-            return  # skip templates that we are generating through the debug toolbar.
-        context_data = kwargs['context']
+        if not self.enabled:
+            return
+
+        template, context = kwargs['template'], kwargs['context']
+
+        # Skip templates that we are generating through the debug toolbar.
+        if template.name and template.name.startswith('debug_toolbar/'):
+            return
 
         context_list = []
-        for context_layer in context_data.dicts:
+        for context_layer in context.dicts:
             temp_layer = {}
             if hasattr(context_layer, 'items'):
                 for key, value in context_layer.items():
@@ -102,6 +106,7 @@ def _store_template_info(self, sender, **kwargs):
                 context_list.append(pformat(temp_layer))
             except UnicodeEncodeError:
                 pass
+
         kwargs['context'] = [force_text(item) for item in context_list]
         self.templates.append(kwargs)
 

debug_toolbar/templates/debug_toolbar/base.html

@@ -24,7 +24,7 @@
 					{% if panel.has_content and panel.enabled %}
 						<a href="{{ panel.url|default:"#" }}" title="{{ panel.title }}" class="{{ panel.dom_id }}">
 					{% else %}
-						<div class="contentless{% if panel.disabled %} disabled{% endif %}">
+						<div class="contentless{% if not panel.enabled %} disabled{% endif %}">
 					{% endif %}
 					{{ panel.nav_title }}
 					{% if panel.enabled %}

debug_toolbar/utils/tracking/__init__.py

@@ -1,19 +0,0 @@
-from __future__ import unicode_literals
-
-
-def replace_method(klass, method_name):
-    original = getattr(klass, method_name)
-
-    def inner(callback):
-        def wrapped(*args, **kwargs):
-            return callback(original, *args, **kwargs)
-
-        actual = getattr(original, '__wrapped__', original)
-        wrapped.__wrapped__ = actual
-        wrapped.__doc__ = getattr(actual, '__doc__', None)
-        wrapped.__name__ = actual.__name__
-
-        setattr(klass, method_name, wrapped)
-        return wrapped
-
-    return inner

tests/panels/test_cache.py

@@ -0,0 +1,28 @@
+# coding: utf-8
+
+from __future__ import unicode_literals
+
+from django.core import cache
+
+from debug_toolbar.panels.cache import CacheDebugPanel
+
+from ..base import BaseTestCase
+
+
+class CachePanelTestCase(BaseTestCase):
+
+    def setUp(self):
+        super(CachePanelTestCase, self).setUp()
+        self.panel = self.toolbar.get_panel(CacheDebugPanel)
+        self.panel.enable_instrumentation()
+
+    def tearDown(self):
+        self.panel.disable_instrumentation()
+        super(CachePanelTestCase, self).tearDown()
+
+    def test_recording(self):
+        self.assertEqual(len(self.panel.calls), 0)
+        cache.cache.set('foo', 'bar')
+        cache.cache.get('foo')
+        cache.cache.delete('foo')
+        self.assertEqual(len(self.panel.calls), 3)

tests/panels/test_sql.py

@@ -17,6 +17,11 @@ class SQLPanelTestCase(BaseTestCase):
     def setUp(self):
         super(SQLPanelTestCase, self).setUp()
         self.panel = self.toolbar.get_panel(SQLDebugPanel)
+        self.panel.enable_instrumentation()
+
+    def tearDown(self):
+        self.panel.disable_instrumentation()
+        super(SQLPanelTestCase, self).tearDown()
 
     def test_recording(self):
         self.assertEqual(len(self.panel._queries), 0)