Canonical SSO provider

Merge lp:~james-w/canonical-identity-provider/remove-logging-middleware into lp:canonical-identity-provider/release

  1. remove-logging-middleware
  2. Merge into trunk
Proposed by James Westby
Status: Merged
Approved by: James Westby
Approved revision: no longer in the source branch.
Merged at revision: 1023
Proposed branch: lp:~james-w/canonical-identity-provider/remove-logging-middleware
Merge into: lp:canonical-identity-provider/release
Diff against target: 193 lines (+1/-120)
5 files modified
django_project/config/main.cfg (+0/-2)
django_project/config/profiling.cfg (+0/-2)
src/identityprovider/middleware/exception.py (+0/-23)
src/identityprovider/middleware/timer.py (+0/-27)
src/identityprovider/tests/test_middleware.py (+1/-66)
To merge this branch: bzr merge lp:~james-w/canonical-identity-provider/remove-logging-middleware
Reviewer Review Type Date Requested Status
Natalia Bidart (community) Approve
Review via email: mp+181828@code.launchpad.net

Commit message

Remove the exception logging and timing middlewares.

Now that we have decent oopses we have a much easier way of getting roughly
the same as these middleware provided. Remove them as they are slowing
down all requests, particularly when they trigger. The oops infrastructure
can be improved if there are extra features these middleware had that
are still desirable.

Description of the change

Hi,

As described in the commit message this drops the logging middlewares
in favour of the oops system.

Thanks,

James

To post a comment you must log in.
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

THIS IS GREAT.

review: Approve
Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :
Download full text (254.5 KiB)

The attempt to merge lp:~james-w/canonical-identity-provider/remove-logging-middleware into lp:canonical-identity-provider failed. Below is the output from the failed tests.

Branching the download cache in dir /mnt/tarmac/cache/canonical-identity-provider/isd-download-cache
[localhost] local: which virtualenv
[localhost] local: /usr/bin/python /usr/bin/virtualenv --distribute --clear --system-site-packages .env
Not deleting .env/bin
New python executable in .env/bin/python
Installing distribute.............................................................................................................................................................................................done.
Installing pip...............done.
[localhost] local: dpkg -l libpq-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l libxml2-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l libxslt1-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l memcached 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l postgresql-plpython-9.1 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-m2crypto 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l swig 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l config-manager 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-egenix-mx-base-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: /usr/lib/config-manager/cm.py update /tmp/tmpdSwRXL
[localhost] local: . /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/.env/bin/activate && make install PACKAGES="-r /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/requirements.txt"
pip install --find-links=file:///mnt/tarmac/cache/canonical-identity-provider/isd-download-cache --no-index pip==dev
Ignoring indexes: http://pypi.python.org/simple/
Downloading/unpacking pip==dev
  Running setup.py egg_info for package pip

    warning: no files found matching '*.html' under directory 'docs'
    warning: no previously-included files matching '*.txt' found under directory 'docs/_build'
    no previously-included directories found matching 'docs/_build/_sources'
Installing collected packages: pip
  Found existing installation: pip 1.1
    Uninstalling pip:
      Successfully uninstalled pip
  Running setup.py install for pip

    warning: no files found matching '*.html' under directory 'docs'
    warning: no previously-included files matching '*.txt' found under directory 'docs/_build'
    no previously-included directories found matching 'docs/_build/_sources'
    Installing pip script to /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/.env/bin
    Installing pip-2.7 script to /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/.env/bin
