feat(analysis): Added additional metadata to the status of AnalysisRun

Signed-off-by: Rohit Agrawal <[email protected]>
argoproj · Feb 1, 2022 · c02f524 · c02f524
1 parent 53643c5
commit c02f524
Show file tree

Hide file tree

Showing 34 changed files with 223 additions and 19 deletions.
diff --git a/analysis/analysis.go b/analysis/analysis.go
@@ -325,16 +325,17 @@ func (c *Controller) runMeasurements(run *v1alpha1.AnalysisRun, tasks []metricTa
 			metricResult := analysisutil.GetResult(run, t.metric.Name)
 			resultsLock.Unlock()
 
+			provider, err := c.newProvider(*logger, t.metric)
 			if metricResult == nil {
 				metricResult = &v1alpha1.MetricResult{
-					Name:   t.metric.Name,
-					Phase:  v1alpha1.AnalysisPhaseRunning,
-					DryRun: dryRunMetricsMap[t.metric.Name],
+					Name:     t.metric.Name,
+					Phase:    v1alpha1.AnalysisPhaseRunning,
+					DryRun:   dryRunMetricsMap[t.metric.Name],
+					Metadata: provider.GetMetadata(t.metric),
 				}
 			}
 
 			var newMeasurement v1alpha1.Measurement
-			provider, err := c.newProvider(*logger, t.metric)
 			if err != nil {
 				if t.incompleteMeasurement != nil {
 					newMeasurement = *t.incompleteMeasurement

diff --git a/analysis/analysis_test.go b/analysis/analysis_test.go
@@ -892,6 +892,7 @@ func TestReconcileAnalysisRunInitial(t *testing.T) {
 		},
 	}
 	f.provider.On("Run", mock.Anything, mock.Anything, mock.Anything).Return(newMeasurement(v1alpha1.AnalysisPhaseSuccessful), nil)
+	f.provider.On("GetMetadata", mock.Anything, mock.Anything).Return(map[string]string{}, nil)
 	{
 		newRun := c.reconcileAnalysisRun(run)
 		assert.Equal(t, v1alpha1.AnalysisPhaseRunning, newRun.Status.MetricResults[0].Phase)
@@ -1128,6 +1129,38 @@ func TestResolveMetricArgsUnableToSubstitute(t *testing.T) {
 	}
 }
 
+func TestGetMetadataIsCalled(t *testing.T) {
+	f := newFixture(t)
+	defer f.Close()
+	c, _, _ := f.newController(noResyncPeriodFunc)
+	arg := "success-rate"
+	run := &v1alpha1.AnalysisRun{
+		Spec: v1alpha1.AnalysisRunSpec{
+			Args: []v1alpha1.Argument{
+				{
+					Name:  "metric-name",
+					Value: &arg,
+				},
+			},
+			Metrics: []v1alpha1.Metric{{
+				Name:             "rate",
+				SuccessCondition: "result[0] > 0",
+				Provider: v1alpha1.MetricProvider{
+					Prometheus: &v1alpha1.PrometheusMetric{
+						Query: "{{args.metric-name}}",
+					},
+				},
+			}},
+		},
+	}
+	metricMetadata := map[string]string{"foo": "bar"}
+	f.provider.On("Run", mock.Anything, mock.Anything, mock.Anything).Return(newMeasurement(v1alpha1.AnalysisPhaseSuccessful), nil)
+	f.provider.On("GetMetadata", mock.Anything, mock.Anything).Return(metricMetadata, nil)
+	newRun := c.reconcileAnalysisRun(run)
+	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, newRun.Status.Phase)
+	assert.Equal(t, metricMetadata, newRun.Status.MetricResults[0].Metadata)
+}
+
 // TestSecretContentReferenceSuccess verifies that secret arguments are properly resolved
 func TestSecretContentReferenceSuccess(t *testing.T) {
 	f := newFixture(t)
@@ -1172,6 +1205,7 @@ func TestSecretContentReferenceSuccess(t *testing.T) {
 		},
 	}
 	f.provider.On("Run", mock.Anything, mock.Anything, mock.Anything).Return(newMeasurement(v1alpha1.AnalysisPhaseSuccessful), nil)
+	f.provider.On("GetMetadata", mock.Anything, mock.Anything).Return(map[string]string{}, nil)
 	newRun := c.reconcileAnalysisRun(run)
 	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, newRun.Status.Phase)
 }
