From 797d83f997fd48bf329087401dacd60bbe7a41b4 Mon Sep 17 00:00:00 2001 From: Jakub Gruszecki Date: Thu, 5 Dec 2024 16:23:13 +0100 Subject: [PATCH] feat: PC-13893 Deprecate usage of objective's value field for Composite SLOs (#549) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Motivation Currently, we have `value` field required for all objectives in an SLO. We currently allow to not pass that field in YAML at all but it causes it to default to 0 anyway and is returned `value: 0`. This is inconsistent because GET yaml is different than APPLY YAML in such case. Composite SLOs always have exactly one objective, so as such `value` doesn’t matter for them at all as long as it doesn't change. Moreover, the existence of value is documented and used in examples which is confusing to new adopters of Composite SLOs because it is required, changing it will restart budget, but it doesn’t do anything. We want to encourage and allow not setting `value` field for Composite SLOs while maintaining backward compatibility with users who perhaps already explicitly set it. ## Summary * `value` will be omitted if it is `null` in API. * `value` field is omitted in all Composite SLO examples and E2E tests. ## Related changes https://github.com/nobl9/nobl9-go/pull/551 https://github.com/nobl9/nobl9-go/pull/549 https://github.com/nobl9/n9/pull/15406 https://github.com/nobl9/terraform-provider-nobl9/pull/312 https://github.com/nobl9/terraform-provider-nobl9/pull/295 ## Testing ```go package main import ( "context" "github.com/nobl9/nobl9-go/manifest" "github.com/nobl9/nobl9-go/manifest/v1alpha/service" "github.com/nobl9/nobl9-go/manifest/v1alpha/slo" "github.com/nobl9/nobl9-go/manifest/v1alpha/twindow" "github.com/nobl9/nobl9-go/sdk" v1 "github.com/nobl9/nobl9-go/sdk/endpoints/objects/v1" "log" "time" ) func main() { ctx := context.Background() client, err := sdk.DefaultClient() if err != nil { log.Fatalf("failed to create sdk client, err: %v", err) } const project = "value-test" svc := service.New(service.Metadata{Name: "my-service", Project: project}, service.Spec{}) composite := slo.New(slo.Metadata{Name: "my-slo", Project: project}, slo.Spec{ BudgetingMethod: slo.BudgetingMethodOccurrences.String(), Objectives: []slo.Objective{ { ObjectiveBase: slo.ObjectiveBase{ Name: "objective-1", // Look MA! No value! }, BudgetTarget: ptr(0.99), Composite: &slo.CompositeSpec{ MaxDelay: time.Hour.String(), Components: slo.Components{ Objectives: []slo.CompositeObjective{}, }, }, }, }, Service: svc.Metadata.Name, TimeWindows: []slo.TimeWindow{ { Unit: twindow.Day.String(), Count: 1, IsRolling: true, }, }, }) objects := []manifest.Object{svc, composite} if err := manifest.Validate(objects); err != nil { log.Fatalf("failed to validate manifest, err: %v", err) } if err := client.Objects().V1().Apply(ctx, objects); err != nil { log.Fatalf("failed to apply objects, err: %v", err) } slos, err := client.Objects().V1().GetV1alphaSLOs(ctx, v1.GetSLOsRequest{ Names: []string{composite.GetName()}, Project: project, }) if err != nil { log.Fatalf("failed to get slo, err: %v", err) } if slos[0].Spec.Objectives[0].Value != nil { log.Fatalf("expected nil, got %v", *slos[0].Spec.Objectives[0].Value) } log.Println("Composite SLO's value is nil") } func ptr[T any](t T) *T { return &t } ``` ## Release Notes Usage of `spec.objective[0].value` field for Composite SLOs becomes deprecated. * New Composite SLOs should not set `value` field. * New Composite SLOs will still accept `value: 0` for backward compatibility with older versions of Nobl9 SDK and Nobl9 Terrafrom Provider. * If `value` was previously set to `0` for Composite SLO then it should be omitted going forward. * If `value` was previously set in Composite SLO to a number other than `0` then it can no longer be updated but still will be accepted for backward compatibility. The usage of `value` for SLOs using ratio or threshold SLIs does not change. --- internal/manifest/v1alpha/examples/slo_composite.go | 1 - manifest/v1alpha/slo/examples/composite-slo.yaml | 4 ---- manifest/v1alpha/slo/slo.go | 2 +- sdk/test_data/reader/expected/composite_v2_slo.tpl.json | 1 - 4 files changed, 1 insertion(+), 7 deletions(-) diff --git a/internal/manifest/v1alpha/examples/slo_composite.go b/internal/manifest/v1alpha/examples/slo_composite.go index 13119005..c9cda79f 100644 --- a/internal/manifest/v1alpha/examples/slo_composite.go +++ b/internal/manifest/v1alpha/examples/slo_composite.go @@ -74,7 +74,6 @@ func (s sloCompositeExample) SLO() v1alphaSLO.SLO { { ObjectiveBase: v1alphaSLO.ObjectiveBase{ DisplayName: "User experience", - Value: ptr(0.0), Name: "user-experience", }, BudgetTarget: ptr(0.95), diff --git a/manifest/v1alpha/slo/examples/composite-slo.yaml b/manifest/v1alpha/slo/examples/composite-slo.yaml index cd7b6d56..aada8be7 100644 --- a/manifest/v1alpha/slo/examples/composite-slo.yaml +++ b/manifest/v1alpha/slo/examples/composite-slo.yaml @@ -29,7 +29,6 @@ budgetingMethod: Occurrences objectives: - displayName: User experience - value: 0.0 name: user-experience target: 0.95 composite: @@ -96,7 +95,6 @@ budgetingMethod: Occurrences objectives: - displayName: User experience - value: 0.0 name: user-experience target: 0.95 composite: @@ -160,7 +158,6 @@ budgetingMethod: Timeslices objectives: - displayName: User experience - value: 0.0 name: user-experience target: 0.95 timeSliceTarget: 0.9 @@ -228,7 +225,6 @@ budgetingMethod: Timeslices objectives: - displayName: User experience - value: 0.0 name: user-experience target: 0.95 timeSliceTarget: 0.9 diff --git a/manifest/v1alpha/slo/slo.go b/manifest/v1alpha/slo/slo.go index e686f2f1..bd1142ce 100644 --- a/manifest/v1alpha/slo/slo.go +++ b/manifest/v1alpha/slo/slo.go @@ -71,7 +71,7 @@ type Attachment struct { // ObjectiveBase base structure representing an objective. type ObjectiveBase struct { DisplayName string `json:"displayName"` - Value *float64 `json:"value"` + Value *float64 `json:"value,omitempty"` Name string `json:"name"` NameChanged bool `json:"-"` } diff --git a/sdk/test_data/reader/expected/composite_v2_slo.tpl.json b/sdk/test_data/reader/expected/composite_v2_slo.tpl.json index fd7f7f86..303f745e 100644 --- a/sdk/test_data/reader/expected/composite_v2_slo.tpl.json +++ b/sdk/test_data/reader/expected/composite_v2_slo.tpl.json @@ -12,7 +12,6 @@ "objectives": [ { "displayName": "composite-obj", - "value": null, "name": "composite-obj", "target": 0.99, "composite": {