Default no_data_timeframe to 2x timeframe

[xp-1000]

Hi

Terraform Version

Terraform v0.12.3

Affected Resource(s)

• datadog_monitor

Terraform Configuration Files

resource "datadog_monitor" "foo" {
  name               = "Name for monitor foo"
  type               = "metric alert"
  message            = "Monitor triggered. Notify: @hipchat-channel"
  escalation_message = "Escalation message @pagerduty"

  query = "avg(last_1h):avg:aws.ec2.cpu{environment:foo,host:foo} by {host} > 4"

  thresholds = {
    critical          = 4
  }

  notify_no_data    = true
  #no_data_timeframe = 120 # 2x alerting window (last_1h)
}

Expected Behavior

What should have happened? Ideally, the provider should respect the documentation and default no_data_timeframe to 2x the alerting window : https://docs.datadoghq.com/monitors/monitor_types/metric/#configuration Set a hardcoded value as 10min could be risky.

Actual Behavior

What actually happened? Since provider v2.0 : https://github.com/terraform-providers/terraform-provider-datadog/blob/master/CHANGELOG.md#200-june-18-2019

datadog_monitor: Add default to no data timeframe of 10 minutes. (#212) And before v2.0 it was 0 mn which was worst. Current behavior is better but will not work in all cases (when monitor alerting window >5min)

Important Factoids

Notice that API documentation suggests the datadog API server will define the default value to 2x alerting window but this is false : https://docs.datadoghq.com/api/?lang=python#create-a-monitor

Default: 2x timeframe for metric alerts, 2 minutes for service checks

References

• https://github.com/terraform-providers/terraform-provider-datadog/pull/212

[bkabrda]

So I've been looking into this and it's almost impossible achieve with current TF internal functionality. Let me explain:

• We basically need a dynamic default for no_data_timeframe, based on type.
• There is a DefaultFunc [1] that can be used on a field to provide a dynamic default, but it doesn't have access to values of other fields, so it's not possible to use it for this usecase.
• We could set this dynamically in create/update methods, but that would always cause TF to provide non-empty plans, as TF would always consider the value returned back from API different than what is current set in your TF config file (==null).
• To aid the above, we could use DiffSuppressFunc [2] to suppress the non-empty plan. The issue is that there's no way of telling in this function if the null in your TF config is result of removal of explicitly set no_data_timeframe or if the attribute hadn't been present before => we have no way of knowing whether to consider this a diff or not.

Based on the above, I'm leaning towards thinking that this is just not possible ATM, but I'll try to think about this some more and come up with a different solution, so leaving this open for now.

[1] https://godoc.org/github.com/hashicorp/terraform/helper/schema#SchemaDefaultFunc [2] https://godoc.org/github.com/hashicorp/terraform/helper/schema#SchemaDiffSuppressFunc

[phillip-dd]

@bkabrda I believe in buildMonitorStruct when we set NoDataTimeframe: https://github.com/terraform-providers/terraform-provider-datadog/blob/36fba07f4a57b264851f7f5206bf095f22335b88/datadog/resource_datadog_monitor.go#L259-L261

we could add something like:

else if o.NotifyNoData {
  // rough check to get "2x the timeframe"
  noData := "last_10m"
  query := d.Get("query").(string)
  if strings.Count(query, "last_5m") > 0 {
    noData = "last_10m"
  } else if strings.Count(query, "last_10m") > 0 {
   noData = "last_30m"
  } else if strings.Count(query, "last_15m") > 0 {
   noData = "last_30m"
  } else if strings.Count(query, "last_30m") > 0 {
   noData = "last_1h"
  } else if strings.Count(query, "last_1h") > 0 {
   noData = "last_2h"
  } else if strings.Count(query, "last_2h") > 0 {
   noData = "last_4h"
  } else if strings.Count(query, "last_4h") > 0 {
   noData = "last_12h"
  } else if strings.Count(query, "last_1d") > 0 {
   noData = "1h_1d_ago"
  }

  o.NoDataTimeframe = noData
}

Which will set NoDataTimeframe to a value thats roughly 2x the timeframe unless specified by the user.

[bkabrda]

@phillip-dd the problem here is that if we do that, the user will always have a non-empty plan on running terraform apply, because the TF config will contain no value for no_data_timeframe, but the API will return a value for it (that's what I tried to explain in the third bullet point in my previous comment).

[alsyia]

I'm not sure why or how, but when we use no_data_timeframe = 0 in our Terraform code, our monitors apparently use the default behaviour, that is to switch to the No Data state if no data has been received for two times the evaluation window. Terraform plan and apply work as expected since the value is hardcoded.

If that helps...