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: PC-13893 Deprecate usage of objective's value field for Composite SLOs #295

Merged
merged 17 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ BIN_DIR=./bin
BINARY=$(BIN_DIR)/terraform-provider-$(NAME)
VERSION=0.32.1
BUILD_FLAGS="-X github.com/nobl9/terraform-provider-nobl9/nobl9.Version=$(VERSION)"
OS_ARCH?=linux_amd64
OS_ARCH?=darwin_arm64

# renovate datasource=github-releases depName=securego/gosec
GOSEC_VERSION := v2.21.4
Expand Down
3 changes: 1 addition & 2 deletions docs/resources/slo.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ resource "nobl9_slo" "composite_slo" {
display_name = "OK"
name = "tf-objective-1"
target = 0.8
value = 1
composite {
max_delay = "45m"
components {
Expand Down Expand Up @@ -227,7 +226,6 @@ resource "nobl9_slo" "composite_slo" {
Required:

- `target` (Number) The numeric target for your objective.
- `value` (Number) For threshold metrics, the threshold value. For ratio metrics, for legacy reasons, this must be a unique value per objective.

Optional:

Expand All @@ -239,6 +237,7 @@ Optional:
- `primary` (Boolean) Is objective marked as primary.
- `raw_metric` (Block Set) Raw data is used to compare objective values. (see [below for nested schema](#nestedblock--objective--raw_metric))
- `time_slice_target` (Number) Designated value for slice.
- `value` (Number) Required for threshold and ratio metrics. Optional for composite SLOs. For thresholdmetrics, the threshold value. For ratio metrics, for legacy reasons, this must be a unique valueper objective. For composite SLOs it should be omitted. If, for composite SLO, it was setpreviously to a non zero value then it should remain set to that value.

<a id="nestedblock--objective--composite"></a>
### Nested Schema for `objective.composite`
Expand Down
1 change: 0 additions & 1 deletion examples/resources/nobl9_slo/resource.tf
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ resource "nobl9_slo" "composite_slo" {
display_name = "OK"
name = "tf-objective-1"
target = 0.8
value = 1
composite {
max_delay = "45m"
components {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ require (
github.com/hashicorp/go-cty v1.4.1-0.20200723130312-85980079f637
github.com/hashicorp/terraform-plugin-docs v0.19.4
github.com/hashicorp/terraform-plugin-sdk/v2 v2.34.0
github.com/nobl9/nobl9-go v0.90.1
github.com/nobl9/nobl9-go v0.92.0
github.com/stretchr/testify v1.9.0
github.com/teambition/rrule-go v1.8.2
)
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ github.com/nobl9/go-yaml v1.0.1 h1:Aj1kSaYdRQTKlvS6ihvXzQJhCpoHhtf9nfA95zqWH4Q=
github.com/nobl9/go-yaml v1.0.1/go.mod h1:t7vCO8ctYdBweZxU5lUgxzAw31+ZcqJYeqRtrv+5RHI=
github.com/nobl9/govy v0.9.1 h1:ht+mgbDB40QEZEmHpzG2xqey+PbfzCwgj3+vdJuHAVQ=
github.com/nobl9/govy v0.9.1/go.mod h1:O+xSiKwZ6gs/orRvH5qLkfkgyT7CkuXprRIq3C5uNXQ=
github.com/nobl9/nobl9-go v0.90.1 h1:OX/0jVrklISXk4Q9FiMcX/SaopLxwXuFNFgvPPsc1fY=
github.com/nobl9/nobl9-go v0.90.1/go.mod h1:SWvmmfSGCairnWDCdRLusRB0zROxF+7XiA9aho2f7rA=
github.com/nobl9/nobl9-go v0.92.0 h1:pw2PiBR60V7tpojN4QGEeJzuNMZx1h8kwOp/dmxz49Q=
github.com/nobl9/nobl9-go v0.92.0/go.mod h1:SWvmmfSGCairnWDCdRLusRB0zROxF+7XiA9aho2f7rA=
github.com/oklog/run v1.1.0 h1:GEenZ1cK0+q0+wsJew9qUg/DyD8k3JzYsZAi5gYi2mA=
github.com/oklog/run v1.1.0/go.mod h1:sVPdnTZT1zYwAJeCMu2Th4T21pA3FPOQRfWjQlk7DVU=
github.com/pjbgf/sha1cd v0.3.0 h1:4D5XXmUUBUl/xQ6IjCkEAbqXskkq/4O7LmGn0AqMDs4=
Expand Down
30 changes: 20 additions & 10 deletions nobl9/resource_slo.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,12 @@ func resourceObjective() *schema.Resource {
Description: "Designated value for slice.",
},
"value": {
Type: schema.TypeFloat,
Required: true,
Description: "For threshold metrics, the threshold value. For ratio metrics, for legacy reasons, this must be a unique value per objective.",
Type: schema.TypeFloat,
Optional: true,
Description: "Required for threshold and ratio metrics. Optional for composite SLOs. For threshold" +
"metrics, the threshold value. For ratio metrics, for legacy reasons, this must be a unique value" +
"per objective. For composite SLOs it should be omitted. If, for composite SLO, it was set" +
"previously to a non zero value then it should remain set to that value.",
},
"name": {
Type: schema.TypeString,
Expand Down Expand Up @@ -362,8 +365,7 @@ func resourceSLOApply(
resultSlo := manifest.SetDefaultProject([]manifest.Object{slo}, config.Project)

if err := retry.RetryContext(ctx, d.Timeout(schema.TimeoutCreate)-time.Minute, func() *retry.RetryError {
err := client.Objects().V1().Apply(ctx, resultSlo)
if err != nil {
if err := client.Objects().V1().Apply(ctx, resultSlo); err != nil {
if errors.Is(err, errConcurrencyIssue) {
return retry.RetryableError(err)
}
Expand Down Expand Up @@ -595,11 +597,11 @@ func marshalIndicator(d *schema.ResourceData) *v1alphaSLO.Indicator {
}

func marshalObjectives(d *schema.ResourceData) ([]v1alphaSLO.Objective, diag.Diagnostics) {
objectivesSchema := d.Get("objective").(*schema.Set).List()
objectivesSchemaSet := d.Get("objective").(*schema.Set)
objectivesSchema := objectivesSchemaSet.List()
objectives := make([]v1alphaSLO.Objective, len(objectivesSchema))
for i, o := range objectivesSchema {
objective := o.(map[string]interface{})
value := objective["value"].(float64)
target := objective["target"].(float64)
timeSliceTarget := objective["time_slice_target"].(float64)
var timeSliceTargetPtr *float64
Expand All @@ -612,11 +614,16 @@ func marshalObjectives(d *schema.ResourceData) ([]v1alphaSLO.Objective, diag.Dia
if err != nil {
return nil, diag.FromErr(err)
}

var valuePtr *float64
value := objective["value"].(float64)
valuePtr = &value
if value == 0 && compositeSpec != nil {
valuePtr = nil
}
objectives[i] = v1alphaSLO.Objective{
ObjectiveBase: v1alphaSLO.ObjectiveBase{
DisplayName: objective["display_name"].(string),
Value: &value,
Value: valuePtr,
Name: objective["name"].(string),
},
BudgetTarget: &target,
Expand Down Expand Up @@ -861,7 +868,9 @@ func unmarshalObjectives(d *schema.ResourceData, spec v1alphaSLO.Spec) error {
objectiveTF["name"] = objective.Name
objectiveTF["display_name"] = objective.DisplayName
objectiveTF["op"] = objective.Operator
objectiveTF["value"] = objective.Value
if objective.Value != nil && (*objective.Value != 0 || objective.Composite == nil) {
objectiveTF["value"] = objective.Value
}
objectiveTF["target"] = objective.BudgetTarget
objectiveTF["time_slice_target"] = objective.TimeSliceTarget
objectiveTF["primary"] = objective.Primary
Expand Down Expand Up @@ -1841,6 +1850,7 @@ func unmarshalHoneycombMetric(metric interface{}) map[string]interface{} {
return nil
}
res := make(map[string]interface{})
// nolint: staticcheck
res["calculation"] = hMetric.Calculation
res["attribute"] = hMetric.Attribute
return res
Expand Down
77 changes: 75 additions & 2 deletions nobl9/resource_slo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func TestAcc_Nobl9SLO(t *testing.T) {
{"test-composite-time-slices-deprecated", testCompositeSLOTimeSlicesDeprecated},
{"test-composite-occurrences", testCompositeSLOOccurrences},
{"test-composite-time-slices", testCompositeSLOTimeSlices},
{"test-composite-with-value", testCompositeSLOValueZeroBackwardCompatibility},
{"test-datadog", testDatadogSLO},
{"test-dynatrace", testDynatraceSLO},
{"test-grafanaloki", testGrafanaLokiSLO},
Expand Down Expand Up @@ -1023,7 +1024,6 @@ resource "nobl9_slo" ":name" {
display_name = "obj1"
name = "tf-objective-1"
target = 0.7
value = 0
composite {
max_delay = "45m"
components {
Expand Down Expand Up @@ -1085,7 +1085,7 @@ resource "nobl9_slo" ":name" {
display_name = "obj1"
name = "tf-objective-1"
target = 0.7
value = 0
value = 1
time_slice_target = 0.7
composite {
max_delay = "45m"
Expand Down Expand Up @@ -1126,6 +1126,79 @@ resource "nobl9_slo" ":name" {
return config
}

func testCompositeSLOValueZeroBackwardCompatibility(name string) string {
var serviceName = name + "-tf-service"
var agentName = name + "-tf-agent"
var sloDependencyName = name + "-dependency-slo-1"
config :=
testService(serviceName) +
testPrometheusAgent(agentName) +
testCompositeDependencySLO(serviceName, agentName, sloDependencyName) + `
resource "nobl9_slo" ":name" {
name = ":name"
display_name = ":name"
project = ":project"
service = nobl9_service.:serviceName.name


depends_on = [nobl9_slo.:sloDependencyName]

label {
key = "team"
values = ["green","sapphire"]
}

label {
key = "env"
values = ["dev", "staging", "prod"]
}

budgeting_method = "Occurrences"

objective {
display_name = "obj1"
name = "tf-objective-1"
target = 0.7
value = 0
composite {
max_delay = "45m"
components {
objectives {
composite_objective {
project = ":project"
slo = ":sloDependencyName"
objective = "objective-1"
weight = 0.8
when_delayed = "CountAsGood"
}
composite_objective {
project = ":project"
slo = ":sloDependencyName"
objective = "objective-2"
weight = 1.0
when_delayed = "CountAsBad"
}
}
}
}
}

time_window {
count = 10
is_rolling = true
unit = "Minute"
}
}
`
config = strings.ReplaceAll(config, ":name", name)
config = strings.ReplaceAll(config, ":serviceName", serviceName)
config = strings.ReplaceAll(config, ":agentName", agentName)
config = strings.ReplaceAll(config, ":project", testProject)
config = strings.ReplaceAll(config, ":sloDependencyName", sloDependencyName)

return config
}

func testDatadogSLO(name string) string {
var serviceName = name + "-tf-service"
var agentName = name + "-tf-agent"
Expand Down
Loading