diff --git a/metricproviders/graphite/api.go b/metricproviders/graphite/api.go index f831b7d196..23e2f4744b 100644 --- a/metricproviders/graphite/api.go +++ b/metricproviders/graphite/api.go @@ -17,10 +17,10 @@ import ( // API represents a Graphite API client type API interface { - Query(query string) (*float64, error) + Query(query string) ([]dataPoint, error) } -// GraphiteAPI is a Graphite API client +// APIClient is a Graphite API client type APIClient struct { url url.URL client *http.Client @@ -28,11 +28,11 @@ type APIClient struct { } // Query performs a Graphite API query with the query it's passed -func (api APIClient) Query(quer string) (*float64, error) { +func (api APIClient) Query(quer string) ([]dataPoint, error) { query := api.trimQuery(quer) u, err := url.Parse(fmt.Sprintf("./render?%s", query)) if err != nil { - return nil, err + return []dataPoint{}, err } q := u.Query() @@ -44,40 +44,31 @@ func (api APIClient) Query(quer string) (*float64, error) { req, err := http.NewRequest("GET", u.String(), nil) if err != nil { - return nil, err + return []dataPoint{}, err } r, err := api.client.Do(req) if err != nil { - return nil, err + return []dataPoint{}, err } defer r.Body.Close() b, err := ioutil.ReadAll(r.Body) if err != nil { - return nil, err + return []dataPoint{}, err } if 400 <= r.StatusCode { - return nil, fmt.Errorf("error response: %s", string(b)) + return []dataPoint{}, fmt.Errorf("error response: %s", string(b)) } var result graphiteResponse err = json.Unmarshal(b, &result) if err != nil { - return nil, err + return []dataPoint{}, err } - var value *float64 - for _, tr := range result { - for _, dp := range tr.DataPoints { - if dp.Value != nil { - value = dp.Value - } - } - } - - return value, nil + return result[0].DataPoints, nil } func (api APIClient) trimQuery(q string) string { @@ -85,12 +76,12 @@ func (api APIClient) trimQuery(q string) string { return space.ReplaceAllString(q, " ") } -type graphiteDataPoint struct { +type dataPoint struct { Value *float64 TimeStamp time.Time } -func (gdp *graphiteDataPoint) UnmarshalJSON(data []byte) error { +func (gdp *dataPoint) UnmarshalJSON(data []byte) error { var v []interface{} if err := json.Unmarshal(data, &v); err != nil { return err @@ -144,8 +135,8 @@ func (gdp *graphiteDataPoint) UnmarshalJSON(data []byte) error { } type graphiteTargetResp struct { - Target string `json:"target"` - DataPoints []graphiteDataPoint `json:"datapoints"` + Target string `json:"target"` + DataPoints []dataPoint `json:"datapoints"` } type graphiteResponse []graphiteTargetResp diff --git a/metricproviders/graphite/api_test.go b/metricproviders/graphite/api_test.go index a925e44ee9..9ce1838c04 100644 --- a/metricproviders/graphite/api_test.go +++ b/metricproviders/graphite/api_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" log "github.com/sirupsen/logrus" @@ -42,14 +43,19 @@ func TestQuery(t *testing.T) { query := "target=sumSeries(app.http.*.*.count)&from=-2min" targetQuery := "sumSeries(app.http.*.*.count)" fromQuery := "-2min" - goodResult := float64(100) + value := float64(100) + timestamp := int64(1621348430) + goodResult := []dataPoint{{ + Value: &value, + TimeStamp: time.Unix(timestamp, 0), + }} tests := []struct { name string query string expectedTarget string expectedFrom string - expectedResult *float64 + expectedResult []dataPoint expectedErr error body string responseCode int @@ -58,26 +64,14 @@ func TestQuery(t *testing.T) { query, targetQuery, fromQuery, - &goodResult, + goodResult, nil, - `[ + fmt.Sprintf(`[ { "datapoints": [ [ - 10, - 1621348400 - ], - [ - 75, - 1621348410 - ], - [ - 25, - 1621348420 - ], - [ - 100, - 1621348430 + %f, + %d ] ], "target": "sumSeries(app.http.*.*.count)", @@ -86,14 +80,14 @@ func TestQuery(t *testing.T) { "name": "sumSeries(app.http.*.*.count)" } } - ]`, + ]`, value, timestamp), 200, }, { "graphite response body with invalid JSON", query, targetQuery, fromQuery, - nil, + []dataPoint{}, errors.New("invalid character 'i' looking for beginning of value"), "invalid JSON", 200, @@ -102,7 +96,7 @@ func TestQuery(t *testing.T) { query, targetQuery, fromQuery, - nil, + []dataPoint{}, errors.New("error response: foo"), "foo", 400, @@ -111,7 +105,7 @@ func TestQuery(t *testing.T) { query, targetQuery, fromQuery, - nil, + []dataPoint{}, errors.New("error response: bar"), "bar", 500, @@ -120,7 +114,7 @@ func TestQuery(t *testing.T) { "target=#$%^&*(proper$#$%%^(password&from=-2min", "#$%^&*(proper$#$%%^(password", "-2min", - nil, + []dataPoint{}, errors.New("parse \"./render?target=#$%^&*(proper$#$%%^(password&from=-2min\": invalid URL escape \"%^&\""), "", 200, @@ -129,7 +123,7 @@ func TestQuery(t *testing.T) { query, targetQuery, fromQuery, - nil, + []dataPoint{}, errors.New("error unmarshaling data point: [10]"), `[ { @@ -151,7 +145,7 @@ func TestQuery(t *testing.T) { query, targetQuery, fromQuery, - nil, + []dataPoint{}, errors.New("strconv.ParseInt: parsing \"f\": invalid syntax"), `[ { @@ -170,26 +164,26 @@ func TestQuery(t *testing.T) { query, targetQuery, fromQuery, - &goodResult, + goodResult, nil, - `[ + fmt.Sprintf(`[ { "datapoints": [ [ - "100", - 1621348420 + "%f", + %d ] ], "target": "sumSeries(app.http.*.*.count)" } - ]`, + ]`, value, timestamp), 200, }, { "graphite response data point JSON triggers unmarshaling error", query, targetQuery, fromQuery, - nil, + []dataPoint{}, errors.New("error unmarshaling value: []"), `[ { @@ -208,26 +202,26 @@ func TestQuery(t *testing.T) { query, targetQuery, fromQuery, - &goodResult, + goodResult, nil, - `[ + fmt.Sprintf(`[ { "datapoints": [ [ - 100, - "1621348420" + %f, + "%d" ] ], "target": "sumSeries(app.http.*.*.count)" } - ]`, + ]`, value, timestamp), 200, }, { "graphite response data point timestamp JSON triggers unmarshaling error", query, targetQuery, fromQuery, - nil, + []dataPoint{}, errors.New("error unmarshaling timestamp: 100"), `[ { @@ -268,6 +262,7 @@ func TestQuery(t *testing.T) { } else { assert.Nil(t, err) } + assert.Equal(t, test.expectedResult, val) }) } diff --git a/metricproviders/graphite/graphite.go b/metricproviders/graphite/graphite.go index 1adc8c0c85..c12fb3142b 100644 --- a/metricproviders/graphite/graphite.go +++ b/metricproviders/graphite/graphite.go @@ -56,18 +56,17 @@ func (p *Provider) Run(run *v1alpha1.AnalysisRun, metric v1alpha1.Metric) v1alph StartedAt: &startTime, } - value, err := p.api.Query(metric.Provider.Graphite.Query) + result, err := p.api.Query(metric.Provider.Graphite.Query) if err != nil { return metricutil.MarkMeasurementError(newMeasurement, err) } - if value == nil { + if len(result) == 0 { return metricutil.MarkMeasurementError(newMeasurement, errors.New("no values found")) } - newMeasurement.Value = fmt.Sprintf("%f", *value) - - newStatus, err := evaluate.EvaluateResult(*value, metric, p.logCtx) + newValue, newStatus, err := p.processResponse(metric, result) + newMeasurement.Value = newValue if err != nil { return metricutil.MarkMeasurementError(newMeasurement, err) } @@ -96,6 +95,28 @@ func (p *Provider) GarbageCollect(run *v1alpha1.AnalysisRun, metric v1alpha1.Met return nil } +func (p *Provider) processResponse(metric v1alpha1.Metric, dataPoints []dataPoint) (string, v1alpha1.AnalysisPhase, error) { + results := make([]float64, 0, len(dataPoints)) + valueStr := "[" + + for _, dp := range dataPoints { + if dp.Value != nil { + valueStr = valueStr + fmt.Sprintf("%f,", *dp.Value) + results = append(results, *dp.Value) + } + } + + // remove the last comma on the '[dp.Value,dp.Value,' string + if len(valueStr) > 1 { + valueStr = valueStr[:len(valueStr)-1] + } + + valueStr = valueStr + "]" + newStatus, err := evaluate.EvaluateResult(results, metric, p.logCtx) + + return valueStr, newStatus, err +} + // NewGraphiteProvider returns a new Graphite provider func NewGraphiteProvider(api API, logCtx log.Entry) *Provider { return &Provider{ diff --git a/metricproviders/graphite/graphite_test.go b/metricproviders/graphite/graphite_test.go index 3fdb4007a3..5d041aa328 100644 --- a/metricproviders/graphite/graphite_test.go +++ b/metricproviders/graphite/graphite_test.go @@ -3,6 +3,7 @@ package graphite import ( "errors" "testing" + "time" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -11,8 +12,17 @@ import ( ) func newMockAPI(response *float64, err error) mockAPI { + dps := []dataPoint{{ + Value: response, + TimeStamp: time.Now(), + }} + + if response == nil { + dps = []dataPoint{} + } + return mockAPI{ - response: response, + response: dps, err: err, } } @@ -20,8 +30,8 @@ func newMockAPI(response *float64, err error) mockAPI { func newTestingMetric() v1alpha1.Metric { return v1alpha1.Metric{ Name: "foo", - SuccessCondition: "result == 10.000000", - FailureCondition: "result != 10.000000", + SuccessCondition: "10.000000 in result", + FailureCondition: "10.000000 not in result", Provider: v1alpha1.MetricProvider{ Graphite: &v1alpha1.GraphiteMetric{ Address: "http://some-graphite.foo", @@ -42,7 +52,7 @@ func TestRunSuccessfulEvaluation(t *testing.T) { g := NewGraphiteProvider(newMockAPI(&response, nil), log.Entry{}) measurement := g.Run(&v1alpha1.AnalysisRun{}, newTestingMetric()) assert.NotNil(t, measurement.StartedAt) - assert.Equal(t, "10.000000", measurement.Value) + assert.Equal(t, "[10.000000]", measurement.Value) assert.NotNil(t, measurement.FinishedAt) assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, measurement.Phase) } @@ -52,7 +62,7 @@ func TestRunFailedEvaluation(t *testing.T) { g := NewGraphiteProvider(newMockAPI(&response, nil), log.Entry{}) measurement := g.Run(&v1alpha1.AnalysisRun{}, newTestingMetric()) assert.NotNil(t, measurement.StartedAt) - assert.Equal(t, "5.000000", measurement.Value) + assert.Equal(t, "[5.000000]", measurement.Value) assert.NotNil(t, measurement.FinishedAt) assert.Equal(t, v1alpha1.AnalysisPhaseFailed, measurement.Phase) } @@ -61,8 +71,8 @@ func TestRunMeasurementError(t *testing.T) { metric := v1alpha1.Metric{ Name: "foo", // Malformed Success and Failure Conditions - SuccessCondition: "result 10.000000", - FailureCondition: "result 10.000000", + SuccessCondition: "result[0] 10.000000", + FailureCondition: "result[0] 10.000000", Provider: v1alpha1.MetricProvider{ Graphite: &v1alpha1.GraphiteMetric{ Address: "http://some-graphite.foo", @@ -74,7 +84,7 @@ func TestRunMeasurementError(t *testing.T) { g := NewGraphiteProvider(newMockAPI(&response, nil), log.Entry{}) measurement := g.Run(&v1alpha1.AnalysisRun{}, metric) assert.NotNil(t, measurement.StartedAt) - assert.Equal(t, "10.000000", measurement.Value) + assert.Equal(t, "[10.000000]", measurement.Value) assert.NotNil(t, measurement.FinishedAt) assert.Equal(t, v1alpha1.AnalysisPhaseError, measurement.Phase) assert.Equal(t, "unexpected token Number(\"10.000000\")", measurement.Message) diff --git a/metricproviders/graphite/mock_test.go b/metricproviders/graphite/mock_test.go index b0b1953144..2e7887e06f 100644 --- a/metricproviders/graphite/mock_test.go +++ b/metricproviders/graphite/mock_test.go @@ -1,11 +1,11 @@ package graphite type mockAPI struct { - response *float64 + response []dataPoint err error } -func (m mockAPI) Query(query string) (*float64, error) { +func (m mockAPI) Query(query string) ([]dataPoint, error) { if m.err != nil { return nil, m.err }