Commit c6f2db35 by Robert Raposa Committed by GitHub

Merge pull request #14774 from edx/rraposa/LEARNER-36

LEARNER-36: Add new relic custom metrics to course home.
parents d9562077 ffe64add
...@@ -89,6 +89,7 @@ from openedx.core.djangoapps.external_auth.login_and_register import ( ...@@ -89,6 +89,7 @@ from openedx.core.djangoapps.external_auth.login_and_register import (
login as external_auth_login, login as external_auth_login,
register as external_auth_register register as external_auth_register
) )
from openedx.core.djangoapps import monitoring_utils
import track.views import track.views
...@@ -120,8 +121,6 @@ from openedx.core.djangoapps.embargo import api as embargo_api ...@@ -120,8 +121,6 @@ from openedx.core.djangoapps.embargo import api as embargo_api
import analytics import analytics
from eventtracking import tracker from eventtracking import tracker
import newrelic_custom_metrics
# Note that this lives in LMS, so this dependency should be refactored. # Note that this lives in LMS, so this dependency should be refactored.
from notification_prefs.views import enable_notifications from notification_prefs.views import enable_notifications
...@@ -654,7 +653,7 @@ def dashboard(request): ...@@ -654,7 +653,7 @@ def dashboard(request):
# Record how many courses there are so that we can get a better # Record how many courses there are so that we can get a better
# understanding of usage patterns on prod. # 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 # sort the enrollment pairs by the enrollment date
course_enrollments.sort(key=lambda x: x.created, reverse=True) course_enrollments.sort(key=lambda x: x.created, reverse=True)
......
...@@ -52,6 +52,9 @@ from openedx.core.djangoapps.bookmarks.services import BookmarksService ...@@ -52,6 +52,9 @@ from openedx.core.djangoapps.bookmarks.services import BookmarksService
from openedx.core.djangoapps.crawlers.models import CrawlersConfig from openedx.core.djangoapps.crawlers.models import CrawlersConfig
from openedx.core.djangoapps.credit.services import CreditService from openedx.core.djangoapps.credit.services import CreditService
from openedx.core.djangoapps.util.user_utils import SystemUser 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 ( from openedx.core.lib.xblock_utils import (
replace_course_urls, replace_course_urls,
replace_jump_to_id_urls, replace_jump_to_id_urls,
...@@ -81,11 +84,6 @@ from .field_overrides import OverrideFieldData ...@@ -81,11 +84,6 @@ from .field_overrides import OverrideFieldData
log = logging.getLogger(__name__) 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: if settings.XQUEUE_INTERFACE.get('basic_auth') is not None:
REQUESTS_AUTH = HTTPBasicAuth(*settings.XQUEUE_INTERFACE['basic_auth']) REQUESTS_AUTH = HTTPBasicAuth(*settings.XQUEUE_INTERFACE['basic_auth'])
else: else:
...@@ -970,10 +968,7 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course ...@@ -970,10 +968,7 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course
except InvalidKeyError: except InvalidKeyError:
raise Http404 raise Http404
if newrelic: set_custom_metrics_for_course_key(course_key)
# 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))
with modulestore().bulk_operations(course_key): with modulestore().bulk_operations(course_key):
instance, tracking_context = get_module_by_usage_id(request, course_id, usage_id, course=course) 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 ...@@ -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". # "handler" in those cases is always just "xmodule_handler".
nr_tx_name = "{}.{}".format(instance.__class__.__name__, handler) nr_tx_name = "{}.{}".format(instance.__class__.__name__, handler)
nr_tx_name += "/{}".format(suffix) if (suffix and handler == "xmodule_handler") else "" nr_tx_name += "/{}".format(suffix) if (suffix and handler == "xmodule_handler") else ""
if newrelic: set_monitoring_transaction_name(nr_tx_name, group="Python/XBlock/Handler")
newrelic.agent.set_transaction_name(nr_tx_name, group="Python/XBlock/Handler")
tracking_context_name = 'module_callback_handler' tracking_context_name = 'module_callback_handler'
req = django_to_webob_request(request) req = django_to_webob_request(request)
......
...@@ -12,7 +12,6 @@ try: ...@@ -12,7 +12,6 @@ try:
except ImportError: except ImportError:
import json import json
import newrelic_custom_metrics
import dogstats_wrapper as dog_stats_api import dogstats_wrapper as dog_stats_api
from django.contrib.auth.models import User from django.contrib.auth.models import User
from django.db import transaction from django.db import transaction
...@@ -20,6 +19,7 @@ from django.db.utils import IntegrityError ...@@ -20,6 +19,7 @@ from django.db.utils import IntegrityError
from xblock.fields import Scope from xblock.fields import Scope
from courseware.models import StudentModule, BaseStudentModuleHistory from courseware.models import StudentModule, BaseStudentModuleHistory
from edx_user_state_client.interface import XBlockUserStateClient, XBlockUserState from edx_user_state_client.interface import XBlockUserStateClient, XBlockUserState
from openedx.core.djangoapps import monitoring_utils
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -136,7 +136,7 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient): ...@@ -136,7 +136,7 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
""" """
Accumulate arbitrary NR stats (not specific to block types). Accumulate arbitrary NR stats (not specific to block types).
""" """
newrelic_custom_metrics.accumulate( monitoring_utils.accumulate(
self._nr_metric_name(function_name, stat_name), self._nr_metric_name(function_name, stat_name),
value value
) )
...@@ -151,11 +151,11 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient): ...@@ -151,11 +151,11 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
""" """
Accumulate NR stats related to block types. Accumulate NR stats related to block types.
""" """
newrelic_custom_metrics.accumulate( monitoring_utils.accumulate(
self._nr_metric_name(function_name, stat_name), self._nr_metric_name(function_name, stat_name),
value, value,
) )
newrelic_custom_metrics.accumulate( monitoring_utils.accumulate(
self._nr_metric_name(function_name, stat_name, block_type=block_type), self._nr_metric_name(function_name, stat_name, block_type=block_type),
value, value,
) )
......
...@@ -22,11 +22,6 @@ import logging ...@@ -22,11 +22,6 @@ import logging
log = logging.getLogger("edx.courseware.views.index") log = logging.getLogger("edx.courseware.views.index")
try:
import newrelic.agent
except ImportError:
newrelic = None # pylint: disable=invalid-name
import urllib import urllib
import waffle import waffle
...@@ -36,6 +31,7 @@ from opaque_keys.edx.keys import CourseKey ...@@ -36,6 +31,7 @@ from opaque_keys.edx.keys import CourseKey
from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY 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.user_api.preferences.api import get_user_preference
from openedx.core.djangoapps.crawlers.models import CrawlersConfig 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 request_cache.middleware import RequestCache
from shoppingcart.models import CourseRegistrationCode from shoppingcart.models import CourseRegistrationCode
from student.models import CourseEnrollment from student.models import CourseEnrollment
...@@ -107,7 +103,7 @@ class CoursewareIndex(View): ...@@ -107,7 +103,7 @@ class CoursewareIndex(View):
self.url = request.path self.url = request.path
try: try:
self._init_new_relic() set_custom_metrics_for_course_key(self.course_key)
self._clean_position() self._clean_position()
with modulestore().bulk_operations(self.course_key): with modulestore().bulk_operations(self.course_key):
self.course = get_course_with_access(request.user, 'load', self.course_key, depth=CONTENT_DEPTH) self.course = get_course_with_access(request.user, 'load', self.course_key, depth=CONTENT_DEPTH)
...@@ -180,15 +176,6 @@ class CoursewareIndex(View): ...@@ -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): def _clean_position(self):
""" """
Verify that the given position is an integer. If it is not positive, set it to 1. Verify that the given position is an integer. If it is not positive, set it to 1.
......
...@@ -87,8 +87,9 @@ from openedx.core.djangoapps.credit.api import ( ...@@ -87,8 +87,9 @@ from openedx.core.djangoapps.credit.api import (
) )
from openedx.core.djangoapps.programs.utils import ProgramMarketingDataExtender from openedx.core.djangoapps.programs.utils import ProgramMarketingDataExtender
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers 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.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.models import UserTestGroup, CourseEnrollment
from student.roles import GlobalStaff from student.roles import GlobalStaff
from util.cache import cache, cache_if_anonymous from util.cache import cache, cache_if_anonymous
...@@ -502,6 +503,7 @@ class CourseTabView(EdxFragmentView): ...@@ -502,6 +503,7 @@ class CourseTabView(EdxFragmentView):
course = get_course_with_access(request.user, 'load', course_key) course = get_course_with_access(request.user, 'load', course_key)
tab = CourseTabList.get_tab_by_type(course.tabs, tab_type) tab = CourseTabList.get_tab_by_type(course.tabs, tab_type)
page_context = self.create_page_context(request, course=course, tab=tab, **kwargs) 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) return super(CourseTabView, self).get(request, course=course, page_context=page_context, **kwargs)
def create_page_context(self, request, course=None, tab=None, **kwargs): def create_page_context(self, request, course=None, tab=None, **kwargs):
......
...@@ -10,10 +10,6 @@ from django.db.utils import DatabaseError ...@@ -10,10 +10,6 @@ from django.db.utils import DatabaseError
from logging import getLogger from logging import getLogger
log = getLogger(__name__) 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.logged_task import LoggedTask
from celery_utils.persist_on_failure import PersistOnFailureTask from celery_utils.persist_on_failure import PersistOnFailureTask
...@@ -22,6 +18,9 @@ from lms.djangoapps.course_blocks.api import get_course_blocks ...@@ -22,6 +18,9 @@ from lms.djangoapps.course_blocks.api import get_course_blocks
from lms.djangoapps.courseware import courses from lms.djangoapps.courseware import courses
from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.keys import CourseKey, UsageKey
from opaque_keys.edx.locator import CourseLocator 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 student.models import CourseEnrollment
from submissions import api as sub_api from submissions import api as sub_api
from track.event_transaction_utils import ( from track.event_transaction_utils import (
...@@ -117,9 +116,8 @@ def _recalculate_subsection_grade(self, **kwargs): ...@@ -117,9 +116,8 @@ def _recalculate_subsection_grade(self, **kwargs):
course_key = CourseLocator.from_string(kwargs['course_id']) course_key = CourseLocator.from_string(kwargs['course_id'])
scored_block_usage_key = UsageKey.from_string(kwargs['usage_id']).replace(course_key=course_key) scored_block_usage_key = UsageKey.from_string(kwargs['usage_id']).replace(course_key=course_key)
if newrelic: set_custom_metrics_for_course_key(course_key)
newrelic.agent.add_custom_parameter('course_id', unicode(course_key)) set_custom_metric('usage_id', unicode(scored_block_usage_key))
newrelic.agent.add_custom_parameter('usage_id', unicode(scored_block_usage_key))
# The request cache is not maintained on celery workers, # The request cache is not maintained on celery workers,
# where this code runs. So we take the values from the # where this code runs. So we take the values from the
......
...@@ -1109,7 +1109,7 @@ MIDDLEWARE_CLASSES = ( ...@@ -1109,7 +1109,7 @@ MIDDLEWARE_CLASSES = (
'crum.CurrentRequestUserMiddleware', 'crum.CurrentRequestUserMiddleware',
'request_cache.middleware.RequestCache', 'request_cache.middleware.RequestCache',
'newrelic_custom_metrics.middleware.NewRelicCustomMetrics', 'openedx.core.djangoapps.monitoring_utils.middleware.MonitoringCustomMetrics',
'mobile_api.middleware.AppVersionUpgrade', 'mobile_api.middleware.AppVersionUpgrade',
'openedx.core.djangoapps.header_control.middleware.HeaderControlMiddleware', 'openedx.core.djangoapps.header_control.middleware.HeaderControlMiddleware',
......
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.
""" """
This is an interface to the newrelic_custom_metrics middleware. Functions This is an interface to the monitoring_utils middleware. Functions
defined in this module can be used to report custom metrics to New Relic. For defined in this module can be used to report monitoring custom metrics.
example:
import newrelic_custom_metrics Usage:
from openedx.core.djangoapps import monitoring_utils
... ...
newrelic_custom_metrics.accumulate('xb_user_state.get_many.num_items', 4) monitoring_utils.accumulate('xb_user_state.get_many.num_items', 4)
There is no need to do anything else. The metrics are automatically cleared There is no need to do anything else. The metrics are automatically cleared
before the next request. before the next request.
...@@ -14,24 +15,31 @@ We try to keep track of our custom metrics at: ...@@ -14,24 +15,31 @@ We try to keep track of our custom metrics at:
https://openedx.atlassian.net/wiki/display/PERF/Custom+Metrics+in+New+Relic 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. TODO: supply additional public functions for storing strings and booleans.
""" """
from newrelic_custom_metrics import middleware from . import middleware
try:
import newrelic.agent
except ImportError:
newrelic = None # pylint: disable=invalid-name
def accumulate(name, value): def accumulate(name, value):
""" """
Accumulate custom New Relic metric for the current request. Accumulate monitoring custom metric for the current request.
The named metric is accumulated by a numerical amount using the sum. All 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 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 the request, the monitoring_utils middleware will batch report all
queued accumulated metrics to NR. queued accumulated metrics to the monitoring tool (e.g. New Relic).
Arguments: Arguments:
name (str): The metric name. It should be period-delimited, and name (str): The metric name. It should be period-delimited, and
increase in specificty from left to right. For example: increase in specificity from left to right. For example:
'xb_user_state.get_many.num_items'. 'xb_user_state.get_many.num_items'.
value (number): The amount to accumulate into the named metric. When value (number): The amount to accumulate into the named metric. When
accumulate() is called multiple times for a given metric name accumulate() is called multiple times for a given metric name
...@@ -39,14 +47,51 @@ def accumulate(name, value): ...@@ -39,14 +47,51 @@ def accumulate(name, value):
for that metric. For metrics which don't make sense to accumulate, for that metric. For metrics which don't make sense to accumulate,
make sure to only call this function once during a request. make sure to only call this function once during a request.
""" """
middleware.NewRelicCustomMetrics.accumulate_metric(name, value) middleware.MonitoringCustomMetrics.accumulate_metric(name, value)
def increment(name): def increment(name):
""" """
Increment a custom New Relic metric representing a counter. Increment a monitoring custom metric representing a counter.
Here we simply accumulate a new custom metric with a value of 1, and the Here we simply accumulate a new custom metric with a value of 1, and the
middleware should automatically aggregate this metric. middleware should automatically aggregate this metric.
""" """
accumulate(name, 1) 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)
""" """
Middleware for handling the storage, aggregation, and reporing of custom New Middleware for handling the storage, aggregation, and reporting of custom
Relic metrics. 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 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 to report for this request, so it will not incur any processing overhead for
request handlers which do not record custom metrics. request handlers which do not record custom metrics.
""" """
import logging import logging
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -16,10 +19,10 @@ except ImportError: ...@@ -16,10 +19,10 @@ except ImportError:
import request_cache 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 The middleware class. Make sure to add below the request cache in
MIDDLEWARE_CLASSES. MIDDLEWARE_CLASSES.
......
""" """
Tests for newrelic custom metrics. Tests for monitoring custom metrics.
""" """
from django.test import TestCase from django.test import TestCase
from mock import patch, call 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') @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) monitoring_utils.accumulate('hello', 10)
newrelic_custom_metrics.accumulate('world', 10) monitoring_utils.accumulate('world', 10)
newrelic_custom_metrics.accumulate('world', 10) monitoring_utils.accumulate('world', 10)
newrelic_custom_metrics.increment('foo') monitoring_utils.increment('foo')
newrelic_custom_metrics.increment('foo') monitoring_utils.increment('foo')
# based on the metric data above, we expect the following calls to newrelic: # based on the metric data above, we expect the following calls to newrelic:
nr_agent_calls_expected = [ nr_agent_calls_expected = [
...@@ -31,7 +32,7 @@ class TestNewRelicCustomMetrics(TestCase): ...@@ -31,7 +32,7 @@ class TestNewRelicCustomMetrics(TestCase):
] ]
# fake a response to trigger metrics reporting # fake a response to trigger metrics reporting
newrelic_custom_metrics.middleware.NewRelicCustomMetrics().process_response( MonitoringCustomMetrics().process_response(
'fake request', 'fake request',
'fake response', 'fake response',
) )
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment