Skip to content

Commit fb4704d

Browse files
committed
Report num. annotations marked deleted to New Relic
1 parent bde2da9 commit fb4704d

File tree

2 files changed

+51
-0
lines changed

2 files changed

+51
-0
lines changed

h/tasks/cleanup.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
# pylint: disable=no-member # Instance of 'Celery' has no 'request' member
22
from datetime import datetime
33

4+
import newrelic
5+
from sqlalchemy import func, select
6+
47
from h import models
58
from h.celery import celery, get_task_logger
69

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

1417

18+
@celery.task
19+
def report_num_deleted_annotations():
20+
"""
21+
Report the number of deleted annotations to New Relic.
22+
23+
Send a custom metric to New Relic that counts the number of annotations
24+
that have been marked as deleted in the database but not yet "purged" (i.e.
25+
actually deleted) from the database by the purge_deleted_annotations() task
26+
above
27+
28+
This is so that we can set a New Relic alert based on this metric so that
29+
we can know if the purge_deleted_annotations() task is unable to keep up
30+
(since it only deletes up to a limited number of annotations per task run)
31+
and we're failing to actually purge data from the database when users ask
32+
us to. Otherwise annotations could be marked as deleted but never actually
33+
purged and we'd never know.
34+
"""
35+
newrelic.agent.record_custom_metric(
36+
"Custom/Annotations/MarkedAsDeleted",
37+
celery.request.db.scalar(
38+
select(
39+
func.count(models.Annotation.id) # pylint:disable=not-callable
40+
).where(models.Annotation.deleted.is_(True))
41+
),
42+
)
43+
44+
1545
@celery.task
1646
def purge_expired_auth_tickets():
1747
celery.request.db.query(models.AuthTicket).filter(

tests/unit/h/tasks/cleanup_test.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
purge_expired_authz_codes,
1111
purge_expired_tokens,
1212
purge_removed_features,
13+
report_num_deleted_annotations,
1314
)
1415

1516

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

2324

25+
@pytest.mark.usefixtures("celery")
26+
class TestReportNumDeletedAnnotations:
27+
def test_report_num_deleted_annotations(self, factories, newrelic):
28+
# Annotations marked as deleted, these should be counted.
29+
factories.Annotation.create_batch(2, deleted=True)
30+
# An annotation not marked as deleted, this should not be counted.
31+
factories.Annotation.create()
32+
33+
report_num_deleted_annotations()
34+
35+
newrelic.agent.record_custom_metric.assert_called_once_with(
36+
"Custom/Annotations/MarkedAsDeleted", 2
37+
)
38+
39+
2440
@pytest.mark.usefixtures("celery")
2541
class TestPurgeExpiredAuthTickets:
2642
def test_it_removes_expired_tickets(self, db_session, factories):
@@ -141,3 +157,8 @@ def celery(patch, pyramid_request):
141157
cel = patch("h.tasks.cleanup.celery", autospec=False)
142158
cel.request = pyramid_request
143159
return cel
160+
161+
162+
@pytest.fixture(autouse=True)
163+
def newrelic(mocker):
164+
return mocker.patch("h.tasks.cleanup.newrelic")

0 commit comments

Comments
 (0)