coralogix / terraform-provider-coralogix

Terraform provider for Coralogix
https://registry.terraform.io/providers/coralogix/coralogix
Apache License 2.0
9 stars 6 forks source link

Outdated documentation for `coralogix_alert` resource #204

Open rudolf-repcin opened 5 months ago

rudolf-repcin commented 5 months ago

Hello,

we've been trying to figure out what's wrong with our configs and we've discovered that there was a change released in v1.11.0 of this provider and following fact is not documented:

exactly one of incident_settings or all of notifications_group..notification.. notify_on and retriggering_period_minutes must be set.

OrNovo commented 5 months ago

Hi @rudolf-repcin. Can you elaborate on what exactly is missing? Because I see that there is documentation about the new field (including examples).

rudolf-repcin commented 5 months ago

Here's the error message we got from TF prepare output:

 Error: Invalid combination of arguments
│ 
│   with module.....omitted........coralogix_alert.max_number_of_tasks_warning[0],
│   on .terraform/.....omitted....../alerts.tf line 104, in resource "coralogix_alert" "max_number_of_tasks_warning":
│  104: resource "coralogix_alert" "max_number_of_tasks_warning" {
│ 
│ "notifications_group.0.notification.0.notify_on": one of
│ `incident_settings,notifications_group.0.notification.0.notify_on` must be
│ specified

and this was our code for the alert

resource "coralogix_alert" "max_number_of_tasks_warning" {
  count    = local.alerts_enabled
  name     = "${local.alert_display_name} [Number of ECS tasks >= 5]"
  severity = "Warning"

  meta_labels = {
    application  = local.df_prod_env
    subsystem    = var.ecs_service_name
  }

  notifications_group {
    notification {
      email_recipients            = [local.email_...omitted..]
      retriggering_period_minutes = 60
    }
  }

  metric {
    promql {
      search_query = "amazonaws_com_ECS_ContainerInsights_RunningTaskCount_sum{ServiceName=\"${var.ecs_service_name}\",account_id=\"${var.aws_account_id}\"}"
      condition {
        more_than                      = true
        threshold                      = 5
        sample_threshold_percentage    = 80
        time_window                    = "1Min"
        min_non_null_values_percentage = 80
      }
    }
  }
}

We had to add notify_on to the notification_group.notification block for it to work (even though notify_on is optional in the documentation). Documentation is missing the fact that either incident_settings or all of notification_group.notification parameters must be configured (they are all optional in the documentation).

OrNovo commented 4 months ago

I see. I'll update the documentations.