feat(kuma-cp) sanitize metrics for StatsD and Prometheus (#562)

CHANGELOG.md

@@ -4,6 +4,8 @@
 
 Changes:
 
+* feature: sanitize metrics for StatsD and Prometheus
+  [#562](https://github.com/Kong/kuma/pull/562)
 * feature: reformat some Envoy metrics available in Prometheus
   [#558](https://github.com/Kong/kuma/pull/558)
 * feature: make maximum number of open connections to Postgres configurable

Makefile.e2e.mk

@@ -352,7 +352,7 @@ verify/example/minikube/mtls: verify/example/minikube/mtls/outbound ## Minikube:
 
 verify/example/minikube/mtls/outbound:
 	@echo "Checking number of Outbound mTLS requests via Envoy ..."
-	test $$( $(call kubectl_exec,kuma-demo,demo-client,kuma-sidecar) wget -qO- http://localhost:9901/stats/prometheus | grep 'envoy_cluster_kuma_demo_svc_8000_ssl_handshake{envoy_cluster_name="demo-app"}' | awk '{print $$2}' | tr -d [:space:] ) -ge 5
+	test $$( $(call kubectl_exec,kuma-demo,demo-client,kuma-sidecar) wget -qO- http://localhost:9901/stats/prometheus | grep 'envoy_cluster_ssl_handshake{envoy_cluster_name="demo-app_kuma-demo_svc_8000"}' | awk '{print $$2}' | tr -d [:space:] ) -ge 5
 	@echo "Check passed!"
 
 kumactl/example/minikube:

pkg/util/xds/metric_sanitizer.go

@@ -0,0 +1,16 @@
+package xds
+
+import (
+	"regexp"
+)
+
+var (
+	illegalChars = regexp.MustCompile(`[^a-zA-Z_\-0-9]`)
+)
+
+// We need to sanitize metrics in order to  not break statsd and prometheus format.
+// StatsD only allow [a-zA-Z_\-0-9.] characters, everything else is removed
+// Extra dots breaks many regexes that converts statsd metric to prometheus one with tags
+func SanitizeMetric(metric string) string {
+	return illegalChars.ReplaceAllString(metric, "_")
+}

pkg/util/xds/metric_sanitizer_test.go

@@ -0,0 +1,20 @@
+package xds_test
+
+import (
+	"github.com/Kong/kuma/pkg/util/xds"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Metric sanitizer", func() {
+	It("should sanitize metrics", func() {
+		// given
+		metric := "some metric with chars :/_-0123{version=3.0}"
+
+		// when
+		sanitized := xds.SanitizeMetric(metric)
+
+		// then
+		Expect(sanitized).To(Equal("some_metric_with_chars____-0123_version_3_0_"))
+	})
+})

pkg/xds/envoy/envoy.go

@@ -15,6 +15,7 @@ import (
 	mesh_core "github.com/Kong/kuma/pkg/core/resources/apis/mesh"
 	core_xds "github.com/Kong/kuma/pkg/core/xds"
 	util_error "github.com/Kong/kuma/pkg/util/error"
+	util_xds "github.com/Kong/kuma/pkg/util/xds"
 	xds_context "github.com/Kong/kuma/pkg/xds/context"
 	v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2"
 	envoy_cluster "github.com/envoyproxy/go-control-plane/envoy/api/v2/cluster"
@@ -115,16 +116,16 @@ func CreateClusterLoadAssignment(clusterName string, endpoints []core_xds.Endpoi
 }
 
 func CreateLocalCluster(clusterName string, address string, port uint32) *v2.Cluster {
-	return &v2.Cluster{
+	return clusterWithAltStatName(&v2.Cluster{
 		Name:                 clusterName,
 		ConnectTimeout:       ptypes.DurationProto(defaultConnectTimeout),
 		ClusterDiscoveryType: &v2.Cluster_Type{Type: v2.Cluster_STATIC},
 		LoadAssignment:       CreateStaticEndpoint(clusterName, address, port),
-	}
+	})
 }
 
 func CreateEdsCluster(ctx xds_context.Context, clusterName string, metadata *core_xds.DataplaneMetadata) *v2.Cluster {
-	return &v2.Cluster{
+	return clusterWithAltStatName(&v2.Cluster{
 		Name:                 clusterName,
 		ConnectTimeout:       ptypes.DurationProto(defaultConnectTimeout),
 		ClusterDiscoveryType: &v2.Cluster_Type{Type: v2.Cluster_EDS},
@@ -136,7 +137,15 @@ func CreateEdsCluster(ctx xds_context.Context, clusterName string, metadata *cor
 			},
 		},
 		TlsContext: CreateUpstreamTlsContext(ctx, metadata),
+	})
+}
+
+func clusterWithAltStatName(cluster *v2.Cluster) *v2.Cluster {
+	sanitizedName := util_xds.SanitizeMetric(cluster.Name)
+	if sanitizedName != cluster.Name {
+		cluster.AltStatName = sanitizedName
 	}
+	return cluster
 }
 
 func ClusterWithHealthChecks(cluster *v2.Cluster, healthCheck *mesh_core.HealthCheckResource) *v2.Cluster {
@@ -166,12 +175,12 @@ func ClusterWithHealthChecks(cluster *v2.Cluster, healthCheck *mesh_core.HealthC
 }
 
 func CreatePassThroughCluster(clusterName string) *v2.Cluster {
-	return &v2.Cluster{
+	return clusterWithAltStatName(&v2.Cluster{
 		Name:                 clusterName,
 		ConnectTimeout:       ptypes.DurationProto(defaultConnectTimeout),
 		ClusterDiscoveryType: &v2.Cluster_Type{Type: v2.Cluster_ORIGINAL_DST},
 		LbPolicy:             v2.Cluster_ORIGINAL_DST_LB,
-	}
+	})
 }
 
 func CreateOutboundListener(ctx xds_context.Context, listenerName string, address string, port uint32, statsName string, clusters []ClusterInfo, virtual bool, sourceService string, destinationService string, backend *v1alpha1.LoggingBackend, proxy *core_xds.Proxy) (*v2.Listener, error) {
@@ -185,7 +194,7 @@ func CreateOutboundListener(ctx xds_context.Context, listenerName string, addres
 	}
 
 	config := &envoy_tcp.TcpProxy{
-		StatPrefix: statsName,
+		StatPrefix: util_xds.SanitizeMetric(statsName),
 		AccessLog:  accessLogs,
 	}
 	if len(clusters) == 1 {
@@ -241,7 +250,7 @@ func CreateOutboundListener(ctx xds_context.Context, listenerName string, addres
 
 func CreateInboundListener(ctx xds_context.Context, listenerName string, address string, port uint32, clusterName string, virtual bool, permissions *mesh_core.TrafficPermissionResourceList, metadata *core_xds.DataplaneMetadata) *v2.Listener {
 	config := &envoy_tcp.TcpProxy{
-		StatPrefix: clusterName,
+		StatPrefix: util_xds.SanitizeMetric(clusterName),
 		ClusterSpecifier: &envoy_tcp.TcpProxy_Cluster{
 			Cluster: clusterName,
 		},
@@ -289,7 +298,7 @@ func CreateInboundListener(ctx xds_context.Context, listenerName string, address
 
 func CreatePrometheusListener(ctx xds_context.Context, listenerName string, address string, port uint32, path string, clusterName string, virtual bool, metadata *core_xds.DataplaneMetadata) *v2.Listener {
 	config := &envoy_hcm.HttpConnectionManager{
-		StatPrefix: listenerName,
+		StatPrefix: util_xds.SanitizeMetric(listenerName),
 		CodecType:  envoy_hcm.HttpConnectionManager_AUTO,
 		HttpFilters: []*envoy_hcm.HttpFilter{{
 			Name: wellknown.Router,
@@ -423,7 +432,7 @@ func sdsSecretConfig(context xds_context.Context, name string, metadata *core_xd
 							TargetSpecifier: &envoy_core.GrpcService_GoogleGrpc_{
 								GoogleGrpc: withCallCredentials(&envoy_core.GrpcService_GoogleGrpc{
 									TargetUri:  context.ControlPlane.SdsLocation,
-									StatPrefix: "sds_" + name,
+									StatPrefix: util_xds.SanitizeMetric("sds_" + name),
 									ChannelCredentials: &envoy_core.GrpcService_GoogleGrpc_ChannelCredentials{
 										CredentialSpecifier: &envoy_core.GrpcService_GoogleGrpc_ChannelCredentials_SslCredentials{
 											SslCredentials: &envoy_core.GrpcService_GoogleGrpc_SslCredentials{
@@ -447,7 +456,7 @@ func sdsSecretConfig(context xds_context.Context, name string, metadata *core_xd
 
 func CreateCatchAllListener(ctx xds_context.Context, listenerName string, address string, port uint32, clusterName string) *v2.Listener {
 	config := &envoy_tcp.TcpProxy{
-		StatPrefix: clusterName,
+		StatPrefix: util_xds.SanitizeMetric(clusterName),
 		ClusterSpecifier: &envoy_tcp.TcpProxy_Cluster{
 			Cluster: clusterName,
 		},

pkg/xds/envoy/envoy_test.go

@@ -48,6 +48,7 @@ var _ = Describe("Envoy", func() {
 		// given
 		expected := `
         name: localhost:8080
+        altStatName: localhost_8080
         type: STATIC
         connectTimeout: 5s
         loadAssignment:
@@ -178,6 +179,7 @@ var _ = Describe("Envoy", func() {
                   edsConfig:
                     ads: {}
                 name: 192.168.0.1:8080
+                altStatName: "192_168_0_1_8080"
                 type: EDS
 `,
 			}),
@@ -204,6 +206,7 @@ var _ = Describe("Envoy", func() {
                   edsConfig:
                     ads: {}
                 name: 192.168.0.1:8080
+                altStatName: "192_168_0_1_8080"
                 tlsContext:
                   commonTlsContext:
                     tlsCertificateSdsSecretConfigs:
@@ -260,6 +263,7 @@ var _ = Describe("Envoy", func() {
                   edsConfig:
                     ads: {}
                 name: 192.168.0.1:8080
+                altStatName: "192_168_0_1_8080"
                 tlsContext:
                   commonTlsContext:
                     tlsCertificateSdsSecretConfigs:
@@ -578,7 +582,7 @@ var _ = Describe("Envoy", func() {
                     typedConfig:
                       '@type': type.googleapis.com/envoy.config.filter.network.tcp_proxy.v2.TcpProxy
                       cluster: localhost:8080
-                      statPrefix: localhost:8080
+                      statPrefix: localhost_8080
 `,
 			}),
 			Entry("with transparent proxying", testCase{
@@ -601,7 +605,7 @@ var _ = Describe("Envoy", func() {
                     typedConfig:
                       '@type': type.googleapis.com/envoy.config.filter.network.tcp_proxy.v2.TcpProxy
                       cluster: localhost:8080
-                      statPrefix: localhost:8080
+                      statPrefix: localhost_8080
                 deprecatedV1:
                   bindToPort: false
 `,
@@ -642,12 +646,12 @@ var _ = Describe("Envoy", func() {
                       - authenticated:
                           principalName:
                             exact: spiffe://default/web1
-                statPrefix: inbound:192.168.0.1:8080.
+                statPrefix: inbound_192_168_0_1_8080.
             - name: envoy.tcp_proxy
               typedConfig:
                 '@type': type.googleapis.com/envoy.config.filter.network.tcp_proxy.v2.TcpProxy
                 cluster: localhost:8080
-                statPrefix: localhost:8080
+                statPrefix: localhost_8080
             tlsContext:
               commonTlsContext:
                 tlsCertificateSdsSecretConfigs:
@@ -719,12 +723,12 @@ var _ = Describe("Envoy", func() {
                       - authenticated:
                           principalName:
                             exact: spiffe://default/web1
-                statPrefix: inbound:192.168.0.1:8080.
+                statPrefix: inbound_192_168_0_1_8080.
             - name: envoy.tcp_proxy
               typedConfig:
                 '@type': type.googleapis.com/envoy.config.filter.network.tcp_proxy.v2.TcpProxy
                 cluster: localhost:8080
-                statPrefix: localhost:8080
+                statPrefix: localhost_8080
             tlsContext:
               commonTlsContext:
                 tlsCertificateSdsSecretConfigs:

pkg/xds/envoy/rbac.go

@@ -5,6 +5,7 @@ import (
 	"github.com/Kong/kuma/api/mesh/v1alpha1"
 	mesh_core "github.com/Kong/kuma/pkg/core/resources/apis/mesh"
 	util_error "github.com/Kong/kuma/pkg/util/error"
+	util_xds "github.com/Kong/kuma/pkg/util/xds"
 	envoy_listener "github.com/envoyproxy/go-control-plane/envoy/api/v2/listener"
 	rbac "github.com/envoyproxy/go-control-plane/envoy/config/filter/network/rbac/v2"
 	rbac_config "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v2"
@@ -38,7 +39,7 @@ func createRbacRule(listenerName string, permissions *mesh_core.TrafficPermissio
 			Action:   rbac_config.RBAC_ALLOW,
 			Policies: policies,
 		},
-		StatPrefix: fmt.Sprintf("%s.", listenerName), // we include dot to change "inbound:127.0.0.1:21011rbac.allowed" metric to "inbound:127.0.0.1:21011.rbac.allowed"
+		StatPrefix: fmt.Sprintf("%s.", util_xds.SanitizeMetric(listenerName)), // we include dot to change "inbound:127.0.0.1:21011rbac.allowed" metric to "inbound:127.0.0.1:21011.rbac.allowed"
 	}
 }
 

pkg/xds/generator/outbound_proxy_generator_test.go

@@ -213,6 +213,81 @@ var _ = Describe("OutboundProxyGenerator", func() {
 		}),
 	)
 
+	It("Add sanitized alternative cluster name for stats", func() {
+		// setup
+		gen := &generator.OutboundProxyGenerator{}
+		dp := `
+        networking:
+          outbound:
+          - interface: :18080
+            service: backend.kuma-system
+          - interface: :54321
+            service: db.kuma-system`
+
+		dataplane := mesh_proto.Dataplane{}
+		Expect(util_proto.FromYAML([]byte(dp), &dataplane)).To(Succeed())
+
+		proxy := &model.Proxy{
+			Id: model.ProxyId{Name: "side-car", Mesh: "default"},
+			Dataplane: &mesh_core.DataplaneResource{
+				Meta: &test_model.ResourceMeta{
+					Version: "1",
+				},
+				Spec: dataplane,
+			},
+			TrafficRoutes: model.RouteMap{
+				"backend.kuma-system": &mesh_core.TrafficRouteResource{
+					Spec: mesh_proto.TrafficRoute{
+						Conf: []*mesh_proto.TrafficRoute_WeightedDestination{{
+							Weight:      100,
+							Destination: mesh_proto.MatchService("backend.kuma-system"),
+						}},
+					},
+				},
+				"db.kuma-system": &mesh_core.TrafficRouteResource{
+					Spec: mesh_proto.TrafficRoute{
+						Conf: []*mesh_proto.TrafficRoute_WeightedDestination{{
+							Weight:      100,
+							Destination: mesh_proto.TagSelector{"service": "db", "version": "3.2.0"},
+						},
+						}},
+				},
+			},
+			OutboundSelectors: model.DestinationMap{
+				"backend.kuma-system": model.TagSelectorSet{
+					{"service": "backend.kuma-system"},
+				},
+				"db.kuma-system": model.TagSelectorSet{
+					{"service": "db", "version": "3.2.0"},
+				},
+			},
+			OutboundTargets: model.EndpointMap{
+				"backend.kuma-system": []model.Endpoint{
+					{Target: "192.168.0.1", Port: 8082},
+				},
+				"db.kuma-system": []model.Endpoint{
+					{Target: "192.168.0.2", Port: 5432, Tags: map[string]string{"service": "db", "role": "master"}},
+				},
+			},
+			Metadata: &model.DataplaneMetadata{},
+		}
+
+		// when
+		rs, err := gen.Generate(plainCtx, proxy)
+
+		// then
+		Expect(err).ToNot(HaveOccurred())
+
+		// then
+		resp := model.ResourceList(rs).ToDeltaDiscoveryResponse()
+		actual, err := util_proto.ToYAML(resp)
+		Expect(err).ToNot(HaveOccurred())
+
+		expected, err := ioutil.ReadFile(filepath.Join("testdata", "outbound-proxy", "cluster-dots.envoy.golden.yaml"))
+		Expect(err).ToNot(HaveOccurred())
+		Expect(actual).To(MatchYAML(expected))
+	})
+
 	Describe("fail when a user-defined configuration (Dataplane, TrafficRoute, etc) is not valid", func() {
 
 		type testCase struct {

pkg/xds/generator/prometheus_endpoint_generator_test.go

@@ -225,6 +225,7 @@ var _ = Describe("PrometheusEndpointGenerator", func() {
                             address: 127.0.0.1
                             portValue: 9902
                 name: kuma:envoy:admin
+                altStatName: kuma_envoy_admin
                 type: STATIC
             - name: kuma:metrics:prometheus
               resource:
@@ -251,7 +252,7 @@ var _ = Describe("PrometheusEndpointGenerator", func() {
                             route:
                               cluster: kuma:envoy:admin
                               prefixRewrite: /stats/prometheus
-                      statPrefix: kuma:metrics:prometheus
+                      statPrefix: kuma_metrics_prometheus
                 name: kuma:metrics:prometheus
 `,
 		}),
@@ -309,6 +310,7 @@ var _ = Describe("PrometheusEndpointGenerator", func() {
                             address: 127.0.0.1
                             portValue: 9902
                 name: kuma:envoy:admin
+                altStatName: kuma_envoy_admin
                 type: STATIC
             - name: kuma:metrics:prometheus
               resource:
@@ -335,7 +337,7 @@ var _ = Describe("PrometheusEndpointGenerator", func() {
                             route:
                               cluster: kuma:envoy:admin
                               prefixRewrite: /stats/prometheus
-                      statPrefix: kuma:metrics:prometheus
+                      statPrefix: kuma_metrics_prometheus
                 name: kuma:metrics:prometheus
 `,
 		}),

pkg/xds/generator/testdata/inbound-proxy/3-envoy-config.golden.yaml

@@ -13,6 +13,7 @@ resources:
                       address: 127.0.0.1
                       portValue: 8080
       name: localhost:8080
+      altStatName: localhost_8080
       type: STATIC
   - name: inbound:192.168.0.1:80
     resource:
@@ -36,12 +37,12 @@ resources:
                       - authenticated:
                           principalName:
                             exact: spiffe://default/web1
-                statPrefix: inbound:192.168.0.1:80.
+                statPrefix: inbound_192_168_0_1_80.
             - name: envoy.tcp_proxy
               typedConfig:
                 '@type': type.googleapis.com/envoy.config.filter.network.tcp_proxy.v2.TcpProxy
                 cluster: localhost:8080
-                statPrefix: localhost:8080
+                statPrefix: localhost_8080
           tlsContext:
             commonTlsContext:
               tlsCertificateSdsSecretConfigs: