From 20c082c33b39e05148c6754aaba020947bf7ce6e Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Mon, 13 May 2024 13:10:46 -0400 Subject: [PATCH] feat: Remove direct New Relic references (use configured telemetry) (#34781) Now that edx-django-utils can report to other telemetry backends, we need to remove the direct newrelic references. --- .../djangoapps/contentserver/middleware.py | 38 ++++++++----------- xmodule/seq_block.py | 38 +++++++------------ 2 files changed, 29 insertions(+), 47 deletions(-) diff --git a/openedx/core/djangoapps/contentserver/middleware.py b/openedx/core/djangoapps/contentserver/middleware.py index 12c63365a475..f159a00a56ba 100644 --- a/openedx/core/djangoapps/contentserver/middleware.py +++ b/openedx/core/djangoapps/contentserver/middleware.py @@ -15,6 +15,7 @@ HttpResponsePermanentRedirect ) from django.utils.deprecation import MiddlewareMixin +from edx_django_utils.monitoring import set_custom_attribute from opaque_keys import InvalidKeyError from opaque_keys.edx.locator import AssetLocator @@ -30,10 +31,6 @@ from .models import CdnUserAgentsConfig, CourseAssetCacheTtlConfig log = logging.getLogger(__name__) -try: - import newrelic.agent -except ImportError: - newrelic = None # pylint: disable=invalid-name # TODO: Soon as we have a reasonable way to serialize/deserialize AssetKeys, we need @@ -99,18 +96,17 @@ def process_request(self, request): if safe_course_key.run is None: safe_course_key = safe_course_key.replace(run='only') - if newrelic: - newrelic.agent.add_custom_attribute('course_id', safe_course_key) - newrelic.agent.add_custom_attribute('org', loc.org) - newrelic.agent.add_custom_attribute('contentserver.path', loc.path) + set_custom_attribute('course_id', safe_course_key) + set_custom_attribute('org', loc.org) + set_custom_attribute('contentserver.path', loc.path) - # Figure out if this is a CDN using us as the origin. - is_from_cdn = StaticContentServer.is_cdn_request(request) - newrelic.agent.add_custom_attribute('contentserver.from_cdn', is_from_cdn) + # Figure out if this is a CDN using us as the origin. + is_from_cdn = StaticContentServer.is_cdn_request(request) + set_custom_attribute('contentserver.from_cdn', is_from_cdn) - # Check if this content is locked or not. - locked = self.is_content_locked(content) - newrelic.agent.add_custom_attribute('contentserver.locked', locked) + # Check if this content is locked or not. + locked = self.is_content_locked(content) + set_custom_attribute('contentserver.locked', locked) # Check that user has access to the content. if not self.is_user_authorized(request, content, loc): @@ -168,8 +164,7 @@ def process_request(self, request): response['Content-Length'] = str(last - first + 1) response.status_code = 206 # Partial Content - if newrelic: - newrelic.agent.add_custom_attribute('contentserver.ranged', True) + set_custom_attribute('contentserver.ranged', True) else: log.warning( "Cannot satisfy ranges in Range header: %s for content: %s", @@ -182,9 +177,8 @@ def process_request(self, request): response = HttpResponse(content.stream_data()) response['Content-Length'] = content.length - if newrelic: - newrelic.agent.add_custom_attribute('contentserver.content_len', content.length) - newrelic.agent.add_custom_attribute('contentserver.content_type', content.content_type) + set_custom_attribute('contentserver.content_len', content.length) + set_custom_attribute('contentserver.content_type', content.content_type) # "Accept-Ranges: bytes" tells the user that only "bytes" ranges are allowed response['Accept-Ranges'] = 'bytes' @@ -213,14 +207,12 @@ def set_caching_headers(self, content, response): # indicate there should be no caching whatsoever. cache_ttl = CourseAssetCacheTtlConfig.get_cache_ttl() if cache_ttl > 0 and not is_locked: - if newrelic: - newrelic.agent.add_custom_attribute('contentserver.cacheable', True) + set_custom_attribute('contentserver.cacheable', True) response['Expires'] = StaticContentServer.get_expiration_value(datetime.datetime.utcnow(), cache_ttl) response['Cache-Control'] = "public, max-age={ttl}, s-maxage={ttl}".format(ttl=cache_ttl) elif is_locked: - if newrelic: - newrelic.agent.add_custom_attribute('contentserver.cacheable', False) + set_custom_attribute('contentserver.cacheable', False) response['Cache-Control'] = "private, no-cache, no-store" diff --git a/xmodule/seq_block.py b/xmodule/seq_block.py index 944c73400ab5..6d1a8c59adeb 100644 --- a/xmodule/seq_block.py +++ b/xmodule/seq_block.py @@ -12,6 +12,7 @@ from functools import reduce from django.conf import settings +from edx_django_utils.monitoring import set_custom_attribute from lxml import etree from opaque_keys.edx.keys import UsageKey from pytz import UTC @@ -43,11 +44,6 @@ log = logging.getLogger(__name__) -try: - import newrelic.agent -except ImportError: - newrelic = None # pylint: disable=invalid-name - # HACK: This shouldn't be hard-coded to two types # OBSOLETE: This obsoletes 'type' class_priority = ['video', 'problem'] @@ -839,58 +835,52 @@ def _locations_in_subtree(self, node): def _capture_basic_metrics(self): """ - Capture basic information about this sequence in New Relic. + Capture basic information about this sequence in telemetry. """ - if not newrelic: - return - newrelic.agent.add_custom_attribute('seq.block_id', str(self.location)) - newrelic.agent.add_custom_attribute('seq.display_name', self.display_name or '') - newrelic.agent.add_custom_attribute('seq.position', self.position) - newrelic.agent.add_custom_attribute('seq.is_time_limited', self.is_time_limited) + set_custom_attribute('seq.block_id', str(self.location)) + set_custom_attribute('seq.display_name', self.display_name or '') + set_custom_attribute('seq.position', self.position) + set_custom_attribute('seq.is_time_limited', self.is_time_limited) def _capture_full_seq_item_metrics(self, children): """ Capture information about the number and types of XBlock content in - the sequence as a whole. We send this information to New Relic so that + the sequence as a whole. We record this information in telemetry so that we can do better performance analysis of courseware. """ - if not newrelic: - return # Basic count of the number of Units (a.k.a. VerticalBlocks) we have in # this learning sequence - newrelic.agent.add_custom_attribute('seq.num_units', len(children)) + set_custom_attribute('seq.num_units', len(children)) # Count of all modules (leaf nodes) in this sequence (e.g. videos, # problems, etc.) The units (verticals) themselves are not counted. all_item_keys = self._locations_in_subtree(self) - newrelic.agent.add_custom_attribute('seq.num_items', len(all_item_keys)) + set_custom_attribute('seq.num_items', len(all_item_keys)) # Count of all modules by block_type (e.g. "video": 2, "discussion": 4) block_counts = collections.Counter(usage_key.block_type for usage_key in all_item_keys) for block_type, count in block_counts.items(): - newrelic.agent.add_custom_attribute(f'seq.block_counts.{block_type}', count) + set_custom_attribute(f'seq.block_counts.{block_type}', count) def _capture_current_unit_metrics(self, children): """ Capture information about the current selected Unit within the Sequence. """ - if not newrelic: - return # Positions are stored with indexing starting at 1. If we get into a # weird state where the saved position is out of bounds (e.g. the # content was changed), avoid going into any details about this unit. if 1 <= self.position <= len(children): # Basic info about the Unit... current = children[self.position - 1] - newrelic.agent.add_custom_attribute('seq.current.block_id', str(current.location)) - newrelic.agent.add_custom_attribute('seq.current.display_name', current.display_name or '') + set_custom_attribute('seq.current.block_id', str(current.location)) + set_custom_attribute('seq.current.display_name', current.display_name or '') # Examining all blocks inside the Unit (or split_test, conditional, etc.) child_locs = self._locations_in_subtree(current) - newrelic.agent.add_custom_attribute('seq.current.num_items', len(child_locs)) + set_custom_attribute('seq.current.num_items', len(child_locs)) curr_block_counts = collections.Counter(usage_key.block_type for usage_key in child_locs) for block_type, count in curr_block_counts.items(): - newrelic.agent.add_custom_attribute(f'seq.current.block_counts.{block_type}', count) + set_custom_attribute(f'seq.current.block_counts.{block_type}', count) def _time_limited_student_view(self): """