diff --git a/cms/envs/common.py b/cms/envs/common.py index 6d0fd9b18d..11d5931a9a 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -480,6 +480,9 @@ MIDDLEWARE_CLASSES = [ 'edx_django_utils.cache.middleware.RequestCacheMiddleware', 'edx_django_utils.monitoring.middleware.MonitoringMemoryMiddleware', + # Cookie monitoring + 'openedx.core.lib.request_utils.CookieMetricsMiddleware', + 'openedx.core.djangoapps.header_control.middleware.HeaderControlMiddleware', 'django.middleware.cache.UpdateCacheMiddleware', 'django.middleware.common.CommonMiddleware', diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py index 4ca9cecf2a..012a5495a5 100644 --- a/lms/djangoapps/course_api/tests/test_views.py +++ b/lms/djangoapps/course_api/tests/test_views.py @@ -387,7 +387,7 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear self.setup_user(self.audit_user) # These query counts were found empirically - query_counts = [64, 51, 51, 51, 51, 51, 51, 51, 51, 51, 21] + query_counts = [65, 52, 52, 52, 52, 52, 52, 52, 52, 52, 22] ordered_course_ids = sorted([str(cid) for cid in (course_ids + [c.id for c in self.courses])]) self.clear_caches() diff --git a/lms/envs/common.py b/lms/envs/common.py index 69e129ea56..764fdb1c6f 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1222,6 +1222,9 @@ MIDDLEWARE_CLASSES = [ 'edx_django_utils.cache.middleware.RequestCacheMiddleware', 'edx_django_utils.monitoring.middleware.MonitoringCustomMetricsMiddleware', + # Cookie monitoring + 'openedx.core.lib.request_utils.CookieMetricsMiddleware', + 'mobile_api.middleware.AppVersionUpgrade', 'openedx.core.djangoapps.header_control.middleware.HeaderControlMiddleware', 'microsite_configuration.middleware.MicrositeMiddleware', diff --git a/openedx/core/djangoapps/bookmarks/tests/test_views.py b/openedx/core/djangoapps/bookmarks/tests/test_views.py index 1e6fb1f284..6a9f8b16d2 100644 --- a/openedx/core/djangoapps/bookmarks/tests/test_views.py +++ b/openedx/core/djangoapps/bookmarks/tests/test_views.py @@ -264,7 +264,7 @@ class BookmarksListViewTests(BookmarksViewsTestsBase): self.assertEqual(response.data['developer_message'], u'Parameter usage_id not provided.') # Send empty data dictionary. - with self.assertNumQueries(8): # No queries for bookmark table. + with self.assertNumQueries(9): # No queries for bookmark table. response = self.send_post( client=self.client, url=reverse('bookmarks'), diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index b0a66dcbc0..bad7effa5a 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -171,7 +171,7 @@ class TestOwnUsernameAPI(CacheIsolationTestCase, UserAPITestCase): Test that a client (logged in) can get her own username. """ self.client.login(username=self.user.username, password=TEST_PASSWORD) - self._verify_get_own_username(16) + self._verify_get_own_username(17) def test_get_username_inactive(self): """ @@ -181,7 +181,7 @@ class TestOwnUsernameAPI(CacheIsolationTestCase, UserAPITestCase): self.client.login(username=self.user.username, password=TEST_PASSWORD) self.user.is_active = False self.user.save() - self._verify_get_own_username(16) + self._verify_get_own_username(17) def test_get_username_not_logged_in(self): """ @@ -190,7 +190,7 @@ class TestOwnUsernameAPI(CacheIsolationTestCase, UserAPITestCase): """ # verify that the endpoint is inaccessible when not logged in - self._verify_get_own_username(12, expected_status=401) + self._verify_get_own_username(13, expected_status=401) @ddt.ddt @@ -335,7 +335,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): """ self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD) self.create_mock_profile(self.user) - with self.assertNumQueries(22): + with self.assertNumQueries(23): response = self.send_get(self.different_client) self._verify_full_shareable_account_response(response, account_privacy=ALL_USERS_VISIBILITY) @@ -350,7 +350,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): """ self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD) self.create_mock_profile(self.user) - with self.assertNumQueries(22): + with self.assertNumQueries(23): response = self.send_get(self.different_client) self._verify_private_account_response(response) @@ -490,12 +490,12 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): self.assertEqual(False, data["accomplishments_shared"]) self.client.login(username=self.user.username, password=TEST_PASSWORD) - verify_get_own_information(20) + verify_get_own_information(21) # Now make sure that the user can get the same information, even if not active self.user.is_active = False self.user.save() - verify_get_own_information(14) + verify_get_own_information(15) def test_get_account_empty_string(self): """ @@ -509,7 +509,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): legacy_profile.save() self.client.login(username=self.user.username, password=TEST_PASSWORD) - with self.assertNumQueries(20): + with self.assertNumQueries(21): response = self.send_get(self.client) for empty_field in ("level_of_education", "gender", "country", "bio"): self.assertIsNone(response.data[empty_field]) diff --git a/openedx/core/lib/request_utils.py b/openedx/core/lib/request_utils.py index fd69bc569c..3041c9da9a 100644 --- a/openedx/core/lib/request_utils.py +++ b/openedx/core/lib/request_utils.py @@ -1,18 +1,30 @@ """ Utility functions related to HTTP requests """ +import logging import re from urlparse import urlparse -import crum +import crum from django.conf import settings +from django.utils.deprecation import MiddlewareMixin from django.test.client import RequestFactory + +from openedx.core.djangoapps.waffle_utils import WaffleFlag, WaffleFlagNamespace from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +try: + import newrelic.agent +except ImportError: + newrelic = None # pylint: disable=invalid-name # accommodates course api urls, excluding any course api routes that do not fall under v*/courses, such as v1/blocks. COURSE_REGEX = re.compile(r'^(.*?/courses/)(?!v[0-9]+/[^/]+){}'.format(settings.COURSE_ID_PATTERN)) +WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name='request_utils') +CAPTURE_COOKIE_SIZES = WaffleFlag(WAFFLE_FLAG_NAMESPACE, 'capture_cookie_sizes') +log = logging.getLogger(__name__) + def get_request_or_stub(): """ @@ -80,3 +92,35 @@ def course_id_from_url(url): return CourseKey.from_string(course_id) except InvalidKeyError: return None + + +class CookieMetricsMiddleware(MiddlewareMixin): + """ + Middleware for monitoring the size and growth of all our cookies, to see if + we're running into browser limits. + """ + def process_request(self, request): + """ + Emit custom metrics for cookie size values for every cookie we have. + + Don't log contents of cookies because that might cause a security issue. + We just want to see if any cookies are growing out of control. + """ + if not newrelic: + return + + if not CAPTURE_COOKIE_SIZES.is_enabled(): + return + + cookie_names_to_size = { + name: len(value) + for name, value in request.COOKIES.items() + } + for name, size in cookie_names_to_size.items(): + metric_name = 'cookies.{}.size'.format(name) + newrelic.agent.add_custom_parameter(metric_name, size) + log.debug("%s = %d", metric_name, size) + + total_cookie_size = sum(cookie_names_to_size.values()) + newrelic.agent.add_custom_parameter('cookies_total_size', total_cookie_size) + log.debug("cookies_total_size = %d", total_cookie_size)