-
Notifications
You must be signed in to change notification settings - Fork 15
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 AppSync alarms and dashboard #104
Conversation
- Remove redundant resources
9396cf3
to
9af9274
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good. Some comments added for now. Will review again when remaining tests and documentation updates are done.
core/dashboard.js
Outdated
function createAppSyncWidgets (appSyncResources) { | ||
const appSyncWidgets = [] | ||
const metricGroups = { | ||
CloudWatch: ['5XXError', '4XXError', 'Latency', 'Requests'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe "API" makes more sense here than "CloudWatch" as a group label
core/dashboard.js
Outdated
function createAppSyncWidgets (appSyncResources) { | ||
const appSyncWidgets = [] | ||
const metricGroups = { | ||
CloudWatch: ['5XXError', '4XXError', 'Latency', 'Requests'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will have to use different y-axes for metrics of different units
@@ -16,7 +16,7 @@ custom: | |||
- Key: Stage | |||
Value: ${self:provider.stage} | |||
slicWatch: | |||
topicArn: ${env:ALARM_TOPIC} | |||
# topicArn: ${env:ALARM_TOPIC} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment #
should be removed before merge
- ALLOW_REFRESH_TOKEN_AUTH | ||
PreventUserExistenceErrors: ENABLED | ||
|
||
#Cognito users pool Admin group |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can add a space after all comment markers (#
).
@@ -8,7 +8,7 @@ plugins: | |||
- serverless-slic-watch-plugin | |||
custom: | |||
slicWatch: | |||
topicArn: ${env:ALARM_TOPIC} | |||
# topicArn: ${env:ALARM_TOPIC} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove comment before merge
ae0fb6c
to
b663cc9
Compare
- Tidy up the branch
b663cc9
to
8afae38
Compare
Already merged as part of #105 |
Description
Add support for AppSync alarms for the metrics are:
Add support for AppSync dashboard for the metrics are:
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: