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

Outdated documentation for coralogix_alert resource #204

Open
rudolf-repcin opened this issue Feb 15, 2024 · 3 comments
Open

Outdated documentation for coralogix_alert resource #204

rudolf-repcin opened this issue Feb 15, 2024 · 3 comments

Comments

@rudolf-repcin
Copy link

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
Copy link
Contributor

OrNovo commented Feb 25, 2024

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
Copy link
Author

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
Copy link
Contributor

OrNovo commented Mar 3, 2024

I see. I'll update the documentations.

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

No branches or pull requests

2 participants