Rewiring how we do statsd

- decorater added to the DAO for notifications

app/dao/notifications_dao.py

@@ -31,8 +31,9 @@ from app.clients import (
     STATISTICS_REQUESTED
 )
 from app.dao.dao_utils import transactional
+from app.statsd_decorators import statsd_timer
 
-
+@statsd_timer(namespace="dao")
 def dao_get_notification_statistics_for_service(service_id, limit_days=None):
     query_filter = [NotificationStatistics.service_id == service_id]
     if limit_days is not None:
@@ -44,6 +45,7 @@ def dao_get_notification_statistics_for_service(service_id, limit_days=None):
     ).all()
 
 
+@statsd_timer(namespace="dao")
 def dao_get_notification_statistics_for_service_and_day(service_id, day):
     return NotificationStatistics.query.filter_by(
         service_id=service_id,
@@ -51,10 +53,12 @@ def dao_get_notification_statistics_for_service_and_day(service_id, day):
     ).order_by(desc(NotificationStatistics.day)).first()
 
 
+@statsd_timer(namespace="dao")
 def dao_get_notification_statistics_for_day(day):
     return NotificationStatistics.query.filter_by(day=day).all()
 
 
+@statsd_timer(namespace="dao")
 def dao_get_potential_notification_statistics_for_day(day):
     all_services = db.session.query(
         Service.id,
@@ -102,6 +106,7 @@ def create_notification_statistics_dict(service_id, day):
     }
 
 
+@statsd_timer(namespace="dao")
 def dao_get_7_day_agg_notification_statistics_for_service(service_id,
                                                           date_from,
                                                           week_count=52):
@@ -129,6 +134,7 @@ def dao_get_7_day_agg_notification_statistics_for_service(service_id,
     )
 
 
+@statsd_timer(namespace="dao")
 def dao_get_template_statistics_for_service(service_id, limit_days=None):
     query_filter = [TemplateStatistics.service_id == service_id]
     if limit_days is not None:
@@ -137,6 +143,7 @@ def dao_get_template_statistics_for_service(service_id, limit_days=None):
         desc(TemplateStatistics.updated_at)).all()
 
 
+@statsd_timer(namespace="dao")
 def dao_get_template_statistics_for_template(template_id):
     return TemplateStatistics.query.filter(
         TemplateStatistics.template_id == template_id
@@ -145,6 +152,7 @@ def dao_get_template_statistics_for_template(template_id):
     ).all()
 
 
+@statsd_timer(namespace="dao")
 @transactional
 def dao_create_notification(notification, notification_type):
     if notification.job_id:
