diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index a1f727ecd..5614c361f 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -8,6 +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 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 @@ -86,7 +87,7 @@ def delete_verify_codes(): current_app.logger.info( "Delete job started {} finished {} deleted {} verify codes".format(start, datetime.utcnow(), deleted) ) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to delete verify codes") raise @@ -105,7 +106,7 @@ def delete_sms_notifications_older_than_seven_days(): deleted ) ) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to delete sms notifications") raise @@ -124,7 +125,7 @@ def delete_email_notifications_older_than_seven_days(): deleted ) ) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to delete sms notifications") raise @@ -143,7 +144,7 @@ def delete_letter_notifications_older_than_seven_days(): deleted ) ) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to delete sms notifications") raise @@ -157,7 +158,7 @@ def delete_invitations(): current_app.logger.info( "Delete job started {} finished {} deleted {} invitations".format(start, datetime.utcnow(), deleted) ) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to delete invitations") raise @@ -175,7 +176,7 @@ def timeout_notifications(): @statsd(namespace="tasks") def send_daily_performance_platform_stats(): if performance_platform_client.active: - count_dict = performance_platform_client.get_total_sent_notifications_yesterday() + 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') @@ -185,18 +186,16 @@ def send_daily_performance_platform_stats(): .format(start_date, email_sent_count, sms_sent_count) ) - performance_platform_client.send_performance_stats( + total_sent_notifications.send_total_notifications_sent_for_day_stats( start_date, 'sms', - sms_sent_count, - 'day' + sms_sent_count ) - performance_platform_client.send_performance_stats( + total_sent_notifications.send_total_notifications_sent_for_day_stats( start_date, 'email', - email_sent_count, - 'day' + email_sent_count ) @@ -253,7 +252,7 @@ def delete_inbound_sms_older_than_seven_days(): deleted ) ) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to delete inbound sms notifications") raise @@ -288,7 +287,7 @@ def delete_dvla_response_files_older_than_seven_days(): len(older_than_seven_days) ) ) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to delete dvla response files") raise diff --git a/app/clients/performance_platform/performance_platform_client.py b/app/clients/performance_platform/performance_platform_client.py index 045991181..d04b22659 100644 --- a/app/clients/performance_platform/performance_platform_client.py +++ b/app/clients/performance_platform/performance_platform_client.py @@ -1,11 +1,10 @@ import base64 import json -from datetime import datetime -import requests from flask import current_app +import requests -from app.utils import get_midnight_for_day_before, get_london_midnight_in_utc, convert_utc_to_bst +from app.utils import convert_utc_to_bst class PerformancePlatformClient: @@ -14,78 +13,70 @@ class PerformancePlatformClient: def active(self): return self._active - @active.setter - def active(self, value): - self._active = value - def init_app(self, app): self._active = app.config.get('PERFORMANCE_PLATFORM_ENABLED') if self.active: - self.bearer_token = app.config.get('PERFORMANCE_PLATFORM_TOKEN') self.performance_platform_url = app.config.get('PERFORMANCE_PLATFORM_URL') + self.performance_platform_endpoints = app.config.get('PERFORMANCE_PLATFORM_ENDPOINTS') - def send_performance_stats(self, date, channel, count, period): + def send_stats_to_performance_platform(self, payload): if self.active: - payload = { - '_timestamp': convert_utc_to_bst(date).isoformat(), - 'service': 'govuk-notify', - 'channel': channel, - 'count': count, - 'dataType': 'notifications', - 'period': period + bearer_token = self.performance_platform_endpoints[payload['dataType']] + headers = { + 'Content-Type': "application/json", + 'Authorization': 'Bearer {}'.format(bearer_token) } - self._add_id_for_payload(payload) - self._send_stats_to_performance_platform(payload) - - def get_total_sent_notifications_yesterday(self): - today = datetime.utcnow() - start_date = get_midnight_for_day_before(today) - end_date = get_london_midnight_in_utc(today) - - from app.dao.notifications_dao import get_total_sent_notifications_in_date_range - email_count = get_total_sent_notifications_in_date_range(start_date, end_date, 'email') - sms_count = get_total_sent_notifications_in_date_range(start_date, end_date, 'sms') - - return { - "start_date": start_date, - "email": { - "count": email_count - }, - "sms": { - "count": sms_count - } - } - - def _send_stats_to_performance_platform(self, payload): - headers = { - 'Content-Type': "application/json", - 'Authorization': 'Bearer {}'.format(self.bearer_token) - } - resp = requests.post( - self.performance_platform_url, - json=payload, - headers=headers - ) - - if resp.status_code == 200: - current_app.logger.info( - "Updated performance platform successfully with payload {}".format(json.dumps(payload)) - ) - else: - current_app.logger.error( - "Performance platform update request failed for payload with response details: {} '{}'".format( - json.dumps(payload), - resp.status_code, - resp.json()) + resp = requests.post( + self.performance_platform_url + payload['dataType'], + json=payload, + headers=headers ) - def _add_id_for_payload(self, payload): + if resp.status_code == 200: + current_app.logger.info( + "Updated performance platform successfully with payload {}".format(json.dumps(payload)) + ) + else: + current_app.logger.error( + "Performance platform update request failed for payload with response details: {} '{}'".format( + json.dumps(payload), + resp.status_code + ) + ) + resp.raise_for_status() + + @staticmethod + def format_payload(*, dataset, date, group_name, group_value, count, period='day'): + """ + :param dataset - the name of the overall graph, as referred to in the endpoint. + :param date - the date we're sending stats for + :param group_name - the name of the individual groups of data, eg "channel" or "status" + :param group_value - the value of the group, eg "sms" or "email" for group_name=channel + :param count - the actual numeric value to send + :param period - the period that this data covers - "day", "week", "month", "quarter". + """ + payload = { + '_timestamp': convert_utc_to_bst(date).isoformat(), + 'service': 'govuk-notify', + 'dataType': dataset, + 'period': period, + 'count': count, + group_name: group_value, + } + payload['_id'] = PerformancePlatformClient.generate_payload_id(payload, group_name) + return payload + + @staticmethod + def generate_payload_id(payload, group_name): + """ + group_name is the name of the group - eg "channel" or "status" + """ payload_string = '{}{}{}{}{}'.format( payload['_timestamp'], payload['service'], - payload['channel'], + payload[group_name], payload['dataType'], payload['period'] ) _id = base64.b64encode(payload_string.encode('utf-8')) - payload.update({'_id': _id.decode('utf-8')}) + return _id.decode('utf-8') diff --git a/app/cloudfoundry_config.py b/app/cloudfoundry_config.py index c890a8333..33cd4f753 100644 --- a/app/cloudfoundry_config.py +++ b/app/cloudfoundry_config.py @@ -34,6 +34,8 @@ def set_config_env_vars(vcap_services): extract_firetext_config(s) elif s['name'] == 'redis': extract_redis_config(s) + elif s['name'] == 'performance-platform': + extract_performance_platform_config(s) def extract_notify_config(notify_config): @@ -42,10 +44,13 @@ def extract_notify_config(notify_config): os.environ['ADMIN_CLIENT_SECRET'] = notify_config['credentials']['admin_client_secret'] os.environ['SECRET_KEY'] = notify_config['credentials']['secret_key'] os.environ['DANGEROUS_SALT'] = notify_config['credentials']['dangerous_salt'] - os.environ['PERFORMANCE_PLATFORM_TOKEN'] = notify_config['credentials'].get('performance_platform_token', '') os.environ['SMS_INBOUND_WHITELIST'] = json.dumps(notify_config['credentials']['allow_ip_inbound_sms']) +def extract_performance_platform_config(performance_platform_config): + os.environ['PERFORMANCE_PLATFORM_ENDPOINTS'] = json.dumps(performance_platform_config['credentials']) + + def extract_notify_aws_config(aws_config): os.environ['NOTIFICATION_QUEUE_PREFIX'] = aws_config['credentials']['sqs_queue_prefix'] os.environ['AWS_ACCESS_KEY_ID'] = aws_config['credentials']['aws_access_key_id'] diff --git a/app/config.py b/app/config.py index 958a0a532..c6c5291eb 100644 --- a/app/config.py +++ b/app/config.py @@ -93,7 +93,7 @@ class Config(object): # Performance platform PERFORMANCE_PLATFORM_ENABLED = False - PERFORMANCE_PLATFORM_URL = 'https://www.performance.service.gov.uk/data/govuk-notify/notifications' + PERFORMANCE_PLATFORM_URL = 'https://www.performance.service.gov.uk/data/govuk-notify/' PERFORMANCE_PLATFORM_TOKEN = os.getenv('PERFORMANCE_PLATFORM_TOKEN') # Logging @@ -273,6 +273,10 @@ class Config(object): SMS_INBOUND_WHITELIST = json.loads(os.environ.get('SMS_INBOUND_WHITELIST', '[]')) + # Format is as follows: + # {"dataset_1": "token_1", ...} + PERFORMANCE_PLATFORM_ENDPOINTS = json.loads(os.environ.get('PERFORMANCE_PLATFORM_ENDPOINTS', '{}')) + ###################### # Config overrides ### diff --git a/app/performance_platform/__init__.py b/app/performance_platform/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/app/performance_platform/total_sent_notifications.py b/app/performance_platform/total_sent_notifications.py new file mode 100644 index 000000000..4ad57171e --- /dev/null +++ b/app/performance_platform/total_sent_notifications.py @@ -0,0 +1,39 @@ +from datetime import datetime + +from app import performance_platform_client +from app.dao.notifications_dao import get_total_sent_notifications_in_date_range +from app.utils import ( + get_london_midnight_in_utc, + get_midnight_for_day_before +) + + +def send_total_notifications_sent_for_day_stats(date, notification_type, count): + payload = performance_platform_client.format_payload( + dataset='notifications', + date=date, + group_name='channel', + group_value=notification_type, + count=count + ) + + performance_platform_client.send_stats_to_performance_platform(payload) + + +def get_total_sent_notifications_yesterday(): + today = datetime.utcnow() + start_date = get_midnight_for_day_before(today) + end_date = get_london_midnight_in_utc(today) + + email_count = get_total_sent_notifications_in_date_range(start_date, end_date, 'email') + sms_count = get_total_sent_notifications_in_date_range(start_date, end_date, 'sms') + + return { + "start_date": start_date, + "email": { + "count": email_count + }, + "sms": { + "count": sms_count + } + } diff --git a/manifest-api-base.yml b/manifest-api-base.yml index 0a79d96bd..19c198d21 100644 --- a/manifest-api-base.yml +++ b/manifest-api-base.yml @@ -10,6 +10,7 @@ services: - firetext - hosted-graphite - redis + - performance-platform env: NOTIFY_APP_NAME: public-api CW_APP_NAME: api diff --git a/manifest-delivery-base.yml b/manifest-delivery-base.yml index 78053a729..6ceeaf201 100644 --- a/manifest-delivery-base.yml +++ b/manifest-delivery-base.yml @@ -11,6 +11,7 @@ services: - firetext - hosted-graphite - redis + - performance-platform instances: 1 memory: 1G diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index f795584b4..a05ea18f9 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -270,13 +270,8 @@ def test_will_remove_csv_files_for_jobs_older_than_seven_days( ] -def test_send_daily_performance_stats_calls_does_not_send_if_inactive( - notify_db, - notify_db_session, - sample_template, - mocker -): - send_mock = mocker.patch('app.celery.scheduled_tasks.performance_platform_client.send_performance_stats') +def test_send_daily_performance_stats_calls_does_not_send_if_inactive(client, mocker): + send_mock = mocker.patch('app.celery.scheduled_tasks.total_sent_notifications.send_total_notifications_sent_for_day_stats') # noqa with patch.object( PerformancePlatformClient, @@ -296,7 +291,7 @@ def test_send_daily_performance_stats_calls_with_correct_totals( sample_template, mocker ): - perf_mock = mocker.patch('app.celery.scheduled_tasks.performance_platform_client.send_performance_stats') + perf_mock = mocker.patch('app.celery.scheduled_tasks.total_sent_notifications.send_total_notifications_sent_for_day_stats') # noqa notification_history = partial( create_notification_history, @@ -327,8 +322,8 @@ def test_send_daily_performance_stats_calls_with_correct_totals( send_daily_performance_platform_stats() perf_mock.assert_has_calls([ - call(get_london_midnight_in_utc(yesterday), 'sms', 2, 'day'), - call(get_london_midnight_in_utc(yesterday), 'email', 3, 'day') + call(get_london_midnight_in_utc(yesterday), 'sms', 2), + call(get_london_midnight_in_utc(yesterday), 'email', 3) ]) diff --git a/tests/app/clients/test_performance_platform.py b/tests/app/clients/test_performance_platform.py index 9913048f8..c55a004cd 100644 --- a/tests/app/clients/test_performance_platform.py +++ b/tests/app/clients/test_performance_platform.py @@ -1,119 +1,58 @@ +import requests import requests_mock import pytest -from datetime import datetime -from freezegun import freeze_time -from functools import partial from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient -from app.utils import ( - get_london_midnight_in_utc, - get_midnight_for_day_before -) -from tests.app.conftest import sample_notification_history as create_notification_history @pytest.fixture(scope='function') -def client(mocker): - client = PerformancePlatformClient() +def perf_client(client, mocker): + perf_client = PerformancePlatformClient() current_app = mocker.Mock(config={ 'PERFORMANCE_PLATFORM_ENABLED': True, - 'PERFORMANCE_PLATFORM_URL': 'https://performance-platform-url/', - 'PERFORMANCE_PLATFORM_TOKEN': 'token' + 'PERFORMANCE_PLATFORM_ENDPOINTS': { + 'foo': 'my_token', + 'bar': 'other_token' + }, + 'PERFORMANCE_PLATFORM_URL': 'https://performance-platform-url/' }) - client.init_app(current_app) - return client + perf_client.init_app(current_app) + return perf_client -def test_should_not_call_if_not_enabled(notify_api, client, mocker): - mocker.patch.object(client, '_send_stats_to_performance_platform') - client.active = False - client.send_performance_stats( - date=datetime(2016, 10, 16, 0, 0, 0), - channel='sms', - count=142, - period='day' - ) - - client._send_stats_to_performance_platform.assert_not_called() - - -def test_should_call_if_enabled(notify_api, client, mocker): - mocker.patch.object(client, '_send_stats_to_performance_platform') - client.send_performance_stats( - date=datetime(2016, 10, 16, 0, 0, 0), - channel='sms', - count=142, - period='day' - ) - - assert client._send_stats_to_performance_platform.call_count == 1 - - -def test_send_platform_stats_creates_correct_call(notify_api, client): +def test_should_not_call_if_not_enabled(perf_client): with requests_mock.Mocker() as request_mock: - request_mock.post( - client.performance_platform_url, - json={}, - status_code=200 - ) - client.send_performance_stats( - date=datetime(2016, 10, 15, 23, 0, 0), - channel='sms', - count=142, - period='day' - ) + request_mock.post('https://performance-platform-url/foo', json={}, status_code=200) + perf_client._active = False + perf_client.send_stats_to_performance_platform({'dataType': 'foo'}) + + assert request_mock.called is False + + +def test_should_call_datatype_endpoint_if_enabled(perf_client): + with requests_mock.Mocker() as request_mock: + request_mock.post('https://performance-platform-url/foo', json={}, status_code=200) + perf_client.send_stats_to_performance_platform({'dataType': 'foo'}) assert request_mock.call_count == 1 - - assert request_mock.request_history[0].url == client.performance_platform_url - assert request_mock.request_history[0].method == 'POST' - - request_args = request_mock.request_history[0].json() - assert request_args['dataType'] == 'notifications' - assert request_args['service'] == 'govuk-notify' - assert request_args['period'] == 'day' - assert request_args['channel'] == 'sms' - assert request_args['_timestamp'] == '2016-10-16T00:00:00' - assert request_args['count'] == 142 - expected_base64_id = 'MjAxNi0xMC0xNlQwMDowMDowMGdvdnVrLW5vdGlmeXNtc25vdGlmaWNhdGlvbnNkYXk=' - assert request_args['_id'] == expected_base64_id + assert request_mock.last_request.method == 'POST' -@freeze_time("2016-01-11 12:30:00") -def test_get_total_sent_notifications_yesterday_returns_expected_totals_dict( - notify_db, - notify_db_session, - client, - sample_template -): - notification_history = partial( - create_notification_history, - notify_db, - notify_db_session, - sample_template, - status='delivered' - ) +@pytest.mark.parametrize('dataset, token', [ + ('foo', 'my_token'), + ('bar', 'other_token') +]) +def test_should_use_correct_token(perf_client, dataset, token): + with requests_mock.Mocker() as request_mock: + request_mock.post('https://performance-platform-url/foo', json={}, status_code=200) + request_mock.post('https://performance-platform-url/bar', json={}, status_code=200) + perf_client.send_stats_to_performance_platform({'dataType': dataset}) - notification_history(notification_type='email') - notification_history(notification_type='sms') + assert request_mock.call_count == 1 + assert request_mock.last_request.headers.get('authorization') == 'Bearer {}'.format(token) - # Create some notifications for the day before - yesterday = datetime(2016, 1, 10, 15, 30, 0, 0) - with freeze_time(yesterday): - notification_history(notification_type='sms') - notification_history(notification_type='sms') - notification_history(notification_type='email') - notification_history(notification_type='email') - notification_history(notification_type='email') - total_count_dict = client.get_total_sent_notifications_yesterday() - - assert total_count_dict == { - "start_date": get_midnight_for_day_before(datetime.utcnow()), - "email": { - "count": 3 - }, - "sms": { - "count": 2 - } - } +def test_should_raise_for_status(perf_client): + with pytest.raises(requests.HTTPError), requests_mock.Mocker() as request_mock: + request_mock.post('https://performance-platform-url/foo', json={}, status_code=403) + perf_client.send_stats_to_performance_platform({'dataType': 'foo'}) diff --git a/tests/app/performance_platform/test_total_sent_notifications.py b/tests/app/performance_platform/test_total_sent_notifications.py new file mode 100644 index 000000000..e4fe597d0 --- /dev/null +++ b/tests/app/performance_platform/test_total_sent_notifications.py @@ -0,0 +1,75 @@ +from datetime import datetime +from functools import partial + +from freezegun import freeze_time + +from app.utils import get_midnight_for_day_before +from app.performance_platform.total_sent_notifications import ( + send_total_notifications_sent_for_day_stats, + get_total_sent_notifications_yesterday +) + +from tests.app.conftest import ( + sample_notification_history as create_notification_history +) + + +def test_send_total_notifications_sent_for_day_stats_stats_creates_correct_call(mocker, client): + send_stats = mocker.patch('app.performance_platform.total_sent_notifications.performance_platform_client.send_stats_to_performance_platform') # noqa + + send_total_notifications_sent_for_day_stats( + date=datetime(2016, 10, 15, 23, 0, 0), + notification_type='sms', + count=142 + ) + + assert send_stats.call_count == 1 + + request_args = send_stats.call_args[0][0] + assert request_args['dataType'] == 'notifications' + assert request_args['service'] == 'govuk-notify' + assert request_args['period'] == 'day' + assert request_args['channel'] == 'sms' + assert request_args['_timestamp'] == '2016-10-16T00:00:00' + assert request_args['count'] == 142 + expected_base64_id = 'MjAxNi0xMC0xNlQwMDowMDowMGdvdnVrLW5vdGlmeXNtc25vdGlmaWNhdGlvbnNkYXk=' + assert request_args['_id'] == expected_base64_id + + +@freeze_time("2016-01-11 12:30:00") +def test_get_total_sent_notifications_yesterday_returns_expected_totals_dict( + notify_db, + notify_db_session, + sample_template +): + notification_history = partial( + create_notification_history, + notify_db, + notify_db_session, + sample_template, + status='delivered' + ) + + notification_history(notification_type='email') + notification_history(notification_type='sms') + + # Create some notifications for the day before + yesterday = datetime(2016, 1, 10, 15, 30, 0, 0) + with freeze_time(yesterday): + notification_history(notification_type='sms') + notification_history(notification_type='sms') + notification_history(notification_type='email') + notification_history(notification_type='email') + notification_history(notification_type='email') + + total_count_dict = get_total_sent_notifications_yesterday() + + assert total_count_dict == { + "start_date": get_midnight_for_day_before(datetime.utcnow()), + "email": { + "count": 3 + }, + "sms": { + "count": 2 + } + } diff --git a/tests/app/test_cloudfoundry_config.py b/tests/app/test_cloudfoundry_config.py index a393fc8bb..72b2f2865 100644 --- a/tests/app/test_cloudfoundry_config.py +++ b/tests/app/test_cloudfoundry_config.py @@ -16,7 +16,6 @@ def notify_config(): 'admin_client_secret': 'admin client secret', 'secret_key': 'secret key', 'dangerous_salt': 'dangerous salt', - 'performance_platform_token': 'performance platform token', 'allow_ip_inbound_sms': ['111.111.111.111', '100.100.100.100'] } } @@ -88,6 +87,17 @@ def redis_config(): } +@pytest.fixture +def performance_platform_config(): + return { + 'name': 'performance-platform', + 'credentials': { + 'foo': 'my_token', + 'bar': 'other_token' + } + } + + @pytest.fixture def cloudfoundry_config( postgres_config, @@ -96,7 +106,8 @@ def cloudfoundry_config( hosted_graphite_config, mmg_config, firetext_config, - redis_config + redis_config, + performance_platform_config ): return { 'postgres': postgres_config, @@ -106,7 +117,8 @@ def cloudfoundry_config( hosted_graphite_config, mmg_config, firetext_config, - redis_config + redis_config, + performance_platform_config ] } @@ -148,16 +160,6 @@ def test_notify_config(): assert os.environ['ADMIN_CLIENT_SECRET'] == 'admin client secret' assert os.environ['SECRET_KEY'] == 'secret key' assert os.environ['DANGEROUS_SALT'] == 'dangerous salt' - assert os.environ['PERFORMANCE_PLATFORM_TOKEN'] == 'performance platform token' - - -@pytest.mark.usefixtures('os_environ', 'cloudfoundry_environ') -def test_notify_config_if_perf_platform_not_set(cloudfoundry_config): - del cloudfoundry_config['user-provided'][0]['credentials']['performance_platform_token'] - - set_config_env_vars(cloudfoundry_config) - - assert os.environ['PERFORMANCE_PLATFORM_TOKEN'] == '' @pytest.mark.usefixtures('os_environ', 'cloudfoundry_environ') @@ -205,3 +207,13 @@ def test_sms_inbound_config(): extract_cloudfoundry_config() assert os.environ['SMS_INBOUND_WHITELIST'] == json.dumps(['111.111.111.111', '100.100.100.100']) + + +@pytest.mark.usefixtures('os_environ', 'cloudfoundry_environ') +def test_performance_platform_config(): + extract_cloudfoundry_config() + + assert os.environ['PERFORMANCE_PLATFORM_ENDPOINTS'] == json.dumps({ + 'foo': 'my_token', + 'bar': 'other_token' + })