From 3727f9934370a28b5e26cb3435c7ab3c633a9c97 Mon Sep 17 00:00:00 2001 From: Tony Compton Date: Wed, 27 Jun 2018 21:37:21 -0400 Subject: [PATCH] Moving the query building logic, adding first unit test. Moving it into its own type to better organize and separate concerns. Also makes unit testing easier. --- .../external_metric_query_builder.go | 94 +++++++++++++++++++ .../external_metric_query_builder_test.go | 23 +++++ pkg/custom-provider/external_provider.go | 83 ++-------------- 3 files changed, 124 insertions(+), 76 deletions(-) create mode 100644 pkg/custom-provider/external_metric_query_builder.go create mode 100644 pkg/custom-provider/external_metric_query_builder_test.go diff --git a/pkg/custom-provider/external_metric_query_builder.go b/pkg/custom-provider/external_metric_query_builder.go new file mode 100644 index 00000000..411f0de2 --- /dev/null +++ b/pkg/custom-provider/external_metric_query_builder.go @@ -0,0 +1,94 @@ +package provider + +import ( + "fmt" + s "strings" + + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" +) + +type ExternalMetricQueryBuilder interface { + BuildPrometheusQuery(namespace string, metricName string, metricSelector labels.Selector) string +} + +type externalMetricQueryBuilder struct { +} + +func NewExternalMetricQueryBuilder() ExternalMetricQueryBuilder { + return &externalMetricQueryBuilder{} +} + +func (p *externalMetricQueryBuilder) BuildPrometheusQuery(namespace string, metricName string, metricSelector labels.Selector) string { + namespaceSelector := p.makeLabelFilter("namespace", "=", namespace) + otherSelectors := p.convertSelectors(metricSelector) + + finalTargets := append([]string{namespaceSelector}, otherSelectors...) + joinedLabels := s.Join(finalTargets, ", ") + return fmt.Sprintf("%s{%s}", metricName, joinedLabels) +} + +func (p *externalMetricQueryBuilder) makeLabelFilter(labelName string, operator string, targetValue string) string { + return fmt.Sprintf("%s%s\"%s\"", labelName, operator, targetValue) +} + +func (p *externalMetricQueryBuilder) convertSelectors(metricSelector labels.Selector) []string { + requirements, _ := metricSelector.Requirements() + + selectors := []string{} + for i := 0; i < len(requirements); i++ { + selector := p.convertRequirement(requirements[i]) + + selectors = append(selectors, selector) + } + + return selectors +} + +func (p *externalMetricQueryBuilder) convertRequirement(requirement labels.Requirement) string { + labelName := requirement.Key() + values := requirement.Values().List() + + stringValues := values[0] + + valueCount := len(values) + if valueCount > 1 { + stringValues = s.Join(values, "|") + } + + operator := p.selectOperator(requirement.Operator(), valueCount) + + return p.makeLabelFilter(labelName, operator, stringValues) +} + +func (p *externalMetricQueryBuilder) selectOperator(operator selection.Operator, valueCount int) string { + if valueCount > 1 { + return p.selectRegexOperator(operator) + } + + return p.selectSingleValueOperator(operator) +} + +func (p *externalMetricQueryBuilder) selectRegexOperator(operator selection.Operator) string { + switch operator { + case selection.Equals: + return "=~" + case selection.NotEquals: + return "!~" + } + + //TODO: Cover more cases, supply appropriate errors for any unhandled cases. + return "=" +} + +func (p *externalMetricQueryBuilder) selectSingleValueOperator(operator selection.Operator) string { + switch operator { + case selection.Equals: + return "=" + case selection.NotEquals: + return "!=" + } + + //TODO: Cover more cases, supply appropriate errors for any unhandled cases. + return "=" +} diff --git a/pkg/custom-provider/external_metric_query_builder_test.go b/pkg/custom-provider/external_metric_query_builder_test.go new file mode 100644 index 00000000..f4037989 --- /dev/null +++ b/pkg/custom-provider/external_metric_query_builder_test.go @@ -0,0 +1,23 @@ +package provider + +import ( + "testing" + + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" +) + +var queryBuilder = NewExternalMetricQueryBuilder() + +func TestBuildPrometheusQuery(t *testing.T) { + fakeSelector := labels.NewSelector() + requirement, _ := labels.NewRequirement("queue_name", selection.Equals, []string{"processing"}) + fakeSelector = fakeSelector.Add(*requirement) + + result := queryBuilder.BuildPrometheusQuery("default", "queue_length", fakeSelector) + + expectedResult := "queue_length{namespace=\"default\", queue_name=\"processing\"}" + if result != expectedResult { + t.Errorf("Incorrect query generated. Expected: %s | Actual %s", result, expectedResult) + } +} diff --git a/pkg/custom-provider/external_provider.go b/pkg/custom-provider/external_provider.go index 544bcd1b..b21035ac 100644 --- a/pkg/custom-provider/external_provider.go +++ b/pkg/custom-provider/external_provider.go @@ -18,13 +18,11 @@ package provider import ( "fmt" - s "strings" "time" "github.com/kubernetes-incubator/custom-metrics-apiserver/pkg/provider" apimeta "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/selection" "k8s.io/client-go/dynamic" "k8s.io/metrics/pkg/apis/external_metrics" @@ -32,14 +30,15 @@ import ( ) type externalPrometheusProvider struct { - mapper apimeta.RESTMapper - kubeClient dynamic.Interface - promClient prom.Client + mapper apimeta.RESTMapper + kubeClient dynamic.Interface + promClient prom.Client + queryBuilder ExternalMetricQueryBuilder SeriesRegistry } -func NewExternalPrometheusProvider(mapper apimeta.RESTMapper, kubeClient dynamic.Interface, promClient prom.Client, namers []MetricNamer, updateInterval time.Duration) (provider.ExternalMetricsProvider, Runnable) { +func NewExternalPrometheusProvider(mapper apimeta.RESTMapper, kubeClient dynamic.Interface, promClient prom.Client, namers []MetricNamer, updateInterval time.Duration, queryBuilder ExternalMetricQueryBuilder) (provider.ExternalMetricsProvider, Runnable) { lister := &cachingMetricsLister{ updateInterval: updateInterval, promClient: promClient, @@ -69,83 +68,15 @@ func (p *externalPrometheusProvider) GetExternalMetric(namespace string, metricN //than for custom metrics because no renaming is applied. //So we'll just start with some simple string operations and see how far that gets us. //Then I'll circle back and figure out how much code reuse I can get out of the original implementation. - namespaceSelector := p.makeLabelFilter("namespace", "=", namespace) - otherSelectors := p.convertSelectors(metricSelector) - - finalTargets := append([]string{namespaceSelector}, otherSelectors...) + query := p.queryBuilder.BuildPrometheusQuery(namespace, metricName, metricSelector) //TODO: Only here to stop compiler issues in this incomplete code. - fmt.Printf("len=%d", len(finalTargets)) + fmt.Printf(query) //TODO: Construct a real result. return nil, nil } -func (p *externalPrometheusProvider) makeLabelFilter(labelName string, operator string, targetValue string) string { - return fmt.Sprintf("%s%s\"%s\"", labelName, operator, targetValue) -} - -func (p *externalPrometheusProvider) convertSelectors(metricSelector labels.Selector) []string { - requirements, _ := metricSelector.Requirements() - - selectors := []string{} - for i := 0; i < len(requirements); i++ { - selector := p.convertRequirement(requirements[i]) - - selectors = append(selectors, selector) - } - - return selectors -} - -func (p *externalPrometheusProvider) convertRequirement(requirement labels.Requirement) string { - labelName := requirement.Key() - values := requirement.Values().List() - - stringValues := values[0] - - valueCount := len(values) - if valueCount > 1 { - stringValues = s.Join(values, "|") - } - - operator := p.selectOperator(requirement.Operator(), valueCount) - - return p.makeLabelFilter(labelName, operator, stringValues) -} - -func (p *externalPrometheusProvider) selectOperator(operator selection.Operator, valueCount int) string { - if valueCount > 1 { - return p.selectRegexOperator(operator) - } - - return p.selectSingleValueOperator(operator) -} - -func (p *externalPrometheusProvider) selectRegexOperator(operator selection.Operator) string { - switch operator { - case selection.Equals: - return "=~" - case selection.NotEquals: - return "!~" - } - - //TODO: Cover more cases, supply appropriate errors for any unhandled cases. - return "=" -} - -func (p *externalPrometheusProvider) selectSingleValueOperator(operator selection.Operator) string { - switch operator { - case selection.Equals: - return "=" - case selection.NotEquals: - return "!=" - } - - //TODO: Cover more cases, supply appropriate errors for any unhandled cases. - return "=" -} - func (p *externalPrometheusProvider) ListAllExternalMetrics() []provider.ExternalMetricInfo { //TODO: Provide a real response. return nil