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

feat(slo): Update Terraform provider to include option to set min Failures #1660

Merged
merged 22 commits into from
Jul 7, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
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
9 changes: 9 additions & 0 deletions docs/data-sources/slos.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,20 @@ Read-Only:

Read-Only:

- `advanced_options` (List of Object) (see [below for nested schema](#nestedobjatt--slos--alerting--advanced_options))
- `annotation` (List of Object) (see [below for nested schema](#nestedobjatt--slos--alerting--annotation))
- `fastburn` (List of Object) (see [below for nested schema](#nestedobjatt--slos--alerting--fastburn))
- `label` (List of Object) (see [below for nested schema](#nestedobjatt--slos--alerting--label))
- `slowburn` (List of Object) (see [below for nested schema](#nestedobjatt--slos--alerting--slowburn))

<a id="nestedobjatt--slos--alerting--advanced_options"></a>
### Nested Schema for `slos.alerting.advanced_options`

Read-Only:

- `min_failures` (Number)


<a id="nestedobjatt--slos--alerting--annotation"></a>
### Nested Schema for `slos.alerting.annotation`

Expand Down
9 changes: 9 additions & 0 deletions docs/resources/slo.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,20 @@ Optional:

Optional:

- `advanced_options` (Block List, Max: 1) Advanced Options for Alert Rules (see [below for nested schema](#nestedblock--alerting--advanced_options))
- `annotation` (Block List) Annotations will be attached to all alerts generated by any of these rules. (see [below for nested schema](#nestedblock--alerting--annotation))
- `fastburn` (Block List, Max: 1) Alerting Rules generated for Fast Burn alerts (see [below for nested schema](#nestedblock--alerting--fastburn))
- `label` (Block List) Labels will be attached to all alerts generated by any of these rules. (see [below for nested schema](#nestedblock--alerting--label))
- `slowburn` (Block List, Max: 1) Alerting Rules generated for Slow Burn alerts (see [below for nested schema](#nestedblock--alerting--slowburn))

<a id="nestedblock--alerting--advanced_options"></a>
### Nested Schema for `alerting.advanced_options`

Optional:

- `min_failures` (Number) Minimum number of failed events to trigger an alert


<a id="nestedblock--alerting--annotation"></a>
### Nested Schema for `alerting.annotation`

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
resource "grafana_slo" "ratio_options" {
name = "Terraform Testing - Ratio Query"
description = "Terraform Description - Ratio Query"
query {
ratio {
success_metric = "kubelet_http_requests_total{status!~\"5..\"}"
total_metric = "kubelet_http_requests_total"
group_by_labels = ["job", "instance"]
}
type = "ratio"
}
objectives {
value = 0.995
window = "30d"
}
destination_datasource {
uid = "grafanacloud-prom"
}
label {
key = "slo"
value = "terraform"
}
alerting {
fastburn {
annotation {
key = "name"
value = "SLO Burn Rate Very High"
}
annotation {
key = "description"
value = "Error budget is burning too fast"
}
}

slowburn {
annotation {
key = "name"
value = "SLO Burn Rate High"
}
annotation {
key = "description"
value = "Error budget is burning too fast"
}
}
advanced_options {
min_failures = 10
}
}
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ require (
github.com/grafana/grafana-com-public-clients/go/gcom v0.0.0-20240322153219-42c6a1d2bcab
github.com/grafana/grafana-openapi-client-go v0.0.0-20240523010106-657d101fcbd9
github.com/grafana/machine-learning-go-client v0.7.0
github.com/grafana/slo-openapi-client/go v0.0.0-20240507015908-bf9e85638f2f
github.com/grafana/slo-openapi-client/go v0.0.0-20240626093634-e6741482b090
github.com/grafana/synthetic-monitoring-agent v0.24.3
github.com/grafana/synthetic-monitoring-api-go-client v0.8.0
github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ github.com/grafana/otel-profiling-go v0.5.1 h1:stVPKAFZSa7eGiqbYuG25VcqYksR6iWvF
github.com/grafana/otel-profiling-go v0.5.1/go.mod h1:ftN/t5A/4gQI19/8MoWurBEtC6gFw8Dns1sJZ9W4Tls=
github.com/grafana/pyroscope-go/godeltaprof v0.1.7 h1:C11j63y7gymiW8VugJ9ZW0pWfxTZugdSJyC48olk5KY=
github.com/grafana/pyroscope-go/godeltaprof v0.1.7/go.mod h1:Tk376Nbldo4Cha9RgiU7ik8WKFkNpfds98aUzS8omLE=
github.com/grafana/slo-openapi-client/go v0.0.0-20240507015908-bf9e85638f2f h1:aUDhr6LmO2W08tEP0Xe694DRfYZjlGp9kUnbeAF+194=
github.com/grafana/slo-openapi-client/go v0.0.0-20240507015908-bf9e85638f2f/go.mod h1:HgbbeH2gFfCk2XZCrCly39DB13WkwWyQ+Ww+HTxePCs=
github.com/grafana/slo-openapi-client/go v0.0.0-20240626093634-e6741482b090 h1:gDkJPpTL84zx+UkSY6a1pPlUm9aDEVBzPlVOkUbXmgM=
github.com/grafana/slo-openapi-client/go v0.0.0-20240626093634-e6741482b090/go.mod h1:HgbbeH2gFfCk2XZCrCly39DB13WkwWyQ+Ww+HTxePCs=
github.com/grafana/synthetic-monitoring-agent v0.24.3 h1:+xscAsGZtWTNTNDxdYqqcz4w1tG6QPaOIgCONsVMoO8=
github.com/grafana/synthetic-monitoring-agent v0.24.3/go.mod h1:CJQmPtKRcJMjb/sDe6fDA4vyS2qFPElu0szI33nKlzk=
github.com/grafana/synthetic-monitoring-api-go-client v0.8.0 h1:Tm4MtwwYmPNInGfnj66l6j6KOshMkNV4emIVKJdlXMg=
Expand Down
16 changes: 16 additions & 0 deletions internal/resources/slo/data_source_slo.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ func unpackAlerting(alertData *slo.SloV00Alerting) []map[string]interface{} {
alertObject["slowburn"] = unpackAlertingMetadata(*alertData.SlowBurn)
}

if alertData.AdvancedOptions != nil {
alertObject["advanced_options"] = unpackAdvancedOptions(*alertData.AdvancedOptions)
}

retAlertData = append(retAlertData, alertObject)
return retAlertData
}
Expand All @@ -212,6 +216,18 @@ func unpackAlertingMetadata(metaData slo.SloV00AlertingMetadata) []map[string]in
return retAlertMetaData
}

func unpackAdvancedOptions(options slo.SloV00AdvancedOptions) []map[string]int {
ellisda marked this conversation as resolved.
Show resolved Hide resolved
retAdvancedOptions := []map[string]int{}
minFailuresStruct := make(map[string]int)

if options.MinFailures != nil {
minFailuresStruct["min_failures"] = int(*options.MinFailures)
}

retAdvancedOptions = append(retAdvancedOptions, minFailuresStruct)
return retAdvancedOptions
}

func unpackDestinationDatasource(destinationDatasource *slo.SloV00DestinationDatasource) []map[string]interface{} {
retDestinationDatasources := []map[string]interface{}{}

Expand Down
32 changes: 31 additions & 1 deletion internal/resources/slo/resource_slo.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,22 @@ Resource manages Grafana SLOs.
},
},
},
"advanced_options": {
Type: schema.TypeList,
MaxItems: 1,
ellisda marked this conversation as resolved.
Show resolved Hide resolved
Optional: true,
Description: "Advanced Options for Alert Rules",
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"min_failures": {
Type: schema.TypeInt,
Optional: true,
Description: "Minimum number of failed events to trigger an alert",
ValidateFunc: validation.IntAtLeast(0),
},
},
},
},
},
},
},
Expand Down Expand Up @@ -393,7 +409,6 @@ func packSloResource(d *schema.ResourceData) (slo.SloV00Slo, error) {
tfalerting = packAlerting(alert)
}
}

req.Alerting = &tfalerting
}

Expand Down Expand Up @@ -524,6 +539,7 @@ func packAlerting(tfAlerting map[string]interface{}) slo.SloV00Alerting {
var tfLabels []slo.SloV00Label
var tfFastBurn slo.SloV00AlertingMetadata
var tfSlowBurn slo.SloV00AlertingMetadata
var tfAdvancedOptions slo.SloV00AdvancedOptions

annots, ok := tfAlerting["annotation"].([]interface{})
if ok {
Expand Down Expand Up @@ -552,6 +568,20 @@ func packAlerting(tfAlerting map[string]interface{}) slo.SloV00Alerting {
SlowBurn: &tfSlowBurn,
}

// All options in advanced options will be optional
// Adding a second feature will need to make a better way of checking what is there
if failures := tfAlerting["advanced_options"]; failures != nil {
lf := failures.([]interface{})
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if failures := tfAlerting["advanced_options"]; failures != nil {
lf := failures.([]interface{})
if advancedOptions, ok := tfAlerting["advanced_options"].([]interface{}); ok {

it seems that the blocks above this all follow the same pattern with type assertion on the "if" line. Can we not follow that pattern here? Note: the suggestion is using []interface{} not []int (type depends on whether you take suggestion above)

if len(lf) > 0 {
lf2 := lf[0].(map[string]interface{})
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: this will panic if the runtime value of lf[0] fails the type assertion.

Don't you need to use the lf2, ok := lf[0].(map[string]interface{}) version and check ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting point. Yeah I should fix that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We actually do this exact same thing for the alerting struct as a whole, the terraform can panic if it doesn't match the expected format. Think I should switch that too?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, if it's easy. If we can handle more gracefully than panic, we should.

where else are we doing it though? The only uncheecked type assertions I see in this function are on line 574 and 576

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually misunderstood. the alerting struct handles it all gracefully. I've replicated it now here.

i64 := int64(lf2["min_failures"].(int))
tfAdvancedOptions = slo.SloV00AdvancedOptions{
MinFailures: &i64,
}
alerting.SetAdvancedOptions(tfAdvancedOptions)
}
}

return alerting
}

Expand Down
34 changes: 34 additions & 0 deletions internal/resources/slo/resource_slo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,16 @@ func TestAccResourceSlo(t *testing.T) {
resource.TestCheckResourceAttr("grafana_slo.ratio", "query.0.ratio.0.group_by_labels.1", "instance"),
),
},
{
// Tests Advanced Options
Config: testutils.TestAccExample(t, "resources/grafana_slo/resource_ratio_advanced_options.tf"),
Check: resource.ComposeTestCheckFunc(
testAccSloCheckExists("grafana_slo.ratio_options", &slo),
testAlertingExists(true, "grafana_slo.ratio_options", &slo),
testAdvancedOptionsExists(true, "grafana_slo.ratio_options", &slo),
resource.TestCheckResourceAttr("grafana_slo.ratio_options", "alerting.0.advanced_options.0.min_failures", "10"),
),
},
},
})
}
Expand Down Expand Up @@ -150,6 +160,30 @@ func testAlertingExists(expectation bool, rn string, slo *slo.SloV00Slo) resourc
}
}

func testAdvancedOptionsExists(expectation bool, rn string, slo *slo.SloV00Slo) resource.TestCheckFunc {
Copy link
Member

Choose a reason for hiding this comment

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

Heh, I guess this works but this could be tested without an additional test function. An import test would've tested that things are written correctly too (it wipes the state and checks that all attributes would be set the same on an import)

return func(s *terraform.State) error {
rs := s.RootModule().Resources[rn]
client := testutils.Provider.Meta().(*common.Client).SLOClient
req := client.DefaultAPI.V1SloIdGet(context.Background(), rs.Primary.ID)
gotSlo, _, err := req.Execute()

if err != nil {
return fmt.Errorf("error getting SLO: %s", err)
}
*slo = *gotSlo

if slo.Alerting.AdvancedOptions == nil && expectation == false {
return nil
}

if slo.Alerting.AdvancedOptions != nil && expectation == true {
return nil
}

return fmt.Errorf("SLO Advanced Options expectation mismatch")
}
}

func testAccSloCheckDestroy(slo *slo.SloV00Slo) resource.TestCheckFunc {
return func(s *terraform.State) error {
client := testutils.Provider.Meta().(*common.Client).SLOClient
Expand Down
Loading