Merge pull request #1355 from njohnstone2/metric_template_vars

Add support for custom variables in metric templates
fluxcd · Feb 8, 2023 · e59e3ae · e59e3ae
2 parents e7d8ade + 2b45c20
commit e59e3ae
Show file tree

Hide file tree

Showing 16 changed files with 253 additions and 17 deletions.
diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml
@@ -970,6 +970,11 @@ spec:
                               namespace:
                                 description: Namespace of this metric template
                                 type: string
+                          templateVariables:
+                            description: Additional variables to be used in the metrics query (key-value pairs)
+                            type: object
+                            additionalProperties:
+                              type: string
                     alerts:
                       description: Alert list for this canary analysis
                       type: array

diff --git a/charts/flagger/crds/crd.yaml b/charts/flagger/crds/crd.yaml
@@ -970,6 +970,11 @@ spec:
                               namespace:
                                 description: Namespace of this metric template
                                 type: string
+                          templateVariables:
+                            description: Additional variables to be used in the metrics query (key-value pairs)
+                            type: object
+                            additionalProperties:
+                              type: string
                     alerts:
                       description: Alert list for this canary analysis
                       type: array

diff --git a/docs/gitbook/usage/metrics.md b/docs/gitbook/usage/metrics.md
@@ -62,6 +62,7 @@ The following variables are available in query templates:
 * `service` (canary.spec.service.name)
 * `ingress` (canary.spec.ingresRef.name)
 * `interval` (canary.spec.analysis.metrics[].interval)
+* `variables` (canary.spec.analysis.metrics[].templateVariables)
 
 A canary analysis metric can reference a template with `templateRef`:
 
