You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by wu...@apache.org on 2021/03/24 13:55:03 UTC

[skywalking-swck] branch master updated: Some enhancements for HPA metric adapter (#25)

This is an automated email from the ASF dual-hosted git repository.

wusheng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking-swck.git


The following commit(s) were added to refs/heads/master by this push:
     new d6f5bfa  Some enhancements for HPA metric adapter (#25)
d6f5bfa is described below

commit d6f5bfae337d645f53eece73b13d34e556015f41
Author: Gao Hongtao <ha...@gmail.com>
AuthorDate: Wed Mar 24 21:54:54 2021 +0800

    Some enhancements for HPA metric adapter (#25)
---
 CHANGES.md                                         |   7 ++
 build/images/Dockerfile.adapter                    |   4 +-
 build/images/Dockerfile.operator                   |   4 +-
 cmd/adapter/adapter.go                             |   5 +-
 .../apiserver_auth_reader_role_binding.yaml        |   4 +-
 config/adapter/kustomization.yaml                  |   2 +-
 config/adapter/namespaced/kustomization.yaml       |   2 +-
 docs/custom-metrics-adapter.md                     |  56 +++++++--
 pkg/provider/provider.go                           |  92 +++++++++++++--
 pkg/provider/provider_test.go                      | 128 +++++++++++++++++++++
 pkg/provider/registry.go                           |   2 +-
 11 files changed, 279 insertions(+), 27 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 31e7092..94597ca 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -2,6 +2,13 @@ Changes by Version
 ==================
 Release Notes.
 
+0.2.1
+------------------
+
+#### Features
+- Support special characters in the metric selector of HPA metric adapter.
+- Add the namespace to HPA metric name.
+
 0.2.0
 ------------------
 
diff --git a/build/images/Dockerfile.adapter b/build/images/Dockerfile.adapter
index 9ba5731..c775f18 100644
--- a/build/images/Dockerfile.adapter
+++ b/build/images/Dockerfile.adapter
@@ -34,8 +34,8 @@ RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o adapter
 # Use distroless as minimal base image to package the manager binary
 # Refer to https://github.com/GoogleContainerTools/distroless for more details
 FROM gcr.io/distroless/static:nonroot
-WORKDIR /
-COPY --from=builder --chown=nonroot:nonroot /workspace/adapter .
+WORKDIR /tmp
+COPY --from=builder --chown=nonroot:nonroot /workspace/adapter /adapter
 USER nonroot:nonroot
 
 ENTRYPOINT ["/adapter"]
diff --git a/build/images/Dockerfile.operator b/build/images/Dockerfile.operator
index 7001cbd..e8de019 100644
--- a/build/images/Dockerfile.operator
+++ b/build/images/Dockerfile.operator
@@ -36,8 +36,8 @@ RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o manager
 # Use distroless as minimal base image to package the manager binary
 # Refer to https://github.com/GoogleContainerTools/distroless for more details
 FROM gcr.io/distroless/static:nonroot
-WORKDIR /
-COPY --from=builder /workspace/manager .
+WORKDIR /tmp
+COPY --from=builder /workspace/manager /manager
 USER nonroot:nonroot
 
 ENTRYPOINT ["/manager"]
diff --git a/cmd/adapter/adapter.go b/cmd/adapter/adapter.go
index 67ba1e6..1db945d 100644
--- a/cmd/adapter/adapter.go
+++ b/cmd/adapter/adapter.go
@@ -44,6 +44,8 @@ type Adapter struct {
 	RefreshRegistryInterval time.Duration
 	// Message is printed on successful startup
 	Message string
+	// Namespace groups metrics into a single set in case of duplicated metric name
+	Namespace string
 }
 
 func main() {
@@ -59,6 +61,7 @@ func main() {
 	cmd.Flags().StringVar(&cmd.Message, "msg", "starting adapter...", "startup message")
 	cmd.Flags().StringVar(&cmd.BaseURL, "oap-addr", "http://oap:12800/graphql", "the address of OAP cluster")
 	cmd.Flags().StringVar(&cmd.MetricRegex, "metric-filter-regex", "", "a regular expression to filter metrics retrieved from OAP cluster")
+	cmd.Flags().StringVar(&cmd.Namespace, "namespace", "skywalking.apache.org", "a prefix to which metrics are appended. The format is 'namespace|metric_name'")
 	cmd.Flags().DurationVar(&cmd.RefreshRegistryInterval, "refresh-interval", 10*time.Second,
 		"the interval at which to update the cache of available metrics from OAP cluster")
 	cmd.Flags().AddGoFlagSet(flag.CommandLine) // make sure we get the klog flags
@@ -66,7 +69,7 @@ func main() {
 		klog.Fatalf("failed to parse arguments: %v", err)
 	}
 
-	p, err := swckprov.NewProvider(cmd.BaseURL, cmd.MetricRegex, cmd.RefreshRegistryInterval)
+	p, err := swckprov.NewProvider(cmd.BaseURL, cmd.MetricRegex, cmd.RefreshRegistryInterval, cmd.Namespace)
 	if err != nil {
 		klog.Fatalf("unable to build p: %v", err)
 	}
diff --git a/config/adapter/apiserver_auth_reader_role_binding.yaml b/config/adapter/apiserver_auth_reader_role_binding.yaml
index 57b6418..240735c 100644
--- a/config/adapter/apiserver_auth_reader_role_binding.yaml
+++ b/config/adapter/apiserver_auth_reader_role_binding.yaml
@@ -26,6 +26,6 @@ roleRef:
   name: extension-apiserver-authentication-reader
 subjects:
   - kind: ServiceAccount
-    name: skywalking-cutome-metrics-apiserver
-    namespace: skywalking-cutome-metrics-system
+    name: skywalking-custom-metrics-apiserver
+    namespace: skywalking-custom-metrics-system
 
diff --git a/config/adapter/kustomization.yaml b/config/adapter/kustomization.yaml
index 36b47a5..f379ab2 100644
--- a/config/adapter/kustomization.yaml
+++ b/config/adapter/kustomization.yaml
@@ -15,7 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 
-namePrefix: skywalking-cutome-metrics-
+namePrefix: skywalking-custom-metrics-
 
 resources:
 - namespaced
diff --git a/config/adapter/namespaced/kustomization.yaml b/config/adapter/namespaced/kustomization.yaml
index 09d479c..b0eaa6e 100644
--- a/config/adapter/namespaced/kustomization.yaml
+++ b/config/adapter/namespaced/kustomization.yaml
@@ -17,7 +17,7 @@
 
 # Adds namespace to all resources.
 # If you update the namespace, ../apiserver_auth_reader_role_binding_patch.yaml should be changed correspondingly.
-namespace: skywalking-cutome-metrics-system
+namespace: skywalking-custom-metrics-system
 
 resources:
 - rbac
diff --git a/docs/custom-metrics-adapter.md b/docs/custom-metrics-adapter.md
index 5e9b6e7..2690ae4 100644
--- a/docs/custom-metrics-adapter.md
+++ b/docs/custom-metrics-adapter.md
@@ -31,6 +31,7 @@ It takes the following addition arguments specific to configuring how the adapte
  * `--oap-addr` The address of OAP cluster.
  * `--metric-filter-regex` A regular expression to filter metrics retrieved from OAP cluster.
  * `--refresh-interval` This is the interval at which to update the cache of available metrics from OAP cluster. 
+ * `--namespace` A prefix to which metrics are appended. The format is 'namespace|metric_name', defaults to `skywalking.apache.org`
  
 ## HPA Configuration
 
@@ -51,13 +52,31 @@ External metrics allow you to autoscale your cluster based on any metric availab
 ```
 
  * metric_name: The name of metric generated by OAL or other subsystem.
- * label: `label_key` is from the arguments of swctl . 
+ * label: `label_key` is the entity name of skywalking metrics. if the label value contains special characters more than
+   `.`, `-` and `_`, `service.str.<number>` represent the literal of label value, and `service.byte.<number>` could 
+   encode these special characters to hex bytes.
+   
+Supposing the service name is `v1|productpage|bookinfo|demo`, the `matchLabels` should be like the below piece:
+
+```yaml
+matchLabels:
+  "service.str.0":  "v1"
+  "service.byte.1": "7c" // the hex byte of "|"
+  "service.str.2":  "productpage"
+  "service.byte.3": "7c"
+  "service.str.4":  "bookinfo"
+  "service.byte.5": "7c"
+  "service.str.6":  "demo"
+```
+
+> Caveats: `byte` label only accept a single character. That means `||` should be transformed to `service.byte.0:"7c"`
+> and `service.byte.1:"7c"` instead of `service.byte.0:"7c7c"`
   
 The options of label keys are:
- * `service` The name of the service.
- * `instance` The name of the service instance.
- * `endpoint` The name of the endpoint.
- * `label` is optional, The labels you need to query, used for querying multi-labels metrics. Unlike [swctl](https://github.com/apache/skywalking-cli#metrics-multiple-linear), 
+ * `service`, `service.str.<number>` or `service.byte.<number>` The name of the service.
+ * `instance`, `instance.str.<number>` or `instance.byte.<number>` The name of the service instance.
+ * `endpoint`, `endpoint.str.<number>` or `endpoint.byte.<number>` The name of the endpoint.
+ * `label`, `label.str.<number>` or `label.byte.<number>` is optional, The labels you need to query, used for querying multi-labels metrics. Unlike [swctl](https://github.com/apache/skywalking-cli#metrics-multiple-linear), 
            this key only supports a single label due to the specification of the custom metrics API.
 
 For example, if your application name is `front_gateway`, you could add the following section to 
@@ -67,13 +86,36 @@ your HorizontalPodAutoscaler manifest to specify that you need less than 80ms of
 - type: External
   external:
     metric:
-      name: service_percentile
+      name: skywalking.apache.org|service_percentile
       metricSelector:
         matchLabels:
            service: front_gateway
             # The index of [P50, P75, P90, P95, P99]. 2 is the index of P90(90%)
            label: "2"
     target:
-      type: value
+      type: Value
       value: 80
 ```
+
+If the service is `v1|productpage|bookinfo|demo|-`:
+
+```yaml
+- type: External
+  external:
+    metric:
+      name: skywalking.apache.org|service_cpm
+      metricSelector:
+        matchLabels:
+          "service.str.0":  "v1"
+          "service.byte.1": "7c"
+          "service.str.2":  "productpage"
+          "service.byte.3": "7c"
+          "service.str.4":  "bookinfo"
+          "service.byte.5": "7c"
+          "service.str.6":  "demo"
+          "service.byte.7": "7c"
+          "service.byte.8": "2d"
+    target:
+      type: Value
+      value: 80
+```
\ No newline at end of file
diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go
index dba9344..0033251 100644
--- a/pkg/provider/provider.go
+++ b/pkg/provider/provider.go
@@ -18,8 +18,11 @@
 package provider
 
 import (
+	"encoding/hex"
 	"flag"
 	"fmt"
+	"strconv"
+	"strings"
 	"sync"
 	"time"
 
@@ -36,11 +39,13 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	apischema "k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/selection"
 	"k8s.io/klog/v2"
 	"k8s.io/metrics/pkg/apis/external_metrics"
 )
 
+const labelValueTypeStr string = "str"
+const labelValueTypeByte string = "byte"
+
 var (
 	NsGroupResource = apischema.GroupResource{Resource: "namespaces"}
 )
@@ -52,6 +57,7 @@ type externalMetricsProvider struct {
 	ctx                     *cli.Context
 	regex                   string
 	refreshRegistryInterval time.Duration
+	namespace               string
 }
 
 type stringValue string
@@ -66,7 +72,7 @@ func (sv *stringValue) Set(s string) error {
 }
 
 // NewProvider returns an instance of externalMetricsProvider
-func NewProvider(baseURL string, metricRegex string, refreshRegistryInterval time.Duration) (apiprovider.ExternalMetricsProvider, error) {
+func NewProvider(baseURL string, metricRegex string, refreshRegistryInterval time.Duration, namespace string) (apiprovider.ExternalMetricsProvider, error) {
 	fs := flag.NewFlagSet("mock", flag.ContinueOnError)
 	var k stringValue
 	if err := k.Set(baseURL); err != nil {
@@ -81,6 +87,7 @@ func NewProvider(baseURL string, metricRegex string, refreshRegistryInterval tim
 		ctx:                     ctx,
 		regex:                   metricRegex,
 		refreshRegistryInterval: refreshRegistryInterval,
+		namespace:               namespace,
 	}
 	provider.sync()
 
@@ -92,11 +99,75 @@ type paramValue struct {
 	val *string
 }
 
+func (pv *paramValue) extractValue(requirements labels.Requirements) error {
+	vv := make([]string, 10)
+	for _, r := range requirements {
+		if !strings.HasPrefix(r.Key(), pv.key) {
+			continue
+		}
+		kk := strings.Split(r.Key(), ".")
+		if len(kk) == 1 {
+			vv, _ = bufferEntity(vv, 0, r, nil)
+			pv.val = &vv[0]
+			return nil
+		} else if len(kk) < 3 {
+			return fmt.Errorf("invalid label key:%s", r.Key())
+		}
+		i, err := strconv.ParseInt(kk[2], 10, 8)
+		if err != nil {
+			return fmt.Errorf("failed to parse index string to int: %v", err)
+		}
+
+		index := int(i)
+		switch kk[1] {
+		case labelValueTypeStr:
+			vv, _ = bufferEntity(vv, index, r, nil)
+		case labelValueTypeByte:
+			vv, err = bufferEntity(vv, index, r, func(encoded string) (string, error) {
+				var bytes []byte
+				bytes, err = hex.DecodeString(encoded)
+				if err != nil {
+					return "", fmt.Errorf("failed to decode hex to string: %v", err)
+				}
+				return string(bytes), nil
+			})
+			if err != nil {
+				return err
+			}
+		}
+	}
+	val := strings.Join(vv, "")
+	pv.val = &val
+	return nil
+}
+
+type decoder func(string) (string, error)
+
+func bufferEntity(buff []string, index int, requirement labels.Requirement, dec decoder) ([]string, error) {
+	if cap(buff) <= index {
+		old := buff
+		buff = make([]string, index+1)
+		copy(buff, old)
+	}
+	if v, exist := requirement.Values().PopAny(); exist {
+		if dec != nil {
+			var err error
+			buff[index], err = dec(v)
+			if err != nil {
+				return nil, err
+			}
+		} else {
+			buff[index] = v
+		}
+	}
+	return buff, nil
+}
+
 func (p *externalMetricsProvider) GetExternalMetric(namespace string, metricSelector labels.Selector,
 	info apiprovider.ExternalMetricInfo) (*external_metrics.ExternalMetricValueList, error) {
 	var md *swctlschema.MetricDefinition
 	for _, m := range p.metricDefines {
-		if m.Name == info.Metric {
+		if p.getMetricNameWithNamespace(m.Name) == info.Metric {
 			md = m
 		}
 	}
@@ -217,13 +288,10 @@ func (p *externalMetricsProvider) GetExternalMetric(namespace string, metricSele
 }
 
 func extractValue(requirement labels.Requirements, paramValues ...*paramValue) {
-	for _, r := range requirement {
-		for _, pv := range paramValues {
-			if r.Key() == pv.key && r.Operator() == selection.Equals {
-				if v, exist := r.Values().PopAny(); exist {
-					pv.val = &v
-				}
-			}
+	for _, pv := range paramValues {
+		err := pv.extractValue(requirement)
+		if err != nil {
+			klog.Errorf("failed to parse label %s: %v ", pv.key, err)
 		}
 	}
 }
@@ -238,3 +306,7 @@ func (p *externalMetricsProvider) selectGroupResource(namespace string) apischem
 		Resource: "",
 	}
 }
+
+func (p *externalMetricsProvider) getMetricNameWithNamespace(metricName string) string {
+	return strings.Join([]string{p.namespace, metricName}, "|")
+}
diff --git a/pkg/provider/provider_test.go b/pkg/provider/provider_test.go
new file mode 100644
index 0000000..9cb2f5a
--- /dev/null
+++ b/pkg/provider/provider_test.go
@@ -0,0 +1,128 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 provider
+
+import (
+	"testing"
+
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/selection"
+)
+
+func Test_paramValue_extractValue(t *testing.T) {
+	type args struct {
+		requirements map[string]string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    string
+		wantErr bool
+	}{
+		{
+			name: "golden path",
+			args: args{requirements: map[string]string{
+				"service.str.0":  "v1",
+				"service.byte.1": "7c",
+				"service.str.2":  "productpage",
+				"service.byte.3": "7c",
+				"service.str.4":  "bookinfo",
+				"service.byte.5": "7c",
+				"service.str.6":  "demo",
+				"service.byte.7": "7c",
+				"service.byte.8": "2d",
+			}},
+			want: "v1|productpage|bookinfo|demo|-",
+		},
+		{
+			name: "empty",
+			args: args{requirements: map[string]string{
+				"instance.str.100": "productpage",
+				"instance.str.0":   "v1",
+				"instance.byte.49": "7c",
+			}},
+			want: "",
+		},
+		{
+			name: "random",
+			args: args{requirements: map[string]string{
+				"service.str.100": "productpage",
+				"service.str.0":   "v1",
+				"service.byte.49": "7c",
+			}},
+			want: "v1|productpage",
+		},
+		{
+			name: "gap",
+			args: args{requirements: map[string]string{
+				"service.str.0":   "v1",
+				"service.byte.50": "7c",
+				"service.str.110": "productpage",
+			}},
+			want: "v1|productpage",
+		},
+		{
+			name: "invalid_byte",
+			args: args{requirements: map[string]string{
+				"service.byte.50": "invalid",
+				"service.str.110": "productpage",
+			}},
+			wantErr: true,
+		},
+		{
+			name: "invalid_key",
+			args: args{requirements: map[string]string{
+				"service.byte": "7c",
+			}},
+			wantErr: true,
+		},
+		{
+			name: "invalid_key_index",
+			args: args{requirements: map[string]string{
+				"service.byte.ab": "7c",
+			}},
+			wantErr: true,
+		},
+		{
+			name: "single_entity",
+			args: args{requirements: map[string]string{
+				"service": "productpage",
+			}},
+			want: "productpage",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			pv := &paramValue{
+				key: "service",
+				val: nil,
+			}
+			requirements := make([]labels.Requirement, len(tt.args.requirements))
+			for key, v := range tt.args.requirements {
+				r, _ := labels.NewRequirement(key, selection.In, []string{v})
+				requirements = append(requirements, *r)
+			}
+			if err := pv.extractValue(requirements); (err != nil) != tt.wantErr {
+				t.Errorf("extractValue() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			if !tt.wantErr && *pv.val != tt.want {
+				t.Errorf("extractValue() val = %v, want %v", *pv.val, tt.want)
+			}
+		})
+	}
+}
diff --git a/pkg/provider/registry.go b/pkg/provider/registry.go
index 4b5bc06..e468a10 100644
--- a/pkg/provider/registry.go
+++ b/pkg/provider/registry.go
@@ -35,7 +35,7 @@ func (p *externalMetricsProvider) ListAllExternalMetrics() (externalMetricsInfo
 
 	for _, md := range p.metricDefines {
 		info := apiprovider.ExternalMetricInfo{
-			Metric: md.Name,
+			Metric: p.getMetricNameWithNamespace(md.Name),
 		}
 		externalMetricsInfo = append(externalMetricsInfo, info)
 	}