Replace culprits with transaction names

- Implemented in Django via middleware
- Defer empty culprit to server (Sentry deals fills it)
- Implemented Celery as task names
- Cleaned up Django and Celery abstractions to ease testing

conftest.py

@@ -81,4 +81,5 @@ def pytest_configure(config):
                 ],
             }],
             ALLOWED_HOSTS=['*'],
+            DISABLE_SENTRY_INSTRUMENTATION=True,
         )

raven/base.py

@@ -37,7 +37,8 @@
 from raven._compat import text_type, iteritems
 from raven.utils.encoding import to_unicode
 from raven.utils.serializer import transform
-from raven.utils.stacks import get_stack_info, iter_stack_frames, get_culprit
+from raven.utils.stacks import get_stack_info, iter_stack_frames
+from raven.utils.transaction import TransactionStack
 from raven.transport.registry import TransportRegistry, default_transports
 
 # enforce imports to avoid obscure stacktraces with MemoryError
@@ -186,6 +187,7 @@ def __init__(self, dsn=None, raise_send_errors=False, transport=None,
         self.tags = o.get('tags') or {}
         self.environment = o.get('environment') or None
         self.release = o.get('release') or os.environ.get('HEROKU_SLUG_COMMIT')
+        self.transaction = TransactionStack()
 
         self.ignore_exceptions = set(o.get('ignore_exceptions') or ())
 
@@ -410,12 +412,7 @@ def build_msg(self, event_type, data=None, date=None,
                     )
 
         if not culprit:
-            if 'stacktrace' in data:
-                culprit = get_culprit(data['stacktrace']['frames'])
-            elif 'exception' in data:
-                stacktrace = data['exception']['values'][0].get('stacktrace')
-                if stacktrace:
-                    culprit = get_culprit(stacktrace['frames'])
+            culprit = self.transaction.peek()
 
         if not data.get('level'):
             data['level'] = kwargs.get('level') or logging.ERROR

raven/contrib/celery/__init__.py

@@ -10,7 +10,9 @@
 import logging
 
 from celery.exceptions import SoftTimeLimitExceeded
-from celery.signals import after_setup_logger, task_failure
+from celery.signals import (
+    after_setup_logger, task_failure, task_prerun, task_postrun
+)
 from raven.handlers.logging import SentryHandler
 
 
@@ -25,26 +27,7 @@ def filter(self, record):
 
 
 def register_signal(client, ignore_expected=False):
-    def process_failure_signal(sender, task_id, args, kwargs, einfo, **kw):
-        if ignore_expected and isinstance(einfo.exception, sender.throws):
-            return
-
-        # This signal is fired inside the stack so let raven do its magic
-        if isinstance(einfo.exception, SoftTimeLimitExceeded):
-            fingerprint = ['celery', 'SoftTimeLimitExceeded', sender]
-        else:
-            fingerprint = None
-        client.captureException(
-            extra={
-                'task_id': task_id,
-                'task': sender,
-                'args': args,
-                'kwargs': kwargs,
-            },
-            fingerprint=fingerprint,
-        )
-
-    task_failure.connect(process_failure_signal, weak=False)
+    SentryCeleryHandler(client, ignore_expected=ignore_expected).install()
 
 
 def register_logger_signal(client, logger=None, loglevel=logging.ERROR):
@@ -67,3 +50,45 @@ def process_logger_event(sender, logger, loglevel, logfile, format,
         logger.addHandler(handler)
 
     after_setup_logger.connect(process_logger_event, weak=False)
+
+
+class SentryCeleryHandler(object):
+    def __init__(self, client, ignore_expected=False):
+        self.client = client
+        self.ignore_expected = ignore_expected
+
+    def install(self):
+        task_prerun.connect(self.handle_task_prerun, weak=False)
+        task_postrun.connect(self.handle_task_postrun, weak=False)
+        task_failure.connect(self.process_failure_signal, weak=False)
+
+    def uninstall(self):
+        task_prerun.disconnect(self.handle_task_prerun)
+        task_postrun.disconnect(self.handle_task_postrun)
+        task_failure.disconnect(self.process_failure_signal)
+
+    def process_failure_signal(self, sender, task_id, args, kwargs, einfo, **kw):
+        if self.ignore_expected and isinstance(einfo.exception, sender.throws):
+            return
+
+        # This signal is fired inside the stack so let raven do its magic
+        if isinstance(einfo.exception, SoftTimeLimitExceeded):
+            fingerprint = ['celery', 'SoftTimeLimitExceeded', sender]
+        else:
+            fingerprint = None
+
+        self.client.captureException(
+            extra={
+                'task_id': task_id,
+                'task': sender,
+                'args': args,
+                'kwargs': kwargs,
+            },
+            fingerprint=fingerprint,
+        )
+
+    def handle_task_prerun(self, sender, task_id, task, **kw):
+        self.client.transaction.push(task.name)
+
+    def handle_task_postrun(self, sender, task_id, task, **kw):
+        self.client.transaction.pop(task.name)

raven/contrib/django/middleware/__init__.py

@@ -8,11 +8,13 @@
 
 from __future__ import absolute_import
 
-import threading
 import logging
+import threading
 
 from django.conf import settings
 
+from raven.contrib.django.resolver import RouteResolver
+
 
 def is_ignorable_404(uri):
     """
@@ -61,9 +63,48 @@ def process_response(self, request, response):
         return response
 
 
-class SentryLogMiddleware(object):
-    # Create a threadlocal variable to store the session in for logging
-    thread = threading.local()
+class SentryMiddleware(threading.local):
+    resolver = RouteResolver()
+
+    # backwards compat
+    @property
+    def thread(self):
+        return self
+
+    def _get_transaction_from_request(self, request):
+        # TODO(dcramer): it'd be nice to pull out parameters
+        # and make this a normalized path
+        return self.resolver.resolve(request.path)
 
     def process_request(self, request):
+        self._txid = None
         self.thread.request = request
+
+    def process_view(self, request, func, args, kwargs):
+        from raven.contrib.django.models import client
+
+        try:
+            self._txid = client.transaction.push(
+                self._get_transaction_from_request(request)
+            )
+        except Exception as exc:
+            raise
+            client.error_logger.exception(repr(exc))
+        return None
+
+    def process_response(self, request, response):
+        from raven.contrib.django.models import client
+
+        if self._txid:
+            client.transaction.pop(self._txid)
+            self._txid = None
+        return response
+
+    # def process_exception(self, request, exception):
+    #     from raven.contrib.django.models import client
+
+    #     if self._txid:
+    #         client.transaction.pop(self._txid)
+    #         self._txid = None
+
+SentryLogMiddleware = SentryMiddleware

raven/contrib/django/models.py

@@ -16,13 +16,17 @@
 import warnings
 
 from django.conf import settings
+from django.core.signals import got_request_exception, request_started
+from threading import Lock
 
-from raven._compat import PY2, binary_type, text_type, string_types
+from raven._compat import PY2, binary_type, text_type
 from raven.utils.conf import convert_options
 from raven.utils.imports import import_string
 
 logger = logging.getLogger('sentry.errors.client')
 
+settings_lock = Lock()
+
 
 def get_installed_apps():
     """
@@ -151,66 +155,95 @@ def sentry_exception_handler(request=None, **kwargs):
             warnings.warn('Unable to process log entry: %s' % (exc,))
 
 
-def register_handlers():
-    from django.core.signals import got_request_exception, request_started
-
-    def before_request(*args, **kwargs):
-        client.context.activate()
-    request_started.connect(before_request, weak=False)
+class SentryDjangoHandler(object):
+    def __init__(self, client=client):
+        self.client = client
 
-    # HACK: support Sentry's internal communication
-    if 'sentry' in settings.INSTALLED_APPS:
-        from django.db import transaction
-        # Django 1.6
-        if hasattr(transaction, 'atomic'):
-            commit_on_success = transaction.atomic
+        try:
+            import celery
+        except ImportError:
+            self.has_celery = False
         else:
-            commit_on_success = transaction.commit_on_success
+            self.has_celery = celery.VERSION >= (2, 5)
 
-        @commit_on_success
-        def wrap_sentry(request, **kwargs):
-            if transaction.is_dirty():
-                transaction.rollback()
-            return sentry_exception_handler(request, **kwargs)
+        self.celery_handler = None
 
-        exception_handler = wrap_sentry
-    else:
-        exception_handler = sentry_exception_handler
+    def install_celery(self):
+        from raven.contrib.celery import (
+            SentryCeleryHandler, register_logger_signal
+        )
 
-    # Connect to Django's internal signal handler
-    got_request_exception.connect(exception_handler, weak=False)
+        self.celery_handler = SentryCeleryHandler(client).install()
 
-    # If Celery is installed, register a signal handler
-    if 'djcelery' in settings.INSTALLED_APPS:
-        try:
-            # Celery < 2.5? is not supported
-            from raven.contrib.celery import (
-                register_signal, register_logger_signal)
-        except ImportError:
-            logger.exception('Failed to install Celery error handler')
-        else:
+        # try:
+        #     ga = lambda x, d=None: getattr(settings, 'SENTRY_%s' % x, d)
+        #     options = getattr(settings, 'RAVEN_CONFIG', {})
+        #     loglevel = options.get('celery_loglevel',
+        #                            ga('CELERY_LOGLEVEL', logging.ERROR))
+
+        #     register_logger_signal(client, loglevel=loglevel)
+        # except Exception:
+        #     logger.exception('Failed to install Celery error handler')
+
+    def install(self):
+        request_started.connect(self.before_request, weak=False)
+        got_request_exception.connect(self.exception_handler, weak=False)
+
+        if self.has_celery:
             try:
-                register_signal(client)
+                self.install_celery()
             except Exception:
                 logger.exception('Failed to install Celery error handler')
 
+    def uninstall(self):
+        request_started.disconnect(self.before_request)
+        got_request_exception.disconnect(self.exception_handler)
+
+        if self.celery_handler:
+            self.celery_handler.uninstall()
+
+    def exception_handler(self, request=None, **kwargs):
+        try:
+            self.client.captureException(exc_info=sys.exc_info(), request=request)
+        except Exception as exc:
             try:
-                ga = lambda x, d=None: getattr(settings, 'SENTRY_%s' % x, d)
-                options = getattr(settings, 'RAVEN_CONFIG', {})
-                loglevel = options.get('celery_loglevel',
-                                       ga('CELERY_LOGLEVEL', logging.ERROR))
+                logger.exception('Unable to process log entry: %s' % (exc,))
+            except Exception as exc:
+                warnings.warn('Unable to process log entry: %s' % (exc,))
 
-                register_logger_signal(client, loglevel=loglevel)
-            except Exception:
-                logger.exception('Failed to install Celery error handler')
+    def before_request(self, *args, **kwargs):
+        self.client.context.activate()
 
 
 def register_serializers():
     # force import so serializers can call register
     import raven.contrib.django.serializers  # NOQA
 
 
-if ('raven.contrib.django' in settings.INSTALLED_APPS
-        or 'raven.contrib.django.raven_compat' in settings.INSTALLED_APPS):
-    register_handlers()
+def install_middleware():
+    """
+    Force installation of SentryMiddlware if it's not explicitly present.
+
+    This ensures things like request context and transaction names are made
+    available.
+    """
+    name = 'raven.contrib.django.middleware.SentryMiddleware'
+    all_names = (name, 'raven.contrib.django.middleware.SentryLogMiddleware')
+    with settings_lock:
+        middleware_list = set(settings.MIDDLEWARE_CLASSES)
+        if not any(n in middleware_list for n in all_names):
+            settings.MIDDLEWARE_CLASSES = (
+                name,
+            ) + tuple(settings.MIDDLEWARE_CLASSES)
+
+
+if (
+    'raven.contrib.django' in settings.INSTALLED_APPS or
+    'raven.contrib.django.raven_compat' in settings.INSTALLED_APPS
+):
     register_serializers()
+    install_middleware()
+
+    if not getattr(settings, 'DISABLE_SENTRY_INSTRUMENTATION', False):
+        handler = SentryDjangoHandler()
+        handler.install()