Warn team about high failure rates #2667
Conversation
CrystalPea
force-pushed
the
warn-team-about-high-failure-rates
branch
from
678cb48 to
180915d
Compare
There was a problem hiding this comment.
I haven't looked at the code but I might suggest adding a runbook entry to https://github.com/alphagov/notifications-manuals/wiki/Support-Runbook telling what a developer should do when they receive this ticket and link to the runbook play from the zendesk ticket
app/celery/scheduled_tasks.py
Outdated
| current_app.logger.exception(message) | ||
|
|
||
| if current_app.config['NOTIFY_ENVIRONMENT'] in ['live', 'production', 'test']: | ||
| message += "\nThings to do: contact service? revoke their key?" |
There was a problem hiding this comment.
Explain what TV numbers are:
These are numbers reserved for the use in TV and drama. The range of TV numbers are 07700 900000 to 900999. When an SMS notification is sent to such a number, we get a 'permanent-failure' status back and we are billed for this notification.
There was a problem hiding this comment.
Thanks. That first sentence would be useful to put in the TODO section or in the runbook so a developer receiving the ticket knows how to deal with it
There was a problem hiding this comment.
yeah, I wrote it to myself here only to remember about including it :). As it only transpired in a spoken conversation.
app/celery/scheduled_tasks.py
Outdated
| len(services_sending_to_tv_numbers) | ||
| ) | ||
| for service in services_sending_to_tv_numbers: | ||
| message += "service id: {}, count of sms to tv numbers: {},\n".format( |
There was a problem hiding this comment.
I would add the url for service id https://www.notifications.service.gov.uk/services/{service_id}
app/celery/scheduled_tasks.py
Outdated
| current_app.logger.exception(message) | ||
|
|
||
| if current_app.config['NOTIFY_ENVIRONMENT'] in ['live', 'production', 'test']: | ||
| message += "\nThings to do: contact service? revoke their key?" |
There was a problem hiding this comment.
To start with we should investigate by looking at the dashboard, then contact the service about the large failure rate or use of tv numbers.
CrystalPea
force-pushed
the
warn-team-about-high-failure-rates
branch
from
538be7d to
635c9cf
Compare
|
@servingUpAces I refactored this code as per our conversations now, what do you think? @idavidmcdonald I created a section for this ticket in the Manual and linked it from the ticket. |
|
@CrystalPea Excellent runbook entry :) |
| @@ -1099,3 +1071,79 @@ def create_email_sms_letter_template(): | |||
| template_two = create_template(service=service, template_name='2', template_type='sms') | |||
| template_three = create_template(service=service, template_name='3', template_type='letter') | |||
| return template_one, template_three, template_two | |||
|
|
|||
|
|
|||
| @freeze_time("2019-12-02 12:00:00.000000") | |||
There was a problem hiding this comment.
Do we need to freeze_time for this test?
There was a problem hiding this comment.
I think we do, to show that most notifications are in good date range, while notifications for service_6 are just a bit too old and do not make it to results.
|
|
||
| @freeze_time("2019-12-02 12:00:00.000000") | ||
| def test_dao_find_services_sending_to_tv_numbers(notify_db_session, fake_uuid): | ||
| service_1 = create_service(service_name="Service 1", service_id=fake_uuid) |
There was a problem hiding this comment.
Do we need to use fake_uuid?
There was a problem hiding this comment.
We need an uuid, as we assert on it below, to check the result we get is for the right service.
There was a problem hiding this comment.
You can just use the id from service_1.id
CrystalPea
force-pushed
the
warn-team-about-high-failure-rates
branch
from
f42f20e to
846abca
Compare
CrystalPea
force-pushed
the
warn-team-about-high-failure-rates
branch
from
846abca to
08b12a6
Compare
Log an exception and send a zendesk ticket when, in the last 24 hours:
Story ticket: https://www.pivotaltracker.com/n/projects/1443052