Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Remove direct New Relic references (use configured telemetry) #34781

Merged
merged 1 commit into from
May 13, 2024
Merged

feat: Remove direct New Relic references (use configured telemetry) #34781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 15 additions & 23 deletions openedx/core/djangoapps/contentserver/middleware.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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",
Expand All @@ -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'
Expand Down Expand Up @@ -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"

Expand Down
38 changes: 14 additions & 24 deletions xmodule/seq_block.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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']
Expand Down Expand Up @@ -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):
"""
Expand Down
Loading