@@ -1236,6 +1270,7 @@ func TestSecretContentReferenceProviderError(t *testing.T) {
 	measurement.Message = error.Error()
 
 	f.provider.On("Run", mock.Anything, mock.Anything, mock.Anything).Return(measurement)
+	f.provider.On("GetMetadata", mock.Anything, mock.Anything).Return(map[string]string{}, nil)
 	newRun := c.reconcileAnalysisRun(run)
 	logMessage := buf.String()
 
@@ -1297,6 +1332,7 @@ func TestSecretContentReferenceAndMultipleArgResolutionSuccess(t *testing.T) {
 	}
 
 	f.provider.On("Run", mock.Anything, mock.Anything, mock.Anything).Return(newMeasurement(v1alpha1.AnalysisPhaseSuccessful), nil)
+	f.provider.On("GetMetadata", mock.Anything, mock.Anything).Return(map[string]string{}, nil)
 	newRun := c.reconcileAnalysisRun(run)
 	assert.Equal(t, v1alpha1.AnalysisPhaseSuccessful, newRun.Status.Phase)
 }

diff --git a/hack/tools.go b/hack/tools.go
@@ -1,3 +1,4 @@
+//go:build tools
 // +build tools
 
 package tools

diff --git a/manifests/crds/analysis-run-crd.yaml b/manifests/crds/analysis-run-crd.yaml
@@ -2817,6 +2817,10 @@ spec:
                       type: array
                     message:
                       type: string
+                    metadata:
+                      additionalProperties:
+                        type: string
+                      type: object
                     name:
                       type: string
                     phase:

diff --git a/manifests/install.yaml b/manifests/install.yaml
@@ -2818,6 +2818,10 @@ spec:
                       type: array
                     message:
                       type: string
+                    metadata:
+                      additionalProperties:
+                        type: string
+                      type: object
                     name:
                       type: string
                     phase:

diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml
@@ -2818,6 +2818,10 @@ spec:
                       type: array
                     message:
                       type: string
+                    metadata:
+                      additionalProperties:
+                        type: string
+                      type: object
                     name:
                       type: string
                     phase:

diff --git a/metricproviders/cloudwatch/cloudwatch.go b/metricproviders/cloudwatch/cloudwatch.go
@@ -50,6 +50,11 @@ func (p *Provider) Type() string {
 	return ProviderType
 }
 
+// GetMetadata returns any additional metadata which needs to be stored & displayed as part of the metrics result.
+func (p *Provider) GetMetadata(metric v1alpha1.Metric) map[string]string {
+	return nil
+}
+
 // Run queries with CloudWatch provider for the metric
 func (p *Provider) Run(run *v1alpha1.AnalysisRun, metric v1alpha1.Metric) v1alpha1.Measurement {
 	startTime := timeutil.MetaNow()

diff --git a/metricproviders/cloudwatch/cloudwatch_test.go b/metricproviders/cloudwatch/cloudwatch_test.go
@@ -78,6 +78,9 @@ func TestRunWithQueryError(t *testing.T) {
 			},
 		},
 	}
+	metricsMetadata := p.GetMetadata(metric)
+	assert.Nil(t, metricsMetadata)
+
 	measurement := p.Run(newAnalysisRun(), metric)
 	assert.Equal(t, expectedErr.Error(), measurement.Message)
 	assert.NotNil(t, measurement.StartedAt)
@@ -103,6 +106,9 @@ func TestRunWithResolveArgsError(t *testing.T) {
 			},
 		},
 	}
+	metricsMetadata := p.GetMetadata(metric)
+	assert.Nil(t, metricsMetadata)
+
 	measurement := p.Run(newAnalysisRun(), metric)
 	assert.Equal(t, expectedErr.Error(), measurement.Message)
 	assert.NotNil(t, measurement.StartedAt)
@@ -131,6 +137,9 @@ constraint: Member must not be null'`)
 			},
 		},
 	}
+	metricsMetadata := p.GetMetadata(metric)
+	assert.Nil(t, metricsMetadata)
+
 	measurement := p.Run(newAnalysisRun(), metric)
 	assert.Equal(t, expectedErr.Error(), measurement.Message)
 	assert.NotNil(t, measurement.StartedAt)

diff --git a/metricproviders/datadog/datadog.go b/metricproviders/datadog/datadog.go
@@ -59,6 +59,11 @@ func (p *Provider) Type() string {
 	return ProviderType
 }
 