Successfully installed pip
Cleaning up...
pip install --find-links=. --no-index -r /mnt/tarmac/cache/canonical-identity-provider/merges/trunk/requirements.txt
Ignoring indexes: http://pypi.python.org/simple/
Downloading/unpacking bzr+http://bazaar.launchpad.net/~canonical-isd-hackers/u1-test-utils/trunk@81 (from -r /mnt/tarmac/cache/canonical-i...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'django_project/config/main.cfg'
2--- django_project/config/main.cfg 2013-08-12 22:08:49 +0000
3+++ django_project/config/main.cfg 2013-08-23 14:50:35 +0000
4@@ -68,7 +68,6 @@
5 middleware_classes =
6 django_statsd.middleware.GraphiteRequestTimingMiddleware
7 django_statsd.middleware.GraphiteMiddleware
8- identityprovider.middleware.timer.TimerMiddleware
9 identityprovider.middleware.readonly.ReadOnlyMiddleware
10 djangosecure.middleware.SecurityMiddleware
11 django.middleware.common.CommonMiddleware
12@@ -79,7 +78,6 @@
13 django.middleware.clickjacking.XFrameOptionsMiddleware
14 identityprovider.middleware.useraccount.UserAccountConversionMiddleware
15 django.middleware.locale.LocaleMiddleware
16- identityprovider.middleware.exception.LogExceptionMiddleware
17 # Uncomment this to enable profiling middleware
18 # identityprovider.middleware.profile.ProfileMiddleware
19 secret_key = CHANGEME
20
21=== modified file 'django_project/config/profiling.cfg'
22--- django_project/config/profiling.cfg 2013-08-21 14:40:33 +0000
23+++ django_project/config/profiling.cfg 2013-08-23 14:50:35 +0000
24@@ -29,7 +29,6 @@
25 middleware_classes =
26 django_statsd.middleware.GraphiteRequestTimingMiddleware
27 django_statsd.middleware.GraphiteMiddleware
28- identityprovider.middleware.timer.TimerMiddleware
29 identityprovider.middleware.readonly.ReadOnlyMiddleware
30 djangosecure.middleware.SecurityMiddleware
31 django.middleware.common.CommonMiddleware
32@@ -40,7 +39,6 @@
33 django.middleware.clickjacking.XFrameOptionsMiddleware
34 identityprovider.middleware.useraccount.UserAccountConversionMiddleware
35 django.middleware.locale.LocaleMiddleware
36- identityprovider.middleware.exception.LogExceptionMiddleware
37 identityprovider.middleware.profile.ProfileMiddleware
38 timelog.middleware.TimeLogMiddleware
39
40
41=== removed file 'src/identityprovider/middleware/exception.py'
42--- src/identityprovider/middleware/exception.py 2013-08-12 15:25:18 +0000
43+++ src/identityprovider/middleware/exception.py 1970-01-01 00:00:00 +0000
44@@ -1,23 +0,0 @@
45-# Copyright 2011 Canonical Ltd. This software is licensed under the
46-# GNU Affero General Public License version 3 (see the file LICENSE).
47-
48-import logging
49-import sys
50-import traceback
51-
52-from .util import log_request
53-
54-
55-logger = logging.getLogger(__name__)
56-
57-
58-class ConsoleExceptionMiddleware(object):
59- def process_exception(self, request, exception):
60- traceback.print_exc(file=sys.stdout)
61-
62-
63-class LogExceptionMiddleware(object):
64-
65- def process_exception(self, request, exception):
66- logger.exception('Unhandled exception in application')
67- log_request(request, log_exception_trace=True)
68
69=== removed file 'src/identityprovider/middleware/timer.py'
70--- src/identityprovider/middleware/timer.py 2013-08-13 17:02:47 +0000
71+++ src/identityprovider/middleware/timer.py 1970-01-01 00:00:00 +0000
72@@ -1,27 +0,0 @@
73-# Copyright 2011 Canonical Ltd. This software is licensed under the
74-# GNU Affero General Public License version 3 (see the file LICENSE).
75-
76-from time import time
77-import logging
78-
79-from django.conf import settings
80-
81-from .util import log_request
82-
83-
84-logger = logging.getLogger(__name__)
85-
86-
87-class TimerMiddleware(object):
88-
89- def process_request(self, request):
90- request._time_start = time()
91- return None
92-
93- def process_response(self, request, response):
94- time_taken_ms = (time() - request._time_start) * 1000
95- if time_taken_ms > settings.HANDLER_TIMEOUT_MILLIS:
96- logger.warning('Response took too long: %s ms', time_taken_ms)
97- log_request(request)
98-
99- return response
100
101=== modified file 'src/identityprovider/tests/test_middleware.py'
102--- src/identityprovider/tests/test_middleware.py 2013-08-12 15:25:18 +0000
103+++ src/identityprovider/tests/test_middleware.py 2013-08-23 14:50:35 +0000
104@@ -2,10 +2,8 @@
105 # GNU Affero General Public License version 3 (see the file LICENSE).
106
107 import re
108-import sys
109
110 from django.conf import settings
111-from django.core.urlresolvers import reverse
112 from django.test import TestCase
113 from django.contrib.auth.models import User, AnonymousUser
114
115@@ -14,7 +12,7 @@
116 from openid.message import IDENTIFIER_SELECT, OPENID1_URL_LIMIT, OPENID2_NS
117
118 from identityprovider.const import SESSION_TOKEN_KEY
119-from identityprovider.middleware import exception, util as middleware_util
120+from identityprovider.middleware import util as middleware_util
121 from identityprovider.middleware.useraccount import (
122 UserAccountConversionMiddleware)
123 from identityprovider.middleware.readonly import ReadOnlyMiddleware
124@@ -160,69 +158,6 @@
125 self.assertEqual(req.GET['get_public'], self.GET_PUBLIC)
126
127
128-class ExceptionMiddlewareTestCase(TestCase):
129-
130- PRIVATE_SETTING = '7&8*9('
131- PUBLIC_SETTING = 'Something public'
132- FRAME_VAR = '*8*8*8*8'
133-
134- def setUp(self):
135- super(ExceptionMiddlewareTestCase, self).setUp()
136- self.request = MockRequest('/login')
137- self.request.META = {'SERVER_NAME': 'localhost', 'SERVER_PORT': 80}
138- self.middleware = exception.LogExceptionMiddleware()
139-
140- def _get_traceback_html(self):
141- reporter = middleware_util._ExceptionReporter(
142- self.request, *sys.exc_info())
143- return reporter.get_traceback_html()
144-
145- def test_settings_sanitization(self):
146- settings.PRIVATE_SETTING = self.PRIVATE_SETTING
147- settings.PUBLIC_SETTING = self.PUBLIC_SETTING
148-
149- try:
150- 2 / 0
151- except:
152- html = self._get_traceback_html()
153-
154- self.assertFalse(self.PRIVATE_SETTING in html)
155- self.assertTrue(self.PUBLIC_SETTING in html)
156-
157- def test_frames_sanitization(self):
158- try:
159- self.FRAME_VAR
160- 2 / 0
161- except:
162- html = self._get_traceback_html()
163-
164- self.assertFalse(self.FRAME_VAR in html)
165- self.assertTrue('public_variable' in html)
166-
167-
168-class TimerMiddlewareTestCase(SSOBaseTestCase):
169-
170- def setUp(self):
171- super(TimerMiddlewareTestCase, self).setUp()
172- self.mock_log_request = self._apply_patch(
173- 'identityprovider.middleware.timer.log_request')
174- self.mock_logger = self._apply_patch(
175- 'identityprovider.middleware.timer.logger')
176-
177- def _check(self, timeout_millis, calls):
178- with patch_settings(HANDLER_TIMEOUT_MILLIS=timeout_millis):
179- self.client.get(reverse('account-index'))
180-
181- self.assertEqual(self.mock_log_request.call_count, calls)
182- self.assertEqual(self.mock_logger.warning.call_count, calls)
183-
184- def test_under_time(self):
185- self._check(5000, 0)
186-
187- def test_over_time(self):
188- self._check(0, 1)
189-
190-
191 # The CSRF middleware requires a session cookie in order to activate.
192 # The tests perform a login in order to acquire this session cookie.
193 class CSRFMiddlewareTestCase(SSOBaseTestCase):