Add cookie size monitoring New Relic metrics.

This adds middleware that will create custom parameter metrics in
New Relic to track the size of all the cookies being received for
our domain. The custom fields are "cookies_total_size" and a
separate named parameter for every cookie size, e.g.
"cookies.csrftoken.size".

This is intended to help us track cookie growth and better diagnose
issues where users lose their sessions. It is toggled by the
'request_utils.capture_cookie_sizes' Waffle Flag.
This commit is contained in:
David Ormsbee
2019-02-22 14:29:31 -05:00
parent 2fc755d43f
commit 76620e0bf2
6 changed files with 61 additions and 11 deletions

View File

@@ -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',

View File

@@ -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()

View File

@@ -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',

View File

@@ -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'),

View File

@@ -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])

View File

@@ -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)