nobl9 / nobl9-go

Nobl9 SDK for Go!
https://www.nobl9.com/
Mozilla Public License 2.0
27 stars 1 forks source link

feat: PC-13893 Deprecate usage of objective's value field for Composite SLOs #549

Open ditrytus opened 1 month ago

ditrytus commented 1 month ago

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

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

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.

The usage of value for SLOs using ratio or threshold SLIs does not change.