cds-snc / notification-planning-core

Project planning for GC Notify Core Team
0 stars 0 forks source link

Get SMS cost per service via Quicksight #309

Closed jimleroyer closed 2 months ago

jimleroyer commented 3 months ago

Description

As a product owner, I want to know how much a service SMS cost for the past week, So that I know what is their financial budget.

WHY are we building?

Provide insights into SMS cost per service. This is a quick solution to a problem that would take longer time to support, but provide quick vital and necessary data for the product to make decisions on billing and limiting services based on a SMS cost perspective.

WHAT are we building?

Wire the SMS usage report into Quicksight as a dataset and join on the notification.reference column.

VALUE created by our solution

Quick and handy solution providing data insights into GCNotify SMS cost broken down by services.

Acceptance Criteria

Privacy considerations

Do not dump the phone numbers from the report into the quicksight sms usage data set.

Measuring success and metrics

Colleagues from growth and product cheering for this change once they got it.

QA Steps

jimleroyer commented 3 months ago

We got wide permissions that worked in staging. I will need to restrict these permissions in the following days. Also we included the SMS usage report CSV as a dataset into Quicksight. Next step is to join with the notifications / services to have a break down of cost per services.

jimleroyer commented 3 months ago

I got an error out of the data set creation. Today I will spend to fix this.

jimleroyer commented 3 months ago

Reintroduced the changes with fixed data type for the PublishTimeUTC column set as a STRING: https://github.com/cds-snc/notification-terraform/pull/1201

jimleroyer commented 3 months ago

The PR was merged and a subsequent change was necessary to finally make it work. After that, I will see if I can do some clickops in quicksight to play a bit, and bring further TF changes along.

sastels commented 3 months ago

Jimmy clickop'd in a joined dataset in staging. Jimmy and Steve will look at converting to CloudFormation for prod

jimleroyer commented 3 months ago

Testing against dev today with the data transformation. I need to bring dev environment up to date with latest QuickSight changes.

sastels commented 3 months ago

data type transformations worked!

jimleroyer commented 3 months ago

Today: to test the left join of smsusage and notifications data set using TF first, then Cloudformation if that fails!

P0NDER0SA commented 3 months ago

have to check if it merged or failed... we can't remember :)

jimleroyer commented 3 months ago

Latest I worked on that, the plan didn't work with the tried TF construct. I was able to get past it for some moment with a variation I saw on forums while debugging the issue but then it went back to the initial TF error. I think it is a bug and I can open a bug in the GH QuickSight plugin forum.

I will put this back into prioritized as I got to work on something else at the moment. Next logical step is to move to cloudformation to get something to work properly.

P0NDER0SA commented 3 months ago

moving back to Prioritized column

sastels commented 3 months ago

I got dev Quicksight sorted out (ie so that main applies cleanly) and started doing the join in CloudFormation. https://github.com/cds-snc/notification-terraform/pull/1231

sastels commented 2 months ago

PR ready for review https://github.com/cds-snc/notification-terraform/pull/1231

sastels commented 2 months ago

merged into staging, will release today

jimleroyer commented 2 months ago

Verified in QuickSight data set editor and it looks as it should be, i.e. with a left join between SMS usage and the notifications table.

image.png

I also proceeded to create a visual with the cost per service.

image.png
sastels commented 2 months ago

So the dataset took 45 minutes to build in production. But CloudFormation timed out the GitHub action after 30 minutes :/

Tried to add a FilterOperation to the notifications logical dataset, ie

locals {
  one_week_ago = timeadd(timestamp(), "-168h")
}
           notifications = {
              Alias = "notifications",
              Source = {
                DataSetArn = aws_quicksight_data_set.notifications.arn
              }
              DataTransforms = [
                {
                  FilterOperation = {
                    ConditionExpression = "{notification_created_at} > ${local.one_week_ago}"
                  }
                }
              ]
            },

But kept getting an error

Error: waiting for CloudFormation Stack (arn:aws:cloudformation:ca-central-1:800095993820:stack/sms-usage-notifications/036ef860-ed43-11ee-b35d-06649295a0c1) update: failed to update CloudFormation stack (UPDATE_ROLLBACK_COMPLETE): ["Resource handler returned message: \"Invalid request provided: Unable to process your request while preparing the table view from your schema. ErrorType: SYNTAX_ERROR (Service: QuickSight, Status Code: 400, Request ID: null)\" (RequestToken: 38794283-eb21-7b89-f09e-1fef7878d7dc, HandlerErrorCode: InvalidRequest)"]
│

So removed the filter and increased the timeout instead as a (temporary) fix.

sastels commented 2 months ago

increased timeout and terragrunt apply succeeded.

ben851 commented 2 months ago

@ben851 to QA

ben851 commented 2 months ago

@sastels to create a new card for adding a filter to the data

ben851 commented 2 months ago

https://ca-central-1.quicksight.aws.amazon.com/sn/analyses/6539c1f5-dc4c-457b-a3f1-14494d5abace

This is the only thing I can see relating to costing in production. It's just usage.

ben851 commented 2 months ago

Steve will refine the QA metrics, so that Ben can try again

sastels commented 2 months ago

dataset is https://ca-central-1.quicksight.aws.amazon.com/sn/data-sets/sms-usage-notifications/view

ben851 commented 2 months ago

Verified the analysis, looks cool!