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

Improve YAML parsing to handle list sequences within SLI spec #325

Closed
Closed
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
4 changes: 3 additions & 1 deletion internal/fmt/fmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 11 additions & 9 deletions pkg/manifest/v1/indicator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
18 changes: 18 additions & 0 deletions pkg/manifest/v1/indicator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
17 changes: 10 additions & 7 deletions pkg/manifest/v1/objective.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
2 changes: 2 additions & 0 deletions pkg/validate/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
21 changes: 21 additions & 0 deletions test/v1/slo/slo-composite-indicatorRef-rolling-no-alerts.yaml
Original file line number Diff line number Diff line change
@@ -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.
57 changes: 57 additions & 0 deletions test/v1/slo/slo-composite-no-indicatorRef-rolling-no-alerts.yaml
Original file line number Diff line number Diff line change
@@ -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()
Loading