@@ -252,6 +260,7 @@ def _update_notification_status(notification, status, notification_statistics_st
     return True
 
 
+@statsd_timer(namespace="dao")
 @transactional
 def update_notification_status_by_id(notification_id, status, notification_statistics_status=None):
     notification = Notification.query.with_lockmode("update").filter(
@@ -270,6 +279,7 @@ def update_notification_status_by_id(notification_id, status, notification_stati
     )
 
 
+@statsd_timer(namespace="dao")
 @transactional
 def update_notification_status_by_reference(reference, status, notification_statistics_status):
     notification = Notification.query.filter(Notification.reference == reference,
@@ -286,6 +296,7 @@ def update_notification_status_by_reference(reference, status, notification_stat
     )
 
 
+@statsd_timer(namespace="dao")
 def dao_update_notification(notification):
     notification.updated_at = datetime.utcnow()
     notification_history = NotificationHistory.query.get(notification.id)
@@ -294,6 +305,7 @@ def dao_update_notification(notification):
     db.session.commit()
 
 
+@statsd_timer(namespace="dao")
 @transactional
 def update_provider_stats(
         id_,
@@ -328,10 +340,12 @@ def update_provider_stats(
         db.session.add(provider_stats)
 
 
+@statsd_timer(namespace="dao")
 def get_notification_for_job(service_id, job_id, notification_id):
     return Notification.query.filter_by(service_id=service_id, job_id=job_id, id=notification_id).one()
 
 
+@statsd_timer(namespace="dao")
 def get_notifications_for_job(service_id, job_id, filter_dict=None, page=1, page_size=None):
     if page_size is None:
         page_size = current_app.config['PAGE_SIZE']
@@ -343,6 +357,7 @@ def get_notifications_for_job(service_id, job_id, filter_dict=None, page=1, page
     )
 
 
+@statsd_timer(namespace="dao")
 def get_notification(service_id, notification_id, key_type=None):
     filter_dict = {'service_id': service_id, 'id': notification_id}
     if key_type:
@@ -351,6 +366,7 @@ def get_notification(service_id, notification_id, key_type=None):
     return Notification.query.filter_by(**filter_dict).one()
 
 
+@statsd_timer(namespace="dao")
 def get_notification_by_id(notification_id):
     return Notification.query.filter_by(id=notification_id).first()
 
@@ -359,6 +375,7 @@ def get_notifications(filter_dict=None):
     return _filter_query(Notification.query, filter_dict=filter_dict)
 
 
+@statsd_timer(namespace="dao")
 def get_notifications_for_service(service_id,
                                   filter_dict=None,
                                   page=1,
@@ -398,6 +415,7 @@ def _filter_query(query, filter_dict=None):
     return query
 
 
+@statsd_timer(namespace="dao")
 def delete_notifications_created_more_than_a_week_ago(status):
     seven_days_ago = date.today() - timedelta(days=7)
     deleted = db.session.query(Notification).filter(

app/statsd_decorators.py

@@ -0,0 +1,30 @@
+import functools
+
+from app import statsd_client
+from flask import current_app
+from monotonic import monotonic
+
+
+def statsd_timer(namespace):
+    def time_function(func):
+        @functools.wraps(func)
+        def wrapper(*args, **kwargs):
+            start_time = monotonic()
+            res = func(*args, **kwargs)
+            elapsed_time = monotonic() - start_time
+            current_app.logger.info(
+                "{namespace} call {func} took {time}".format(
+                    namespace=namespace, func=func.__name__, time="{0:.4f}".format(elapsed_time)
+                )
+            )
+            statsd_client.incr('notifications.{namespace}.{func}'.format(
+                namespace=namespace, func=func.__name__)
+            )
+            statsd_client.timing('notifications.{namespace}.{func}'.format(
+                namespace=namespace, func=func.__name__), elapsed_time
+            )
+            return res
+        wrapper.__wrapped__.__name__ = func.__name__
+        return wrapper
+
+    return time_function

tests/app/conftest.py

@@ -367,6 +367,16 @@ def sample_notification(notify_db,
     return notification
 
 
+@pytest.fixture(scope='function')
+def mock_statsd_inc(mocker):
+    return mocker.patch('app.statsd_client.incr')
+
+
+@pytest.fixture(scope='function')
+def mock_statsd_timing(mocker):
+    return mocker.patch('app.statsd_client.timing')
+
+
 @pytest.fixture(scope='function')
 def mock_celery_send_sms_code(mocker):
     return mocker.patch('app.celery.tasks.send_sms_code.apply_async')

tests/app/dao/test_notification_dao.py

@@ -5,6 +5,7 @@ from functools import partial
 import pytest
 
 from freezegun import freeze_time
+from mock import ANY
 from sqlalchemy.exc import SQLAlchemyError, IntegrityError
 
 from app import db
@@ -32,14 +33,36 @@ from app.dao.notifications_dao import (
     update_provider_stats,
     update_notification_status_by_reference,
     dao_get_template_statistics_for_service,
-    get_notifications_for_service
+    get_notifications_for_service, dao_get_7_day_agg_notification_statistics_for_service,
-)
+    dao_get_potential_notification_statistics_for_day, dao_get_notification_statistics_for_day,
+    dao_get_template_statistics_for_template, get_notification_by_id)
 
 from notifications_utils.template import get_sms_fragment_count
 
 from tests.app.conftest import (sample_notification)
 
 
+def test_should_have_decorated_notifications_dao_functions():
+    assert dao_get_notification_statistics_for_service.__wrapped__.__name__ == 'dao_get_notification_statistics_for_service'  # noqa
+    assert dao_get_notification_statistics_for_service_and_day.__wrapped__.__name__ == 'dao_get_notification_statistics_for_service_and_day'  # noqa
+    assert dao_get_notification_statistics_for_day.__wrapped__.__name__ == 'dao_get_notification_statistics_for_day'  # noqa
+    assert dao_get_potential_notification_statistics_for_day.__wrapped__.__name__ == 'dao_get_potential_notification_statistics_for_day'  # noqa
+    assert dao_get_7_day_agg_notification_statistics_for_service.__wrapped__.__name__ == 'dao_get_7_day_agg_notification_statistics_for_service'  # noqa
+    assert dao_get_template_statistics_for_service.__wrapped__.__name__ == 'dao_get_template_statistics_for_service'  # noqa
+    assert dao_get_template_statistics_for_template.__wrapped__.__name__ == 'dao_get_template_statistics_for_template'  # noqa
+    assert dao_create_notification.__wrapped__.__name__ == 'dao_create_notification'  # noqa
+    assert update_notification_status_by_id.__wrapped__.__name__ == 'update_notification_status_by_id'  # noqa
+    assert dao_update_notification.__wrapped__.__name__ == 'dao_update_notification'  # noqa
+    assert update_provider_stats.__wrapped__.__name__ == 'update_provider_stats'  # noqa
+    assert update_notification_status_by_reference.__wrapped__.__name__ == 'update_notification_status_by_reference'  # noqa
+    assert get_notification_for_job.__wrapped__.__name__ == 'get_notification_for_job'  # noqa
+    assert get_notifications_for_job.__wrapped__.__name__ == 'get_notifications_for_job'  # noqa
+    assert get_notification.__wrapped__.__name__ == 'get_notification'  # noqa
+    assert get_notifications_for_service.__wrapped__.__name__ == 'get_notifications_for_service'  # noqa
+    assert get_notification_by_id.__wrapped__.__name__ == 'get_notification_by_id'  # noqa
+    assert delete_notifications_created_more_than_a_week_ago.__wrapped__.__name__ == 'delete_notifications_created_more_than_a_week_ago'  # noqa
+
+
 def test_should_by_able_to_update_status_by_reference(sample_email_template, ses_provider):
     data = _notification_json(sample_email_template, status='sending')
 
@@ -163,7 +186,7 @@ def test_should_not_update_status_one_notification_status_is_delivered(notify_db
     _assert_job_stats(notification.job_id, sent=1, count=1, delivered=1, failed=0)
 
 
-def test_should_be_able_to_record_statistics_failure_for_sms(notify_db, notify_db_session,):
+def test_should_be_able_to_record_statistics_failure_for_sms(notify_db, notify_db_session, ):
     notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, status='sending')
     assert Notification.query.get(notification.id).status == 'sending'
 
@@ -313,7 +336,6 @@ def test_should_be_able_to_get_all_statistics_for_a_service_for_several_days_pre
 def test_create_notification_creates_notification_with_personalisation(notify_db, notify_db_session,
                                                                        sample_template_with_placeholders,
                                                                        sample_job, mmg_provider):
-
     assert Notification.query.count() == 0
     assert NotificationStatistics.query.count() == 0
     assert TemplateStatistics.query.count() == 0
@@ -664,7 +686,6 @@ def test_get_all_notifications_for_job(notify_db, notify_db_session, sample_job)
 
 
 def test_get_all_notifications_for_job_by_status(notify_db, notify_db_session, sample_job):
-
     notifications = partial(get_notifications_for_job, sample_job.service.id, sample_job.id)
 
     for status in NOTIFICATION_STATUS_TYPES: