matchCondition metrics

staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go

@@ -54,6 +54,8 @@ var (
 type ObserverFunc func(ctx context.Context, elapsed time.Duration, rejected bool, attr admission.Attributes, stepType string, extraLabels ...string)
 
 const (
+	kindWebhook  = "webhook"
+	kindPolicy   = "policy"
 	stepValidate = "validate"
 	stepAdmit    = "admit"
 )
@@ -112,13 +114,15 @@ func (p pluginHandlerWithMetrics) Validate(ctx context.Context, a admission.Attr
 
 // AdmissionMetrics instruments admission with prometheus metrics.
 type AdmissionMetrics struct {
-	step                     *metricSet
+	step                            *metricSet
-	controller               *metricSet
+	controller                      *metricSet
-	webhook                  *metricSet
+	webhook                         *metricSet
-	webhookRejection         *metrics.CounterVec
+	webhookRejection                *metrics.CounterVec
-	webhookFailOpen          *metrics.CounterVec
+	webhookFailOpen                 *metrics.CounterVec
-	webhookRequest           *metrics.CounterVec
+	webhookRequest                  *metrics.CounterVec
-	matchConditionEvalErrors *metrics.CounterVec
+	matchConditionEvalErrors        *metrics.CounterVec
+	matchConditionExclusions        *metrics.CounterVec
+	matchConditionEvaluationSeconds *metricSet
 }
 
 // newAdmissionMetrics create a new AdmissionMetrics, configured with default metric names.
@@ -222,20 +226,47 @@ func newAdmissionMetrics() *AdmissionMetrics {
 		&metrics.CounterOpts{
 			Namespace:      namespace,
 			Subsystem:      subsystem,
-			Name:           "admission_match_condition_evaluation_errors_total",
+			Name:           "match_condition_evaluation_errors_total",
-			Help:           "Admission match condition evaluation errors count, identified by name of resource containing the match condition and broken out for each admission type (validating or mutating).",
+			Help:           "Admission match condition evaluation errors count, identified by name of resource containing the match condition and broken out for each kind containing matchConditions (webhook or policy), operation and admission type (validate or admit).",
 			StabilityLevel: metrics.ALPHA,
 		},
-		[]string{"name", "type"})
+		[]string{"name", "kind", "type", "operation"})
+
+	matchConditionExclusions := metrics.NewCounterVec(
+		&metrics.CounterOpts{
+			Namespace:      namespace,
+			Subsystem:      subsystem,
+			Name:           "match_condition_exclusions_total",
+			Help:           "Admission match condition evaluation exclusions count, identified by name of resource containing the match condition and broken out for each kind containing matchConditions (webhook or policy), operation and admission type (validate or admit).",
+			StabilityLevel: metrics.ALPHA,
+		},
+		[]string{"name", "kind", "type", "operation"})
+
+	matchConditionEvaluationSeconds := &metricSet{
+		latencies: metrics.NewHistogramVec(
+			&metrics.HistogramOpts{
+				Namespace:      namespace,
+				Subsystem:      subsystem,
+				Name:           "match_condition_evaluation_seconds",
+				Help:           "Admission match condition evaluation time in seconds, identified by name and broken out for each kind containing matchConditions (webhook or policy), operation and type (validate or admit).",
+				Buckets:        []float64{0.005, 0.025, 0.1, 0.5, 1.0, 2.5},
+				StabilityLevel: metrics.ALPHA,
+			},
+			[]string{"name", "kind", "type", "operation"},
+		),
+		latenciesSummary: nil,
+	}
 
 	step.mustRegister()
 	controller.mustRegister()
 	webhook.mustRegister()
+	matchConditionEvaluationSeconds.mustRegister()
 	legacyregistry.MustRegister(webhookRejection)
 	legacyregistry.MustRegister(webhookFailOpen)
 	legacyregistry.MustRegister(webhookRequest)
 	legacyregistry.MustRegister(matchConditionEvalError)
-	return &AdmissionMetrics{step: step, controller: controller, webhook: webhook, webhookRejection: webhookRejection, webhookFailOpen: webhookFailOpen, webhookRequest: webhookRequest, matchConditionEvalErrors: matchConditionEvalError}
+	legacyregistry.MustRegister(matchConditionExclusions)
+	return &AdmissionMetrics{step: step, controller: controller, webhook: webhook, webhookRejection: webhookRejection, webhookFailOpen: webhookFailOpen, webhookRequest: webhookRequest, matchConditionEvalErrors: matchConditionEvalError, matchConditionExclusions: matchConditionExclusions, matchConditionEvaluationSeconds: matchConditionEvaluationSeconds}
 }
 
 func (m *AdmissionMetrics) reset() {
@@ -280,8 +311,18 @@ func (m *AdmissionMetrics) ObserveWebhookFailOpen(ctx context.Context, name, ste
 }
 
 // ObserveMatchConditionEvalError records validating or mutating webhook that are not called due to match conditions
-func (m *AdmissionMetrics) ObserveMatchConditionEvalError(ctx context.Context, name, stepType string) {
+func (m *AdmissionMetrics) ObserveMatchConditionEvalError(ctx context.Context, name, kind, stepType, operation string) {
-	m.matchConditionEvalErrors.WithContext(ctx).WithLabelValues(name, stepType).Inc()
+	m.matchConditionEvalErrors.WithContext(ctx).WithLabelValues(name, kind, stepType, operation).Inc()
+}
+
+// ObserveMatchConditionExclusion records validating or mutating webhook that are not called due to match conditions
+func (m *AdmissionMetrics) ObserveMatchConditionExclusion(ctx context.Context, name, kind, stepType, operation string) {
+	m.matchConditionExclusions.WithContext(ctx).WithLabelValues(name, kind, stepType, operation).Inc()
+}
+
+// ObserveMatchConditionEvaluationTime records duration of match condition evaluation process.
+func (m *AdmissionMetrics) ObserveMatchConditionEvaluationTime(ctx context.Context, elapsed time.Duration, name, kind, stepType, operation string) {
+	m.matchConditionEvaluationSeconds.observe(ctx, elapsed, name, kind, stepType, operation)
 }
 
 type metricSet struct {

staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go

@@ -176,6 +176,121 @@ func TestObserveWebhookFailOpen(t *testing.T) {
 	expectCounterValue(t, "apiserver_admission_webhook_fail_open_count", wantLabelsCounterValidate, 1)
 }
 
+func TestObserveMatchConditionEvalError(t *testing.T) {
+	defer Metrics.reset()
+	defer legacyregistry.Reset()
+	Metrics.ObserveMatchConditionEvalError(context.TODO(), "x", kindWebhook, stepAdmit, string(admission.Create))
+	Metrics.ObserveMatchConditionEvalError(context.TODO(), "x", kindWebhook, stepValidate, string(admission.Create))
+	Metrics.ObserveMatchConditionEvalError(context.TODO(), "x", kindPolicy, stepAdmit, string(admission.Create))
+	Metrics.ObserveMatchConditionEvalError(context.TODO(), "x", kindPolicy, stepValidate, string(admission.Create))
+	wantLabelsCounterPolicyAdmit := map[string]string{
+		"name":      "x",
+		"type":      "admit",
+		"kind":      "policy",
+		"operation": string(admission.Create),
+	}
+	wantLabelsCounterPolicyValidate := map[string]string{
+		"name":      "x",
+		"type":      "validate",
+		"kind":      "policy",
+		"operation": string(admission.Create),
+	}
+	wantLabelsCounterWebhookAdmit := map[string]string{
+		"name":      "x",
+		"type":      "admit",
+		"kind":      "webhook",
+		"operation": string(admission.Create),
+	}
+	wantLabelsCounterWebhookValidate := map[string]string{
+		"name":      "x",
+		"type":      "validate",
+		"kind":      "webhook",
+		"operation": string(admission.Create),
+	}
+	expectCounterValue(t, "apiserver_admission_match_condition_evaluation_errors_total", wantLabelsCounterPolicyAdmit, 1)
+	expectCounterValue(t, "apiserver_admission_match_condition_evaluation_errors_total", wantLabelsCounterPolicyValidate, 1)
+	expectCounterValue(t, "apiserver_admission_match_condition_evaluation_errors_total", wantLabelsCounterWebhookAdmit, 1)
+	expectCounterValue(t, "apiserver_admission_match_condition_evaluation_errors_total", wantLabelsCounterWebhookValidate, 1)
+}
+
+func TestObserveMatchConditionExclusion(t *testing.T) {
+	defer Metrics.reset()
+	defer legacyregistry.Reset()
+	Metrics.ObserveMatchConditionExclusion(context.TODO(), "x", kindWebhook, stepAdmit, string(admission.Create))
+	Metrics.ObserveMatchConditionExclusion(context.TODO(), "x", kindWebhook, stepValidate, string(admission.Create))
+	Metrics.ObserveMatchConditionExclusion(context.TODO(), "x", kindPolicy, stepAdmit, string(admission.Create))
+	Metrics.ObserveMatchConditionExclusion(context.TODO(), "x", kindPolicy, stepValidate, string(admission.Create))
+	wantLabelsCounterPolicyAdmit := map[string]string{
+		"name":      "x",
+		"type":      "admit",
+		"kind":      "policy",
+		"operation": string(admission.Create),
+	}
+	wantLabelsCounterPolicyValidate := map[string]string{
+		"name":      "x",
+		"type":      "validate",
+		"kind":      "policy",
+		"operation": string(admission.Create),
+	}
+	wantLabelsCounterWebhookAdmit := map[string]string{
+		"name":      "x",
+		"type":      "admit",
+		"kind":      "webhook",
+		"operation": string(admission.Create),
+	}
+	wantLabelsCounterWebhookValidate := map[string]string{
+		"name":      "x",
+		"type":      "validate",
+		"kind":      "webhook",
+		"operation": string(admission.Create),
+	}
+	metrics, _ := legacyregistry.DefaultGatherer.Gather()
+	for _, mf := range metrics {
+		t.Logf("%v", *mf.Name)
+	}
+	expectCounterValue(t, "apiserver_admission_match_condition_exclusions_total", wantLabelsCounterPolicyAdmit, 1)
+	expectCounterValue(t, "apiserver_admission_match_condition_exclusions_total", wantLabelsCounterPolicyValidate, 1)
+	expectCounterValue(t, "apiserver_admission_match_condition_exclusions_total", wantLabelsCounterWebhookAdmit, 1)
+	expectCounterValue(t, "apiserver_admission_match_condition_exclusions_total", wantLabelsCounterWebhookValidate, 1)
+}
+
+func TestObserveMatchConditionEvaluationTime(t *testing.T) {
+	defer Metrics.reset()
+	defer legacyregistry.Reset()
+	Metrics.ObserveMatchConditionEvaluationTime(context.TODO(), 2*time.Second, "x", kindPolicy, stepAdmit, string(admission.Create))
+	Metrics.ObserveMatchConditionEvaluationTime(context.TODO(), 2*time.Second, "x", kindPolicy, stepValidate, string(admission.Create))
+	Metrics.ObserveMatchConditionEvaluationTime(context.TODO(), 2*time.Second, "x", kindWebhook, stepAdmit, string(admission.Create))
+	Metrics.ObserveMatchConditionEvaluationTime(context.TODO(), 2*time.Second, "x", kindWebhook, stepValidate, string(admission.Create))
+	wantLabelsCounterPolicyAdmit := map[string]string{
+		"name":      "x",
+		"type":      "admit",
+		"kind":      "policy",
+		"operation": string(admission.Create),
+	}
+	wantLabelsCounterPolicyValidate := map[string]string{
+		"name":      "x",
+		"type":      "validate",
+		"kind":      "policy",
+		"operation": string(admission.Create),
+	}
+	wantLabelsCounterWebhookAdmit := map[string]string{
+		"name":      "x",
+		"type":      "admit",
+		"kind":      "webhook",
+		"operation": string(admission.Create),
+	}
+	wantLabelsCounterWebhookValidate := map[string]string{
+		"name":      "x",
+		"type":      "validate",
+		"kind":      "webhook",
+		"operation": string(admission.Create),
+	}
+	expectHistogramCountTotal(t, "apiserver_admission_match_condition_evaluation_seconds", wantLabelsCounterPolicyAdmit, 1)
+	expectHistogramCountTotal(t, "apiserver_admission_match_condition_evaluation_seconds", wantLabelsCounterPolicyValidate, 1)
+	expectHistogramCountTotal(t, "apiserver_admission_match_condition_evaluation_seconds", wantLabelsCounterWebhookAdmit, 1)
+	expectHistogramCountTotal(t, "apiserver_admission_match_condition_evaluation_seconds", wantLabelsCounterWebhookValidate, 1)
+}
+
 func TestWithMetrics(t *testing.T) {
 	defer Metrics.reset()
 	defer legacyregistry.Reset()

staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller_reconcile.go

@@ -481,7 +481,7 @@ func (c *policyController) latestPolicyData() []policyData {
 					for i := range matchConditions {
 						matchExpressionAccessors[i] = (*matchconditions.MatchCondition)(&matchConditions[i])
 					}
-					matcher = matchconditions.NewMatcher(filterCompiler.Compile(matchExpressionAccessors, optionalVars, environment.StoredExpressions), failurePolicy, "validatingadmissionpolicy", definitionInfo.lastReconciledValue.Name)
+					matcher = matchconditions.NewMatcher(filterCompiler.Compile(matchExpressionAccessors, optionalVars, environment.StoredExpressions), failurePolicy, "policy", "validate", definitionInfo.lastReconciledValue.Name)
 				}
 				bindingInfo.validator = c.newValidator(
 					filterCompiler.Compile(convertv1alpha1Validations(definitionInfo.lastReconciledValue.Spec.Validations), optionalVars, environment.StoredExpressions),

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go

@@ -80,6 +80,9 @@ type WebhookAccessor interface {
 	GetMutatingWebhook() (*v1.MutatingWebhook, bool)
 	// GetValidatingWebhook if the accessor contains a ValidatingWebhook, returns it and true, else returns false.
 	GetValidatingWebhook() (*v1.ValidatingWebhook, bool)
+
+	// GetType returns the type of the accessor (validate or admit)
+	GetType() string
 }
 
 // NewMutatingWebhookAccessor creates an accessor for a MutatingWebhook.
@@ -123,6 +126,10 @@ func (m *mutatingWebhookAccessor) GetRESTClient(clientManager *webhookutil.Clien
 	return m.client, m.clientErr
 }
 
+func (m *mutatingWebhookAccessor) GetType() string {
+	return "admit"
+}
+
 func (m *mutatingWebhookAccessor) GetCompiledMatcher(compiler cel.FilterCompiler) matchconditions.Matcher {
 	m.compileMatcher.Do(func() {
 		expressions := make([]cel.ExpressionAccessor, len(m.MutatingWebhook.MatchConditions))
@@ -139,7 +146,7 @@ func (m *mutatingWebhookAccessor) GetCompiledMatcher(compiler cel.FilterCompiler
 				HasAuthorizer: true,
 			},
 			environment.StoredExpressions,
-		), m.FailurePolicy, "validating", m.Name)
+		), m.FailurePolicy, "webhook", "admit", m.Name)
 	})
 	return m.compiledMatcher
 }
@@ -267,7 +274,7 @@ func (v *validatingWebhookAccessor) GetCompiledMatcher(compiler cel.FilterCompil
 				HasAuthorizer: true,
 			},
 			environment.StoredExpressions,
-		), v.FailurePolicy, "validating", v.Name)
+		), v.FailurePolicy, "webhook", "validating", v.Name)
 	})
 	return v.compiledMatcher
 }
