sastels commented 5 months ago

Description

As a Notify team member, I need to be able to decide which SMS gets sent not using the short code.

WHY are we building?

Currently we can specify templates that are sent with the shortcode. However the other templates will also often get sent with the shortcode. We would like to have the other templates sent with the long codes.

WHAT are we building?

Create / turn on the Pinpoint default pool.

VALUE created by our solution

Fewer messages sent via short code. This will save money and allow us to reserve the short code for high priority messages.

Acceptance Criteria

Given some context, when (X) action occurs, then (Y) outcome is achieved.

[x] default pool created
[x] default pool code added but disabled
[x] long codes have keywords set appropriately (ie same as short code)
[x] long codes added to the default pool
[x] fix the toll free US number in staging (UPDATE: new registration "under review")
[x] default pool enabled and tested in staging
[x] default pool enabled and tested in production

QA Steps

Staging

[x] Send an sms using a template in the shortcode list. It should be sent from the Pinpoint shortcode pool.
[x] Send an sms using a template not in the list. It should be sent from the Pinpoint default pool.
[x] Send an sms using a service with a dedicated number. If should be sent through SNS / us-west-2 (and from the dedicated number)
[x] Send an sms to a US number. It should be sent through SNS / us-west-2 (and fail until we sort out our TFN)
[x] Send an sms to an international number. It should be sent by SNS (and received by Proovl!)
[x] Send an SMS to a fake Guam number (1 671 555 0123). Should be sent with SNS (but fail since it's a fake number)
[x] Send an email to verify that they still work.
[x] Unsubscribe via STOP or ARRET and verify that you're on the opt out list.
[x] Remove your self from the opt out list.

Production

[x] Send an sms using a template in the shortcode list. It should be sent from the Pinpoint shortcode pool (and the shortcode!).
[x] Send an sms using a template not in the list. It should be sent from the Pinpoint default pool.
[x] Send an sms using a service with a dedicated number. If should be sent through SNS / us-west-2 (and from the dedicated number)
- [x] Send an sms to a US number. It should be sent through SNS / us-west-2
[x] Send an sms to an international number. It should be sent by SNS (and received by Proovl!)
[x] Send an SMS to a fake Guam number (1 671 555 0123). Should be sent with SNS (but fail since it's a fake number)
[x] Send an email to verify that they still work.
[x] Unsubscribe via STOP or ARRET and verify that you're on the opt out list.
[x] Wait 10 seconds and then try to send to yourself again. The notification should immediately be marked as opted out.
[x] Remove your self from the opt out list.

sastels commented 5 months ago

Add the default pool to k8s but set AWS_PINPOINT_DEFAULT_POOL_ID to "": https://github.com/cds-snc/notification-manifests/pull/2668

sastels commented 5 months ago

Will release tomorrow. After that, will turn on in staging.

sastels commented 5 months ago

PR to set phone number keywords correctly https://github.com/cds-snc/notification-terraform/pull/1358

PR to turn on the default pool in staging: https://github.com/cds-snc/notification-manifests/pull/2682

sastels commented 5 months ago

Default pool enabled in staging, looks pretty good!

sastels commented 5 months ago

PR for script to set phone number keywords correctly is ready for :eyes: : https://github.com/cds-snc/notification-terraform/pull/1358

jimleroyer commented 5 months ago

This was merged and moving in QA. Jimmy will review the keywords messages in staging env.

sastels commented 5 months ago

Long codes in production have been moved to the default pool and had their keywords set appropriately.

Trying to set the ARRET keyword to OPT_OUT for the shortcode pool gives the error

Error
Conflict Occurred - Reason="RESOURCE_MODIFICATION_NOT_ALLOWED" ResourceType="keyword" ResourceId="ARRET" RequestId: 7b4280c0-c1d5-4950-a40d-da679818a1a9 HttpStatusCode: 400

sastels commented 5 months ago

Having a bug bash on staging Tuesday.

jimleroyer commented 5 months ago

Jimmy reviewed the keywords and these look good!

sastels commented 5 months ago

Bug bash went well, no blockers found.

Tested sending with the default pool in production with

aws pinpoint-sms-voice-v2 send-text-message --destination-phone-number +16136192797 --message-body "test from pinpoint" --origination-identity pool-1240066b32d34ce896fc233a9c1c02fe

Verified that the SMS was sent from one of the long codes in the default pool.

sastels commented 5 months ago

Targeting release to production Wednesday June 19, 10:30.

special [MANIFEST] PR to switch it on (revert this PR to turn back off)
run through the QA on prod steps ASAP afterwards

sastels commented 5 months ago

Had a problem sending to non-US international numbers :disappointed: PR to fix https://github.com/cds-snc/notification-api/pull/2195

testing in staging.....

sastels commented 5 months ago

Also found that in the repo and staging SNS is not allowed to send internationally. It has been manually enabled in the prod database. We should fix this with a migration. At the same time we can probably turn on international sending for Pinpoint.

sastels commented 5 months ago

Discovered what the NO_ORIGINATION_IDENTITIES_FOUND error is that we were seeing in production

we were trying to use Pinpoint to send to international numbers
in Pinpoint or SNS, if you send to a number in a different country (ie a country you don't own a number in), AWS will use one of their own numbers in that country to send. This happens automagically with SNS, but for Pinpoint there's an explicit setting (shared_routes) that is disabled by default.
I reproduced the error in staging, enabled the setting for the default pool, then tried again and this time it was sent.

sastels commented 5 months ago

Enable international sending for SNS and Pinpoint in our provider database via a migration: https://github.com/cds-snc/notification-api/pull/2197

Tweak the pool creation script to enable shared routes for new Pinpoint pools: https://github.com/cds-snc/notification-terraform/pull/1396

After these two are merged (and the existing Pinpoint pools in staging and production have shared_routes enabled) we can turn on international Pinpoint sending again: https://github.com/cds-snc/notification-api/pull/2198

sastels commented 5 months ago

ok, everything's been reviewed and approved! Will merge into staging and test today.

sastels commented 5 months ago

merged into staging and tested (Pinpoint configured there for default pool)

Next steps:

Release to prod (Tues).
Verify that SNS and Pinpoint have supports_international set to True.
turn on "shared routes" for both pools.
Configure the default pool (ie turn it on) on prod and QA

sastels commented 5 months ago

code released in prod
Pinpoint is marked in Admin / providers as an international provider
in AWS "shared routes" are enabled in both shortcode and default pools.

Ready to enable the default pools.

sastels commented 4 months ago

Will we need to have specific sender ids in pinpoint? :thinking: https://docs.aws.amazon.com/sms-voice/latest/userguide/sender-id-request.html

sastels commented 4 months ago

There's a set-default-sender-id for pinpoint that I'll test in staging today

sastels commented 4 months ago

that didn't work as expected, I put in a support ticket.

I might change back to using Pinpoint for everything except international (and designated numbers) until we get this sorted.

sastels commented 4 months ago

Use SNS for international: https://github.com/cds-snc/notification-api/pull/2204

Turn the default pool back on in staging https://github.com/cds-snc/notification-manifests/pull/2724

sastels commented 4 months ago

PRs merged, updated and went through staging QA steps again :tada: Should be ready to turn on in prod again once the "SNS for international" api PR is released.

sastels commented 4 months ago

note from AWS: to use the default sender id in Pinpoint we need to send without an origination-identity. So, essentially, to send internationally we should not supply a pool.

sastels commented 4 months ago

PR to turn on default pool in prod https://github.com/cds-snc/notification-manifests/pull/2733

sastels commented 4 months ago

Turned on in prod and prod QA tests pass. Continuing to monitor...

sastels commented 4 months ago

Pinpoint SendTextMessage throws a DESTINATION_PHONE_NUMBER_OPTED_OUT error when a number is opted out, we should handle this without an error

botocore.errorfactory.ConflictException: An error occurred (ConflictException) when calling the SendTextMessage operation: Conflict Occurred - Reason="DESTINATION_PHONE_NUMBER_OPTED_OUT" ResourceType="opt-out-list" ResourceId="Default"

This notification has status "... is opted out", but I think that's because after the Pinpoint celery error it retried with SNS. SNS tries to send it (and charges us) and returns that status. So with Pinpoint we should catch the celery error and update the status without having SNS send it.

sastels commented 4 months ago

WIP PR to fix the opted out error: https://github.com/cds-snc/notification-api/pull/2207

PR to fix a couple things on the Pinpoint dashboard: https://github.com/cds-snc/notification-terraform/pull/1415

sastels commented 4 months ago

Also I noticed that https://ca-central-1.console.aws.amazon.com/cloudwatch/home?region=ca-central-1#alarmsV2:alarm/pinpoint-sms-success-rate-warning?~(search~'pinpoint) did not have data - we should look through all the Pinpoint alarms and make sure they all have data as appropriate.
the sns-sms-success-rate-canadian-numbers-warning alarm has been going off - getting expired TTLs from SMS sent 2 days ago (before switch). They're pushing the success rate under 60% because there's so few new (successful) SMS being send through SNS now. Looks like TTLs come in about 2 days after the original send, so we should see these go away by Saturday noon. :crossed_fingers:
Unknown error attempting to reach phone Can also take 2 days to come in, we're seeing a few of them as well in SNS/SMS

sastels commented 4 months ago

We should add a card wrt SNS alarms, ie at least make critical alarm non-paging

sastels commented 4 months ago

releasing the opted out numbers fix to prod today

sastels commented 4 months ago

released and opted out number tested. Ready for QA!

jimleroyer commented 4 months ago

Jimmy to QA.

jimleroyer commented 4 months ago

QA notes.

Send an sms using a template in the shortcode list. It should be sent from the Pinpoint shortcode pool (and the shortcode!). ✅
Send an sms using a template not in the list. It should be sent from the Pinpoint default pool. ✅
Send an sms using a service with a dedicated number. If should be sent through SNS / us-west-2 (and from the dedicated number) ✅
Send an sms to a US number. It should be sent through SNS / us-west-2 ✅ Sent using a proovl American number. The SNS logs indicate a success although it cannot be displayed by Proovl (it is not a reliable tool/website).
Send an sms to an international number. It should be sent by SNS (and received by Proovl!) ✅ Sent using proovl UK number. The international numbers are now sent with pinpoint and not SNS. The logs were in pinpoint and indicated it was delivered using our custom sender ID of CANADACA.
Send an SMS to a fake Guam number (1 671 555 0123). Should be sent with SNS (but fail since it's a fake number) ✅
Send an email to verify that they still work. ✅
Unsubscribe via STOP or ARRET and verify that you're on the opt out list. ✅
Wait 10 seconds and then try to send to yourself again. The notification should immediately be marked as opted out. ✅
Remove your self from the opt out list. ✅

P0NDER0SA commented 4 months ago

Jimmy experienced some issues with Proovl (as per the steps above). Steve says Proovl might have been experiencing issues yesterday, so Jimmy might try again today.

jimleroyer commented 4 months ago

I updated the test cases with the results along with some notes. This issue can be considered QA'ed successfully.

cds-snc / notification-planning-core

Enable the Pinpoint default pool #370

Description

WHY are we building?

WHAT are we building?

VALUE created by our solution

Acceptance Criteria

QA Steps

Staging

Production