diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 14a3821b3..9f79a36de 100644 --- a/app/dao/notifications_dao.py +++ b/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( diff --git a/app/statsd_decorators.py b/app/statsd_decorators.py new file mode 100644 index 000000000..b379d9430 --- /dev/null +++ b/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 diff --git a/notifications-api.zip b/notifications-api.zip new file mode 100644 index 000000000..83298c812 Binary files /dev/null and b/notifications-api.zip differ diff --git a/tests/app/conftest.py b/tests/app/conftest.py index e6295860d..3ef74cfff 100644 --- a/tests/app/conftest.py +++ b/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') diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index a963dd648..664c37f2e 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/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: