Skip to content

Commit fc758ff

Browse files
y-rabiezachaller
authored andcommitted
feat(metricprovider): add support for multiple formulas in dd metricprovider
Signed-off-by: Youssef Rabie <[email protected]>
1 parent 21d5716 commit fc758ff

18 files changed

+936
-611
lines changed

docs/features/kustomize/rollout_cr_schema.json

+18
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,12 @@
279279
"formula": {
280280
"type": "string"
281281
},
282+
"formulas": {
283+
"items": {
284+
"type": "string"
285+
},
286+
"type": "array"
287+
},
282288
"interval": {
283289
"default": "5m",
284290
"type": "string"
@@ -5156,6 +5162,12 @@
51565162
"formula": {
51575163
"type": "string"
51585164
},
5165+
"formulas": {
5166+
"items": {
5167+
"type": "string"
5168+
},
5169+
"type": "array"
5170+
},
51595171
"interval": {
51605172
"default": "5m",
51615173
"type": "string"
@@ -10046,6 +10058,12 @@
1004610058
"formula": {
1004710059
"type": "string"
1004810060
},
10061+
"formulas": {
10062+
"items": {
10063+
"type": "string"
10064+
},
10065+
"type": "array"
10066+
},
1004910067
"interval": {
1005010068
"default": "5m",
1005110069
"type": "string"

manifests/crds/analysis-run-crd.yaml

+4
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,10 @@ spec:
203203
type: string
204204
formula:
205205
type: string
206+
formulas:
207+
items:
208+
type: string
209+
type: array
206210
interval:
207211
default: 5m
208212
type: string

manifests/crds/analysis-template-crd.yaml

+4
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,10 @@ spec:
199199
type: string
200200
formula:
201201
type: string
202+
formulas:
203+
items:
204+
type: string
205+
type: array
202206
interval:
203207
default: 5m
204208
type: string

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

+4
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,10 @@ spec:
199199
type: string
200200
formula:
201201
type: string
202+
formulas:
203+
items:
204+
type: string
205+
type: array
202206
interval:
203207
default: 5m
204208
type: string

manifests/install.yaml

+12
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,10 @@ spec:
204204
type: string
205205
formula:
206206
type: string
207+
formulas:
208+
items:
209+
type: string
210+
type: array
207211
interval:
208212
default: 5m
209213
type: string
@@ -3520,6 +3524,10 @@ spec:
35203524
type: string
35213525
formula:
35223526
type: string
3527+
formulas:
3528+
items:
3529+
type: string
3530+
type: array
35233531
interval:
35243532
default: 5m
35253533
type: string
@@ -6711,6 +6719,10 @@ spec:
67116719
type: string
67126720
formula:
67136721
type: string
6722+
formulas:
6723+
items:
6724+
type: string
6725+
type: array
67146726
interval:
67156727
default: 5m
67166728
type: string

metricproviders/datadog/datadog.go

+127-19
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"net/url"
1111
"reflect"
1212
"strconv"
13+
"strings"
1314
"time"
1415

1516
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
@@ -184,7 +185,7 @@ func (p *Provider) createRequest(dd *v1alpha1.DatadogMetric, now int64, interval
184185
dd.Queries = map[string]string{"query": dd.Query}
185186
}
186187

187-
return p.createRequestV2(dd.Queries, dd.Formula, now, interval, dd.Aggregator, url)
188+
return p.createRequestV2(dd.Queries, dd.Formula, dd.Formulas, now, interval, dd.Aggregator, url)
188189
}
189190

190191
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
211212
return qp
212213
}
213214

214-
func (p *Provider) createRequestV2(queries map[string]string, formula string, now int64, interval int64, aggregator string, url *url.URL) (*http.Request, error) {
215-
formulas := []map[string]string{}
216-
// ddAPI supports multiple formulas but doesn't make sense in our context
217-
// can't have a 'blank' formula, so have to guard
215+
func (p *Provider) createRequestV2(queries map[string]string, formula string, formulas []string, now int64, interval int64, aggregator string, url *url.URL) (*http.Request, error) {
216+
217+
var fp []map[string]string
218+
219+
// We know either formula formulas are provided, but not both.
218220
if formula != "" {
219-
formulas = []map[string]string{{
221+
fp = []map[string]string{{
220222
"formula": formula,
221223
}}
224+
} else if len(formulas) != 0 {
225+
fp = make([]map[string]string, len(formulas))
226+
for i, v := range formulas {
227+
// can't have a 'blank' formula, so have to guard
228+
// This won't happen though since we check in validateIncomingProps
229+
if v != "" {
230+
p := map[string]string{
231+
"formula": v,
232+
}
233+
fp[i] = p
234+
}
235+
}
236+
} else {
237+
fp = []map[string]string{}
222238
}
239+
223240
// we cannot leave aggregator empty as it will be passed as such to datadog API and fail
224241
if aggregator == "" {
225242
aggregator = "last"
@@ -230,7 +247,7 @@ func (p *Provider) createRequestV2(queries map[string]string, formula string, no
230247
From: (now - interval) * 1000,
231248
To: now * 1000,
232249
Queries: buildQueriesPayload(queries, aggregator),
233-
Formulas: formulas,
250+
Formulas: fp,
234251
}
235252

236253
queryBody, err := json.Marshal(datadogRequest{
@@ -296,6 +313,36 @@ func (p *Provider) parseResponseV1(metric v1alpha1.Metric, response *http.Respon
296313
return strconv.FormatFloat(value, 'f', -1, 64), status, err
297314
}
298315

316+
func valuesToResultStr(value interface{}) string {
317+
318+
if v, ok := value.(float64); ok {
319+
return strconv.FormatFloat(v, 'f', -1, 64)
320+
}
321+
322+
if valueSlice, ok := value.([]interface{}); ok {
323+
324+
results := []string{}
325+
326+
for _, v := range valueSlice {
327+
// This never happens
328+
if v == nil {
329+
continue
330+
}
331+
332+
// This is always true
333+
if vFloat, ok := v.(float64); ok {
334+
results = append(results, strconv.FormatFloat(vFloat, 'f', -1, 64))
335+
}
336+
}
337+
338+
return fmt.Sprintf("[%s]", strings.Join(results, ","))
339+
}
340+
341+
// We should never reach here
342+
return ""
343+
344+
}
345+
299346
func (p *Provider) parseResponseV2(metric v1alpha1.Metric, response *http.Response) (string, v1alpha1.AnalysisPhase, error) {
300347
bodyBytes, err := io.ReadAll(response.Body)
301348
if err != nil {
@@ -319,17 +366,65 @@ func (p *Provider) parseResponseV2(metric v1alpha1.Metric, response *http.Respon
319366
return "", v1alpha1.AnalysisPhaseError, fmt.Errorf("There were errors in your query: %v", res.Data.Errors)
320367
}
321368

369+
var somethingNil, allNil bool
370+
var value interface{}
371+
var nilFloat64 *float64
372+
373+
// formulasLength is the length of formulas array provided
374+
formulasLength := len(metric.Provider.Datadog.Formulas)
375+
// valuesLength is the length of returned values to access
376+
// if no formulas array provided, that means it is only 1 value
377+
// (the result of the signle formula or query)
378+
valuesLength := formulasLength
379+
if formulasLength == 0 {
380+
valuesLength = 1
381+
}
382+
383+
// Evalulate whether all formulas are nil
384+
allNil = reflect.ValueOf(res.Data.Attributes).IsZero() || len(res.Data.Attributes.Columns) == 0
385+
386+
// Populate value and evalulate somethingNil
387+
// value is a slice of interface, which is really a slice of float64 (and nils)
388+
// somethingNil indicates whether there exists a formula with null response
389+
value = make([]interface{}, valuesLength)
390+
valueAsSlice := value.([]interface{})
391+
392+
if allNil {
393+
for i := range len(valueAsSlice) {
394+
valueAsSlice[i] = nilFloat64
395+
}
396+
} else {
397+
for i := range len(valueAsSlice) {
398+
if len(res.Data.Attributes.Columns[i].Values) == 0 || res.Data.Attributes.Columns[i].Values[0] == nil {
399+
valueAsSlice[i] = nilFloat64
400+
somethingNil = true
401+
} else {
402+
valueAsSlice[i] = *res.Data.Attributes.Columns[i].Values[0]
403+
}
404+
}
405+
}
406+
407+
// To preserve backward conditions accessing `result` directly
408+
// instead of `result[0]`. Cast value back to float64 instead of a slice
409+
// when no `formulas` array is provided
410+
if formulasLength == 0 {
411+
value = valueAsSlice[0]
412+
}
413+
322414
// Handle an empty query result
323-
if reflect.ValueOf(res.Data.Attributes).IsZero() || len(res.Data.Attributes.Columns) == 0 || len(res.Data.Attributes.Columns[0].Values) == 0 || res.Data.Attributes.Columns[0].Values[0] == nil {
324-
var nilFloat64 *float64
325-
status, err := evaluate.EvaluateResult(nilFloat64, metric, p.logCtx)
415+
if allNil || somethingNil {
416+
status, err := evaluate.EvaluateResult(value, metric, p.logCtx)
326417

327418
var attributesBytes []byte
328419
var jsonErr error
329420
// Should be impossible for this to not be true, based on dd openapi spec.
330421
// But in this case, better safe than sorry
331-
if len(res.Data.Attributes.Columns) == 1 {
332-
attributesBytes, jsonErr = json.Marshal(res.Data.Attributes.Columns[0].Values)
422+
if len(res.Data.Attributes.Columns) >= 1 {
423+
allValues := []*float64{}
424+
for i := range len(res.Data.Attributes.Columns) {
425+
allValues = append(allValues, res.Data.Attributes.Columns[i].Values...)
426+
}
427+
attributesBytes, jsonErr = json.Marshal(allValues)
333428
} else {
334429
attributesBytes, jsonErr = json.Marshal(res.Data.Attributes)
335430
}
@@ -342,10 +437,9 @@ func (p *Provider) parseResponseV2(metric v1alpha1.Metric, response *http.Respon
342437
}
343438

344439
// Handle a populated query result
345-
column := res.Data.Attributes.Columns[0]
346-
value := *column.Values[0]
347440
status, err := evaluate.EvaluateResult(value, metric, p.logCtx)
348-
return strconv.FormatFloat(value, 'f', -1, 64), status, err
441+
return valuesToResultStr(value), status, err
442+
349443
}
350444

351445
// Resume should not be used the Datadog provider since all the work should occur in the Run method
@@ -381,21 +475,35 @@ func validateIncomingProps(dd *v1alpha1.DatadogMetric) error {
381475
return errors.New("Cannot have both a query and queries. Please review the Analysis Template.")
382476
}
383477

478+
// check that we have ONE OF formula/formulas
479+
if dd.Formula != "" && len(dd.Formulas) > 0 {
480+
return errors.New("Cannot have both a formula and formulas. Please review the Analysis Template.")
481+
}
482+
384483
// check that query is set for apiversion v1
385484
if dd.ApiVersion == "v1" && dd.Query == "" {
386485
return errors.New("Query is empty. API Version v1 only supports using the query parameter in your Analysis Template.")
387486
}
388487

389488
// formula <3 queries. won't go anywhere without them
390-
if dd.Formula != "" && len(dd.Queries) == 0 {
391-
return errors.New("Formula are only valid when queries are set. Please review the Analysis Template.")
489+
if (dd.Formula != "" || len(dd.Formulas) != 0) && len(dd.Queries) == 0 {
490+
return errors.New("Formula/Formulas are only valid when queries are set. Please review the Analysis Template.")
491+
}
492+
493+
// validate that if formulas are set, no one of them is an empty string
494+
if len(dd.Formulas) != 0 {
495+
for _, f := range dd.Formulas {
496+
if f == "" {
497+
return errors.New("All formulas within Formulas field must be non-empty strings.")
498+
}
499+
}
392500
}
393501

394-
// Reject queries with more than 1 when NO formula provided. While this would technically work
502+
// Reject queries with more than 1 when NO formula/formulas provided. While this would technically work
395503
// DD will return 2 columns of data, and there is no guarantee what order they would be in, so
396504
// there is no way to guess at intention of user. Since this is about metrics and monitoring, we should
397505
// avoid ambiguity.
398-
if dd.Formula == "" && len(dd.Queries) > 1 {
506+
if (dd.Formula == "" && len(dd.Formulas) == 0) && len(dd.Queries) > 1 {
399507
return errors.New("When multiple queries are provided you must include a formula.")
400508
}
401509

0 commit comments

Comments
 (0)