Skip to content

Commit

Permalink
Report num. annotations marked deleted to New Relic
Browse files Browse the repository at this point in the history
  • Loading branch information
seanh committed May 30, 2024
1 parent bde2da9 commit fb4704d
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 0 deletions.
30 changes: 30 additions & 0 deletions h/tasks/cleanup.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# pylint: disable=no-member # Instance of 'Celery' has no 'request' member
from datetime import datetime

import newrelic
from sqlalchemy import func, select

from h import models
from h.celery import celery, get_task_logger

Expand All @@ -12,6 +15,33 @@ def purge_deleted_annotations():
celery.request.find_service(name="annotation_delete").bulk_delete()


@celery.task
def report_num_deleted_annotations():
"""
Report the number of deleted annotations to New Relic.
Send a custom metric to New Relic that counts the number of annotations
that have been marked as deleted in the database but not yet "purged" (i.e.
actually deleted) from the database by the purge_deleted_annotations() task
above
This is so that we can set a New Relic alert based on this metric so that
we can know if the purge_deleted_annotations() task is unable to keep up
(since it only deletes up to a limited number of annotations per task run)
and we're failing to actually purge data from the database when users ask
us to. Otherwise annotations could be marked as deleted but never actually
purged and we'd never know.
"""
newrelic.agent.record_custom_metric(
"Custom/Annotations/MarkedAsDeleted",
celery.request.db.scalar(
select(
func.count(models.Annotation.id) # pylint:disable=not-callable
).where(models.Annotation.deleted.is_(True))
),
)


@celery.task
def purge_expired_auth_tickets():
celery.request.db.query(models.AuthTicket).filter(
Expand Down
21 changes: 21 additions & 0 deletions tests/unit/h/tasks/cleanup_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
purge_expired_authz_codes,
purge_expired_tokens,
purge_removed_features,
report_num_deleted_annotations,
)


Expand All @@ -21,6 +22,21 @@ def test_purge(self, annotation_delete_service):
annotation_delete_service.bulk_delete.assert_called_once()


@pytest.mark.usefixtures("celery")
class TestReportNumDeletedAnnotations:
def test_report_num_deleted_annotations(self, factories, newrelic):
# Annotations marked as deleted, these should be counted.
factories.Annotation.create_batch(2, deleted=True)
# An annotation not marked as deleted, this should not be counted.
factories.Annotation.create()

report_num_deleted_annotations()

newrelic.agent.record_custom_metric.assert_called_once_with(
"Custom/Annotations/MarkedAsDeleted", 2
)


@pytest.mark.usefixtures("celery")
class TestPurgeExpiredAuthTickets:
def test_it_removes_expired_tickets(self, db_session, factories):
Expand Down Expand Up @@ -141,3 +157,8 @@ def celery(patch, pyramid_request):
cel = patch("h.tasks.cleanup.celery", autospec=False)
cel.request = pyramid_request
return cel


@pytest.fixture(autouse=True)
def newrelic(mocker):
return mocker.patch("h.tasks.cleanup.newrelic")

0 comments on commit fb4704d

Please sign in to comment.