launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/maas-logging into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/maas-logging/+merge/96874

This branch adds a proper logging configuration in demo mode (LOGGING in demo.py).  It also changes the error middleware so that it is now properly logging errors instead of bluntly using print().  This branch also contains a sample prod conf that will log errors in /var/log/maas/maas.log.
-- 
https://code.launchpad.net/~rvb/maas/maas-logging/+merge/96874
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-logging into lp:maas.

=== modified file 'contrib/maas_local_settings_sample.py'
--- contrib/maas_local_settings_sample.py	2012-03-08 18:01:03 +0000
+++ contrib/maas_local_settings_sample.py	2012-03-12 08:41:18 +0000
@@ -12,6 +12,40 @@
 # Where to store the user uploaded files.
 MEDIA_ROOT = '/var/lib/maas/media/'
 
+# See http://docs.djangoproject.com/en/dev/topics/logging for
+# more details on how to customize the logging configuration.
+LOGGING = {
+    'version': 1,
+    'disable_existing_loggers': False,
+    'formatters': {
+        'simple': {
+            'format': '%(levelname)s %(asctime)s %(name)s %(message)s'
+        },
+    },
+    'handlers': {
+        'log': {
+            'level': 'ERROR',
+            'class': 'logging.handlers.RotatingFileHandler',
+            'filename': '/var/log/maas/maas.log',
+            'formatter': 'simple',
+        },
+    },
+    'loggers': {
+        'maas': {
+            'handlers': ['log'],
+            'propagate': True,
+        },
+        'django.request': {
+            'handlers': ['log'],
+            'propagate': True,
+        },
+        'django.db.backends': {
+            'handlers': ['log'],
+            'propagate': True,
+        },
+     }
+}
+
 # Database access configuration.
 DATABASES = {
     'default': {

=== modified file 'src/maas/demo.py'
--- src/maas/demo.py	2012-03-05 15:13:25 +0000
+++ src/maas/demo.py	2012-03-12 08:41:18 +0000
@@ -15,9 +15,40 @@
 from maas.development import *
 
 MIDDLEWARE_CLASSES += (
-    'maasserver.middleware.ConsoleExceptionMiddleware',
     'debug_toolbar.middleware.DebugToolbarMiddleware',
 )
 
 # This should match the setting in Makefile:pserv.pid.
 PSERV_URL = "http://localhost:8001/api";
+
+
+LOGGING = {
+    'version': 1,
+    'disable_existing_loggers': False,
+    'formatters': {
+        'simple': {
+            'format': '%(levelname)s %(asctime)s %(name)s %(message)s'
+        },
+    },
+    'handlers': {
+        'console': {
+            'level': 'INFO',
+            'class': 'logging.StreamHandler',
+            'formatter': 'simple',
+        },
+    },
+    'loggers': {
+        'maas': {
+            'handlers': ['console'],
+            'propagate': True,
+        },
+        'django.request': {
+            'handlers': ['console'],
+            'propagate': True,
+        },
+        'django.db.backends': {
+            'handlers': ['console'],
+            'propagate': True,
+        },
+     }
+}

=== modified file 'src/maas/settings.py'
--- src/maas/settings.py	2012-03-09 13:00:59 +0000
+++ src/maas/settings.py	2012-03-12 08:41:18 +0000
@@ -181,6 +181,7 @@
     'django.middleware.transaction.TransactionMiddleware',
     'django.middleware.csrf.CsrfViewMiddleware',
     'django.middleware.csrf.CsrfResponseMiddleware',
+    'maasserver.middleware.ExceptionLoggerMiddleware',
     'django.contrib.auth.middleware.AuthenticationMiddleware',
     'django.contrib.messages.middleware.MessageMiddleware',
     'maasserver.middleware.AccessMiddleware',
@@ -214,27 +215,11 @@
     INSTALLED_APPS += (
         'django.contrib.admin',
     )
-# A sample logging configuration. The only tangible logging
-# performed by this configuration is to send an email to
-# the site admins on every HTTP 500 error.
+
 # See http://docs.djangoproject.com/en/dev/topics/logging for
-# more details on how to customize your logging configuration.
+# more details on how to customize the logging configuration.
 LOGGING = {
     'version': 1,
-    'disable_existing_loggers': False,
-    'handlers': {
-        'mail_admins': {
-            'level': 'ERROR',
-            'class': 'django.utils.log.AdminEmailHandler'
-        }
-    },
-    'loggers': {
-        'django.request': {
-            'handlers': ['mail_admins'],
-            'level': 'ERROR',
-            'propagate': True,
-        },
-    }
 }
 
 # The location of the Provisioning API XML-RPC endpoint. If PSERV_URL is None,

=== modified file 'src/maasserver/middleware.py'
--- src/maasserver/middleware.py	2012-03-08 15:53:39 +0000
+++ src/maasserver/middleware.py	2012-03-12 08:41:18 +0000
@@ -20,6 +20,7 @@
     abstractproperty,
     )
 import json
+import logging
 import re
 
 from django.conf import settings
@@ -152,11 +153,12 @@
     path_regex = settings.API_URL_REGEXP
 
 
-class ConsoleExceptionMiddleware:
+class ExceptionLoggerMiddleware:
+
     def process_exception(self, request, exception):
         import traceback
         import sys
         exc_info = sys.exc_info()
-        print(" Exception ".center(79, "#"))
-        print(''.join(traceback.format_exception(*exc_info)))
-        print("#" * 79)
+        logger = logging.getLogger('maas.maasserver')
+        logger.error(" Exception ".center(79, "#"))
+        logger.error(''.join(traceback.format_exception(*exc_info)))

=== modified file 'src/maasserver/tests/test_middleware.py'
--- src/maasserver/tests/test_middleware.py	2012-02-27 07:47:28 +0000
+++ src/maasserver/tests/test_middleware.py	2012-03-12 08:41:18 +0000
@@ -13,6 +13,8 @@
 
 import httplib
 import json
+import logging
+from tempfile import NamedTemporaryFile
 
 from django.core.exceptions import ValidationError
 from django.test.client import RequestFactory
@@ -22,6 +24,7 @@
     )
 from maasserver.middleware import (
     APIErrorsMiddleware,
+    ExceptionLoggerMiddleware,
     ExceptionMiddleware,
     )
 from maasserver.testing import (
@@ -129,3 +132,20 @@
         exception = MaaSAPINotFound("Have you looked under the couch?")
         self.assertIsNone(
             middleware.process_exception(non_api_request, exception))
+
+
+class ExceptionLoggerMiddlewareTest(TestCase):
+
+    def setup_logger(self):
+        tempfile = NamedTemporaryFile()
+        logger = logging.getLogger('maas')
+        logger.addHandler(logging.handlers.RotatingFileHandler(tempfile.name))
+        return tempfile
+
+    def test_exception_logger_logs_error(self):
+        log_file = self.setup_logger()
+        middleware = ExceptionLoggerMiddleware()
+        request = fake_request("/middleware/api/hello")
+        middleware.process_exception(request, ValueError())
+        logger_content = open(log_file.name).read()
+        self.assertIn(" Exception ".center(79, "#"), logger_content)