+// GetMetadata returns any additional metadata which needs to be stored & displayed as part of the metrics result.
+func (p *Provider) GetMetadata(metric v1alpha1.Metric) map[string]string {
+	return nil
+}
+
 func (p *Provider) Run(run *v1alpha1.AnalysisRun, metric v1alpha1.Metric) v1alpha1.Measurement {
 	startTime := timeutil.MetaNow()
 

diff --git a/metricproviders/datadog/datadog_test.go b/metricproviders/datadog/datadog_test.go
@@ -314,6 +314,9 @@ func TestRunSuite(t *testing.T) {
 
 		provider, _ := NewDatadogProvider(*logCtx, fakeClient)
 
+		metricsMetadata := provider.GetMetadata(test.metric)
+		assert.Nil(t, metricsMetadata)
+
 		// Get our result
 		measurement := provider.Run(newAnalysisRun(), test.metric)
 

diff --git a/metricproviders/graphite/graphite.go b/metricproviders/graphite/graphite.go
@@ -49,6 +49,11 @@ func (p *Provider) Type() string {
 	return ProviderType
 }
 
+// GetMetadata returns any additional metadata which needs to be stored & displayed as part of the metrics result.
+func (p *Provider) GetMetadata(metric v1alpha1.Metric) map[string]string {
+	return nil
+}
+
 // Run queries Graphite for the metric.
 func (p *Provider) Run(run *v1alpha1.AnalysisRun, metric v1alpha1.Metric) v1alpha1.Measurement {
 	startTime := timeutil.MetaNow()

diff --git a/metricproviders/graphite/graphite_test.go b/metricproviders/graphite/graphite_test.go
@@ -50,6 +50,9 @@ func TestType(t *testing.T) {
 func TestRunSuccessfulEvaluation(t *testing.T) {
 	response := 10.000000
 	g := NewGraphiteProvider(newMockAPI(&response, nil), log.Entry{})
+	metricsMetadata := g.GetMetadata(newTestingMetric())
+	assert.Nil(t, metricsMetadata)
+
 	measurement := g.Run(&v1alpha1.AnalysisRun{}, newTestingMetric())
 	assert.NotNil(t, measurement.StartedAt)
 	assert.Equal(t, "[10.000000]", measurement.Value)
@@ -60,6 +63,9 @@ func TestRunSuccessfulEvaluation(t *testing.T) {
 func TestRunFailedEvaluation(t *testing.T) {
 	response := 5.000000
 	g := NewGraphiteProvider(newMockAPI(&response, nil), log.Entry{})
+	metricsMetadata := g.GetMetadata(newTestingMetric())
+	assert.Nil(t, metricsMetadata)
+
 	measurement := g.Run(&v1alpha1.AnalysisRun{}, newTestingMetric())
 	assert.NotNil(t, measurement.StartedAt)
 	assert.Equal(t, "[5.000000]", measurement.Value)

diff --git a/metricproviders/job/job.go b/metricproviders/job/job.go
@@ -55,6 +55,11 @@ func (p *JobProvider) Type() string {
 	return ProviderType
 }
 
+// GetMetadata returns any additional metadata which needs to be stored & displayed as part of the metrics result.
+func (p *JobProvider) GetMetadata(metric v1alpha1.Metric) map[string]string {
+	return nil
+}
+
 // newJobName returns a new job name for the run and metric. Names must be shortened so that it can
 // fit into a 63 character label, since the k8s job controller incorporates the job name into the
 // pod spec labels.

diff --git a/metricproviders/job/job_test.go b/metricproviders/job/job_test.go
@@ -120,6 +120,9 @@ func TestRun(t *testing.T) {
 	p := newTestJobProvider()
 	run := newRunWithJobMetric()
 	metric := run.Spec.Metrics[0]
+	metricsMetadata := p.GetMetadata(metric)
+	assert.Nil(t, metricsMetadata)
+
 	measurement := p.Run(run, metric)
 
 	assert.Equal(t, v1alpha1.AnalysisPhaseRunning, measurement.Phase)
@@ -160,6 +163,8 @@ func TestRunCreateFail(t *testing.T) {
 	fakeClient.PrependReactor("create", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
 		return true, nil, fmt.Errorf(errMsg)
 	})
+	metricsMetadata := p.GetMetadata(run.Spec.Metrics[0])
+	assert.Nil(t, metricsMetadata)
 
 	measurement := p.Run(run, run.Spec.Metrics[0])
 	assert.Equal(t, v1alpha1.AnalysisPhaseError, measurement.Phase)

diff --git a/metricproviders/kayenta/kayenta.go b/metricproviders/kayenta/kayenta.go
@@ -61,6 +61,11 @@ func (p *Provider) Type() string {
 	return ProviderType
 }
 
+// GetMetadata returns any additional metadata which needs to be stored & displayed as part of the metrics result.
+func (p *Provider) GetMetadata(metric v1alpha1.Metric) map[string]string {
+	return nil
+}
+
 func getCanaryConfigId(metric v1alpha1.Metric, p *Provider) (string, error) {
 
 	configIdLookupURL := fmt.Sprintf(configIdLookupURLFormat, metric.Provider.Kayenta.Address, metric.Provider.Kayenta.Application, metric.Provider.Kayenta.StorageAccountName)

diff --git a/metricproviders/kayenta/kayenta_test.go b/metricproviders/kayenta/kayenta_test.go
@@ -206,6 +206,8 @@ func TestRunSuccessfully(t *testing.T) {
 		{Name: "stable-hash", Value: &stableHash},
 		{Name: "canary-hash", Value: &canaryHash},
 	}
+	metricsMetadata := p.GetMetadata(metric)
+	assert.Nil(t, metricsMetadata)
 
 	measurement := p.Run(run, metric)
 
@@ -268,6 +270,8 @@ func TestRunBadJobResponse(t *testing.T) {
 		{Name: "stable-hash", Value: &stableHash},
 		{Name: "canary-hash", Value: &canaryHash},
 	}
+	metricsMetadata := p.GetMetadata(metric)
+	assert.Nil(t, metricsMetadata)
 
 	measurement := p.Run(run, metric)
 

diff --git a/metricproviders/metricproviders.go b/metricproviders/metricproviders.go
@@ -34,6 +34,9 @@ type Provider interface {
 	GarbageCollect(*v1alpha1.AnalysisRun, v1alpha1.Metric, int) error
 	// Type gets the provider type
 	Type() string
+	// GetMetadata returns any additional metadata which providers need to store/display as part
+	// of the metric result. For example, Prometheus uses is to store the final resolved queries.
+	GetMetadata(metric v1alpha1.Metric) map[string]string
 }
 
 type ProviderFactory struct {

diff --git a/metricproviders/mocks/Provider.go b/metricproviders/mocks/Provider.go
diff --git a/metricproviders/newrelic/newrelic.go b/metricproviders/newrelic/newrelic.go
@@ -131,6 +131,11 @@ func (p *Provider) Type() string {
 	return ProviderType
 }
 
+// GetMetadata returns any additional metadata which needs to be stored & displayed as part of the metrics result.
+func (p *Provider) GetMetadata(metric v1alpha1.Metric) map[string]string {
+	return nil
+}
+
 //NewNewRelicProvider creates a new NewRelic provider
 func NewNewRelicProvider(api NewRelicClientAPI, logCtx log.Entry) *Provider {
 	return &Provider{

diff --git a/metricproviders/newrelic/newrelic_test.go b/metricproviders/newrelic/newrelic_test.go
@@ -44,6 +44,9 @@ func TestRunSuccessfully(t *testing.T) {
 			},
 		},
 	}
+	metricsMetadata := p.GetMetadata(metric)
+	assert.Nil(t, metricsMetadata)
+
 	measurement := p.Run(newAnalysisRun(), metric)
 	assert.NotNil(t, measurement.StartedAt)
 	assert.Equal(t, `{"count":10}`, measurement.Value)
@@ -70,6 +73,9 @@ func TestRunWithTimeseries(t *testing.T) {
 			},
 		},
 	}
+	metricsMetadata := p.GetMetadata(metric)
+	assert.Nil(t, metricsMetadata)
+
 	measurement := p.Run(newAnalysisRun(), metric)
 	assert.NotNil(t, measurement.StartedAt)
 	assert.Equal(t, `[{"count":10},{"count":20},{"count":30}]`, measurement.Value)
@@ -93,6 +99,9 @@ func TestRunWithFacet(t *testing.T) {
 			},
 		},
 	}
+	metricsMetadata := p.GetMetadata(metric)
+	assert.Nil(t, metricsMetadata)
+
 	measurement := p.Run(newAnalysisRun(), metric)
 	assert.NotNil(t, measurement.StartedAt)
 	assert.Equal(t, `{"average.duration":12.34,"count":10}`, measurement.Value)
@@ -116,6 +125,9 @@ func TestRunWithMultipleSelectTerms(t *testing.T) {
 			},
 		},
 	}
+	metricsMetadata := p.GetMetadata(metric)
+	assert.Nil(t, metricsMetadata)
+
 	measurement := p.Run(newAnalysisRun(), metric)
 	assert.NotNil(t, measurement.StartedAt)
 	assert.Equal(t, `{"count":10}`, measurement.Value)