@@ -82,6 +83,50 @@ A canary analysis metric can reference a template with `templateRef`:
         interval: 1m
 ```
 
+A canary analysis metric can reference a set of custom variables with `templateVariables`. These variables will be then injected into the query defined in the referred `MetricTemplate` object during canary analysis:
+
+```yaml
+  analysis:
+    metrics:
+      - name: "my metric"
+        templateRef:
+          name: my-metric
+          namespace: flagger
+        # accepted values
+        thresholdRange:
+          min: 10
+          max: 1000
+        # metric query time window
+        interval: 1m
+        # custom variables used within the referenced metric template
+        templateVariables:
+          direction: inbound
+```
+
+```yaml
+apiVersion: flagger.app/v1beta1
+kind: MetricTemplate
+metadata:
+  name: my-metric
+spec:
+  provider:
+    type: prometheus
+    address: http://prometheus.linkerd-viz:9090
+  query: |
+    histogram_quantile(
+      0.99,
+      sum(
+        rate(
+          response_latency_ms_bucket{
+            namespace="{{ namespace }}",
+            deployment=~"{{ target }}",
+            direction="{{ variables.direction }}"
+          }[{{ interval }}]
+        )
+      ) by (le)
+    )
+```
+
 ## Prometheus
 
 You can create custom metric checks targeting a Prometheus server by

diff --git a/kustomize/base/flagger/crd.yaml b/kustomize/base/flagger/crd.yaml
@@ -970,6 +970,11 @@ spec:
                               namespace:
                                 description: Namespace of this metric template
                                 type: string
+                          templateVariables:
+                            description: Additional variables to be used in the metrics query (key-value pairs)
+                            type: object
+                            additionalProperties:
+                              type: string
                     alerts:
                       description: Alert list for this canary analysis
                       type: array

diff --git a/pkg/apis/flagger/v1beta1/canary.go b/pkg/apis/flagger/v1beta1/canary.go
@@ -304,6 +304,10 @@ type CanaryMetric struct {
 	// TemplateRef references a metric template object
 	// +optional
 	TemplateRef *CrossNamespaceObjectReference `json:"templateRef,omitempty"`
+
+	// TemplateVariables provides a map of key/value pairs that can be used to inject variables into a metric query.
+	// +optional
+	TemplateVariables map[string]string `json:"templateVariables,omitempty"`
 }
 
 // CanaryThresholdRange defines the range used for metrics validation

diff --git a/pkg/apis/flagger/v1beta1/metric.go b/pkg/apis/flagger/v1beta1/metric.go
@@ -82,13 +82,14 @@ type MetricTemplateProvider struct {
 
 // MetricTemplateModel is the query template model
 type MetricTemplateModel struct {
-	Name      string `json:"name"`
-	Namespace string `json:"namespace"`
-	Target    string `json:"target"`
-	Service   string `json:"service"`
-	Ingress   string `json:"ingress"`
-	Route     string `json:"route"`
-	Interval  string `json:"interval"`
+	Name      string            `json:"name"`
+	Namespace string            `json:"namespace"`
+	Target    string            `json:"target"`
+	Service   string            `json:"service"`
+	Ingress   string            `json:"ingress"`
+	Route     string            `json:"route"`
+	Interval  string            `json:"interval"`
+	Variables map[string]string `json:"variables"`
 }
 
 // TemplateFunctions returns a map of functions, one for each model field
@@ -101,6 +102,7 @@ func (mtm *MetricTemplateModel) TemplateFunctions() template.FuncMap {
 		"ingress":   func() string { return mtm.Ingress },
 		"route":     func() string { return mtm.Route },
 		"interval":  func() string { return mtm.Interval },
+		"variables": func() map[string]string { return mtm.Variables },
 	}
 }
 

diff --git a/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go b/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go
diff --git a/pkg/controller/scheduler_deployment_fixture_test.go b/pkg/controller/scheduler_deployment_fixture_test.go
@@ -91,6 +91,7 @@ func newDeploymentFixture(c *flaggerv1.Canary) fixture {
 	flaggerClient := fakeFlagger.NewSimpleClientset(
 		c,
 		newDeploymentTestMetricTemplate(),
+		newDeploymentTestMetricTemplateCustomVars(),
 		newDeploymentTestAlertProvider(),
 	)
 
@@ -152,6 +153,7 @@ func newDeploymentFixture(c *flaggerv1.Canary) fixture {
 	ctrl.flaggerSynced = alwaysReady
 	ctrl.flaggerInformers.CanaryInformer.Informer().GetIndexer().Add(c)
 	ctrl.flaggerInformers.MetricInformer.Informer().GetIndexer().Add(newDeploymentTestMetricTemplate())
+	ctrl.flaggerInformers.MetricInformer.Informer().GetIndexer().Add(newDeploymentTestMetricTemplateCustomVars())
 	ctrl.flaggerInformers.AlertInformer.Informer().GetIndexer().Add(newDeploymentTestAlertProvider())
 
 	meshRouter := rf.MeshRouter("istio", "")
@@ -746,6 +748,29 @@ func newDeploymentTestMetricTemplate() *flaggerv1.MetricTemplate {
 	return template
 }
 
+func newDeploymentTestMetricTemplateCustomVars() *flaggerv1.MetricTemplate {
+	provider := flaggerv1.MetricTemplateProvider{
+		Type:    "prometheus",
+		Address: testMetricsServerURL,
+		SecretRef: &corev1.LocalObjectReference{
+			Name: "podinfo-secret-env",
+		},
+	}
+
+	template := &flaggerv1.MetricTemplate{
+		TypeMeta: metav1.TypeMeta{APIVersion: flaggerv1.SchemeGroupVersion.String()},
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "default",
+			Name:      "custom-vars",
+		},
+		Spec: flaggerv1.MetricTemplateSpec{
+			Provider: provider,
+			Query:    `sum(envoy_cluster_upstream_rq{envoy_cluster_name=~"{{ namespace }}_{{ target }},custom_label!={{ variables.second }}"})`,
+		},
+	}
+	return template
+}
+
 func newDeploymentTestAlertProviderSecret() *corev1.Secret {
 	return &corev1.Secret{
 		TypeMeta: metav1.TypeMeta{APIVersion: corev1.SchemeGroupVersion.String()},

diff --git a/pkg/controller/scheduler_metrics.go b/pkg/controller/scheduler_metrics.go
@@ -135,7 +135,7 @@ func (c *Controller) runBuiltinMetricChecks(canary *flaggerv1.Canary) bool {
 		}
 
 		if metric.Name == "request-success-rate" {
-			val, err := observer.GetRequestSuccessRate(toMetricModel(canary, metric.Interval))
+			val, err := observer.GetRequestSuccessRate(toMetricModel(canary, metric.Interval, metric.TemplateVariables))
 			if err != nil {
 				if errors.Is(err, providers.ErrNoValuesFound) {
 					c.recordEventWarningf(canary,
@@ -167,7 +167,7 @@ func (c *Controller) runBuiltinMetricChecks(canary *flaggerv1.Canary) bool {
 		}
 
 		if metric.Name == "request-duration" {
-			val, err := observer.GetRequestDuration(toMetricModel(canary, metric.Interval))
+			val, err := observer.GetRequestDuration(toMetricModel(canary, metric.Interval, metric.TemplateVariables))
 			if err != nil {
 				if errors.Is(err, providers.ErrNoValuesFound) {
 					c.recordEventWarningf(canary, "Halt advancement no values found for %s metric %s probably %s.%s is not receiving traffic",
@@ -199,7 +199,7 @@ func (c *Controller) runBuiltinMetricChecks(canary *flaggerv1.Canary) bool {
 
 		// in-line PromQL
 		if metric.Query != "" {
-			query, err := observers.RenderQuery(metric.Query, toMetricModel(canary, metric.Interval))
+			query, err := observers.RenderQuery(metric.Query, toMetricModel(canary, metric.Interval, metric.TemplateVariables))
 			val, err := observerFactory.Client.RunQuery(query)
 			if err != nil {
 				if errors.Is(err, providers.ErrNoValuesFound) {
@@ -267,7 +267,9 @@ func (c *Controller) runMetricChecks(canary *flaggerv1.Canary) bool {
 				return false
 			}
 
-			query, err := observers.RenderQuery(template.Spec.Query, toMetricModel(canary, metric.Interval))
+			query, err := observers.RenderQuery(template.Spec.Query, toMetricModel(canary, metric.Interval, metric.TemplateVariables))
+			c.logger.With("canary", fmt.Sprintf("%s.%s", canary.Name, namespace)).
+				Debugf("Metric template %s.%s query: %s", metric.TemplateRef.Name, namespace, query)
 			if err != nil {
 				c.recordEventErrorf(canary, "Metric template %s.%s query render error: %v",
 					metric.TemplateRef.Name, namespace, err)
@@ -310,7 +312,7 @@ func (c *Controller) runMetricChecks(canary *flaggerv1.Canary) bool {
 	return true
 }
 
-func toMetricModel(r *flaggerv1.Canary, interval string) flaggerv1.MetricTemplateModel {
+func toMetricModel(r *flaggerv1.Canary, interval string, variables map[string]string) flaggerv1.MetricTemplateModel {
 	service := r.Spec.TargetRef.Name
 	if r.Spec.Service.Name != "" {
 		service = r.Spec.Service.Name
@@ -331,5 +333,6 @@ func toMetricModel(r *flaggerv1.Canary, interval string) flaggerv1.MetricTemplat
 		Ingress:   ingress,
 		Route:     route,
 		Interval:  interval,
+		Variables: variables,
 	}
 }
diff --git a/pkg/controller/scheduler_metrics_test.go b/pkg/controller/scheduler_metrics_test.go
@@ -19,6 +19,7 @@ package controller
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 	"k8s.io/client-go/tools/record"
@@ -82,3 +83,28 @@ func TestController_checkMetricProviderAvailability(t *testing.T) {
 		require.NoError(t, ctrl.checkMetricProviderAvailability(canary))
 	})
 }
+
+func TestController_runMetricChecks(t *testing.T) {
+	t.Run("customVariables", func(t *testing.T) {
+		ctrl := newDeploymentFixture(nil).ctrl
+		analysis := &flaggerv1.CanaryAnalysis{Metrics: []flaggerv1.CanaryMetric{{
+			Name: "", TemplateVariables: map[string]string{
+				"first":  "abc",
+				"second": "def",
+			},
+			TemplateRef: &flaggerv1.CrossNamespaceObjectReference{
+				Name:      "custom-vars",
+				Namespace: "default",
+			},
+			ThresholdRange: &flaggerv1.CanaryThresholdRange{
+				Min: toFloatPtr(0),
+				Max: toFloatPtr(100),
+			},
+		}}}
+		canary := &flaggerv1.Canary{
+			ObjectMeta: metav1.ObjectMeta{Namespace: "default"},
+			Spec:       flaggerv1.CanarySpec{Analysis: analysis},
+		}
+		assert.Equal(t, true, ctrl.runMetricChecks(canary))
+	})
+}
diff --git a/pkg/metrics/observers/render.go b/pkg/metrics/observers/render.go
@@ -26,7 +26,7 @@ import (
 )
 
 func RenderQuery(queryTemplate string, model flaggerv1.MetricTemplateModel) (string, error) {
-	t, err := template.New("tmpl").Funcs(model.TemplateFunctions()).Parse(queryTemplate)
+	t, err := template.New("tmpl").Option("missingkey=error").Funcs(model.TemplateFunctions()).Parse(queryTemplate)
 	if err != nil {
 		return "", fmt.Errorf("template parsing failed: %w", err)
 	}

diff --git a/pkg/metrics/observers/render_test.go b/pkg/metrics/observers/render_test.go
@@ -0,0 +1,82 @@
+/*
+Copyright 2020 The Flux authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package observers
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1"
+)
+
+func Test_RenderQuery(t *testing.T) {
+	t.Run("ok_without_variables", func(t *testing.T) {
+		expected := `sum(envoy_cluster_upstream_rq{envoy_cluster_name=~"default_myapp"})`
+		templateQuery := `sum(envoy_cluster_upstream_rq{envoy_cluster_name=~"{{ namespace }}_{{ target }}"})`
+
+		model := &flaggerv1.MetricTemplateModel{
+			Name:      "standard",
+			Namespace: "default",
+			Target:    "myapp",
+			Interval:  "1m",
+		}
+
+		actual, err := RenderQuery(templateQuery, *model)
+		require.NoError(t, err)
+
+		assert.Equal(t, expected, actual)
+	})
+
+	t.Run("ok_with_variables", func(t *testing.T) {
+		expected := `delta(max by (consumer_group) (kafka_consumer_current_offset{cluster="dev", consumer_group="my_consumer"}[1m]))`
+		templateQuery := `delta(max by (consumer_group) (kafka_consumer_current_offset{cluster="{{ variables.cluster }}", consumer_group="{{ variables.consumer_group }}"}[{{ interval }}]))`
+
+		model := &flaggerv1.MetricTemplateModel{
+			Name:      "kafka_consumer_offset",
+			Namespace: "default",
+			Interval:  "1m",
+			Variables: map[string]string{
+				"cluster":        "dev",
+				"consumer_group": "my_consumer",
+			},
+		}
+
+		actual, err := RenderQuery(templateQuery, *model)
+		require.NoError(t, err)
+
+		assert.Equal(t, expected, actual)
+	})
+
+	t.Run("missing_variable_key", func(t *testing.T) {
+		templateQuery := `delta(max by (consumer_group) (kafka_consumer_current_offset{cluster="{{ variables.cluster }}", consumer_group="{{ variables.consumer_group }}"}[{{ interval }}]))`
+
+		model := &flaggerv1.MetricTemplateModel{
+			Name:      "kafka_consumer_offset",
+			Namespace: "default",
+			Interval:  "1m",
+			Variables: map[string]string{
+				"invalid":        "dev",
+				"consumer_group": "my_consumer",
+			},
+		}
+
+		_, err := RenderQuery(templateQuery, *model)
+		require.Error(t, err)
+	})
+}