diff --git a/common/djangoapps/newrelic_custom_metrics/__init__.py b/common/djangoapps/newrelic_custom_metrics/__init__.py deleted file mode 100644 index a695a4a5b0..0000000000 --- a/common/djangoapps/newrelic_custom_metrics/__init__.py +++ /dev/null @@ -1,52 +0,0 @@ -""" -This is an interface to the newrelic_custom_metrics middleware. Functions -defined in this module can be used to report custom metrics to New Relic. For -example: - - import newrelic_custom_metrics - ... - newrelic_custom_metrics.accumulate('xb_user_state.get_many.num_items', 4) - -There is no need to do anything else. The metrics are automatically cleared -before the next request. - -We try to keep track of our custom metrics at: - -https://openedx.atlassian.net/wiki/display/PERF/Custom+Metrics+in+New+Relic - -TODO: supply additional public functions for storing strings and booleans. -""" - -from newrelic_custom_metrics import middleware - - -def accumulate(name, value): - """ - Accumulate custom New Relic metric for the current request. - - The named metric is accumulated by a numerical amount using the sum. All - metrics are queued up in the request_cache for this request. At the end of - the request, the newrelic_custom_metrics middleware will batch report all - queued accumulated metrics to NR. - - Arguments: - name (str): The metric name. It should be period-delimited, and - increase in specificty from left to right. For example: - 'xb_user_state.get_many.num_items'. - value (number): The amount to accumulate into the named metric. When - accumulate() is called multiple times for a given metric name - during a request, the sum of the values for each call is reported - for that metric. For metrics which don't make sense to accumulate, - make sure to only call this function once during a request. - """ - middleware.NewRelicCustomMetrics.accumulate_metric(name, value) - - -def increment(name): - """ - Increment a custom New Relic metric representing a counter. - - Here we simply accumulate a new custom metric with a value of 1, and the - middleware should automatically aggregate this metric. - """ - accumulate(name, 1) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 696b7ff29f..571386da59 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -89,6 +89,7 @@ from openedx.core.djangoapps.external_auth.login_and_register import ( login as external_auth_login, register as external_auth_register ) +from openedx.core.djangoapps import monitoring_utils import track.views @@ -120,8 +121,6 @@ from openedx.core.djangoapps.embargo import api as embargo_api import analytics from eventtracking import tracker -import newrelic_custom_metrics - # Note that this lives in LMS, so this dependency should be refactored. from notification_prefs.views import enable_notifications @@ -654,7 +653,7 @@ def dashboard(request): # Record how many courses there are so that we can get a better # understanding of usage patterns on prod. - newrelic_custom_metrics.accumulate('num_courses', len(course_enrollments)) + monitoring_utils.accumulate('num_courses', len(course_enrollments)) # sort the enrollment pairs by the enrollment date course_enrollments.sort(key=lambda x: x.created, reverse=True) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 4de76a77bb..c85a613422 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -52,6 +52,9 @@ from openedx.core.djangoapps.bookmarks.services import BookmarksService from openedx.core.djangoapps.crawlers.models import CrawlersConfig from openedx.core.djangoapps.credit.services import CreditService from openedx.core.djangoapps.util.user_utils import SystemUser +from openedx.core.djangoapps.monitoring_utils import ( + set_custom_metrics_for_course_key, set_monitoring_transaction_name +) from openedx.core.lib.xblock_utils import ( replace_course_urls, replace_jump_to_id_urls, @@ -81,11 +84,6 @@ from .field_overrides import OverrideFieldData log = logging.getLogger(__name__) -try: - import newrelic.agent -except ImportError: - newrelic = None # pylint: disable=invalid-name - if settings.XQUEUE_INTERFACE.get('basic_auth') is not None: REQUESTS_AUTH = HTTPBasicAuth(*settings.XQUEUE_INTERFACE['basic_auth']) else: @@ -970,10 +968,7 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course except InvalidKeyError: raise Http404 - if newrelic: - # Gather metrics for New Relic so we can slice data in New Relic Insights - newrelic.agent.add_custom_parameter('course_id', unicode(course_key)) - newrelic.agent.add_custom_parameter('org', unicode(course_key.org)) + set_custom_metrics_for_course_key(course_key) with modulestore().bulk_operations(course_key): instance, tracking_context = get_module_by_usage_id(request, course_id, usage_id, course=course) @@ -983,8 +978,7 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course # "handler" in those cases is always just "xmodule_handler". nr_tx_name = "{}.{}".format(instance.__class__.__name__, handler) nr_tx_name += "/{}".format(suffix) if (suffix and handler == "xmodule_handler") else "" - if newrelic: - newrelic.agent.set_transaction_name(nr_tx_name, group="Python/XBlock/Handler") + set_monitoring_transaction_name(nr_tx_name, group="Python/XBlock/Handler") tracking_context_name = 'module_callback_handler' req = django_to_webob_request(request) diff --git a/lms/djangoapps/courseware/user_state_client.py b/lms/djangoapps/courseware/user_state_client.py index c1dfe42b82..ac316b876f 100644 --- a/lms/djangoapps/courseware/user_state_client.py +++ b/lms/djangoapps/courseware/user_state_client.py @@ -12,7 +12,6 @@ try: except ImportError: import json -import newrelic_custom_metrics import dogstats_wrapper as dog_stats_api from django.contrib.auth.models import User from django.db import transaction @@ -20,6 +19,7 @@ from django.db.utils import IntegrityError from xblock.fields import Scope from courseware.models import StudentModule, BaseStudentModuleHistory from edx_user_state_client.interface import XBlockUserStateClient, XBlockUserState +from openedx.core.djangoapps import monitoring_utils log = logging.getLogger(__name__) @@ -136,7 +136,7 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient): """ Accumulate arbitrary NR stats (not specific to block types). """ - newrelic_custom_metrics.accumulate( + monitoring_utils.accumulate( self._nr_metric_name(function_name, stat_name), value ) @@ -151,11 +151,11 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient): """ Accumulate NR stats related to block types. """ - newrelic_custom_metrics.accumulate( + monitoring_utils.accumulate( self._nr_metric_name(function_name, stat_name), value, ) - newrelic_custom_metrics.accumulate( + monitoring_utils.accumulate( self._nr_metric_name(function_name, stat_name, block_type=block_type), value, ) diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index 33c0a076d3..7fae4a491a 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -22,11 +22,6 @@ import logging log = logging.getLogger("edx.courseware.views.index") -try: - import newrelic.agent -except ImportError: - newrelic = None # pylint: disable=invalid-name - import urllib import waffle @@ -36,6 +31,7 @@ from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.user_api.preferences.api import get_user_preference from openedx.core.djangoapps.crawlers.models import CrawlersConfig +from openedx.core.djangoapps.monitoring_utils import set_custom_metrics_for_course_key from request_cache.middleware import RequestCache from shoppingcart.models import CourseRegistrationCode from student.models import CourseEnrollment @@ -107,7 +103,7 @@ class CoursewareIndex(View): self.url = request.path try: - self._init_new_relic() + set_custom_metrics_for_course_key(self.course_key) self._clean_position() with modulestore().bulk_operations(self.course_key): self.course = get_course_with_access(request.user, 'load', self.course_key, depth=CONTENT_DEPTH) @@ -180,15 +176,6 @@ class CoursewareIndex(View): ) ) - def _init_new_relic(self): - """ - Initialize metrics for New Relic so we can slice data in New Relic Insights - """ - if not newrelic: - return - newrelic.agent.add_custom_parameter('course_id', unicode(self.course_key)) - newrelic.agent.add_custom_parameter('org', unicode(self.course_key.org)) - def _clean_position(self): """ Verify that the given position is an integer. If it is not positive, set it to 1. diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 947881853b..ff7f2d9ced 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -87,8 +87,9 @@ from openedx.core.djangoapps.credit.api import ( ) from openedx.core.djangoapps.programs.utils import ProgramMarketingDataExtender from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers -from shoppingcart.utils import is_shopping_cart_enabled from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration +from openedx.core.djangoapps.monitoring_utils import set_custom_metrics_for_course_key +from shoppingcart.utils import is_shopping_cart_enabled from student.models import UserTestGroup, CourseEnrollment from student.roles import GlobalStaff from util.cache import cache, cache_if_anonymous @@ -502,6 +503,7 @@ class CourseTabView(EdxFragmentView): course = get_course_with_access(request.user, 'load', course_key) tab = CourseTabList.get_tab_by_type(course.tabs, tab_type) page_context = self.create_page_context(request, course=course, tab=tab, **kwargs) + set_custom_metrics_for_course_key(course_key) return super(CourseTabView, self).get(request, course=course, page_context=page_context, **kwargs) def create_page_context(self, request, course=None, tab=None, **kwargs): diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index e3acd10acb..c7d0704168 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -10,10 +10,6 @@ from django.db.utils import DatabaseError from logging import getLogger log = getLogger(__name__) -try: - import newrelic.agent -except ImportError: - newrelic = None # pylint: disable=invalid-name from celery_utils.logged_task import LoggedTask from celery_utils.persist_on_failure import PersistOnFailureTask @@ -22,6 +18,9 @@ from lms.djangoapps.course_blocks.api import get_course_blocks from lms.djangoapps.courseware import courses from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import CourseLocator +from openedx.core.djangoapps.monitoring_utils import ( + set_custom_metrics_for_course_key, set_custom_metric +) from student.models import CourseEnrollment from submissions import api as sub_api from track.event_transaction_utils import ( @@ -117,9 +116,8 @@ def _recalculate_subsection_grade(self, **kwargs): course_key = CourseLocator.from_string(kwargs['course_id']) scored_block_usage_key = UsageKey.from_string(kwargs['usage_id']).replace(course_key=course_key) - if newrelic: - newrelic.agent.add_custom_parameter('course_id', unicode(course_key)) - newrelic.agent.add_custom_parameter('usage_id', unicode(scored_block_usage_key)) + set_custom_metrics_for_course_key(course_key) + set_custom_metric('usage_id', unicode(scored_block_usage_key)) # The request cache is not maintained on celery workers, # where this code runs. So we take the values from the diff --git a/lms/envs/common.py b/lms/envs/common.py index 0f81a8c9c2..bb0fc9a00b 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1109,7 +1109,7 @@ MIDDLEWARE_CLASSES = ( 'crum.CurrentRequestUserMiddleware', 'request_cache.middleware.RequestCache', - 'newrelic_custom_metrics.middleware.NewRelicCustomMetrics', + 'openedx.core.djangoapps.monitoring_utils.middleware.MonitoringCustomMetrics', 'mobile_api.middleware.AppVersionUpgrade', 'openedx.core.djangoapps.header_control.middleware.HeaderControlMiddleware', diff --git a/openedx/core/djangoapps/monitoring/README.rst b/openedx/core/djangoapps/monitoring/README.rst new file mode 100644 index 0000000000..4b7617d88b --- /dev/null +++ b/openedx/core/djangoapps/monitoring/README.rst @@ -0,0 +1,6 @@ +This djangoapp is incorrectly named 'monitoring'. + +The name is related to old functionality that used to be a part of this app. + +TODO: The current contents of this app should be joined with other generic +platform utilities and renamed appropriately. diff --git a/openedx/core/djangoapps/monitoring_utils/__init__.py b/openedx/core/djangoapps/monitoring_utils/__init__.py new file mode 100644 index 0000000000..bd581daf52 --- /dev/null +++ b/openedx/core/djangoapps/monitoring_utils/__init__.py @@ -0,0 +1,97 @@ +""" +This is an interface to the monitoring_utils middleware. Functions +defined in this module can be used to report monitoring custom metrics. + +Usage: + + from openedx.core.djangoapps import monitoring_utils + ... + monitoring_utils.accumulate('xb_user_state.get_many.num_items', 4) + +There is no need to do anything else. The metrics are automatically cleared +before the next request. + +We try to keep track of our custom metrics at: + +https://openedx.atlassian.net/wiki/display/PERF/Custom+Metrics+in+New+Relic + +At this time, these custom metrics will only be reported to New Relic. + +TODO: supply additional public functions for storing strings and booleans. + +""" + +from . import middleware +try: + import newrelic.agent +except ImportError: + newrelic = None # pylint: disable=invalid-name + + +def accumulate(name, value): + """ + Accumulate monitoring custom metric for the current request. + + The named metric is accumulated by a numerical amount using the sum. All + metrics are queued up in the request_cache for this request. At the end of + the request, the monitoring_utils middleware will batch report all + queued accumulated metrics to the monitoring tool (e.g. New Relic). + + Arguments: + name (str): The metric name. It should be period-delimited, and + increase in specificity from left to right. For example: + 'xb_user_state.get_many.num_items'. + value (number): The amount to accumulate into the named metric. When + accumulate() is called multiple times for a given metric name + during a request, the sum of the values for each call is reported + for that metric. For metrics which don't make sense to accumulate, + make sure to only call this function once during a request. + """ + middleware.MonitoringCustomMetrics.accumulate_metric(name, value) + + +def increment(name): + """ + Increment a monitoring custom metric representing a counter. + + Here we simply accumulate a new custom metric with a value of 1, and the + middleware should automatically aggregate this metric. + """ + accumulate(name, 1) + + +def set_custom_metrics_for_course_key(course_key): + """ + Set monitoring custom metrics related to a course key. + + This is not cached, and only support reporting to New Relic Insights. + + """ + if not newrelic: + return + newrelic.agent.add_custom_parameter('course_id', unicode(course_key)) + newrelic.agent.add_custom_parameter('org', unicode(course_key.org)) + + +def set_custom_metric(key, value): + """ + Set monitoring custom metric. + + This is not cached, and only support reporting to New Relic Insights. + + """ + if not newrelic: + return + newrelic.agent.add_custom_parameter(key, value) + + +def set_monitoring_transaction_name(name, group=None, priority=None): + """ + Sets the transaction name for monitoring. + + This is not cached, and only support reporting to New Relic. + + """ + if not newrelic: + return + newrelic.agent.set_transaction_name(name, group, priority) diff --git a/common/djangoapps/newrelic_custom_metrics/middleware.py b/openedx/core/djangoapps/monitoring_utils/middleware.py similarity index 89% rename from common/djangoapps/newrelic_custom_metrics/middleware.py rename to openedx/core/djangoapps/monitoring_utils/middleware.py index 3d6bcd89f9..67155bb21f 100644 --- a/common/djangoapps/newrelic_custom_metrics/middleware.py +++ b/openedx/core/djangoapps/monitoring_utils/middleware.py @@ -1,10 +1,13 @@ """ -Middleware for handling the storage, aggregation, and reporing of custom New -Relic metrics. +Middleware for handling the storage, aggregation, and reporting of custom +metrics for monitoring. + +At this time, the custom metrics can only reported to New Relic. This middleware will only call on the newrelic agent if there are any metrics to report for this request, so it will not incur any processing overhead for request handlers which do not record custom metrics. + """ import logging log = logging.getLogger(__name__) @@ -16,10 +19,10 @@ except ImportError: import request_cache -REQUEST_CACHE_KEY = 'newrelic_custom_metrics' +REQUEST_CACHE_KEY = 'monitoring_custom_metrics' -class NewRelicCustomMetrics(object): +class MonitoringCustomMetrics(object): """ The middleware class. Make sure to add below the request cache in MIDDLEWARE_CLASSES. diff --git a/common/djangoapps/newrelic_custom_metrics/tests.py b/openedx/core/djangoapps/monitoring_utils/tests.py similarity index 58% rename from common/djangoapps/newrelic_custom_metrics/tests.py rename to openedx/core/djangoapps/monitoring_utils/tests.py index b1fc931b1a..4410e234a3 100644 --- a/common/djangoapps/newrelic_custom_metrics/tests.py +++ b/openedx/core/djangoapps/monitoring_utils/tests.py @@ -1,27 +1,28 @@ """ -Tests for newrelic custom metrics. +Tests for monitoring custom metrics. """ from django.test import TestCase from mock import patch, call -import newrelic_custom_metrics +from openedx.core.djangoapps import monitoring_utils +from openedx.core.djangoapps.monitoring_utils.middleware import MonitoringCustomMetrics -class TestNewRelicCustomMetrics(TestCase): +class TestMonitoringCustomMetrics(TestCase): """ - Test the newrelic_custom_metrics middleware and helpers + Test the monitoring_utils middleware and helpers """ @patch('newrelic.agent') - def test_cache_normal_contents(self, mock_newrelic_agent): + def test_custom_metrics_with_new_relic(self, mock_newrelic_agent): """ - Test normal usage of collecting and reporting custom New Relic metrics + Test normal usage of collecting custom metrics and reporting to New Relic """ - newrelic_custom_metrics.accumulate('hello', 10) - newrelic_custom_metrics.accumulate('world', 10) - newrelic_custom_metrics.accumulate('world', 10) - newrelic_custom_metrics.increment('foo') - newrelic_custom_metrics.increment('foo') + monitoring_utils.accumulate('hello', 10) + monitoring_utils.accumulate('world', 10) + monitoring_utils.accumulate('world', 10) + monitoring_utils.increment('foo') + monitoring_utils.increment('foo') # based on the metric data above, we expect the following calls to newrelic: nr_agent_calls_expected = [ @@ -31,7 +32,7 @@ class TestNewRelicCustomMetrics(TestCase): ] # fake a response to trigger metrics reporting - newrelic_custom_metrics.middleware.NewRelicCustomMetrics().process_response( + MonitoringCustomMetrics().process_response( 'fake request', 'fake response', )