mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-04 18:31:13 -05:00
Add prometheus client metric for number of inbound text messages
As we gradually move from statsd to prometheus, we change the metric to be a prometheus metric rather than statsd. The change worth pointing out is that we have dropped the 'successful' and 'failed' statuses from the metrics. I don't think it's useful to have these statuses. It's very rare for an inbound message to fail when we receive it and when it does, we raise an error and see it in our logs. We aren't going to be looking at a graph of it as it's a rare event, not typical behaviour that we want to monitor with a graph.
This commit is contained in:
@@ -3,9 +3,9 @@ from urllib.parse import unquote
|
|||||||
|
|
||||||
import iso8601
|
import iso8601
|
||||||
from flask import jsonify, Blueprint, current_app, request, abort
|
from flask import jsonify, Blueprint, current_app, request, abort
|
||||||
|
from gds_metrics.metrics import Counter
|
||||||
from notifications_utils.recipients import try_validate_and_format_phone_number
|
from notifications_utils.recipients import try_validate_and_format_phone_number
|
||||||
|
|
||||||
from app import statsd_client
|
|
||||||
from app.celery import tasks
|
from app.celery import tasks
|
||||||
from app.config import QueueNames
|
from app.config import QueueNames
|
||||||
from app.dao.services_dao import dao_fetch_service_by_inbound_number
|
from app.dao.services_dao import dao_fetch_service_by_inbound_number
|
||||||
@@ -17,6 +17,13 @@ receive_notifications_blueprint = Blueprint('receive_notifications', __name__)
|
|||||||
register_errors(receive_notifications_blueprint)
|
register_errors(receive_notifications_blueprint)
|
||||||
|
|
||||||
|
|
||||||
|
INBOUND_SMS_COUNTER = Counter(
|
||||||
|
'inbound_sms',
|
||||||
|
'Total number of inbound SMS received',
|
||||||
|
['provider']
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@receive_notifications_blueprint.route('/notifications/sms/receive/mmg', methods=['POST'])
|
@receive_notifications_blueprint.route('/notifications/sms/receive/mmg', methods=['POST'])
|
||||||
def receive_mmg_sms():
|
def receive_mmg_sms():
|
||||||
"""
|
"""
|
||||||
@@ -48,7 +55,7 @@ def receive_mmg_sms():
|
|||||||
# we should still tell MMG that we received it successfully
|
# we should still tell MMG that we received it successfully
|
||||||
return 'RECEIVED', 200
|
return 'RECEIVED', 200
|
||||||
|
|
||||||
statsd_client.incr('inbound.mmg.successful')
|
INBOUND_SMS_COUNTER.labels("mmg").inc()
|
||||||
|
|
||||||
inbound = create_inbound_sms_object(service,
|
inbound = create_inbound_sms_object(service,
|
||||||
content=format_mmg_message(post_data["Message"]),
|
content=format_mmg_message(post_data["Message"]),
|
||||||
@@ -93,7 +100,7 @@ def receive_firetext_sms():
|
|||||||
date_received=post_data['time'],
|
date_received=post_data['time'],
|
||||||
provider_name="firetext")
|
provider_name="firetext")
|
||||||
|
|
||||||
statsd_client.incr('inbound.firetext.successful')
|
INBOUND_SMS_COUNTER.labels("firetext").inc()
|
||||||
|
|
||||||
tasks.send_inbound_sms_to_service.apply_async([str(inbound.id), str(service.id)], queue=QueueNames.NOTIFY)
|
tasks.send_inbound_sms_to_service.apply_async([str(inbound.id), str(service.id)], queue=QueueNames.NOTIFY)
|
||||||
current_app.logger.debug(
|
current_app.logger.debug(
|
||||||
@@ -155,7 +162,6 @@ def fetch_potential_service(inbound_number, provider_name):
|
|||||||
current_app.logger.error('Inbound number "{}" from {} not associated with a service'.format(
|
current_app.logger.error('Inbound number "{}" from {} not associated with a service'.format(
|
||||||
inbound_number, provider_name
|
inbound_number, provider_name
|
||||||
))
|
))
|
||||||
statsd_client.incr('inbound.{}.failed'.format(provider_name))
|
|
||||||
return False
|
return False
|
||||||
|
|
||||||
if not has_inbound_sms_permissions(service.permissions):
|
if not has_inbound_sms_permissions(service.permissions):
|
||||||
|
|||||||
@@ -1,6 +1,5 @@
|
|||||||
import base64
|
import base64
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
from unittest.mock import call
|
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from flask import json
|
from flask import json
|
||||||
@@ -54,6 +53,7 @@ def mmg_post(client, data, auth=True, password='testkey'):
|
|||||||
|
|
||||||
def test_receive_notification_returns_received_to_mmg(client, mocker, sample_service_full_permissions):
|
def test_receive_notification_returns_received_to_mmg(client, mocker, sample_service_full_permissions):
|
||||||
mocked = mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async")
|
mocked = mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async")
|
||||||
|
prom_counter_labels_mock = mocker.patch('app.notifications.receive_notifications.INBOUND_SMS_COUNTER.labels')
|
||||||
data = {
|
data = {
|
||||||
"ID": "1234",
|
"ID": "1234",
|
||||||
"MSISDN": "447700900855",
|
"MSISDN": "447700900855",
|
||||||
@@ -69,6 +69,9 @@ def test_receive_notification_returns_received_to_mmg(client, mocker, sample_ser
|
|||||||
result = json.loads(response.get_data(as_text=True))
|
result = json.loads(response.get_data(as_text=True))
|
||||||
assert result['status'] == 'ok'
|
assert result['status'] == 'ok'
|
||||||
|
|
||||||
|
prom_counter_labels_mock.assert_called_once_with("mmg")
|
||||||
|
prom_counter_labels_mock.return_value.inc.assert_called_once_with()
|
||||||
|
|
||||||
inbound_sms_id = InboundSms.query.all()[0].id
|
inbound_sms_id = InboundSms.query.all()[0].id
|
||||||
mocked.assert_called_once_with(
|
mocked.assert_called_once_with(
|
||||||
[str(inbound_sms_id), str(sample_service_full_permissions.id)], queue="notify-internal-tasks")
|
[str(inbound_sms_id), str(sample_service_full_permissions.id)], queue="notify-internal-tasks")
|
||||||
@@ -299,7 +302,7 @@ def test_receive_notification_error_if_not_single_matching_service(client, notif
|
|||||||
|
|
||||||
def test_receive_notification_returns_received_to_firetext(notify_db_session, client, mocker):
|
def test_receive_notification_returns_received_to_firetext(notify_db_session, client, mocker):
|
||||||
mocked = mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async")
|
mocked = mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async")
|
||||||
mock = mocker.patch('app.notifications.receive_notifications.statsd_client.incr')
|
prom_counter_labels_mock = mocker.patch('app.notifications.receive_notifications.INBOUND_SMS_COUNTER.labels')
|
||||||
|
|
||||||
service = create_service_with_inbound_number(
|
service = create_service_with_inbound_number(
|
||||||
service_name='b', inbound_number='07111111111', service_permissions=[EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE])
|
service_name='b', inbound_number='07111111111', service_permissions=[EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE])
|
||||||
@@ -311,7 +314,8 @@ def test_receive_notification_returns_received_to_firetext(notify_db_session, cl
|
|||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
result = json.loads(response.get_data(as_text=True))
|
result = json.loads(response.get_data(as_text=True))
|
||||||
|
|
||||||
mock.assert_has_calls([call('inbound.firetext.successful')])
|
prom_counter_labels_mock.assert_called_once_with("firetext")
|
||||||
|
prom_counter_labels_mock.return_value.inc.assert_called_once_with()
|
||||||
|
|
||||||
assert result['status'] == 'ok'
|
assert result['status'] == 'ok'
|
||||||
inbound_sms_id = InboundSms.query.all()[0].id
|
inbound_sms_id = InboundSms.query.all()[0].id
|
||||||
@@ -320,7 +324,7 @@ def test_receive_notification_returns_received_to_firetext(notify_db_session, cl
|
|||||||
|
|
||||||
def test_receive_notification_from_firetext_persists_message(notify_db_session, client, mocker):
|
def test_receive_notification_from_firetext_persists_message(notify_db_session, client, mocker):
|
||||||
mocked = mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async")
|
mocked = mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async")
|
||||||
mocker.patch('app.notifications.receive_notifications.statsd_client.incr')
|
mocker.patch('app.notifications.receive_notifications.INBOUND_SMS_COUNTER')
|
||||||
|
|
||||||
service = create_service_with_inbound_number(
|
service = create_service_with_inbound_number(
|
||||||
inbound_number='07111111111',
|
inbound_number='07111111111',
|
||||||
@@ -347,7 +351,7 @@ def test_receive_notification_from_firetext_persists_message(notify_db_session,
|
|||||||
|
|
||||||
def test_receive_notification_from_firetext_persists_message_with_normalized_phone(notify_db_session, client, mocker):
|
def test_receive_notification_from_firetext_persists_message_with_normalized_phone(notify_db_session, client, mocker):
|
||||||
mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async")
|
mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async")
|
||||||
mocker.patch('app.notifications.receive_notifications.statsd_client.incr')
|
mocker.patch('app.notifications.receive_notifications.INBOUND_SMS_COUNTER')
|
||||||
|
|
||||||
create_service_with_inbound_number(
|
create_service_with_inbound_number(
|
||||||
inbound_number='07111111111', service_name='b', service_permissions=[EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE])
|
inbound_number='07111111111', service_name='b', service_permissions=[EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE])
|
||||||
@@ -367,7 +371,7 @@ def test_receive_notification_from_firetext_persists_message_with_normalized_pho
|
|||||||
|
|
||||||
def test_returns_ok_to_firetext_if_mismatched_sms_sender(notify_db_session, client, mocker):
|
def test_returns_ok_to_firetext_if_mismatched_sms_sender(notify_db_session, client, mocker):
|
||||||
mocked = mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async")
|
mocked = mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async")
|
||||||
mock = mocker.patch('app.notifications.receive_notifications.statsd_client.incr')
|
mocker.patch('app.notifications.receive_notifications.INBOUND_SMS_COUNTER')
|
||||||
|
|
||||||
create_service_with_inbound_number(
|
create_service_with_inbound_number(
|
||||||
inbound_number='07111111199', service_name='b', service_permissions=[EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE])
|
inbound_number='07111111199', service_name='b', service_permissions=[EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE])
|
||||||
@@ -381,7 +385,6 @@ def test_returns_ok_to_firetext_if_mismatched_sms_sender(notify_db_session, clie
|
|||||||
|
|
||||||
assert not InboundSms.query.all()
|
assert not InboundSms.query.all()
|
||||||
assert result['status'] == 'ok'
|
assert result['status'] == 'ok'
|
||||||
mock.assert_has_calls([call('inbound.firetext.failed')])
|
|
||||||
mocked.call_count == 0
|
mocked.call_count == 0
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user