Skip to content

Feedback api alarm threshold 1395#131

Closed
AnJuHyppolite wants to merge 25 commits intodevfrom
feedback-api-alarm-threshold-1395
Closed

Feedback api alarm threshold 1395#131
AnJuHyppolite wants to merge 25 commits intodevfrom
feedback-api-alarm-threshold-1395

Conversation

@AnJuHyppolite
Copy link
Copy Markdown
Contributor

Description

This PR updates the alarm threshold.

Ticket

[chore]: Update Feedback Widget Alarm Threshold

Approach

Steps to Test

Notes

sannidhishukla and others added 25 commits August 25, 2025 21:12
<!-- Please complete the following sections as necessary. -->

### Description

Creates a docker compose file that defines a local postgres database to
use for development and testing. Also updates the README with
instructions for how to run the local db.

### Ticket
Resolves [Ticket
#424](newjersey/innovation-platform-pm#424)
<!-- Please complete the following sections as necessary. -->

### Description

Creates the `Feedback` Prisma data model, as well as the initial
migration to load this schema into our postgres database.

### Ticket

Resolves [Ticket
#430](newjersey/innovation-platform-pm#430)
(Define a basic schema for the feedback database using Prisma/Prisma
Migrate)

### Approach

- Added the `@prisma/client` package
- Added `DATABASE_URL` to .env
- This is set to the local db URL
(`postgresql://postgres:postgres@localhost:5432/postgres`)
    - I've also updated the Bitwarden with the new .env
- I changed the database server time zone from `UTC` to `US/Eastern` by
editing the `TZ` env var in the `docker-compose.yml`
- We originally chose UTC because we didn't want to manage between
switching from EDT/EST, but the`US/Eastern` time zone automatically
handles this for us.
- With the current database schema, when we create a new feedback
record, the `createdAt` column defaults to the time the record was
created. By setting the database time zone to `US/Eastern`, it will
automatically generate the timestamp in Eastern time.
- So when we're looking up records in the database, the dates will be
more intuitive since they will match NJ's time zone, which is generally
the timezone that people are submitting feedback from.
- This might also make integrating with Looker easier as we don't have
to convert the time from another timezone when displaying dates.

### Steps to Test
#### Loading the schema into the local db
If this is your first time running the local db, follow the
[instructions in the
README](https://github.com/newjersey/feedback-api/blob/dev/README.md#running-the-local-dev-database)
first.

1. Run `npm install`
2. Delete the `feedback-api_postgres-data` volume since we don't want
old persisted data from the local db
    - Run `docker volume ls` to see a list of volumes
- If the volume is present, run `docker volume rm
feedback-api_postgres-data` to delete it
3. Start the local db by running `colima start` then `docker compose up
-d`
4. Run `npx prisma migrate dev` to apply the migrations in
`prisma/migrations/` to the local db
5. Generate the Prisma Client containing our database schema's data
models with `npx prisma generate`

#### Manual testing strategy
I didn't want to open the integration testing can of works yet, I
decided to manually test that inserting into the Feedback table with
Prisma works using a ts script, specifically paying attention that the
`createdAt` column is correct.

1. Installed ts-node without saving it to package.json by running `npm
install ts-node --no-save`
2. Created a `test.ts` file in the `prisma/` directory with the
following code:
```ts
import { PrismaClient } from '..//generated/prisma/client';

const prisma = new PrismaClient();
async function main() {
  await prisma.feedback.deleteMany({});

  const feedbackWithoutTime = await prisma.feedback.create({
    data: {
      rating: true,
      pageUrl: 'testUrl',
      createdAtFromApi: new Date()
    }
  });

  const feedbackWithTime = await prisma.feedback.create({
    data: {
      rating: true,
      pageUrl: 'testUrl',
      createdAt: new Date(1671717203269), // Thursday, December 22, 2022 1:53:23.269 PM UTC
      createdAtFromApi: new Date()
    }
  });

  console.log({ feedbackWithoutTime, feedbackWithTime });
}

main()
  .then(async () => {
    await prisma.$disconnect();
  })
  .catch(async (e) => {
    console.error(e);
    await prisma.$disconnect();
    process.exit(1);
  });
```

3. Ran the script with `npx ts-node prisma/test.ts`. This produced the
following console output:
```bash
{
  feedbackWithoutTime: {
    id: 1,
    createdAt: 2025-07-22T19:43:28.141Z,
    createdAtFromApi: 2025-07-22T19:43:28.139Z,
    rating: true,
    pageUrl: 'testUrl',
    comment: null,
    email: null
  },
  feedbackWithTime: {
    id: 2,
    createdAt: 2022-12-22T13:53:23.269Z,
    createdAtFromApi: 2025-07-22T19:43:28.147Z,
    rating: true,
    pageUrl: 'testUrl',
    comment: null,
    email: null
  }
}
```
  - Success criteria
    - The ids are being generated sequentially
- For `feedbackWithoutTime`, the createdAt matches the time at which I
ran the script (formatted in UTC)
- For `feedbackWith`, the createdAt matches the inputted DateTime object
(formatted in UTC)

4. Then, checked that the objects were created as expected in the
database itself. Ran `psql
postgresql://postgres:postgres@localhost:5432/postgres` to start the
psql console.
5. Within the psql console, ran `SELECT * FROM feedback;`. This gave the
following output
```
 id |         created_at         |    created_at_from_api     | rating | page_url | comment | email 
----+----------------------------+----------------------------+--------+----------+---------+-------
  1 | 2025-07-22 15:43:28.141-04 | 2025-07-22 15:43:28.139-04 | t      | testUrl  |         | 
  2 | 2022-12-22 08:53:23.269-05 | 2025-07-22 15:43:28.147-04 | t      | testUrl  |         | 
(2 rows)
```
  - Success criteria
- The `id`, `rating`, and `pageUrl` fields match what we inserted with
prisma.
- The `createdAt` and `createdAtFromApi` fields are displayed in
"US/Eastern" time, i.e. it automatically selects the appropriate UTC
offset depending on whether DST was in effect on the given date.
- The Prisma camelCase fields (e.g. `createdAt`) are mapped to
snake_case based on the `@map` attributes in prisma.schema.
<!-- Please complete the following sections as necessary. -->

### Description

<!-- Summary of the changes, related issue, relevant motivation, and
context -->

Implements [#578 Set up alerts for new Feedback API (in
Innov-Platform-Prod AWS account)
](newjersey/innovation-platform-pm#578).

#### Context
As part of migrating the Feedback API resources from the `Innov-RES-Dev`
to the Platforms AWS accounts, we realized that our alarm for API errors
was not defined in CDK (nor Serverless before we migrated to CDK).

Previously in the `Innov-RES-Dev` account, we had an alarm named
"Feedback API - Comments - Error" which tracked a metric by the same
name. With @sannidhishukla's help we tracked this metric back down a
metric filter on the [/aws/lambda/feedback-api-dev-comment log
group](https://us-east-1.console.aws.amazon.com/cloudwatch/home?region=us-east-1#logsV2:log-groups/log-group/$252Faws$252Flambda$252Ffeedback-api-dev-comment$23metric-filters)
log group which matches when the string "Error:" appears in logs within
that log stream.

For the sake of simplicity, instead of copying the previous approach, I
decided to set up two new alarms that instead track two built-in
metrics, `5XXError` and `4XXError`, on the API gateway. An additional
benefit is that the alarm will trigger on all of our endpoints rather
than just the comment endpoint.

### Approach

<!-- Any changed dependencies, e.g. requires an
install/update/migration, etc. -->
**Using AWS Cloud Development Kit (CDK) and TypeScript:**
- Defined two alarm constructs for the 5XXError and 4XXError alarms and
added them to the existing FeedbackApiStack.
- Created an SNS topic and added a subscription to send alerts to our
#platform-eng-alerts Slack channel.

#### AWS Resource Documentation
- [Set a CloudWatch
alarm](https://docs.aws.amazon.com/cdk/v2/guide/how-to-set-cw-alarm.html)
- [Alarm
construct](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_cloudwatch.Alarm.html)
- [enum
ComparisonOperator](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_cloudwatch.ComparisonOperator.html)
- [enum
TreatMissingData](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_cloudwatch.TreatMissingData.html)
- [class Topic
(construct)](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_sns.Topic.html)
- [class Subscription
(construct)](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_sns.Subscription.html)
- [CDK Construct Library for Amazon Simple Notification Service
Subscriptions](https://docs.aws.amazon.com/cdk/api/v2/python/aws_cdk.aws_sns_subscriptions/README.html)
- [Metric
objects](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_cloudwatch-readme.html#metric-objects)
- [class
Metric](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_cloudwatch.Metric.html)

### Steps to Test

<!-- If this work affects a user's experience, provide steps to test
these changes in-app. -->
1. Make sure you've authenticated to the correct AWS account using
short-term credentials (see README for instructions on how to do this).
2. Run `npm install` to install dependencies.
3. Run `npm run synth:api` to synthesize the Feedback API into a
CloudFormation template.
4. Run `npm run deploy:alarm` to deploy the Feedback API, which includes
the alarms
5. Verify that the CloudWatch alarms are created as expected using the
AWS console

---------

Co-authored-by: elizabeth zhang <elizabeth.zhangey@gmail.com>
This PR fixes the tests written in #75 as well as longer-standing type
errors that were causing our test suite to fail.
Reverts #76 (merged by rebasing, but should have
merged with a merge commit instead)
Merges the following PRs to main: 
- #68 
- #70 
- #69 
- #72 
- #71 
- #73 
- #75 
- #77
Reverts #81 (should have merged with a merge
commit, but instead squashed and merged)
Reverts #82

#82 reverts #81, which I did because I wanted to merge with a merge
commit instead of squashing and merging. But afterwards, I couldn't
create a new PR to merge dev into main that contained all the code
changes we wanted (not sure why). So reverting the revert so we can at
least get back the code changes.
<!-- Please complete the following sections as necessary. -->

### Description

<!-- Summary of the changes, related issue, relevant motivation, and
context -->
Merges the following PRs to main: 
- #101
- #103
- #104
-  #107
- #106
<!-- Please complete the following sections as necessary. -->

### Description

<!-- Summary of the changes, related issue, relevant motivation, and
context -->
Merges the following PRs to main:
- #109
@AnJuHyppolite
Copy link
Copy Markdown
Contributor Author

closing to pull from dev and create a chore branch off dev

@AnJuHyppolite AnJuHyppolite deleted the feedback-api-alarm-threshold-1395 branch April 2, 2026 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants