-
Notifications
You must be signed in to change notification settings - Fork 6
Feat/tr/cluster dlq alarms #137
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
base: main
Are you sure you want to change the base?
Conversation
…nsearch cluster lives or not
| resource "aws_cloudwatch_metric_alarm" "critical_stac_server_dlq_alarm" { | ||
| alarm_name = "CRITICAL: ${local.name_prefix}-stac-server-dead-letter SQS DLQ Critical Alarm" | ||
| alarm_description = "CRITICAL: 10 or more messages are persisting in the ${local.name_prefix}-stac-server SQS dead letter queue" | ||
| evaluation_periods = 5 |
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.
might be high. Thought is that if a bunch of messages are there for over 5 minutes its really properly stuck there, thus a CRITICAL alarm
…iption to accurately reflect threshold
| deploy_stac_server_outside_vpc = var.deploy_stac_server_outside_vpc | ||
| fd_web_acl_id = var.deploy_waf_rule ? module.base_infra.web_acl_id : var.ext_web_acl_id | ||
| warning_sns_topic_arn = module.base_infra.warning_sns_topic_arn | ||
| critical_sns_topic_arn = module.base_infra.critical_sns_topic_arn |
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 should allow the user to toggle whether these stac-server alarms are deployed or not via boolean flag; automatically deploying alarms can be handy but they may prefer to manage all tracked dimensions themselves, too.
Suggest using the same approach used in the cirrus module for consistency:
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.
added deploy_alarms trigger similar to cirrus module
Proposed Changes
This PR is filling in some gaps in alarm systems that were exposed by some infra failures.
stac-serverfor the dead letter queuestac-serverOpenSearch cluster healthOne outstanding question - optional deployment of alarms. I'm choosing to have them deployed automatically but open to suggestions for why they could be optionally deployed.
Checklist