diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 5614c361f..bf10ceec7 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -8,7 +8,7 @@ from sqlalchemy.exc import SQLAlchemyError from app.aws import s3 from app import notify_celery -from app.performance_platform import total_sent_notifications +from app.performance_platform import total_sent_notifications, processing_time from app import performance_platform_client from app.dao.date_util import get_month_start_and_end_date_in_utc from app.dao.inbound_sms_dao import delete_inbound_sms_created_more_than_a_week_ago @@ -176,27 +176,32 @@ def timeout_notifications(): @statsd(namespace="tasks") def send_daily_performance_platform_stats(): if performance_platform_client.active: - count_dict = total_sent_notifications.get_total_sent_notifications_yesterday() - email_sent_count = count_dict.get('email').get('count') - sms_sent_count = count_dict.get('sms').get('count') - start_date = count_dict.get('start_date') + send_total_sent_notifications_to_performance_platform() + processing_time.send_processing_time_to_performance_platform() - current_app.logger.info( - "Attempting to update performance platform for date {} with email count {} and sms count {}" - .format(start_date, email_sent_count, sms_sent_count) - ) - total_sent_notifications.send_total_notifications_sent_for_day_stats( - start_date, - 'sms', - sms_sent_count - ) +def send_total_sent_notifications_to_performance_platform(): + count_dict = total_sent_notifications.get_total_sent_notifications_yesterday() + email_sent_count = count_dict.get('email').get('count') + sms_sent_count = count_dict.get('sms').get('count') + start_date = count_dict.get('start_date') - total_sent_notifications.send_total_notifications_sent_for_day_stats( - start_date, - 'email', - email_sent_count - ) + current_app.logger.info( + "Attempting to update performance platform for date {} with email count {} and sms count {}" + .format(start_date, email_sent_count, sms_sent_count) + ) + + total_sent_notifications.send_total_notifications_sent_for_day_stats( + start_date, + 'sms', + sms_sent_count + ) + + total_sent_notifications.send_total_notifications_sent_for_day_stats( + start_date, + 'email', + email_sent_count + ) @notify_celery.task(name='switch-current-sms-provider-on-slow-delivery') diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 4514d4310..17e388422 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -16,6 +16,8 @@ from notifications_utils.recipients import ( from werkzeug.datastructures import MultiDict from sqlalchemy import (desc, func, or_, and_, asc) from sqlalchemy.orm import joinedload +from sqlalchemy.sql.expression import case +from sqlalchemy.sql import functions from notifications_utils.international_billing_rates import INTERNATIONAL_BILLING_RATES from app import db, create_uuid @@ -42,7 +44,6 @@ from app.models import ( from app.dao.dao_utils import transactional from app.statsd_decorators import statsd -from app.utils import get_london_month_from_utc_column def dao_get_notification_statistics_for_service_and_day(service_id, day): @@ -519,3 +520,38 @@ def set_scheduled_notification_to_processed(notification_id): {'pending': False} ) db.session.commit() + + +def dao_get_total_notifications_sent_per_day_for_performance_platform(start_date, end_date): + """ + SELECT + count(notification_history), + coalesce(sum(CASE WHEN sent_at - created_at <= interval '10 seconds' THEN 1 ELSE 0 END), 0) + FROM notification_history + WHERE + created_at > 'START DATE' AND + created_at < 'END DATE' AND + api_key_id IS NOT NULL AND + key_type != 'test' AND + notification_type != 'letter'; + """ + under_10_secs = NotificationHistory.sent_at - NotificationHistory.created_at <= timedelta(seconds=10) + sum_column = functions.coalesce(functions.sum( + case( + [ + (under_10_secs, 1) + ], + else_=0 + ) + ), 0) + + return db.session.query( + func.count(NotificationHistory.id).label('messages_total'), + sum_column.label('messages_within_10_secs') + ).filter( + NotificationHistory.created_at >= start_date, + NotificationHistory.created_at < end_date, + NotificationHistory.api_key_id.isnot(None), + NotificationHistory.key_type != KEY_TYPE_TEST, + NotificationHistory.notification_type != LETTER_TYPE + ).one() diff --git a/app/performance_platform/processing_time.py b/app/performance_platform/processing_time.py new file mode 100644 index 000000000..320c0d454 --- /dev/null +++ b/app/performance_platform/processing_time.py @@ -0,0 +1,36 @@ +from datetime import datetime + +from flask import current_app + +from app.utils import get_midnight_for_day_before, get_london_midnight_in_utc +from app.dao.notifications_dao import dao_get_total_notifications_sent_per_day_for_performance_platform +from app import performance_platform_client + + +def send_processing_time_to_performance_platform(): + today = datetime.utcnow() + start_date = get_midnight_for_day_before(today) + end_date = get_london_midnight_in_utc(today) + + result = dao_get_total_notifications_sent_per_day_for_performance_platform(start_date, end_date) + + current_app.logger.info( + 'Sending processing-time to performance platform for date {}. Total: {}, under 10 secs {}'.format( + start_date, result.messages_total, result.messages_within_10_secs + ) + ) + + send_processing_time_data(start_date, 'messages-total', result.messages_total) + send_processing_time_data(start_date, 'messages-within-10-secs', result.messages_within_10_secs) + + +def send_processing_time_data(date, status, count): + payload = performance_platform_client.format_payload( + dataset='processing-time', + date=date, + group_name='status', + group_value=status, + count=count + ) + + performance_platform_client.send_stats_to_performance_platform(payload) diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index a05ea18f9..c77785e53 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -29,7 +29,8 @@ from app.celery.scheduled_tasks import ( switch_current_sms_provider_on_slow_delivery, timeout_job_statistics, timeout_notifications, - populate_monthly_billing) + populate_monthly_billing, + send_total_sent_notifications_to_performance_platform) from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient from app.config import QueueNames, TaskNames from app.dao.jobs_dao import dao_get_job_by_id @@ -285,7 +286,7 @@ def test_send_daily_performance_stats_calls_does_not_send_if_inactive(client, mo @freeze_time("2016-01-11 12:30:00") -def test_send_daily_performance_stats_calls_with_correct_totals( +def test_send_total_sent_notifications_to_performance_platform_calls_with_correct_totals( notify_db, notify_db_session, sample_template, @@ -319,7 +320,7 @@ def test_send_daily_performance_stats_calls_with_correct_totals( new_callable=PropertyMock ) as mock_active: mock_active.return_value = True - send_daily_performance_platform_stats() + send_total_sent_notifications_to_performance_platform() perf_mock.assert_has_calls([ call(get_london_midnight_in_utc(yesterday), 'sms', 2), diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 86437e7a1..91954dc1b 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -136,24 +136,30 @@ def sample_service( restricted=False, limit=1000, email_from=None, - permissions=[SMS_TYPE, EMAIL_TYPE] + permissions=[SMS_TYPE, EMAIL_TYPE], + research_mode=None ): if user is None: user = create_user() if email_from is None: email_from = service_name.lower().replace(' ', '.') + data = { 'name': service_name, 'message_limit': limit, 'restricted': restricted, 'email_from': email_from, 'created_by': user, - 'letter_contact_block': 'London,\nSW1A 1AA', + 'letter_contact_block': 'London,\nSW1A 1AA' } service = Service.query.filter_by(name=service_name).first() if not service: service = Service(**data) dao_create_service(service, user, service_permissions=permissions) + + if research_mode: + service.research_mode = research_mode + else: if user not in service.users: dao_add_user_to_service(service, user) @@ -206,6 +212,7 @@ def sample_template( }) template = Template(**data) dao_create_template(template) + return template @@ -631,14 +638,22 @@ def sample_notification_history( status='created', created_at=None, notification_type=None, - key_type=KEY_TYPE_NORMAL + key_type=KEY_TYPE_NORMAL, + sent_at=None, + api_key=None ): if created_at is None: created_at = datetime.utcnow() + if sent_at is None: + sent_at = datetime.utcnow() + if notification_type is None: notification_type = sample_template.template_type + if not api_key: + api_key = create_api_key(sample_template.service, key_type=key_type) + notification_history = NotificationHistory( id=uuid.uuid4(), service=sample_template.service, @@ -647,7 +662,10 @@ def sample_notification_history( status=status, created_at=created_at, notification_type=notification_type, - key_type=key_type + key_type=key_type, + api_key=api_key, + api_key_id=api_key and api_key.id, + sent_at=sent_at ) notify_db.session.add(notification_history) notify_db.session.commit() diff --git a/tests/app/dao/notification_dao/__init__.py b/tests/app/dao/notification_dao/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py similarity index 100% rename from tests/app/dao/test_notification_dao.py rename to tests/app/dao/notification_dao/test_notification_dao.py diff --git a/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py b/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py new file mode 100644 index 000000000..125223e2a --- /dev/null +++ b/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py @@ -0,0 +1,135 @@ +from datetime import date, datetime, timedelta + +from freezegun import freeze_time + +from app.dao.notifications_dao import dao_get_total_notifications_sent_per_day_for_performance_platform +from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST + +from tests.app.db import create_notification +from tests.app.conftest import ( + sample_notification_history, + sample_service, + sample_template +) + +BEGINNING_OF_DAY = date(2016, 10, 18) +END_OF_DAY = date(2016, 10, 19) + + +def test_get_total_notifications_filters_on_date_within_date_range(sample_template): + create_notification(sample_template, created_at=datetime(2016, 10, 17, 23, 59, 59)) + create_notification(sample_template, created_at=BEGINNING_OF_DAY) + create_notification(sample_template, created_at=datetime(2016, 10, 18, 23, 59, 59)) + create_notification(sample_template, created_at=END_OF_DAY) + + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) + + assert result.messages_total == 2 + + +@freeze_time('2016-10-18T10:00') +def test_get_total_notifications_only_counts_api_notifications(sample_template, sample_job, sample_api_key): + create_notification(sample_template, one_off=True) + create_notification(sample_template, one_off=True) + create_notification(sample_template, job=sample_job) + create_notification(sample_template, job=sample_job) + create_notification(sample_template, api_key=sample_api_key) + + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) + + assert result.messages_total == 1 + + +@freeze_time('2016-10-18T10:00') +def test_get_total_notifications_ignores_test_keys(sample_template): + # Creating multiple templates with normal and team keys but only 1 template + # with a test key to test that the count ignores letters + create_notification(sample_template, key_type=KEY_TYPE_NORMAL) + create_notification(sample_template, key_type=KEY_TYPE_NORMAL) + create_notification(sample_template, key_type=KEY_TYPE_TEAM) + create_notification(sample_template, key_type=KEY_TYPE_TEAM) + create_notification(sample_template, key_type=KEY_TYPE_TEST) + + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) + + assert result.messages_total == 4 + + +@freeze_time('2016-10-18T10:00') +def test_get_total_notifications_ignores_letters( + sample_template, + sample_email_template, + sample_letter_template +): + # Creating multiple sms and email templates but only 1 letter template to + # test that the count ignores letters + create_notification(sample_template) + create_notification(sample_template) + create_notification(sample_email_template) + create_notification(sample_email_template) + create_notification(sample_letter_template) + + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) + + assert result.messages_total == 4 + + +@freeze_time('2016-10-18T10:00') +def test_get_total_notifications_counts_messages_within_10_seconds(sample_template): + created_at = datetime.utcnow() + + create_notification(sample_template, sent_at=created_at + timedelta(seconds=5)) + create_notification(sample_template, sent_at=created_at + timedelta(seconds=10)) + create_notification(sample_template, sent_at=created_at + timedelta(seconds=15)) + + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) + + assert result.messages_total == 3 + assert result.messages_within_10_secs == 2 + + +@freeze_time('2016-10-18T10:00') +def test_get_total_notifications_counts_messages_that_have_not_sent(sample_template): + create_notification(sample_template, status='created', sent_at=None) + + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) + + assert result.messages_total == 1 + assert result.messages_within_10_secs == 0 + + +@freeze_time('2016-10-18T10:00') +def test_get_total_notifications_returns_zero_if_no_data(notify_db_session): + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) + + assert result.messages_total == 0 + assert result.messages_within_10_secs == 0 + + +@freeze_time('2016-10-18T10:00') +def test_get_total_notifications_counts_ignores_research_mode(notify_db, notify_db_session): + created_at = datetime.utcnow() + service = sample_service(notify_db, notify_db_session, research_mode=True) + template = sample_template(notify_db, notify_db_session, service=service) + + create_notification(template, status='created', sent_at=None) + + sample_notification_history( + notify_db, + notify_db_session, + template, + notification_type='email', + sent_at=created_at + timedelta(seconds=5) + ) + sample_notification_history( + notify_db, + notify_db_session, + template, + notification_type='sms', + sent_at=created_at + timedelta(seconds=5) + ) + + result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) + + assert result.messages_total == 2 + assert result.messages_within_10_secs == 2 diff --git a/tests/app/db.py b/tests/app/db.py index 8283428c9..e61270b81 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -123,7 +123,8 @@ def create_notification( international=False, phone_prefix=None, scheduled_for=None, - normalised_to=None + normalised_to=None, + one_off=False, ): if created_at is None: created_at = datetime.utcnow() @@ -132,7 +133,7 @@ def create_notification( sent_at = sent_at or datetime.utcnow() updated_at = updated_at or datetime.utcnow() - if job is None and api_key is None: + if not one_off and (job is None and api_key is None): # we didn't specify in test - lets create it api_key = ApiKey.query.filter(ApiKey.service == template.service, ApiKey.key_type == key_type).first() if not api_key: diff --git a/tests/app/performance_platform/test_processing_time.py b/tests/app/performance_platform/test_processing_time.py new file mode 100644 index 000000000..07f78859e --- /dev/null +++ b/tests/app/performance_platform/test_processing_time.py @@ -0,0 +1,47 @@ +from datetime import datetime, timedelta + +from freezegun import freeze_time + +from tests.app.db import create_notification +from app.performance_platform.processing_time import ( + send_processing_time_to_performance_platform, + send_processing_time_data +) + + +@freeze_time('2016-10-18T02:00') +def test_send_processing_time_to_performance_platform_generates_correct_calls(mocker, sample_template): + send_mock = mocker.patch('app.performance_platform.processing_time.send_processing_time_data') + + created_at = datetime.utcnow() - timedelta(days=1) + + create_notification(sample_template, created_at=created_at, sent_at=created_at + timedelta(seconds=5)) + create_notification(sample_template, created_at=created_at, sent_at=created_at + timedelta(seconds=15)) + create_notification(sample_template, created_at=datetime.utcnow() - timedelta(days=2)) + + send_processing_time_to_performance_platform() + + send_mock.assert_any_call(datetime(2016, 10, 16, 23, 0), 'messages-total', 2) + send_mock.assert_any_call(datetime(2016, 10, 16, 23, 0), 'messages-within-10-secs', 1) + + +def test_send_processing_time_to_performance_platform_creates_correct_call_to_perf_platform(mocker): + send_stats = mocker.patch('app.performance_platform.total_sent_notifications.performance_platform_client.send_stats_to_performance_platform') # noqa + + send_processing_time_data( + date=datetime(2016, 10, 15, 23, 0, 0), + status='foo', + count=142 + ) + + assert send_stats.call_count == 1 + + request_args = send_stats.call_args[0][0] + assert request_args['dataType'] == 'processing-time' + assert request_args['service'] == 'govuk-notify' + assert request_args['period'] == 'day' + assert request_args['status'] == 'foo' + assert request_args['_timestamp'] == '2016-10-16T00:00:00' + assert request_args['count'] == 142 + expected_base64_id = 'MjAxNi0xMC0xNlQwMDowMDowMGdvdnVrLW5vdGlmeWZvb3Byb2Nlc3NpbmctdGltZWRheQ==' + assert request_args['_id'] == expected_base64_id