diff --git a/internal/fmt/fmt.go b/internal/fmt/fmt.go index 7b63479..802f570 100644 --- a/internal/fmt/fmt.go +++ b/internal/fmt/fmt.go @@ -34,7 +34,9 @@ func Files(out io.Writer, sources []string) error { return err } if i != count-1 { - fmt.Fprintln(out, "---") + if _, err := fmt.Fprintln(out, "---"); err != nil { + return err + } } } return nil diff --git a/pkg/manifest/v1/indicator.go b/pkg/manifest/v1/indicator.go index 844456e..4c45ed4 100644 --- a/pkg/manifest/v1/indicator.go +++ b/pkg/manifest/v1/indicator.go @@ -70,15 +70,15 @@ type MetricSource struct { MetricSourceSpec map[string]string `yaml:"spec" validate:"required_without=MetricSourceRef"` } -// UnmarshalYAML is used to override the default unmarshal behavior -// Since MetricSources don't have a determined structure, we need to do a few things here: -// 1. Pull out the MetricSourceRef and Type separately, and add them to the MetricSource -// 2. Attempt to unmarshal the MetricSourceSpec, which can be either a string or an array. -// 2a. If its a string, add it as a single string -// 2b. If its an array, flatten it to a single string -// -// This also assumes a certain flat structure that we can revisit if the need arises. +// UnmarshalYAML is used to override the default unmarshal behavior. +// MetricSources can have varying structures, so this method performs the following tasks: +// 1. Extracts the MetricSourceRef and Type separately, and assigns them to the MetricSource. +// 2. Attempts to unmarshal the MetricSourceSpec, which can be either a string, a list, or a more complex structure: +// 2a. If it's a scalar string, it is added directly as a single string. +// 2b. If it's a list of scalar values, they are concatenated into a single semicolon separated string. +// 2c. If it's a more complex sequence (e.g., containing mappings), the values are processed and flattened appropriately. // +// The current implementation assumes a relatively flat structure but can be adapted if more complex nesting becomes necessary. func (m *MetricSource) UnmarshalYAML(value *yaml.Node) error { // temp struct to unmarshal the string values @@ -108,7 +108,9 @@ func (m *MetricSource) UnmarshalYAML(value *yaml.Node) error { // top level string that we will join with a semicolon seqStrings := []string{} for _, node := range v.Content { - if node.Kind == yaml.MappingNode { + if node.Kind == yaml.ScalarNode { + seqStrings = append(seqStrings, node.Value) + } else if node.Kind == yaml.MappingNode { // each of these are k/v pairs that we will join with a comma kvPairs := []string{} for i := 0; i < len(node.Content); i += 2 { diff --git a/pkg/manifest/v1/indicator_test.go b/pkg/manifest/v1/indicator_test.go index 4e8bbde..b987faf 100644 --- a/pkg/manifest/v1/indicator_test.go +++ b/pkg/manifest/v1/indicator_test.go @@ -81,6 +81,24 @@ spec: }, }, }, + { + name: "Prometheus - Two labels", + yaml: `metricSourceRef: thanos +type: Prometheus +spec: + query: http_requests_total{} + dimensions: + - following + - another`, + want: MetricSource{ + MetricSourceRef: "thanos", + Type: "Prometheus", + MetricSourceSpec: map[string]string{ + "query": "http_requests_total{}", + "dimensions": "following;another", + }, + }, + }, } for _, tt := range tests { diff --git a/pkg/manifest/v1/objective.go b/pkg/manifest/v1/objective.go index 27dd2d2..3015084 100644 --- a/pkg/manifest/v1/objective.go +++ b/pkg/manifest/v1/objective.go @@ -32,19 +32,22 @@ type TimeWindow struct { // Objective represents single threshold for SLO, for internal usage. type Objective struct { - DisplayName string `yaml:"displayName,omitempty"` - Op string `yaml:"op,omitempty" example:"lte"` - Value float64 `yaml:"value,omitempty" validate:"numeric,omitempty"` - Target float64 `yaml:"target" validate:"required,numeric,gte=0,lt=1" example:"0.9"` - TimeSliceTarget float64 `yaml:"timeSliceTarget,omitempty" validate:"gte=0,lte=1,omitempty" example:"0.9"` - TimeSliceWindow string `yaml:"timeSliceWindow,omitempty" example:"5m"` + DisplayName string `yaml:"displayName,omitempty"` + Op string `yaml:"op,omitempty" example:"lte"` + Value float64 `yaml:"value,omitempty" validate:"numeric,omitempty"` + Target float64 `yaml:"target" validate:"required,numeric,gte=0,lt=1" example:"0.9"` + TimeSliceTarget float64 `yaml:"timeSliceTarget,omitempty" validate:"gte=0,lte=1,omitempty" example:"0.9"` + TimeSliceWindow string `yaml:"timeSliceWindow,omitempty" example:"5m"` + Indicator *SLIInline `yaml:"indicator,omitempty"` + IndicatorRef *string `yaml:"indicatorRef,omitempty"` + CompositeWeight float64 `yaml:"compositeWeight,omitempty" validate:"gte=0,omitempty"` } // SLOSpec struct which mapped one to one with kind: slo yaml definition, internal use. type SLOSpec struct { Description string `yaml:"description,omitempty" validate:"max=1050,omitempty"` Service string `yaml:"service" validate:"required" example:"webapp-service"` - Indicator *SLIInline `yaml:"indicator,omitempty" validate:"required_without=IndicatorRef"` + Indicator *SLIInline `yaml:"indicator,omitempty"` IndicatorRef *string `yaml:"indicatorRef,omitempty"` BudgetingMethod string `yaml:"budgetingMethod" validate:"required,oneof=Occurrences Timeslices" example:"Occurrences"` //nolint:lll TimeWindow []TimeWindow `yaml:"timeWindow" validate:"required,len=1,dive"` diff --git a/pkg/validate/validate_test.go b/pkg/validate/validate_test.go index 2af9cc1..c727bef 100644 --- a/pkg/validate/validate_test.go +++ b/pkg/validate/validate_test.go @@ -180,6 +180,8 @@ func Test_validateFiles(t *testing.T) { "../../test/v1/slo/slo-no-indicatorref-calendar-no-alerts.yaml", "../../test/v1/slo/slo-no-indicatorref-rolling-alerts.yaml", "../../test/v1/slo/slo-no-indicatorref-rolling-no-alerts.yaml", + "../../test/v1/slo/slo-composite-indicatorRef-rolling-no-alerts.yaml", + "../../test/v1/slo/slo-composite-no-indicatorRef-rolling-no-alerts.yaml", }, }, wantErr: false, diff --git a/test/v1/slo/slo-composite-indicatorRef-rolling-no-alerts.yaml b/test/v1/slo/slo-composite-indicatorRef-rolling-no-alerts.yaml new file mode 100644 index 0000000..925d2e6 --- /dev/null +++ b/test/v1/slo/slo-composite-indicatorRef-rolling-no-alerts.yaml @@ -0,0 +1,21 @@ +apiVersion: openslo/v1 +kind: SLO +metadata: + name: TestSLO + displayName: Test SLO # optional +spec: + description: This is a great description # optional + service: TheServiceName # name of the service to associate this SLO with + timeWindow: + - duration: 1d + isRolling: true + budgetingMethod: Occurrences + objectives: + - displayName: Foo Total Errors + target: 0.98 + compositeWeight: 1 + indicatorRef: indicatorRefString # name of the SLI. Required if indicator is not given. + - displayName: Bar Total Errors + target: 0.99 + compositeWeight: 2 + indicatorRef: indicatorRefString # name of the SLI. Required if indicator is not given. diff --git a/test/v1/slo/slo-composite-no-indicatorRef-rolling-no-alerts.yaml b/test/v1/slo/slo-composite-no-indicatorRef-rolling-no-alerts.yaml new file mode 100644 index 0000000..5d8c0b0 --- /dev/null +++ b/test/v1/slo/slo-composite-no-indicatorRef-rolling-no-alerts.yaml @@ -0,0 +1,57 @@ +apiVersion: openslo/v1 +kind: SLO +metadata: + name: TestSLO + displayName: Test SLO +spec: + description: This is a great description + service: TheServiceName + timeWindow: + - duration: 1M + isRolling: true + budgetingMethod: Occurrences + objectives: + - displayName: Foo Total Errors + target: 0.98 + compositeWeight: 1 + indicator: + metadata: + name: foo-error + displayName: Foo Error + spec: + ratioMetric: + counter: true + good: + metricSource: + metricSourceRef: datadog-datasource + type: Datadog + spec: + query: sum:trace.http.request.hits.by_http_status{http.status_code:200}.as_count() + total: + metricSource: + metricSourceRef: datadog-datasource + type: Datadog + spec: + query: sum:trace.http.request.hits.by_http_status{*}.as_count() + - displayName: Bar Total Errors + target: 0.99 + compositeWeight: 2 + indicator: + metadata: + name: foo-error + displayName: Foo Error + spec: + ratioMetric: + counter: true + good: + metricSource: + metricSourceRef: datadog-datasource + type: Datadog + spec: + query: sum:trace.http.request.hits.by_http_status{http.status_code:200}.as_count() + total: + metricSource: + metricSourceRef: datadog-datasource + type: Datadog + spec: + query: sum:trace.http.request.hits.by_http_status{*}.as_count()