Merge pull request #583 from alphagov/stats-db-updates

Stats db updates

tests/app/celery/test_provider_tasks.py

@@ -20,6 +20,11 @@ from app.models import Notification, NotificationStatistics, Job, KEY_TYPE_NORMA
 from tests.app.conftest import sample_notification
 
 
+def test_should_have_decorated_tasks_functions():
+    assert send_sms_to_provider.__wrapped__.__name__ == 'send_sms_to_provider'
+    assert send_email_to_provider.__wrapped__.__name__ == 'send_email_to_provider'
+
+
 def test_should_by_10_second_delay_as_default():
     assert provider_tasks.retry_iteration_to_delay() == 10
 
@@ -91,9 +96,6 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist(
 
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.mmg_client.get_name', return_value="mmg")
-    mocker.patch('app.statsd_client.incr')
-    mocker.patch('app.statsd_client.timing_with_dates')
-    mocker.patch('app.statsd_client.timing')
 
     send_sms_to_provider(
         db_notification.service_id,
@@ -130,9 +132,6 @@ def test_should_send_personalised_template_to_correct_email_provider_and_persist
 
     mocker.patch('app.aws_ses_client.send_email', return_value='reference')
     mocker.patch('app.aws_ses_client.get_name', return_value="ses")
-    mocker.patch('app.statsd_client.incr')
-    mocker.patch('app.statsd_client.timing_with_dates')
-    mocker.patch('app.statsd_client.timing')
 
     send_email_to_provider(
         db_notification.service_id,
@@ -296,29 +295,6 @@ def test_should_not_send_to_provider_when_status_is_not_created(notify_db, notif
     app.celery.research_mode_tasks.send_sms_response.apply_async.assert_not_called()
 
 
-def test_send_sms_statsd_updates(notify_db, notify_db_session, sample_service, sample_notification, mocker):
-    mocker.patch('app.statsd_client.incr')
-    mocker.patch('app.statsd_client.timing')
-    mocker.patch('app.mmg_client.send_sms')
-    mocker.patch('app.mmg_client.get_name', return_value="mmg")
-
-    send_sms_to_provider(
-        sample_notification.service_id,
-        sample_notification.id
-    )
-
-    statsd_client.incr.assert_called_once_with("notifications.tasks.send-sms-to-provider")
-
-    statsd_client.timing.assert_has_calls([
-        call("notifications.tasks.send-sms-to-provider.task-time", ANY),
-        call("notifications.sms.total-time", ANY)
-    ])
-
-    # assert that the ANYs above are at least floats
-    for call_arg in statsd_client.timing.call_args_list:
-        assert isinstance(call_arg[0][1], float)
-
-
 def test_should_go_into_technical_error_if_exceeds_retries(
         notify_db,
         notify_db_session,
@@ -328,8 +304,6 @@ def test_should_go_into_technical_error_if_exceeds_retries(
     notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session,
                                        service=sample_service, status='created')
 
-    mocker.patch('app.statsd_client.incr')
-    mocker.patch('app.statsd_client.timing')
     mocker.patch('app.mmg_client.send_sms', side_effect=SmsClientException("EXPECTED"))
     mocker.patch('app.celery.provider_tasks.send_sms_to_provider.retry', side_effect=MaxRetriesExceededError())
 
@@ -339,8 +313,6 @@ def test_should_go_into_technical_error_if_exceeds_retries(
     )
 
     provider_tasks.send_sms_to_provider.retry.assert_called_with(queue='retry', countdown=10)
-    assert statsd_client.incr.assert_not_called
-    assert statsd_client.timing.assert_not_called
 
     db_notification = Notification.query.filter_by(id=notification.id).one()
     assert db_notification.status == 'technical-failure'
@@ -368,9 +340,6 @@ def test_should_send_sms_sender_from_service_if_present(
 
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.mmg_client.get_name', return_value="mmg")
-    mocker.patch('app.statsd_client.incr')
-    mocker.patch('app.statsd_client.timing_with_dates')
-    mocker.patch('app.statsd_client.timing')
 
     send_sms_to_provider(
         db_notification.service_id,
@@ -449,8 +418,6 @@ def test_send_email_to_provider_should_go_into_technical_error_if_exceeds_retrie
     notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session,
                                        service=sample_service, status='created', template=sample_email_template)
 
-    mocker.patch('app.statsd_client.incr')
-    mocker.patch('app.statsd_client.timing')
     mocker.patch('app.aws_ses_client.send_email', side_effect=EmailClientException("EXPECTED"))
     mocker.patch('app.celery.provider_tasks.send_email_to_provider.retry', side_effect=MaxRetriesExceededError())
 
@@ -460,8 +427,6 @@ def test_send_email_to_provider_should_go_into_technical_error_if_exceeds_retrie
     )
 
     provider_tasks.send_email_to_provider.retry.assert_called_with(queue='retry', countdown=10)
-    assert statsd_client.incr.assert_not_called
-    assert statsd_client.timing.assert_not_called
 
     db_notification = Notification.query.filter_by(id=notification.id).one()
     assert db_notification.status == 'technical-failure'
@@ -473,31 +438,6 @@ def test_send_email_to_provider_should_go_into_technical_error_if_exceeds_retrie
     assert job.notifications_failed == 1
 
 
-def test_send_email_to_provider_statsd_updates(notify_db, notify_db_session, sample_service,
-                                               sample_email_template, mocker):
-    mocker.patch('app.statsd_client.incr')
-    mocker.patch('app.statsd_client.timing')
-    mocker.patch('app.aws_ses_client.send_email', return_value='reference')
-    mocker.patch('app.aws_ses_client.get_name', return_value="ses")
-    notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session,
-                                       template=sample_email_template)
-    send_email_to_provider(
-        notification.service_id,
-        notification.id
-    )
-
-    statsd_client.incr.assert_called_once_with("notifications.tasks.send-email-to-provider")
-
-    statsd_client.timing.assert_has_calls([
-        call("notifications.tasks.send-email-to-provider.task-time", ANY),
-        call("notifications.email.total-time", ANY)
-    ])
-
-    # assert that the ANYs above are at least floats
-    for call_arg in statsd_client.timing.call_args_list:
-        assert isinstance(call_arg[0][1], float)
-
-
 def test_send_email_to_provider_should_not_send_to_provider_when_status_is_not_created(notify_db, notify_db_session,
                                                                                        sample_service,
                                                                                        sample_email_template,
@@ -524,8 +464,6 @@ def test_send_email_should_use_service_reply_to_email(
         sample_service,
         sample_email_template,
         mocker):
-    mocker.patch('app.statsd_client.incr')
-    mocker.patch('app.statsd_client.timing')
     mocker.patch('app.aws_ses_client.send_email', return_value='reference')
     mocker.patch('app.aws_ses_client.get_name', return_value="ses")
 

tests/app/celery/test_scheduled_tasks.py

@@ -11,6 +11,14 @@ from app.celery.scheduled_tasks import (delete_verify_codes,
 from tests.app.conftest import sample_notification
 
 
+def test_should_have_decorated_tasks_functions():
+    assert delete_verify_codes.__wrapped__.__name__ == 'delete_verify_codes'
+    assert delete_successful_notifications.__wrapped__.__name__ == 'delete_successful_notifications'
+    assert delete_failed_notifications.__wrapped__.__name__ == 'delete_failed_notifications'
+    assert timeout_notifications.__wrapped__.__name__ == 'timeout_notifications'
+    assert delete_invitations.__wrapped__.__name__ == 'delete_invitations'
+
+
 def test_should_call_delete_notifications_more_than_week_in_task(notify_api, mocker):
     mocked = mocker.patch('app.celery.scheduled_tasksgit .delete_notifications_created_more_than_a_week_ago')
     delete_successful_notifications()

tests/app/celery/test_tasks.py

@@ -7,10 +7,10 @@ from mock import ANY
 from sqlalchemy.exc import SQLAlchemyError
 from sqlalchemy.orm.exc import NoResultFound
 
-from app import (encryption, DATETIME_FORMAT, statsd_client)
+from app import (encryption, DATETIME_FORMAT)
 from app.celery import provider_tasks
 from app.celery import tasks
-from app.celery.tasks import s3
+from app.celery.tasks import s3, remove_job
 from app.celery.tasks import (
     send_sms,
     process_job,
@@ -53,10 +53,15 @@ def _notification_json(template, to, personalisation=None, job_id=None, row_numb
     return notification
 
 
+def test_should_have_decorated_tasks_functions():
+    assert process_job.__wrapped__.__name__ == 'process_job'
+    assert remove_job.__wrapped__.__name__ == 'remove_job'
+    assert send_sms.__wrapped__.__name__ == 'send_sms'
+    assert send_email.__wrapped__.__name__ == 'send_email'
+
+
 @freeze_time("2016-01-01 11:09:00.061258")
 def test_should_process_sms_job(sample_job, mocker, mock_celery_remove_job):
-    mocker.patch('app.statsd_client.incr')
-    mocker.patch('app.statsd_client.timing')
     mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('sms'))
     mocker.patch('app.celery.tasks.send_sms.apply_async')
     mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
@@ -81,8 +86,6 @@ def test_should_process_sms_job(sample_job, mocker, mock_celery_remove_job):
     )
     job = jobs_dao.dao_get_job_by_id(sample_job.id)
     assert job.status == 'finished'
-    statsd_client.incr.assert_called_once_with("notifications.tasks.process-job")
-    statsd_client.timing.assert_called_once_with("notifications.tasks.process-job.task-time", ANY)
 
 
 @freeze_time("2016-01-01 11:09:00.061258")
@@ -278,9 +281,6 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi
     notification = _notification_json(sample_template_with_placeholders,
                                       to="+447234123123", personalisation={"name": "Jo"})
 
-    mocker.patch('app.statsd_client.incr')
-    mocker.patch('app.statsd_client.timing_with_dates')
-    mocker.patch('app.statsd_client.timing')
     mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
 
     notification_id = uuid.uuid4()
@@ -292,15 +292,12 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi
         datetime.utcnow().strftime(DATETIME_FORMAT)
     )
 
-    statsd_client.timing.assert_called_once_with("notifications.tasks.send-sms.task-time", ANY)
-
     provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with(
         (sample_template_with_placeholders.service_id,
          notification_id),
         queue="sms"
     )
 
-    statsd_client.incr.assert_called_once_with("notifications.tasks.send-sms")
     persisted_notification = Notification.query.filter_by(id=notification_id).one()
     assert persisted_notification.id == notification_id
     assert persisted_notification.to == '+447234123123'
@@ -436,9 +433,6 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh
         "my_email@my_email.com",
         {"name": "Jo"},
         row_number=1)
-    mocker.patch('app.statsd_client.incr')
-    mocker.patch('app.statsd_client.timing_with_dates')
-    mocker.patch('app.statsd_client.timing')
     mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async')
 
     notification_id = uuid.uuid4()
@@ -456,8 +450,6 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh
             key_type=KEY_TYPE_TEAM
         )
 
-    statsd_client.incr.assert_called_once_with("notifications.tasks.send-email")
-    statsd_client.timing.assert_called_once_with("notifications.tasks.send-email.task-time", ANY)
     persisted_notification = Notification.query.filter_by(id=notification_id).one()
     provider_tasks.send_email_to_provider.apply_async.assert_called_once_with(
         (sample_email_template_with_placeholders.service_id, notification_id), queue='email')