Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for SNS alarms and dashboard #83

Merged
merged 13 commits into from
Jun 21, 2022
Merged

Add support for SNS alarms and dashboard #83

merged 13 commits into from
Jun 21, 2022

Conversation

direnakkoc
Copy link
Collaborator

@direnakkoc direnakkoc commented Jun 13, 2022

Description

Add support for SNS alarms and dashboard for the metrics are

  • NumberOfNotificationsFilteredOut-InvalidAttributes
  • NumberOfNotificationsFailed

Motivation and Context

  • NumberOfNotificationsFilteredOut-InvalidAttributes - This metric keeps track of messages that were filtered out because they carried invalid or malformed attributes and, thus, didn’t match the subscription filter policy.

  • NumberOfNotificationsFailed – This last metric tracks all the messages that failed to be delivered to subscribing endpoints, regardless of whether a filter policy had been set for the endpoint. This metric is emitted after the message delivery retry policy is exhausted, and SNS stops attempting to deliver the message. At that moment, the subscribing endpoint is likely no longer reachable. For example, the subscribing SQS queue or Lambda function has been deleted by its owner. You may want to closely monitor this metric to address message delivery issues quickly.

How Has This Been Tested?

  • Unit test added
  • Integration test is completed

Screenshots (if appropriate):

Screenshot 2022-06-21 at 15 28 36
Screenshot 2022-06-21 at 15 31 24

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING.md document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@direnakkoc direnakkoc requested a review from eoinsha June 13, 2022 10:53
@coveralls
Copy link

coveralls commented Jun 13, 2022

Coverage Status

Coverage decreased (-100.0%) to 0.0% when pulling 6e8e217 on sns-metrics into 18c409e on main.

- Doing integration test with real resources by adding https endpoint and creating lambda function to handle confirmSubscription for SNS topic
README.md Outdated Show resolved Hide resolved
serverless-plugin/default-config.yaml Show resolved Hide resolved
"gitHead": "c4300240f9e854c4eb1c5503b839882ff0cd1cae"
"gitHead": "c4300240f9e854c4eb1c5503b839882ff0cd1cae",
"dependencies": {
"https": "^1.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node.js has a built-in https module, so I don't think we need to add an NPM module. Does it add anything?
I'd like to suggest replacing this with axios in any case and changing the handler to use async/await.

serverless-test-project/package.json Outdated Show resolved Hide resolved
- Remove usage of sns-validator as it was overkill
Copy link
Contributor

@eoinsha eoinsha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LTGM!

@direnakkoc direnakkoc merged commit 03dcfb6 into main Jun 21, 2022
@direnakkoc direnakkoc deleted the sns-metrics branch June 21, 2022 16:29
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