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 #326

Merged
merged 1 commit into from
Aug 28, 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
4 changes: 3 additions & 1 deletion internal/convert/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,9 @@ func printYaml(out io.Writer, object interface{}) error {
return fmt.Errorf("issue marshaling content: %w", err)
}

fmt.Fprint(out, "---\n")
if _, err = fmt.Fprint(out, "---\n"); err != nil {
return err
}
_, err = out.Write(yml)
if err != nil {
return fmt.Errorf("issue writing content: %w", err)
Expand Down
18 changes: 10 additions & 8 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
// 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, the values are processed and flattened appropriately.
//
// This also assumes a certain flat structure that we can revisit if the need arises.
//

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
Loading