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

fix(aws-backup): avoid duplicated and ill-routed alerts #508

Merged
merged 3 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions modules/integration_aws-backup/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ existing [stack](https://github.com/claranet/terraform-signalfx-detectors/wiki/G
module "signalfx-detectors-integration-aws-backup" {
source = "github.com/claranet/terraform-signalfx-detectors.git//modules/integration_aws-backup?ref={revision}"

environment = var.environment
notifications = local.notifications
environment = var.environment
notifications = local.notifications
aws_account_id = "fillme"
}
```

Expand Down Expand Up @@ -57,7 +58,7 @@ Note the following parameters:

These 3 parameters along with all variables defined in [common-variables.tf](common-variables.tf) are common to all
[modules](../) in this repository. Other variables, specific to this module, are available in
[variables-gen.tf](variables-gen.tf).
[variables.tf](variables.tf) and [variables-gen.tf](variables-gen.tf).
In general, the default configuration "works" but all of these Terraform
[variables](https://www.terraform.io/language/values/variables) make it possible to
customize the detectors behavior to better fit your needs.
Expand Down
12 changes: 10 additions & 2 deletions modules/integration_aws-backup/common-filters.tf
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
# Common filters at ../../common/module/filters-integration-aws.tf are not used
# with AWS Backup because recovery points (snapshots) and jobs are unlikely to
# be tagged with `env` and `sfx_monitored`.
#
# We are also assuming that we want to be notified for every backup vault and
# resource types. `match_missing=False` is required to avoid duplicated signal
# and alerts, since CloudWatch returns aggregated metric, and per vault and
# resource type.
locals {
filters = "filter('ResourceType', '*')"
}
filters = format("filter('namespace', 'AWS/Backup') and filter('stat', 'sum') and filter('BackupVaultName', '*', match_missing=False) and filter('ResourceType', '*', match_missing=False) and filter('aws_account_id', '%s')", var.aws_account_id)
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ id: backup_failed

transformation: ".max(over='1d').fill(0)"
aggregation: true
filtering: "filter('namespace', 'AWS/Backup') and filter('stat', 'sum')"
filtering: "filter('namespace', 'AWS/Backup') and filter('stat', 'sum') and filter('BackupVaultName', '*', match_missing=False) and filter('ResourceType', '*', match_missing=False)"

signals:
signal:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ id: backup_job_expired

transformation: ".max(over='1d').fill(0)"
aggregation: true
filtering: "filter('namespace', 'AWS/Backup') and filter('stat', 'sum')"
filtering: "filter('namespace', 'AWS/Backup') and filter('stat', 'sum') and filter('BackupVaultName', '*', match_missing=False) and filter('ResourceType', '*', match_missing=False)"

signals:
signal:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ id: backup_successful

transformation: ".min(over='23h')"
aggregation: true
filtering: "filter('namespace', 'AWS/Backup') and filter('stat', 'sum')"
filtering: "filter('namespace', 'AWS/Backup') and filter('stat', 'sum') and filter('BackupVaultName', '*', match_missing=False) and filter('ResourceType', '*', match_missing=False)"

signals:
created:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ id: backup_rp_partial

transformation: ".max(over='1d').fill(0)"
aggregation: true
filtering: "filter('namespace', 'AWS/Backup') and filter('stat', 'sum')"
filtering: "filter('namespace', 'AWS/Backup') and filter('stat', 'sum') and filter('BackupVaultName', '*', match_missing=False) and filter('ResourceType', '*', match_missing=False)"

signals:
signal:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ id: backup_rp_expired

transformation: ".max(over='1d').fill(0)"
aggregation: true
filtering: "filter('namespace', 'AWS/Backup') and filter('stat', 'sum')"
filtering: "filter('namespace', 'AWS/Backup') and filter('stat', 'sum') and filter('BackupVaultName', '*', match_missing=False) and filter('ResourceType', '*', match_missing=False)"

signals:
signal:
Expand Down
10 changes: 5 additions & 5 deletions modules/integration_aws-backup/detectors-gen.tf
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ resource "signalfx_detector" "backup_failed" {
tags = compact(concat(local.common_tags, local.tags, var.extra_tags))

program_text = <<-EOF
base_filtering = filter('namespace', 'AWS/Backup') and filter('stat', 'sum')
base_filtering = filter('namespace', 'AWS/Backup') and filter('stat', 'sum') and filter('BackupVaultName', '*', match_missing=False) and filter('ResourceType', '*', match_missing=False)
signal = data('NumberOfBackupJobsFailed', filter=base_filtering and ${module.filtering.signalflow})${var.backup_failed_aggregation_function}${var.backup_failed_transformation_function}.publish('signal')
detect(when(signal > ${var.backup_failed_threshold_critical}, lasting=%{if var.backup_failed_lasting_duration_critical == null}None%{else}'${var.backup_failed_lasting_duration_critical}'%{endif}, at_least=${var.backup_failed_at_least_percentage_critical})).publish('CRIT')
EOF
Expand Down Expand Up @@ -34,7 +34,7 @@ resource "signalfx_detector" "backup_job_expired" {
tags = compact(concat(local.common_tags, local.tags, var.extra_tags))

program_text = <<-EOF
base_filtering = filter('namespace', 'AWS/Backup') and filter('stat', 'sum')
base_filtering = filter('namespace', 'AWS/Backup') and filter('stat', 'sum') and filter('BackupVaultName', '*', match_missing=False) and filter('ResourceType', '*', match_missing=False)
signal = data('NumberOfBackupJobsExpired', filter=base_filtering and ${module.filtering.signalflow}, extrapolation='zero')${var.backup_job_expired_aggregation_function}${var.backup_job_expired_transformation_function}.publish('signal')
detect(when(signal > ${var.backup_job_expired_threshold_critical}, lasting=%{if var.backup_job_expired_lasting_duration_critical == null}None%{else}'${var.backup_job_expired_lasting_duration_critical}'%{endif}, at_least=${var.backup_job_expired_at_least_percentage_critical})).publish('CRIT')
EOF
Expand Down Expand Up @@ -90,7 +90,7 @@ resource "signalfx_detector" "backup_successful" {
tags = compact(concat(local.common_tags, local.tags, var.extra_tags))

program_text = <<-EOF
base_filtering = filter('namespace', 'AWS/Backup') and filter('stat', 'sum')
base_filtering = filter('namespace', 'AWS/Backup') and filter('stat', 'sum') and filter('BackupVaultName', '*', match_missing=False) and filter('ResourceType', '*', match_missing=False)
created = data('NumberOfBackupJobsCreated', filter=base_filtering and ${module.filtering.signalflow}, extrapolation='zero')${var.backup_successful_aggregation_function}${var.backup_successful_transformation_function}
completed = data('NumberOfBackupJobsCompleted', filter=base_filtering and ${module.filtering.signalflow}, extrapolation='zero')${var.backup_successful_aggregation_function}${var.backup_successful_transformation_function}
signal = (created-completed).publish('signal')
Expand Down Expand Up @@ -120,7 +120,7 @@ resource "signalfx_detector" "backup_rp_partial" {
tags = compact(concat(local.common_tags, local.tags, var.extra_tags))

program_text = <<-EOF
base_filtering = filter('namespace', 'AWS/Backup') and filter('stat', 'sum')
base_filtering = filter('namespace', 'AWS/Backup') and filter('stat', 'sum') and filter('BackupVaultName', '*', match_missing=False) and filter('ResourceType', '*', match_missing=False)
signal = data('NumberOfRecoveryPointsPartial', filter=base_filtering and ${module.filtering.signalflow})${var.backup_rp_partial_aggregation_function}${var.backup_rp_partial_transformation_function}.publish('signal')
detect(when(signal > ${var.backup_rp_partial_threshold_minor}, lasting=%{if var.backup_rp_partial_lasting_duration_minor == null}None%{else}'${var.backup_rp_partial_lasting_duration_minor}'%{endif}, at_least=${var.backup_rp_partial_at_least_percentage_minor})).publish('MINOR')
EOF
Expand Down Expand Up @@ -148,7 +148,7 @@ resource "signalfx_detector" "backup_rp_expired" {
tags = compact(concat(local.common_tags, local.tags, var.extra_tags))

program_text = <<-EOF
base_filtering = filter('namespace', 'AWS/Backup') and filter('stat', 'sum')
base_filtering = filter('namespace', 'AWS/Backup') and filter('stat', 'sum') and filter('BackupVaultName', '*', match_missing=False) and filter('ResourceType', '*', match_missing=False)
signal = data('NumberOfRecoveryPointsExpired', filter=base_filtering and ${module.filtering.signalflow})${var.backup_rp_expired_aggregation_function}${var.backup_rp_expired_transformation_function}.publish('signal')
detect(when(signal > ${var.backup_rp_expired_threshold_major}, lasting=%{if var.backup_rp_expired_lasting_duration_major == null}None%{else}'${var.backup_rp_expired_lasting_duration_major}'%{endif}, at_least=${var.backup_rp_expired_at_least_percentage_major})).publish('MAJOR')
EOF
Expand Down
3 changes: 3 additions & 0 deletions modules/integration_aws-backup/variables.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
variable "aws_account_id" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think forcing aws_account_id is a good idea, not everyone wants to filter by aws_account_id
It's best to let people choose if they want to by setting the appropriate filter in their own terraform

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello, we don't have much choice because recovery points are not automatically tagged according to the convention used with the detectors (see comment in common-filters.tf).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Variable removed after further discussions.

type = string
}
Loading