refactor: kyverno_policy_execution_duration_seconds metric management (#6782)

* refactor: kyverno_policy_results metric management Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com> * refactor: kyverno_policy_results metric management Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com> * fix Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com> * fix Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com> --------- Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
2025-03-31 03:45:17 +00:00 · 2023-04-04 18:34:58 +02:00 · 2023-04-04 18:34:58 +02:00 · 5d6b987eec
commit 5d6b987eec
parent b2340785fc
9 changed files with 81 additions and 210 deletions
--- a/pkg/engine/engine.go
+++ b/pkg/engine/engine.go
@ -33,7 +33,8 @@ type engine struct {
 	contextLoader        engineapi.ContextLoaderFactory
 	exceptionSelector    engineapi.PolicyExceptionSelector
 	// metrics
-	resultCounter instrument.Int64Counter
+	resultCounter     instrument.Int64Counter
+	durationHistogram instrument.Float64Histogram
 }

 type handlerFactory = func() (handlers.Handler, error)
@ -54,6 +55,13 @@ func NewEngine(
 	if err != nil {
 		logging.Error(err, "failed to register metric kyverno_policy_results")
 	}
+	durationHistogram, err := meter.Float64Histogram(
+		"kyverno_policy_execution_duration_seconds",
+		instrument.WithDescription("can be used to track the latencies (in seconds) associated with the execution/processing of the individual rules under Kyverno policies whenever they evaluate incoming resource requests"),
+	)
+	if err != nil {
+		logging.Error(err, "failed to register metric kyverno_policy_execution_duration_seconds")
+	}
 	return &engine{
 		configuration:        configuration,
 		metricsConfiguration: metricsConfiguration,
@ -62,6 +70,7 @@ func NewEngine(
 		contextLoader:        contextLoader,
 		exceptionSelector:    exceptionSelector,
 		resultCounter:        resultCounter,
+		durationHistogram:    durationHistogram,
 	}
 }

--- a/pkg/engine/metrics.go
+++ b/pkg/engine/metrics.go
@ -18,56 +18,77 @@ func (e *engine) reportMetrics(
 	admissionOperation bool,
 	response engineapi.EngineResponse,
 ) {
-	if e.resultCounter != nil {
-		policy := response.Policy
-		if name, namespace, policyType, backgroundMode, validationMode, err := metrics.GetPolicyInfos(policy); err != nil {
-			logger.Error(err, "failed to get policy infos for metrics reporting")
-		} else {
-			if policyType == metrics.Cluster {
-				namespace = "-"
+	if e.resultCounter == nil && e.durationHistogram == nil {
+		return
+	}
+	policy := response.Policy
+	if name, namespace, policyType, backgroundMode, validationMode, err := metrics.GetPolicyInfos(policy); err != nil {
+		logger.Error(err, "failed to get policy infos for metrics reporting")
+	} else {
+		if policyType == metrics.Cluster {
+			namespace = "-"
+		}
+		if !e.metricsConfiguration.CheckNamespace(namespace) {
+			return
+		}
+		resourceSpec := response.Resource
+		resourceKind := resourceSpec.GetKind()
+		resourceNamespace := resourceSpec.GetNamespace()
+		for _, rule := range response.PolicyResponse.Rules {
+			ruleName := rule.Name
+			ruleType := metrics.ParseRuleTypeFromEngineRuleResponse(rule)
+			var ruleResult metrics.RuleResult
+			switch rule.Status {
+			case engineapi.RuleStatusPass:
+				ruleResult = metrics.Pass
+			case engineapi.RuleStatusFail:
+				ruleResult = metrics.Fail
+			case engineapi.RuleStatusWarn:
+				ruleResult = metrics.Warn
+			case engineapi.RuleStatusError:
+				ruleResult = metrics.Error
+			case engineapi.RuleStatusSkip:
+				ruleResult = metrics.Skip
+			default:
+				ruleResult = metrics.Fail
 			}
-			if e.metricsConfiguration.CheckNamespace(namespace) {
-				resourceSpec := response.Resource
-				resourceKind := resourceSpec.GetKind()
-				resourceNamespace := resourceSpec.GetNamespace()
-				for _, rule := range response.PolicyResponse.Rules {
-					ruleName := rule.Name
-					ruleType := metrics.ParseRuleTypeFromEngineRuleResponse(rule)
-					var ruleResult metrics.RuleResult
-					switch rule.Status {
-					case engineapi.RuleStatusPass:
-						ruleResult = metrics.Pass
-					case engineapi.RuleStatusFail:
-						ruleResult = metrics.Fail
-					case engineapi.RuleStatusWarn:
-						ruleResult = metrics.Warn
-					case engineapi.RuleStatusError:
-						ruleResult = metrics.Error
-					case engineapi.RuleStatusSkip:
-						ruleResult = metrics.Skip
-					default:
-						ruleResult = metrics.Fail
-					}
-					executionCause := metrics.AdmissionRequest
-					if !admissionOperation {
-						executionCause = metrics.BackgroundScan
-					}
-					commonLabels := []attribute.KeyValue{
-						attribute.String("policy_validation_mode", string(validationMode)),
-						attribute.String("policy_type", string(policyType)),
-						attribute.String("policy_background_mode", string(backgroundMode)),
-						attribute.String("policy_namespace", namespace),
-						attribute.String("policy_name", name),
-						attribute.String("resource_kind", resourceKind),
-						attribute.String("resource_namespace", resourceNamespace),
-						attribute.String("resource_request_operation", strings.ToLower(string(operation))),
-						attribute.String("rule_name", ruleName),
-						attribute.String("rule_result", string(ruleResult)),
-						attribute.String("rule_type", string(ruleType)),
-						attribute.String("rule_execution_cause", string(executionCause)),
-					}
-					e.resultCounter.Add(ctx, 1, commonLabels...)
+			executionCause := metrics.AdmissionRequest
+			if !admissionOperation {
+				executionCause = metrics.BackgroundScan
+			}
+			if e.resultCounter != nil {
+				commonLabels := []attribute.KeyValue{
+					attribute.String("policy_validation_mode", string(validationMode)),
+					attribute.String("policy_type", string(policyType)),
+					attribute.String("policy_background_mode", string(backgroundMode)),
+					attribute.String("policy_namespace", namespace),
+					attribute.String("policy_name", name),
+					attribute.String("resource_kind", resourceKind),
+					attribute.String("resource_namespace", resourceNamespace),
+					attribute.String("resource_request_operation", strings.ToLower(string(operation))),
+					attribute.String("rule_name", ruleName),
+					attribute.String("rule_result", string(ruleResult)),
+					attribute.String("rule_type", string(ruleType)),
+					attribute.String("rule_execution_cause", string(executionCause)),
 				}
+				e.resultCounter.Add(ctx, 1, commonLabels...)
+			}
+			if e.durationHistogram != nil {
+				commonLabels := []attribute.KeyValue{
+					attribute.String("policy_validation_mode", string(validationMode)),
+					attribute.String("policy_type", string(policyType)),
+					attribute.String("policy_background_mode", string(backgroundMode)),
+					attribute.String("policy_namespace", namespace),
+					attribute.String("policy_name", name),
+					attribute.String("resource_kind", resourceKind),
+					attribute.String("resource_namespace", resourceNamespace),
+					attribute.String("resource_request_operation", strings.ToLower(string(operation))),
+					attribute.String("rule_name", ruleName),
+					attribute.String("rule_result", string(ruleResult)),
+					attribute.String("rule_type", string(ruleType)),
+					attribute.String("rule_execution_cause", string(executionCause)),
+				}
+				e.durationHistogram.Record(ctx, rule.Stats.ProcessingTime.Seconds(), commonLabels...)
 			}
 		}
 	}
--- a/pkg/metrics/metrics.go
+++ b/pkg/metrics/metrics.go
@ -29,9 +29,8 @@ const (

 type MetricsConfig struct {
 	// instruments
-	policyChangesMetric           instrument.Int64Counter
-	policyExecutionDurationMetric instrument.Float64Histogram
-	clientQueriesMetric           instrument.Int64Counter
+	policyChangesMetric instrument.Int64Counter
+	clientQueriesMetric instrument.Int64Counter

 	// config
 	config kconfig.MetricsConfiguration
@ -41,7 +40,6 @@ type MetricsConfig struct {
 type MetricsConfigManager interface {
 	Config() kconfig.MetricsConfiguration
 	RecordPolicyChanges(ctx context.Context, policyValidationMode PolicyValidationMode, policyType PolicyType, policyBackgroundMode PolicyBackgroundMode, policyNamespace string, policyName string, policyChangeType string)
-	RecordPolicyExecutionDuration(ctx context.Context, policyValidationMode PolicyValidationMode, policyType PolicyType, policyBackgroundMode PolicyBackgroundMode, policyNamespace string, policyName string, ruleName string, ruleResult RuleResult, ruleType RuleType, ruleExecutionCause RuleExecutionCause, ruleExecutionLatency float64)
 	RecordClientQueries(ctx context.Context, clientQueryOperation ClientQueryOperation, clientType ClientType, resourceKind string, resourceNamespace string)
 }

@ -57,11 +55,6 @@ func (m *MetricsConfig) initializeMetrics(meterProvider metric.MeterProvider) er
 		m.Log.Error(err, "Failed to create instrument, kyverno_policy_changes")
 		return err
 	}
-	m.policyExecutionDurationMetric, err = meter.Float64Histogram("kyverno_policy_execution_duration_seconds", instrument.WithDescription("can be used to track the latencies (in seconds) associated with the execution/processing of the individual rules under Kyverno policies whenever they evaluate incoming resource requests"))
-	if err != nil {
-		m.Log.Error(err, "Failed to create instrument, kyverno_policy_execution_duration_seconds")
-		return err
-	}
 	m.clientQueriesMetric, err = meter.Int64Counter("kyverno_client_queries", instrument.WithDescription("can be used to track the number of client queries sent from Kyverno to the API-server"))
 	if err != nil {
 		m.Log.Error(err, "Failed to create instrument, kyverno_client_queries")
@ -174,23 +167,6 @@ func (m *MetricsConfig) RecordPolicyChanges(ctx context.Context, policyValidatio
 	m.policyChangesMetric.Add(ctx, 1, commonLabels...)
 }

-func (m *MetricsConfig) RecordPolicyExecutionDuration(ctx context.Context, policyValidationMode PolicyValidationMode, policyType PolicyType, policyBackgroundMode PolicyBackgroundMode, policyNamespace string, policyName string,
-	ruleName string, ruleResult RuleResult, ruleType RuleType, ruleExecutionCause RuleExecutionCause, ruleExecutionLatency float64,
-) {
-	commonLabels := []attribute.KeyValue{
-		attribute.String("policy_validation_mode", string(policyValidationMode)),
-		attribute.String("policy_type", string(policyType)),
-		attribute.String("policy_background_mode", string(policyBackgroundMode)),
-		attribute.String("policy_namespace", policyNamespace),
-		attribute.String("policy_name", policyName),
-		attribute.String("rule_name", ruleName),
-		attribute.String("rule_result", string(ruleResult)),
-		attribute.String("rule_type", string(ruleType)),
-		attribute.String("rule_execution_cause", string(ruleExecutionCause)),
-	}
-	m.policyExecutionDurationMetric.Record(ctx, ruleExecutionLatency, commonLabels...)
-}
-
 func (m *MetricsConfig) RecordClientQueries(ctx context.Context, clientQueryOperation ClientQueryOperation, clientType ClientType, resourceKind string, resourceNamespace string) {
 	commonLabels := []attribute.KeyValue{
 		attribute.String("operation", string(clientQueryOperation)),
--- a/pkg/metrics/policyexecutionduration/policyExecutionDuration.go
+++ b/pkg/metrics/policyexecutionduration/policyExecutionDuration.go
@ -1,78 +0,0 @@
-package policyexecutionduration
-
-import (
-	"context"
-
-	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
-	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
-	"github.com/kyverno/kyverno/pkg/metrics"
-)
-
-func registerPolicyExecutionDurationMetric(
-	ctx context.Context,
-	m metrics.MetricsConfigManager,
-	policyValidationMode metrics.PolicyValidationMode,
-	policyType metrics.PolicyType,
-	policyBackgroundMode metrics.PolicyBackgroundMode,
-	policyNamespace, policyName string,
-	resourceNamespace string,
-	ruleName string,
-	ruleResult metrics.RuleResult,
-	ruleType metrics.RuleType,
-	ruleExecutionCause metrics.RuleExecutionCause,
-	ruleExecutionLatency float64,
-) {
-	if policyType == metrics.Cluster {
-		policyNamespace = "-"
-	}
-	if m.Config().CheckNamespace(policyNamespace) {
-		m.RecordPolicyExecutionDuration(ctx, policyValidationMode, policyType, policyBackgroundMode, policyNamespace, policyName, ruleName, ruleResult, ruleType, ruleExecutionCause, ruleExecutionLatency)
-	}
-}
-
-// policy - policy related data
-// engineResponse - resource and rule related data
-func ProcessEngineResponse(ctx context.Context, m metrics.MetricsConfigManager, policy kyvernov1.PolicyInterface, engineResponse engineapi.EngineResponse, executionCause metrics.RuleExecutionCause, resourceRequestOperation metrics.ResourceRequestOperation) error {
-	name, namespace, policyType, backgroundMode, validationMode, err := metrics.GetPolicyInfos(policy)
-	if err != nil {
-		return err
-	}
-	resourceSpec := engineResponse.Resource
-	resourceNamespace := resourceSpec.GetNamespace()
-	ruleResponses := engineResponse.PolicyResponse.Rules
-	for _, rule := range ruleResponses {
-		ruleName := rule.Name
-		ruleType := metrics.ParseRuleTypeFromEngineRuleResponse(rule)
-		var ruleResult metrics.RuleResult
-		switch rule.Status {
-		case engineapi.RuleStatusPass:
-			ruleResult = metrics.Pass
-		case engineapi.RuleStatusFail:
-			ruleResult = metrics.Fail
-		case engineapi.RuleStatusWarn:
-			ruleResult = metrics.Warn
-		case engineapi.RuleStatusError:
-			ruleResult = metrics.Error
-		case engineapi.RuleStatusSkip:
-			ruleResult = metrics.Skip
-		default:
-			ruleResult = metrics.Fail
-		}
-		ruleExecutionLatencyInSeconds := float64(rule.Stats.ProcessingTime) / float64(1000*1000*1000)
-		registerPolicyExecutionDurationMetric(
-			ctx,
-			m,
-			validationMode,
-			policyType,
-			backgroundMode,
-			namespace, name,
-			resourceNamespace,
-			ruleName,
-			ruleResult,
-			ruleType,
-			executionCause,
-			ruleExecutionLatencyInSeconds,
-		)
-	}
-	return nil
-}
--- a/pkg/webhooks/resource/generation/handler.go
+++ b/pkg/webhooks/resource/generation/handler.go
@ -19,7 +19,6 @@ import (
 	"github.com/kyverno/kyverno/pkg/metrics"
 	engineutils "github.com/kyverno/kyverno/pkg/utils/engine"
 	webhookgenerate "github.com/kyverno/kyverno/pkg/webhooks/updaterequest"
-	webhookutils "github.com/kyverno/kyverno/pkg/webhooks/utils"
 	admissionv1 "k8s.io/api/admission/v1"
 	corev1listers "k8s.io/client-go/listers/core/v1"
 )
@ -123,8 +122,6 @@ func (h *generationHandler) handleTrigger(

 		h.applyGeneration(ctx, request, policy, appliedRules, policyContext)
 		h.syncTriggerAction(ctx, request, policy, failedRules, policyContext)
-
-		go webhookutils.RegisterPolicyExecutionDurationMetricGenerate(ctx, h.log, h.metrics, string(request.Operation), policy, engineResponse)
 	}
 }

--- a/pkg/webhooks/resource/mutation/mutation.go
+++ b/pkg/webhooks/resource/mutation/mutation.go
@ -122,9 +122,6 @@ func (v *mutationHandler) applyMutations(
 					engineResponses = append(engineResponses, *engineResponse)
 				}

-				// registering the kyverno_policy_execution_duration_seconds metric concurrently
-				go webhookutils.RegisterPolicyExecutionDurationMetricMutate(context.TODO(), v.log, v.metrics, string(request.Operation), policy, *engineResponse)
-
 				return nil
 			},
 		)
--- a/pkg/webhooks/resource/updaterequest.go
+++ b/pkg/webhooks/resource/updaterequest.go
@ -13,7 +13,6 @@ import (
 	"github.com/kyverno/kyverno/pkg/event"
 	"github.com/kyverno/kyverno/pkg/utils/wildcard"
 	"github.com/kyverno/kyverno/pkg/webhooks/resource/generation"
-	webhookutils "github.com/kyverno/kyverno/pkg/webhooks/utils"
 	admissionv1 "k8s.io/api/admission/v1"
 )

@ -60,9 +59,6 @@ func (h *resourceHandlers) handleMutateExisting(ctx context.Context, logger logr
 			engineResponse.PolicyResponse.Rules = rules
 			engineResponses = append(engineResponses, &engineResponse)
 		}
-
-		// registering the kyverno_policy_execution_duration_seconds metric concurrently
-		go webhookutils.RegisterPolicyExecutionDurationMetricMutate(context.TODO(), logger, h.metricsConfig, string(request.Operation), policy, engineResponse)
 	}

 	if failedResponse := applyUpdateRequest(ctx, request, kyvernov1beta1.Mutate, h.urGenerator, policyContext.AdmissionInfo(), request.Operation, engineResponses...); failedResponse != nil {
--- a/pkg/webhooks/resource/validation/validation.go
+++ b/pkg/webhooks/resource/validation/validation.go
@ -110,8 +110,6 @@ func (v *validationHandler) HandleValidation(
 					return
 				}

-				go webhookutils.RegisterPolicyExecutionDurationMetricValidate(ctx, logger, v.metrics, string(request.Operation), policyContext.Policy(), engineResponse)
-
 				engineResponses = append(engineResponses, engineResponse)
 				if !engineResponse.IsSuccessful() {
 					logger.V(2).Info("validation failed", "action", policy.GetSpec().ValidationFailureAction, "policy", policy.GetName(), "failed rules", engineResponse.GetFailedRules())
@ -164,7 +162,6 @@ func (v *validationHandler) buildAuditResponses(
 				policyContext := policyContext.WithPolicy(policy).WithNamespaceLabels(namespaceLabels)
 				response := v.engine.Validate(ctx, policyContext)
 				responses = append(responses, response)
-				go webhookutils.RegisterPolicyExecutionDurationMetricValidate(ctx, v.log, v.metrics, string(request.Operation), policyContext.Policy(), response)
 			},
 		)
 	}
--- a/pkg/webhooks/utils/metrics.go
+++ b/pkg/webhooks/utils/metrics.go
@ -1,44 +0,0 @@
-package utils
-
-import (
-	"context"
-	"fmt"
-
-	"github.com/go-logr/logr"
-	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
-	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
-	"github.com/kyverno/kyverno/pkg/metrics"
-	policyExecutionDuration "github.com/kyverno/kyverno/pkg/metrics/policyexecutionduration"
-)
-
-type reporterFunc func(metrics.ResourceRequestOperation) error
-
-func registerMetric(logger logr.Logger, m string, requestOperation string, r reporterFunc) {
-	if op, err := metrics.ParseResourceRequestOperation(requestOperation); err != nil {
-		logger.Error(err, fmt.Sprintf("error occurred while registering %s metrics", m))
-	} else {
-		if err := r(op); err != nil {
-			logger.Error(err, fmt.Sprintf("error occurred while registering %s metrics", m))
-		}
-	}
-}
-
-// POLICY EXECUTION
-
-func RegisterPolicyExecutionDurationMetricMutate(ctx context.Context, logger logr.Logger, metricsConfig metrics.MetricsConfigManager, requestOperation string, policy kyvernov1.PolicyInterface, engineResponse engineapi.EngineResponse) {
-	registerMetric(logger, "kyverno_policy_execution_duration_seconds", requestOperation, func(op metrics.ResourceRequestOperation) error {
-		return policyExecutionDuration.ProcessEngineResponse(ctx, metricsConfig, policy, engineResponse, metrics.AdmissionRequest, op)
-	})
-}
-
-func RegisterPolicyExecutionDurationMetricValidate(ctx context.Context, logger logr.Logger, metricsConfig metrics.MetricsConfigManager, requestOperation string, policy kyvernov1.PolicyInterface, engineResponse engineapi.EngineResponse) {
-	registerMetric(logger, "kyverno_policy_execution_duration_seconds", requestOperation, func(op metrics.ResourceRequestOperation) error {
-		return policyExecutionDuration.ProcessEngineResponse(ctx, metricsConfig, policy, engineResponse, metrics.AdmissionRequest, op)
-	})
-}
-
-func RegisterPolicyExecutionDurationMetricGenerate(ctx context.Context, logger logr.Logger, metricsConfig metrics.MetricsConfigManager, requestOperation string, policy kyvernov1.PolicyInterface, engineResponse engineapi.EngineResponse) {
-	registerMetric(logger, "kyverno_policy_execution_duration_seconds", requestOperation, func(op metrics.ResourceRequestOperation) error {
-		return policyExecutionDuration.ProcessEngineResponse(ctx, metricsConfig, policy, engineResponse, metrics.AdmissionRequest, op)
-	})
-}