feat(metricprovider): add support for multiple formulas in dd metricp…

…rovider

Signed-off-by: Youssef Rabie <[email protected]>

docs/features/kustomize/rollout_cr_schema.json

@@ -268,6 +268,12 @@
                                                     "formula": {
                                                         "type": "string"
                                                     },
+                                                    "formulas": {
+                                                        "items": {
+                                                            "type": "string"
+                                                        },
+                                                        "type": "array"
+                                                    },
                                                     "interval": {
                                                         "default": "5m",
                                                         "type": "string"
@@ -5134,6 +5140,12 @@
                                                     "formula": {
                                                         "type": "string"
                                                     },
+                                                    "formulas": {
+                                                        "items": {
+                                                            "type": "string"
+                                                        },
+                                                        "type": "array"
+                                                    },
                                                     "interval": {
                                                         "default": "5m",
                                                         "type": "string"
@@ -10013,6 +10025,12 @@
                                                     "formula": {
                                                         "type": "string"
                                                     },
+                                                    "formulas": {
+                                                        "items": {
+                                                            "type": "string"
+                                                        },
+                                                        "type": "array"
+                                                    },
                                                     "interval": {
                                                         "default": "5m",
                                                         "type": "string"

manifests/crds/analysis-run-crd.yaml

@@ -198,6 +198,10 @@ spec:
                               type: string
                             formula:
                               type: string
+                            formulas:
+                              items:
+                                type: string
+                              type: array
                             interval:
                               default: 5m
                               type: string

manifests/crds/analysis-template-crd.yaml

@@ -194,6 +194,10 @@ spec:
                               type: string
                             formula:
                               type: string
+                            formulas:
+                              items:
+                                type: string
+                              type: array
                             interval:
                               default: 5m
                               type: string

manifests/crds/cluster-analysis-template-crd.yaml

@@ -194,6 +194,10 @@ spec:
                               type: string
                             formula:
                               type: string
+                            formulas:
+                              items:
+                                type: string
+                              type: array
                             interval:
                               default: 5m
                               type: string

manifests/install.yaml

@@ -199,6 +199,10 @@ spec:
                               type: string
                             formula:
                               type: string
+                            formulas:
+                              items:
+                                type: string
+                              type: array
                             interval:
                               default: 5m
                               type: string
@@ -3507,6 +3511,10 @@ spec:
                               type: string
                             formula:
                               type: string
+                            formulas:
+                              items:
+                                type: string
+                              type: array
                             interval:
                               default: 5m
                               type: string
@@ -6693,6 +6701,10 @@ spec:
                               type: string
                             formula:
                               type: string
+                            formulas:
+                              items:
+                                type: string
+                              type: array
                             interval:
                               default: 5m
                               type: string

metricproviders/datadog/datadog.go

@@ -10,6 +10,7 @@ import (
 	"net/url"
 	"reflect"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
@@ -184,7 +185,7 @@ func (p *Provider) createRequest(dd *v1alpha1.DatadogMetric, now int64, interval
 		dd.Queries = map[string]string{"query": dd.Query}
 	}
 
-	return p.createRequestV2(dd.Queries, dd.Formula, now, interval, dd.Aggregator, url)
+	return p.createRequestV2(dd.Queries, dd.Formula, dd.Formulas, now, interval, dd.Aggregator, url)
 }
 
 func (p *Provider) createRequestV1(query string, now int64, interval int64, url *url.URL) (*http.Request, error) {
@@ -211,15 +212,31 @@ func buildQueriesPayload(queries map[string]string, aggregator string) []map[str
 	return qp
 }
 
-func (p *Provider) createRequestV2(queries map[string]string, formula string, now int64, interval int64, aggregator string, url *url.URL) (*http.Request, error) {
-	formulas := []map[string]string{}
-	// ddAPI supports multiple formulas but doesn't make sense in our context
-	// can't have a 'blank' formula, so have to guard
+func (p *Provider) createRequestV2(queries map[string]string, formula string, formulas []string, now int64, interval int64, aggregator string, url *url.URL) (*http.Request, error) {
+
+	var fp []map[string]string
+
+	// We know either formula formulas are provided, but not both.
 	if formula != "" {
-		formulas = []map[string]string{{
+		fp = []map[string]string{{
 			"formula": formula,
 		}}
+	} else if len(formulas) != 0 {
+		fp = make([]map[string]string, len(formulas))
+		for i, v := range formulas {
+			// can't have a 'blank' formula, so have to guard
+			// This won't happen though since we check in validateIncomingProps
+			if v != "" {
+				p := map[string]string{
+					"formula": v,
+				}
+				fp[i] = p
+			}
+		}
+	} else {
+		fp = []map[string]string{}
 	}
+
 	// we cannot leave aggregator empty as it will be passed as such to datadog API and fail
 	if aggregator == "" {
 		aggregator = "last"
@@ -230,7 +247,7 @@ func (p *Provider) createRequestV2(queries map[string]string, formula string, no
 		From:     (now - interval) * 1000,
 		To:       now * 1000,
 		Queries:  buildQueriesPayload(queries, aggregator),
-		Formulas: formulas,
+		Formulas: fp,
 	}
 
 	queryBody, err := json.Marshal(datadogRequest{
@@ -296,6 +313,36 @@ func (p *Provider) parseResponseV1(metric v1alpha1.Metric, response *http.Respon
 	return strconv.FormatFloat(value, 'f', -1, 64), status, err
 }
 
+func valuesToResultStr(value interface{}) string {
+
+	if v, ok := value.(float64); ok {
+		return strconv.FormatFloat(v, 'f', -1, 64)
+	}
+
+	if valueSlice, ok := value.([]interface{}); ok {
+
+		results := []string{}
+
+		for _, v := range valueSlice {
+			// This never happens
+			if v == nil {
+				continue
+			}
+
+			// This is always true
+			if vFloat, ok := v.(float64); ok {
+				results = append(results, strconv.FormatFloat(vFloat, 'f', -1, 64))
+			}
+		}
+
+		return fmt.Sprintf("[%s]", strings.Join(results, ","))
+	}
+
+	// We should never reach here
+	return ""
+
+}
+
 func (p *Provider) parseResponseV2(metric v1alpha1.Metric, response *http.Response) (string, v1alpha1.AnalysisPhase, error) {
 	bodyBytes, err := io.ReadAll(response.Body)
 	if err != nil {
@@ -319,17 +366,81 @@ func (p *Provider) parseResponseV2(metric v1alpha1.Metric, response *http.Respon
 		return "", v1alpha1.AnalysisPhaseError, fmt.Errorf("There were errors in your query: %v", res.Data.Errors)
 	}
 
+	var somethingNil, allNil bool
+	var value interface{}
+
+	formulasLength := len(metric.Provider.Datadog.Formulas)
+
+	// Evalulate whether all formulas are nil, and whether something in them is nil
+	allNil = reflect.ValueOf(res.Data.Attributes).IsZero() || len(res.Data.Attributes.Columns) == 0
+
+	// If all is nil, something is nil
+	if allNil {
+		somethingNil = true
+	} else {
+		// if there are no formulas provided (that is, either a query or a single formula),
+		// then we just check the first column
+		if formulasLength == 0 {
+			somethingNil = len(res.Data.Attributes.Columns[0].Values) == 0
+		} else {
+			// if there are multiple formulas provided, we set somethingNil to true
+			// if any of them are missing values.
+			for i := range formulasLength {
+				if len(res.Data.Attributes.Columns[i].Values) == 0 {
+					somethingNil = true
+					break
+				}
+			}
+		}
+	}
+
+	var nilFloat64 *float64
+
+	// Populate value
+	// In the case of a single formula, or a query, it is of type float64
+	// In the case of multiple formulas, it is a slice of interface, which is really
+	// a slice of float64
+	if formulasLength == 0 {
+
+		if allNil || somethingNil {
+			value = nilFloat64
+		} else {
+			value = res.Data.Attributes.Columns[0].Values[0]
+		}
+	} else {
+
+		value = make([]interface{}, formulasLength)
+		valueAsSlice := value.([]interface{})
+
+		if allNil {
+			for i := range len(valueAsSlice) {
+				valueAsSlice[i] = nilFloat64
+			}
+		} else {
+			for i := range len(valueAsSlice) {
+				if len(res.Data.Attributes.Columns[i].Values) == 0 {
+					valueAsSlice[i] = nilFloat64
+				} else {
+					valueAsSlice[i] = res.Data.Attributes.Columns[i].Values[0]
+				}
+			}
+		}
+	}
+
 	// Handle an empty query result
-	if reflect.ValueOf(res.Data.Attributes).IsZero() || len(res.Data.Attributes.Columns) == 0 || len(res.Data.Attributes.Columns[0].Values) == 0 {
-		var nilFloat64 *float64
-		status, err := evaluate.EvaluateResult(nilFloat64, metric, p.logCtx)
+	if allNil || somethingNil {
+		status, err := evaluate.EvaluateResult(value, metric, p.logCtx)
 
 		var attributesBytes []byte
 		var jsonErr error
 		// Should be impossible for this to not be true, based on dd openapi spec.
 		// But in this case, better safe than sorry
-		if len(res.Data.Attributes.Columns) == 1 {
-			attributesBytes, jsonErr = json.Marshal(res.Data.Attributes.Columns[0].Values)
+		if len(res.Data.Attributes.Columns) >= 1 {
+			allValues := []float64{}
+			for i := range len(res.Data.Attributes.Columns) {
+				allValues = append(allValues, res.Data.Attributes.Columns[i].Values...)
+			}
+			attributesBytes, jsonErr = json.Marshal(allValues)
 		} else {
 			attributesBytes, jsonErr = json.Marshal(res.Data.Attributes)
 		}
@@ -342,10 +453,9 @@ func (p *Provider) parseResponseV2(metric v1alpha1.Metric, response *http.Respon
 	}
 
 	// Handle a populated query result
-	column := res.Data.Attributes.Columns[0]
-	value := column.Values[0]
 	status, err := evaluate.EvaluateResult(value, metric, p.logCtx)
-	return strconv.FormatFloat(value, 'f', -1, 64), status, err
+	return valuesToResultStr(value), status, err
+
 }
 
 // Resume should not be used the Datadog provider since all the work should occur in the Run method
@@ -381,21 +491,35 @@ func validateIncomingProps(dd *v1alpha1.DatadogMetric) error {
 		return errors.New("Cannot have both a query and queries. Please review the Analysis Template.")
 	}
 
+	// check that we have ONE OF formula/formulas
+	if dd.Formula != "" && len(dd.Formulas) > 0 {
+		return errors.New("Cannot have both a formula and formulas. Please review the Analysis Template.")
+	}
+
 	// check that query is set for apiversion v1
 	if dd.ApiVersion == "v1" && dd.Query == "" {
 		return errors.New("Query is empty. API Version v1 only supports using the query parameter in your Analysis Template.")
 	}
 
 	// formula <3 queries. won't go anywhere without them
-	if dd.Formula != "" && len(dd.Queries) == 0 {
-		return errors.New("Formula are only valid when queries are set. Please review the Analysis Template.")
+	if (dd.Formula != "" || len(dd.Formulas) != 0) && len(dd.Queries) == 0 {
+		return errors.New("Formula/Formulas are only valid when queries are set. Please review the Analysis Template.")
+	}
+
+	// validate that if formulas are set, no one of them is an empty string
+	if len(dd.Formulas) != 0 {
+		for _, f := range dd.Formulas {
+			if f == "" {
+				return errors.New("All formulas within Formulas field must be non-empty strings.")
+			}
+		}
 	}
 
-	// Reject queries with more than 1 when NO formula provided. While this would technically work
+	// Reject queries with more than 1 when NO formula/formulas provided. While this would technically work
 	// DD will return 2 columns of data, and there is no guarantee what order they would be in, so
 	// there is no way to guess at intention of user. Since this is about metrics and monitoring, we should
 	// avoid ambiguity.
-	if dd.Formula == "" && len(dd.Queries) > 1 {
+	if (dd.Formula == "" && len(dd.Formulas) == 0) && len(dd.Queries) > 1 {
 		return errors.New("When multiple queries are provided you must include a formula.")
 	}