Instrumenting Proxy-Injector (#3354)

* add proxy injection prometheus counters

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

* formatted injection reasons

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

* update proxy injection report tests

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

* keep the structure, and add global ownerKind

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

* increase request count, when owner is nil

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

* add readable reasons using map

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

* fix linting issues

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

* add proxy config override annotations as labels

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

* remove space for machine reasons

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

* use correct proxy image override annotation

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

* add annotation_at label to prom metrics

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

* refactor disablebyannotation function

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

controller/proxy-injector/metrics.go

@@ -0,0 +1,74 @@
+package injector
+
+import (
+	"strings"
+
+	"github.com/linkerd/linkerd2/pkg/k8s"
+
+	"github.com/linkerd/linkerd2/pkg/inject"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
+)
+
+const (
+	labelOwnerKind    = "owner_kind"
+	labelNamespace    = "namespace"
+	labelSkip         = "skip"
+	labelAnnotationAt = "annotation_at"
+	labelReason       = "skip_reason"
+)
+
+var (
+	requestLabels  = []string{labelOwnerKind, labelNamespace, labelAnnotationAt}
+	responseLabels = []string{labelOwnerKind, labelNamespace, labelSkip, labelReason, labelAnnotationAt}
+
+	proxyInjectionAdmissionRequests = promauto.NewCounterVec(prometheus.CounterOpts{
+		Name: "proxy_inject_admission_requests_total",
+		Help: "A counter for number of admission requests to proxy injector.",
+	}, append(requestLabels, validLabelNames(inject.ProxyAnnotations)...))
+
+	proxyInjectionAdmissionResponses = promauto.NewCounterVec(prometheus.CounterOpts{
+		Name: "proxy_inject_admission_responses_total",
+		Help: "A counter for number of admission responses from proxy injector.",
+	}, append(responseLabels, validLabelNames(inject.ProxyAnnotations)...))
+)
+
+func admissionRequestLabels(ownerKind, namespace, annotationAt string, configLabels prometheus.Labels) prometheus.Labels {
+	configLabels[labelOwnerKind] = ownerKind
+	configLabels[labelNamespace] = namespace
+	configLabels[labelAnnotationAt] = annotationAt
+	return configLabels
+}
+
+func admissionResponseLabels(owner, namespace, skip, reason, annotationAt string, configLabels prometheus.Labels) prometheus.Labels {
+	configLabels[labelOwnerKind] = owner
+	configLabels[labelNamespace] = namespace
+	configLabels[labelSkip] = skip
+	configLabels[labelReason] = reason
+	configLabels[labelAnnotationAt] = annotationAt
+	return configLabels
+}
+
+func configToPrometheusLabels(conf *inject.ResourceConfig) prometheus.Labels {
+	labels := conf.GetOverriddenConfiguration()
+	promLabels := map[string]string{}
+
+	for label, value := range labels {
+		promLabels[validProxyConfigurationLabel(label)] = value
+
+	}
+	return promLabels
+}
+
+func validLabelNames(labels []string) []string {
+	var validLabels []string
+
+	for _, label := range labels {
+		validLabels = append(validLabels, validProxyConfigurationLabel(label))
+	}
+	return validLabels
+}
+
+func validProxyConfigurationLabel(label string) string {
+	return strings.Replace(label[len(k8s.ProxyConfigAnnotationsPrefix)+1:], "-", "_", -1)
+}

controller/proxy-injector/webhook.go

@@ -2,6 +2,7 @@ package injector
 
 import (
 	"fmt"
+	"strings"
 
 	pb "github.com/linkerd/linkerd2/controller/gen/config"
 	"github.com/linkerd/linkerd2/controller/k8s"
@@ -61,7 +62,9 @@ func Inject(api *k8s.API,
 		Allowed: true,
 	}
 
+	configLabels := configToPrometheusLabels(resourceConfig)
 	var parent *runtime.Object
+	ownerKind := ""
 	if ownerRef := resourceConfig.GetOwnerRef(); ownerRef != nil {
 		objs, err := api.GetObjects(request.Namespace, ownerRef.Kind, ownerRef.Name)
 		if err != nil {
@@ -71,13 +74,23 @@ func Inject(api *k8s.API,
 		} else {
 			parent = &objs[0]
 		}
+		ownerKind = strings.ToLower(ownerRef.Kind)
 	}
+	proxyInjectionAdmissionRequests.With(admissionRequestLabels(ownerKind, request.Namespace, report.InjectAnnotationAt, configLabels)).Inc()
 
-	if injectable, reason := report.Injectable(); !injectable {
+	if injectable, reasons := report.Injectable(); !injectable {
-		if parent != nil {
+		var readableReasons, metricReasons string
-			recorder.Eventf(*parent, v1.EventTypeNormal, eventTypeSkipped, "Linkerd sidecar proxy injection skipped: %s", reason)
+		metricReasons = strings.Join(reasons, ",")
+		for _, reason := range reasons {
+			readableReasons = readableReasons + ", " + inject.Reasons[reason]
 		}
-		log.Infof("skipped %s: %s", report.ResName(), reason)
+		// removing the initial comma, space
+		readableReasons = readableReasons[2:]
+		if parent != nil {
+			recorder.Eventf(*parent, v1.EventTypeNormal, eventTypeSkipped, "Linkerd sidecar proxy injection skipped: %s", readableReasons)
+		}
+		log.Infof("skipped %s: %s", report.ResName(), readableReasons)
+		proxyInjectionAdmissionResponses.With(admissionResponseLabels(ownerKind, request.Namespace, "true", metricReasons, report.InjectAnnotationAt, configLabels)).Inc()
 		return admissionResponse, nil
 	}
 
@@ -98,7 +111,7 @@ func Inject(api *k8s.API,
 	}
 	log.Infof("patch generated for: %s", report.ResName())
 	log.Debugf("patch: %s", patchJSON)
-
+	proxyInjectionAdmissionResponses.With(admissionResponseLabels(ownerKind, request.Namespace, "false", "", report.InjectAnnotationAt, configLabels)).Inc()
 	patchType := admissionv1beta1.PatchTypeJSONPatch
 	admissionResponse.Patch = patchJSON
 	admissionResponse.PatchType = &patchType

pkg/inject/inject.go

@@ -29,7 +29,34 @@ const (
 	proxyInitResourceLimitMemory   = "50Mi"
 )
 
-var rTrail = regexp.MustCompile(`\},\s*\]`)
+var (
+	rTrail = regexp.MustCompile(`\},\s*\]`)
+
+	// ProxyAnnotations is the list of possible annotations that can be applied on a pod or namespace
+	ProxyAnnotations = []string{
+		k8s.ProxyAdminPortAnnotation,
+		k8s.ProxyControlPortAnnotation,
+		k8s.ProxyDisableIdentityAnnotation,
+		k8s.ProxyDisableTapAnnotation,
+		k8s.ProxyEnableDebugAnnotation,
+		k8s.ProxyEnableExternalProfilesAnnotation,
+		k8s.ProxyImagePullPolicyAnnotation,
+		k8s.ProxyInboundPortAnnotation,
+		k8s.ProxyInitImageAnnotation,
+		k8s.ProxyInitImageVersionAnnotation,
+		k8s.ProxyOutboundPortAnnotation,
+		k8s.ProxyCPULimitAnnotation,
+		k8s.ProxyCPURequestAnnotation,
+		k8s.ProxyImageAnnotation,
+		k8s.ProxyLogLevelAnnotation,
+		k8s.ProxyMemoryLimitAnnotation,
+		k8s.ProxyMemoryRequestAnnotation,
+		k8s.ProxyUIDAnnotation,
+		k8s.ProxyVersionOverrideAnnotation,
+		k8s.ProxyIgnoreInboundPortsAnnotation,
+		k8s.ProxyIgnoreOutboundPortsAnnotation,
+	}
+)
 
 // Origin defines where the input YAML comes from. Refer the ResourceConfig's
 // 'origin' field
@@ -783,6 +810,16 @@ func (conf *ResourceConfig) proxyOutboundSkipPorts() string {
 	return strings.Join(ports, ",")
 }
 
+// GetOverriddenConfiguration returns a map of the overridden proxy annotations
+func (conf *ResourceConfig) GetOverriddenConfiguration() map[string]string {
+	proxyOverrideConfig := map[string]string{}
+	for _, annotation := range ProxyAnnotations {
+		proxyOverrideConfig[annotation] = conf.getOverride(annotation)
+	}
+
+	return proxyOverrideConfig
+}
+
 func sortedKeys(m map[string]string) []string {
 	keys := []string{}
 	for k := range m {

pkg/inject/report.go

@@ -9,6 +9,27 @@ import (
 	v1 "k8s.io/api/core/v1"
 )
 
+const (
+	hostNetworkEnabled             = "host_network_enabled"
+	sidecarExists                  = "sidecar_already_exists"
+	unsupportedResource            = "unsupported_resource"
+	injectEnableAnnotationAbsent   = "injection_enable_annotation_absent"
+	injectDisableAnnotationPresent = "injection_disable_annotation_present"
+	annotationAtNamespace          = "namespace"
+	annotationAtWorkload           = "workload"
+)
+
+var (
+	// Reasons is a map of inject skip reasons with human readable sentences
+	Reasons = map[string]string{
+		hostNetworkEnabled:             "hostNetwork is enabled",
+		sidecarExists:                  "pod has a sidecar injected already",
+		unsupportedResource:            "this resource kind is unsupported",
+		injectEnableAnnotationAbsent:   fmt.Sprintf("neither the namespace nor the pod have the annotation \"%s:%s\"", k8s.ProxyInjectAnnotation, k8s.ProxyInjectEnabled),
+		injectDisableAnnotationPresent: fmt.Sprintf("pod has the annotation \"%s:%s\"", k8s.ProxyInjectAnnotation, k8s.ProxyInjectDisabled),
+	}
+)
+
 // Report contains the Kind and Name for a given workload along with booleans
 // describing the result of the injection transformation
 type Report struct {
@@ -20,6 +41,7 @@ type Report struct {
 	UnsupportedResource  bool
 	InjectDisabled       bool
 	InjectDisabledReason string
+	InjectAnnotationAt   string
 
 	// Uninjected consists of two boolean flags to indicate if a proxy and
 	// proxy-init containers have been uninjected in this report
@@ -51,7 +73,7 @@ func newReport(conf *ResourceConfig) *Report {
 	}
 
 	if conf.pod.meta != nil && conf.pod.spec != nil {
-		report.InjectDisabled, report.InjectDisabledReason = report.disableByAnnotation(conf)
+		report.InjectDisabled, report.InjectDisabledReason, report.InjectAnnotationAt = report.disableByAnnotation(conf)
 		report.HostNetwork = conf.pod.spec.HostNetwork
 		report.Sidecar = healthcheck.HasExistingSidecars(conf.pod.spec)
 		report.UDP = checkUDPPorts(conf.pod.spec)
@@ -70,25 +92,25 @@ func (r *Report) ResName() string {
 // Injectable returns false if the report flags indicate that the workload is on a host network
 // or there is already a sidecar or the resource is not supported or inject is explicitly disabled.
 // If false, the second returned value describes the reason.
-func (r *Report) Injectable() (bool, string) {
+func (r *Report) Injectable() (bool, []string) {
-	reasons := []string{}
+	var reasons []string
 	if r.HostNetwork {
-		reasons = append(reasons, "hostNetwork is enabled")
+		reasons = append(reasons, hostNetworkEnabled)
 	}
 	if r.Sidecar {
-		reasons = append(reasons, "pod has a sidecar injected already")
+		reasons = append(reasons, sidecarExists)
 	}
 	if r.UnsupportedResource {
-		reasons = append(reasons, "this resource kind is unsupported")
+		reasons = append(reasons, unsupportedResource)
 	}
 	if r.InjectDisabled {
 		reasons = append(reasons, r.InjectDisabledReason)
 	}
 
 	if len(reasons) > 0 {
-		return false, strings.Join(reasons, ", ")
+		return false, reasons
 	}
-	return true, ""
+	return true, nil
 }
 
 func checkUDPPorts(t *v1.PodSpec) bool {
@@ -103,7 +125,9 @@ func checkUDPPorts(t *v1.PodSpec) bool {
 	return false
 }
 
-func (r *Report) disableByAnnotation(conf *ResourceConfig) (bool, string) {
+// disabledByAnnotation checks annotations for both workload, namespace and returns
+// if disabled, Inject Disabled reason and the resource where that annotation was present
+func (r *Report) disableByAnnotation(conf *ResourceConfig) (bool, string, string) {
 	// truth table of the effects of the inject annotation:
 	//
 	// origin  | namespace | pod      | inject?  | return
@@ -125,19 +149,19 @@ func (r *Report) disableByAnnotation(conf *ResourceConfig) (bool, string) {
 	nsAnnotation := conf.nsAnnotations[k8s.ProxyInjectAnnotation]
 
 	if conf.origin == OriginCLI {
-		return podAnnotation == k8s.ProxyInjectDisabled, ""
+		return podAnnotation == k8s.ProxyInjectDisabled, "", ""
 	}
 
 	if nsAnnotation == k8s.ProxyInjectEnabled {
 		if podAnnotation == k8s.ProxyInjectDisabled {
-			return true, fmt.Sprintf("pod has the annotation \"%s:%s\"", k8s.ProxyInjectAnnotation, k8s.ProxyInjectDisabled)
+			return true, injectDisableAnnotationPresent, annotationAtWorkload
 		}
-		return false, ""
+		return false, "", annotationAtNamespace
 	}
 
 	if podAnnotation != k8s.ProxyInjectEnabled {
-		return true, fmt.Sprintf("neither the namespace nor the pod have the annotation \"%s:%s\"", k8s.ProxyInjectAnnotation, k8s.ProxyInjectEnabled)
+		return true, injectEnableAnnotationAbsent, ""
 	}
 
-	return false, ""
+	return false, "", annotationAtWorkload
 }

pkg/inject/report_test.go

@@ -16,7 +16,7 @@ func TestInjectable(t *testing.T) {
 		nsAnnotations       map[string]string
 		unsupportedResource bool
 		injectable          bool
-		reason              string
+		reasons             []string
 	}{
 		{
 			podSpec: &corev1.PodSpec{HostNetwork: false},
@@ -35,7 +35,7 @@ func TestInjectable(t *testing.T) {
 				},
 			},
 			injectable: false,
-			reason:     "hostNetwork is enabled",
+			reasons:    []string{hostNetworkEnabled},
 		},
 		{
 			podSpec: &corev1.PodSpec{
@@ -52,7 +52,7 @@ func TestInjectable(t *testing.T) {
 				},
 			},
 			injectable: false,
-			reason:     "pod has a sidecar injected already",
+			reasons:    []string{sidecarExists},
 		},
 		{
 			podSpec: &corev1.PodSpec{
@@ -69,7 +69,7 @@ func TestInjectable(t *testing.T) {
 				},
 			},
 			injectable: false,
-			reason:     "pod has a sidecar injected already",
+			reasons:    []string{sidecarExists},
 		},
 		{
 			unsupportedResource: true,
@@ -80,7 +80,73 @@ func TestInjectable(t *testing.T) {
 				},
 			},
 			injectable: false,
-			reason:     "this resource kind is unsupported",
+			reasons:    []string{unsupportedResource},
+		},
+		{
+			unsupportedResource: true,
+			podSpec:             &corev1.PodSpec{HostNetwork: true},
+			podMeta: &metav1.ObjectMeta{
+				Annotations: map[string]string{
+					k8s.ProxyInjectAnnotation: k8s.ProxyInjectEnabled,
+				},
+			},
+
+			injectable: false,
+			reasons:    []string{hostNetworkEnabled, unsupportedResource},
+		},
+		{
+			nsAnnotations: map[string]string{
+				k8s.ProxyInjectAnnotation: k8s.ProxyInjectEnabled,
+			},
+			podSpec: &corev1.PodSpec{HostNetwork: true},
+			podMeta: &metav1.ObjectMeta{
+				Annotations: map[string]string{
+					k8s.ProxyInjectAnnotation: k8s.ProxyInjectDisabled,
+				},
+			},
+
+			injectable: false,
+			reasons:    []string{hostNetworkEnabled, injectDisableAnnotationPresent},
+		},
+		{
+			nsAnnotations: map[string]string{
+				k8s.ProxyInjectAnnotation: k8s.ProxyInjectEnabled,
+			},
+			unsupportedResource: true,
+			podSpec:             &corev1.PodSpec{HostNetwork: true},
+			podMeta: &metav1.ObjectMeta{
+				Annotations: map[string]string{
+					k8s.ProxyInjectAnnotation: k8s.ProxyInjectDisabled,
+				},
+			},
+
+			injectable: false,
+			reasons:    []string{hostNetworkEnabled, unsupportedResource, injectDisableAnnotationPresent},
+		},
+		{
+			unsupportedResource: true,
+			podSpec:             &corev1.PodSpec{HostNetwork: true},
+			podMeta: &metav1.ObjectMeta{
+				Annotations: map[string]string{},
+			},
+
+			injectable: false,
+			reasons:    []string{hostNetworkEnabled, unsupportedResource, injectEnableAnnotationAbsent},
+		},
+		{
+			podSpec: &corev1.PodSpec{HostNetwork: true,
+				Containers: []corev1.Container{
+					{
+						Name:  k8s.ProxyContainerName,
+						Image: "gcr.io/linkerd-io/proxy:",
+					},
+				}},
+			podMeta: &metav1.ObjectMeta{
+				Annotations: map[string]string{},
+			},
+
+			injectable: false,
+			reasons:    []string{hostNetworkEnabled, sidecarExists, injectEnableAnnotationAbsent},
 		},
 	}
 
@@ -90,18 +156,27 @@ func TestInjectable(t *testing.T) {
 			resourceConfig := &ResourceConfig{}
 			resourceConfig.WithNsAnnotations(testCase.nsAnnotations)
 			resourceConfig.pod.spec = testCase.podSpec
+			resourceConfig.origin = OriginWebhook
 			resourceConfig.pod.meta = testCase.podMeta
 
 			report := newReport(resourceConfig)
 			report.UnsupportedResource = testCase.unsupportedResource
 
-			actual, reason := report.Injectable()
+			actual, reasons := report.Injectable()
 			if testCase.injectable != actual {
 				t.Errorf("Expected %t. Actual %t", testCase.injectable, actual)
 			}
-			if testCase.reason != reason {
+
-				t.Errorf("Expected reason '%s'. Actual reason '%s'", testCase.reason, reason)
+			if len(reasons) != len(testCase.reasons) {
+				t.Errorf("Expected %d number of reasons. Actual %d", len(testCase.reasons), len(reasons))
 			}
+
+			for i := range reasons {
+				if testCase.reasons[i] != reasons[i] {
+					t.Errorf("Expected reason '%s'. Actual reason '%s'", testCase.reasons[i], reasons[i])
+				}
+			}
+
 		})
 	}
 }
@@ -203,7 +278,7 @@ func TestDisableByAnnotation(t *testing.T) {
 				resourceConfig.pod.meta = testCase.podMeta
 
 				report := newReport(resourceConfig)
-				if actual, _ := report.disableByAnnotation(resourceConfig); testCase.expected != actual {
+				if actual, _, _ := report.disableByAnnotation(resourceConfig); testCase.expected != actual {
 					t.Errorf("Expected %t. Actual %t", testCase.expected, actual)
 				}
 			})
@@ -244,7 +319,7 @@ func TestDisableByAnnotation(t *testing.T) {
 				resourceConfig.pod.meta = testCase.podMeta
 
 				report := newReport(resourceConfig)
-				if actual, _ := report.disableByAnnotation(resourceConfig); testCase.expected != actual {
+				if actual, _, _ := report.disableByAnnotation(resourceConfig); testCase.expected != actual {
 					t.Errorf("Expected %t. Actual %t", testCase.expected, actual)
 				}
 			})