Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions notesapi/v1/views/common.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
# pylint:disable=possibly-used-before-assignment
import json
import logging
import newrelic.agent

from django.conf import settings
from django.core.exceptions import ValidationError
from django.db.models import Q
from django.urls import reverse
from django.utils.translation import gettext as _
from edx_django_utils.monitoring import set_custom_attribute
from rest_framework import status
from rest_framework.generics import GenericAPIView, ListAPIView
from rest_framework.response import Response
Expand Down Expand Up @@ -398,8 +399,7 @@ def post(self, *args, **kwargs):
note = Note.create(self.request.data)
note.full_clean()

# Gather metrics for New Relic so we can slice data in New Relic Insights
newrelic.agent.add_custom_parameter("notes.count", total_notes)
set_custom_attribute("notes.count", total_notes)
except ValidationError as error:
log.debug(error, exc_info=True)
return Response(status=status.HTTP_400_BAD_REQUEST)
Expand Down
9 changes: 2 additions & 7 deletions notesserver/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,12 @@
from django.db import connection
from django.http import JsonResponse
from django.http import HttpResponse
from edx_django_utils.monitoring import ignore_transaction
from rest_framework import status
from rest_framework.decorators import api_view, permission_classes
from rest_framework.permissions import AllowAny
from rest_framework.response import Response

try:
import newrelic.agent
except ImportError: # pragma: no cover
newrelic = None # pylint: disable=invalid-name

from notesapi.v1.views import get_annotation_search_view_class
from notesapi.v1.views import SearchViewRuntimeError

Expand Down Expand Up @@ -45,8 +41,7 @@ def heartbeat(request):
"""
ElasticSearch and database are reachable and ready to handle requests.
"""
if newrelic: # pragma: no cover
newrelic.agent.ignore_transaction()
ignore_transaction() # no need to record telemetry for heartbeats
try:
db_status()
except Exception: # pylint: disable=broad-exception-caught
Expand Down
1 change: 0 additions & 1 deletion requirements/base.in
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ PyJWT
gunicorn # MIT
path.py
python-dateutil
newrelic
edx-django-release-util
edx-django-utils
edx-drf-extensions
Expand Down
4 changes: 1 addition & 3 deletions requirements/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,7 @@ meilisearch==0.34.0
mysqlclient==2.2.7
# via -r requirements/base.in
newrelic==10.7.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like newrelic is still included in the requirements, is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes -- if you look at the comments below that line, you can see that it's still being pulled in via edx-django-utils (for now).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the future plan for removing it from requirements?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell, this was the last instance in the edx and openedx orgs where we still had a direct import newrelic -- so after this merges, I think we're clear to remove newrelic from the edx-django-utils base.in as well. I don't have a specific plan for that, but I did want to clear the path for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: Filed a DEPR for this: openedx/public-engineering#360

# via
# -r requirements/base.in
# edx-django-utils
# via edx-django-utils
packaging==24.2
# via
# django-nine
Expand Down