@@ -286,6 +293,10 @@ func (v *validatingWebhookAccessor) GetParsedObjectSelector() (labels.Selector,
 	return v.objectSelector, v.objectSelectorErr
 }
 
+func (m *validatingWebhookAccessor) GetType() string {
+	return "validate"
+}
+
 func (v *validatingWebhookAccessor) GetName() string {
 	return v.Name
 }

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go

@@ -21,6 +21,8 @@ import (
 	"fmt"
 	"io"
 
+	admissionmetrics "k8s.io/apiserver/pkg/admission/metrics"
+
 	admissionv1 "k8s.io/api/admission/v1"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	v1 "k8s.io/api/admissionregistration/v1"
@@ -217,7 +219,7 @@ func (a *Webhook) ShouldCallHook(ctx context.Context, h webhook.WebhookAccessor,
 	if matchObjErr != nil {
 		return nil, matchObjErr
 	}
-
+	//TODO: maybe this sould be before the invocations are created
 	matchConditions := h.GetMatchConditions()
 	if len(matchConditions) > 0 {
 		versionedAttr, err := v.VersionedAttribute(invocation.Kind)
@@ -232,6 +234,7 @@ func (a *Webhook) ShouldCallHook(ctx context.Context, h webhook.WebhookAccessor,
 			klog.Warningf("Failed evaluating match conditions, failing closed %v: %v", h.GetName(), matchResult.Error)
 			return nil, apierrors.NewForbidden(attr.GetResource().GroupResource(), attr.GetName(), matchResult.Error)
 		} else if !matchResult.Matches {
+			admissionmetrics.Metrics.ObserveMatchConditionExclusion(ctx, h.GetName(), "webhook", h.GetType(), string(attr.GetOperation()))
 			// if no match, always skip webhook
 			return nil, nil
 		}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/matchconditions/matcher.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"time"
 
 	"github.com/google/cel-go/cel"
 	celtypes "github.com/google/cel-go/common/types"
@@ -55,10 +56,11 @@ type matcher struct {
 	filter      celplugin.Filter
 	failPolicy  v1.FailurePolicyType
 	matcherType string
+	matcherKind string
 	objectName  string
 }
 
-func NewMatcher(filter celplugin.Filter, failPolicy *v1.FailurePolicyType, matcherType, objectName string) Matcher {
+func NewMatcher(filter celplugin.Filter, failPolicy *v1.FailurePolicyType, matcherKind, matcherType, objectName string) Matcher {
 	var f v1.FailurePolicyType
 	if failPolicy == nil {
 		f = v1.Fail
@@ -68,18 +70,21 @@ func NewMatcher(filter celplugin.Filter, failPolicy *v1.FailurePolicyType, match
 	return &matcher{
 		filter:      filter,
 		failPolicy:  f,
+		matcherKind: matcherKind,
 		matcherType: matcherType,
 		objectName:  objectName,
 	}
 }
 
 func (m *matcher) Match(ctx context.Context, versionedAttr *admission.VersionedAttributes, versionedParams runtime.Object, authz authorizer.Authorizer) MatchResult {
+	t := time.Now()
 	evalResults, _, err := m.filter.ForInput(ctx, versionedAttr, celplugin.CreateAdmissionRequest(versionedAttr.Attributes), celplugin.OptionalVariableBindings{
 		VersionedParams: versionedParams,
 		Authorizer:      authz,
 	}, celconfig.RuntimeCELCostBudgetMatchConditions)
 
 	if err != nil {
+		admissionmetrics.Metrics.ObserveMatchConditionEvaluationTime(ctx, time.Since(t), m.objectName, m.matcherKind, m.matcherType, string(versionedAttr.GetOperation()))
 		// filter returning error is unexpected and not an evaluation error so not incrementing metric here
 		if m.failPolicy == v1.Fail {
 			return MatchResult{
@@ -104,10 +109,10 @@ func (m *matcher) Match(ctx context.Context, versionedAttr *admission.VersionedA
 		}
 		if evalResult.Error != nil {
 			errorList = append(errorList, evalResult.Error)
-			//TODO: what's the best way to handle this metric since its reused by VAP for match conditions
+			admissionmetrics.Metrics.ObserveMatchConditionEvalError(ctx, m.objectName, m.matcherKind, m.matcherType, string(versionedAttr.GetOperation()))
-			admissionmetrics.Metrics.ObserveMatchConditionEvalError(ctx, m.objectName, m.matcherType)
 		}
 		if evalResult.EvalResult == celtypes.False {
+			admissionmetrics.Metrics.ObserveMatchConditionEvaluationTime(ctx, time.Since(t), m.objectName, m.matcherKind, m.matcherType, string(versionedAttr.GetOperation()))
 			// If any condition false, skip calling webhook always
 			return MatchResult{
 				Matches:             false,
@@ -116,6 +121,7 @@ func (m *matcher) Match(ctx context.Context, versionedAttr *admission.VersionedA
 		}
 	}
 	if len(errorList) > 0 {
+		admissionmetrics.Metrics.ObserveMatchConditionEvaluationTime(ctx, time.Since(t), m.objectName, m.matcherKind, m.matcherType, string(versionedAttr.GetOperation()))
 		// If mix of true and eval errors then resort to fail policy
 		if m.failPolicy == v1.Fail {
 			// mix of true and errors with fail policy fail should fail request without calling webhook

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/matchconditions/matcher_test.go

@@ -334,7 +334,7 @@ func TestMatch(t *testing.T) {
 			m := NewMatcher(&fakeCelFilter{
 				evaluations: tc.evaluations,
 				throwError:  tc.throwError,
-			}, tc.failPolicy, "test", "testhook")
+			}, tc.failPolicy, "webhook", "test", "testhook")
 			ctx := context.TODO()
 			matchResult := m.Match(ctx, fakeVersionedAttr, nil, nil)