From 2159ea2d3d141b78bdc66b4063b092d618907bc5 Mon Sep 17 00:00:00 2001 From: James Edouard Date: Mon, 8 May 2023 14:16:46 +0000 Subject: [PATCH 01/10] Add support for retriggering notifications in AlertPolicy --- mmv1/products/monitoring/AlertPolicy.yaml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/mmv1/products/monitoring/AlertPolicy.yaml b/mmv1/products/monitoring/AlertPolicy.yaml index 828bae9b7d98..2390cf688bcb 100644 --- a/mmv1/products/monitoring/AlertPolicy.yaml +++ b/mmv1/products/monitoring/AlertPolicy.yaml @@ -830,6 +830,25 @@ properties: name: autoClose description: | If an alert policy that was active has no data for this long, any open incidents will close. + - !ruby/object:Api::Type::Array + name: notificationChannelStrategy + description: | + Control over how the notification channels in `notification_channels` + are notified when this alert fires, on a per-channel basis. + item_type: !ruby/object:Api::Type::NestedObject + properties: + - !ruby/object:Api::Type::Array + name: notificationChannelNames + description: | + The notification channels that these settings apply to. Each of these + correspond to the name field in one of the NotificationChannel objects + referenced in the notification_channels field of this AlertPolicy. The format is + `projects/[PROJECT_ID_OR_NUMBER]/notificationChannels/[CHANNEL_ID]` + item_type: Api::Type::String + - !ruby/object:Api::Type::String + name: renotifyInterval + description: | + The frequency at which to send reminder notifications for open incidents. - !ruby/object:Api::Type::KeyValuePairs name: userLabels description: | From f26234db69b4e53a7f56af484d03d4dbc756f5c0 Mon Sep 17 00:00:00 2001 From: James Edouard Date: Thu, 11 May 2023 15:13:43 +0000 Subject: [PATCH 02/10] Add support for forecast options in AlertPolicy --- mmv1/products/monitoring/AlertPolicy.yaml | 26 ++++++++++++++++++- ...oring_alert_policy_forecast_options.tf.erb | 23 ++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 mmv1/templates/terraform/examples/monitoring_alert_policy_forecast_options.tf.erb diff --git a/mmv1/products/monitoring/AlertPolicy.yaml b/mmv1/products/monitoring/AlertPolicy.yaml index 2390cf688bcb..729810c420ae 100644 --- a/mmv1/products/monitoring/AlertPolicy.yaml +++ b/mmv1/products/monitoring/AlertPolicy.yaml @@ -38,13 +38,18 @@ examples: vars: alert_policy_display_name: 'My Alert Policy' - # skipping tests because the API is full of race conditions + # skipping tests because the API is full of race conditions - !ruby/object:Provider::Terraform::Examples skip_test: true name: 'monitoring_alert_policy_evaluation_missing_data' primary_resource_id: 'alert_policy' vars: alert_policy_display_name: 'My Alert Policy' + - !ruby/object:Provider::Terraform::Examples + name: "monitoring_alert_policy_forecast_options" + primary_resource_id: "alert_policy" + vars: + alert_policy_display_name: "My Alert Policy" custom_code: !ruby/object:Provider::Terraform::CustomCode constants: templates/terraform/constants/monitoring_alert_policy.go.erb custom_import: templates/terraform/custom_import/self_link_as_name.erb @@ -560,6 +565,25 @@ properties: generate spurious alerts, but short enough that unhealthy states are detected and alerted on quickly. + - !ruby/object:Api::Type::NestedObject + name: forecastOptions + description: | + When this field is present, the `MetricThreshold` + condition forecasts whether the time series is + predicted to violate the threshold within the + `forecastHorizon`. When this field is not set, the + `MetricThreshold` tests the current value of the + timeseries against the threshold. + properties: + - !ruby/object:Api::Type::String + name: forecastHorizon + description: | + The length of time into the future to forecast + whether a timeseries will violate the threshold. + If the predicted value is found to violate the + threshold, and the violation is observed in all + forecasts made for the Configured `duration`, + then the timeseries is considered to be failing. - !ruby/object:Api::Type::Enum name: comparison description: | diff --git a/mmv1/templates/terraform/examples/monitoring_alert_policy_forecast_options.tf.erb b/mmv1/templates/terraform/examples/monitoring_alert_policy_forecast_options.tf.erb new file mode 100644 index 000000000000..b624462640cb --- /dev/null +++ b/mmv1/templates/terraform/examples/monitoring_alert_policy_forecast_options.tf.erb @@ -0,0 +1,23 @@ +resource "google_monitoring_alert_policy" "<%= ctx[:primary_resource_id] %>" { + display_name = "<%= ctx[:vars]["alert_policy_display_name"] %>" + combiner = "OR" + conditions { + display_name = "test condition" + condition_threshold { + filter = "metric.type=\"compute.googleapis.com/instance/disk/write_bytes_count\" AND resource.type=\"gce_instance\"" + duration = "60s" + forecast_options { + forecast_horizon = "3600s" + } + comparison = "COMPARISON_GT" + aggregations { + alignment_period = "60s" + per_series_aligner = "ALIGN_RATE" + } + } + } + + user_labels = { + foo = "bar" + } +} From b1cd4135a649263d8854e30475e14c586549780f Mon Sep 17 00:00:00 2001 From: James Edouard Date: Fri, 12 May 2023 19:33:11 +0000 Subject: [PATCH 03/10] Move tests for forecast alerts into the handwritten file --- mmv1/products/monitoring/AlertPolicy.yaml | 3 + .../resource_monitoring_alert_policy_test.go | 63 +++++++++++++++++-- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/mmv1/products/monitoring/AlertPolicy.yaml b/mmv1/products/monitoring/AlertPolicy.yaml index 729810c420ae..0831ff4fdd30 100644 --- a/mmv1/products/monitoring/AlertPolicy.yaml +++ b/mmv1/products/monitoring/AlertPolicy.yaml @@ -45,7 +45,9 @@ examples: primary_resource_id: 'alert_policy' vars: alert_policy_display_name: 'My Alert Policy' + # skipping tests because the API is full of race conditions - !ruby/object:Provider::Terraform::Examples + skip_test: true name: "monitoring_alert_policy_forecast_options" primary_resource_id: "alert_policy" vars: @@ -584,6 +586,7 @@ properties: threshold, and the violation is observed in all forecasts made for the Configured `duration`, then the timeseries is considered to be failing. + required: true - !ruby/object:Api::Type::Enum name: comparison description: | diff --git a/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go b/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go index 6cd8f6e594f2..8bbb7508f69e 100644 --- a/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go +++ b/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go @@ -15,11 +15,12 @@ import ( func TestAccMonitoringAlertPolicy(t *testing.T) { testCases := map[string]func(t *testing.T){ - "basic": testAccMonitoringAlertPolicy_basic, - "full": testAccMonitoringAlertPolicy_full, - "update": testAccMonitoringAlertPolicy_update, - "mql": testAccMonitoringAlertPolicy_mql, - "log": testAccMonitoringAlertPolicy_log, + "basic": testAccMonitoringAlertPolicy_basic, + "full": testAccMonitoringAlertPolicy_full, + "update": testAccMonitoringAlertPolicy_update, + "mql": testAccMonitoringAlertPolicy_mql, + "log": testAccMonitoringAlertPolicy_log, + "forecast": testAccMonitoringAlertPolicy_forecast, } for name, tc := range testCases { @@ -181,6 +182,29 @@ func testAccCheckAlertPolicyDestroyProducer(t *testing.T) func(s *terraform.Stat } } +func testAccMonitoringAlertPolicy_forecast(t *testing.T) { + + alertName := fmt.Sprintf("tf-test-%s", RandString(t, 10)) + conditionName := fmt.Sprintf("tf-test-%s", RandString(t, 10)) + filter := `metric.type=\"compute.googleapis.com/instance/disk/write_bytes_count\" AND resource.type=\"gce_instance\"` + + VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: ProtoV5ProviderFactories(t), + CheckDestroy: testAccCheckAlertPolicyDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccMonitoringAlertPolicy_forecastCfg(alertName, conditionName, "ALIGN_RATE", filter), + }, + { + ResourceName: "google_monitoring_alert_policy.forecast", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func testAccMonitoringAlertPolicy_basicCfg(alertName, conditionName, aligner, filter string) string { return fmt.Sprintf(` resource "google_monitoring_alert_policy" "basic" { @@ -335,3 +359,32 @@ resource "google_monitoring_alert_policy" "log" { } `, alertName, conditionName) } + +func testAccMonitoringAlertPolicy_forecastCfg(alertName, conditionName, aligner, filter string) string { + return fmt.Sprintf(` +resource "google_monitoring_alert_policy" "forecast" { + display_name = "%s" + enabled = true + combiner = "OR" + + conditions { + display_name = "%s" + + condition_threshold { + aggregations { + alignment_period = "60s" + per_series_aligner = "%s" + } + + duration = "60s" + forecast_options { + forecast_horizon = "3600s" + } + comparison = "COMPARISON_GT" + filter = "%s" + threshold_value = "0.5" + } + } +} +`, alertName, conditionName, aligner, filter) +} From 04bf6136b42a75678e8262201de8033b72737424 Mon Sep 17 00:00:00 2001 From: James Edouard Date: Thu, 20 Jul 2023 12:23:38 +0000 Subject: [PATCH 04/10] Add support for PromQL condition type in AlertPolicy --- mmv1/products/monitoring/AlertPolicy.yaml | 72 +++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/mmv1/products/monitoring/AlertPolicy.yaml b/mmv1/products/monitoring/AlertPolicy.yaml index 2d0f77c58978..cf8b642f6a8c 100644 --- a/mmv1/products/monitoring/AlertPolicy.yaml +++ b/mmv1/products/monitoring/AlertPolicy.yaml @@ -825,6 +825,78 @@ properties: a separate rule for the purposes of triggering notifications. Label keys and corresponding values can be used in notifications generated by this condition. + - !ruby/object:Api::Type::NestedObject + name: conditionPrometheusQueryLanguage + description: | + A Monitoring Query Language query that outputs a boolean stream + + A condition type that allows alert policies to be defined using + Prometheus Query Language (PromQL). + + The PrometheusQueryLanguageCondition message contains information + from a Prometheus alerting rule and its associated rule group. + properties: + - !ruby/object:Api::Type::String + name: query + description: | + The PromQL expression to evaluate. Every evaluation cycle this + expression is evaluated at the current time, and all resultant time + series become pending/firing alerts. This field must not be empty. + required: true + - !ruby/object:Api::Type::String + name: duration + description: | + Alerts are considered firing once their PromQL expression evaluated + to be "true" for this long. Alerts whose PromQL expression was not + evaluated to be "true" for long enough are considered pending. The + default value is zero. Must be zero or positive. + - !ruby/object:Api::Type::String + name: evaluationInterval + required: true + description: | + How often this rule should be evaluated. Must be a positive multiple + of 30 seconds or missing. The default value is 30 seconds. If this + PrometheusQueryLanguageCondition was generated from a Prometheus + alerting rule, then this value should be taken from the enclosing + rule group. + - !ruby/object:Api::Type::KeyValuePairs + name: labels + description: | + Labels to add to or overwrite in the PromQL query result. Label names + must be valid. + + Label values can be templatized by using variables. The only available + variable names are the names of the labels in the PromQL result, including + "__name__" and "value". "labels" may be empty. This field is intended to be + used for organizing and identifying the AlertPolicy + - !ruby/object:Api::Type::String + name: ruleGroup + description: | + The rule group name of this alert in the corresponding Prometheus + configuration file. + + Some external tools may require this field to be populated correctly + in order to refer to the original Prometheus configuration file. + The rule group name and the alert name are necessary to update the + relevant AlertPolicies in case the definition of the rule group changes + in the future. + + This field is optional. If this field is not empty, then it must be a + valid Prometheus label name. + - !ruby/object:Api::Type::String + name: alertRule + description: | + The alerting rule name of this alert in the corresponding Prometheus + configuration file. + + Some external tools may require this field to be populated correctly + in order to refer to the original Prometheus configuration file. + The rule group name and the alert name are necessary to update the + relevant AlertPolicies in case the definition of the rule group changes + in the future. + + This field is optional. If this field is not empty, then it must be a + valid Prometheus label name. required: true - !ruby/object:Api::Type::Array name: 'notificationChannels' From ed33813e707eacb3894553504d76911d5f603273 Mon Sep 17 00:00:00 2001 From: James Edouard Date: Tue, 25 Jul 2023 20:17:55 +0000 Subject: [PATCH 05/10] Added test for promql alerts into the handwritten file --- .../resource_monitoring_alert_policy_test.go | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go b/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go index c9441f0c3fad..8b2363d18736 100644 --- a/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go +++ b/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go @@ -21,6 +21,7 @@ func TestAccMonitoringAlertPolicy(t *testing.T) { "mql": testAccMonitoringAlertPolicy_mql, "log": testAccMonitoringAlertPolicy_log, "forecast": testAccMonitoringAlertPolicy_forecast, + "promql": testAccMonitoringAlertPolicy_promql, } for name, tc := range testCases { @@ -210,6 +211,28 @@ func testAccMonitoringAlertPolicy_forecast(t *testing.T) { }) } +func testAccMonitoringAlertPolicy_promql(t *testing.T) { + + alertName := fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10)) + conditionName := fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10)) + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccCheckAlertPolicyDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccMonitoringAlertPolicy_promqlCfg(alertName, conditionName), + }, + { + ResourceName: "google_monitoring_alert_policy.promql", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func testAccMonitoringAlertPolicy_basicCfg(alertName, conditionName, aligner, filter string) string { return fmt.Sprintf(` resource "google_monitoring_alert_policy" "basic" { @@ -393,3 +416,30 @@ resource "google_monitoring_alert_policy" "forecast" { } `, alertName, conditionName, aligner, filter) } + +func testAccMonitoringAlertPolicy_promqlCfg(alertName, conditionName string) string { + return fmt.Sprintf(` +resource "google_monitoring_alert_policy" "promql" { + display_name = "%s" + combiner = "OR" + enabled = true + + conditions { + display_name = "%s" + + condition_prometheus_query_language { + query = "vector(1)" + duration = "60s" + evaluation_interval = "60s" + alert_rule = "AlwaysOn" + rule_group = "abc" + } + } + + documentation { + content = "test content" + mime_type = "text/markdown" + } +} +`, alertName, conditionName) +} From e269a8060ffa052060dc0e668523ffbeea4f560b Mon Sep 17 00:00:00 2001 From: James Edouard Date: Wed, 26 Jul 2023 02:31:24 +0000 Subject: [PATCH 06/10] Add test for condition_prometheus_query_language.labels --- .../terraform/tests/resource_monitoring_alert_policy_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go b/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go index 8b2363d18736..5ce46ba0b173 100644 --- a/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go +++ b/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go @@ -431,6 +431,9 @@ resource "google_monitoring_alert_policy" "promql" { query = "vector(1)" duration = "60s" evaluation_interval = "60s" + labels = { + "severity" = "page" + } alert_rule = "AlwaysOn" rule_group = "abc" } From b01b178e4f2845e13700b758e98ec381e2022d3e Mon Sep 17 00:00:00 2001 From: Cameron Thornton Date: Wed, 26 Jul 2023 12:01:16 -0500 Subject: [PATCH 07/10] Remove trailing whitespace --- mmv1/products/monitoring/AlertPolicy.yaml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/mmv1/products/monitoring/AlertPolicy.yaml b/mmv1/products/monitoring/AlertPolicy.yaml index cf8b642f6a8c..1cf75857f914 100644 --- a/mmv1/products/monitoring/AlertPolicy.yaml +++ b/mmv1/products/monitoring/AlertPolicy.yaml @@ -829,10 +829,10 @@ properties: name: conditionPrometheusQueryLanguage description: | A Monitoring Query Language query that outputs a boolean stream - + A condition type that allows alert policies to be defined using Prometheus Query Language (PromQL). - + The PrometheusQueryLanguageCondition message contains information from a Prometheus alerting rule and its associated rule group. properties: @@ -864,7 +864,7 @@ properties: description: | Labels to add to or overwrite in the PromQL query result. Label names must be valid. - + Label values can be templatized by using variables. The only available variable names are the names of the labels in the PromQL result, including "__name__" and "value". "labels" may be empty. This field is intended to be @@ -874,13 +874,13 @@ properties: description: | The rule group name of this alert in the corresponding Prometheus configuration file. - + Some external tools may require this field to be populated correctly in order to refer to the original Prometheus configuration file. The rule group name and the alert name are necessary to update the relevant AlertPolicies in case the definition of the rule group changes in the future. - + This field is optional. If this field is not empty, then it must be a valid Prometheus label name. - !ruby/object:Api::Type::String @@ -888,13 +888,13 @@ properties: description: | The alerting rule name of this alert in the corresponding Prometheus configuration file. - + Some external tools may require this field to be populated correctly in order to refer to the original Prometheus configuration file. The rule group name and the alert name are necessary to update the relevant AlertPolicies in case the definition of the rule group changes in the future. - + This field is optional. If this field is not empty, then it must be a valid Prometheus label name. required: true From 7ae3ece1261d28a9a6a86c3944a5dc79110bbdaa Mon Sep 17 00:00:00 2001 From: Cameron Thornton Date: Wed, 26 Jul 2023 12:33:20 -0500 Subject: [PATCH 08/10] Update mmv1/products/monitoring/AlertPolicy.yaml --- mmv1/products/monitoring/AlertPolicy.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mmv1/products/monitoring/AlertPolicy.yaml b/mmv1/products/monitoring/AlertPolicy.yaml index 1cf75857f914..6d1f9deb45f2 100644 --- a/mmv1/products/monitoring/AlertPolicy.yaml +++ b/mmv1/products/monitoring/AlertPolicy.yaml @@ -865,9 +865,9 @@ properties: Labels to add to or overwrite in the PromQL query result. Label names must be valid. - Label values can be templatized by using variables. The only available - variable names are the names of the labels in the PromQL result, including - "__name__" and "value". "labels" may be empty. This field is intended to be + Label values can be templatized by using variables. The only available + variable names are the names of the labels in the PromQL result, including + "__name__" and "value". "labels" may be empty. This field is intended to be used for organizing and identifying the AlertPolicy - !ruby/object:Api::Type::String name: ruleGroup From fe38827155c3d705a35258335d8c43fa4af50c42 Mon Sep 17 00:00:00 2001 From: James Edouard Date: Fri, 4 Aug 2023 20:37:12 +0000 Subject: [PATCH 09/10] Make condition_prometheus_query_language.evaluation_interval optional --- mmv1/products/monitoring/AlertPolicy.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/mmv1/products/monitoring/AlertPolicy.yaml b/mmv1/products/monitoring/AlertPolicy.yaml index 6d1f9deb45f2..6ec4e791e16f 100644 --- a/mmv1/products/monitoring/AlertPolicy.yaml +++ b/mmv1/products/monitoring/AlertPolicy.yaml @@ -852,7 +852,6 @@ properties: default value is zero. Must be zero or positive. - !ruby/object:Api::Type::String name: evaluationInterval - required: true description: | How often this rule should be evaluated. Must be a positive multiple of 30 seconds or missing. The default value is 30 seconds. If this From 5d3d5376305e1e63a3f2f97518423b1e439eb698 Mon Sep 17 00:00:00 2001 From: James Edouard Date: Sat, 5 Aug 2023 00:17:21 +0000 Subject: [PATCH 10/10] Remove test for condition_prometheus_query_language.evaluation_interval --- .../terraform/tests/resource_monitoring_alert_policy_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go b/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go index 5ce46ba0b173..758bdaefe8b7 100644 --- a/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go +++ b/mmv1/third_party/terraform/tests/resource_monitoring_alert_policy_test.go @@ -430,7 +430,6 @@ resource "google_monitoring_alert_policy" "promql" { condition_prometheus_query_language { query = "vector(1)" duration = "60s" - evaluation_interval = "60s" labels = { "severity" = "page" }