refactor: engine response stats (#6796)

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

pkg/engine/api/engineresponse.go

@@ -2,7 +2,6 @@ package api
 
 import (
 	"fmt"
-	"time"
 
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
 	datautils "github.com/kyverno/kyverno/pkg/utils/data"
@@ -24,7 +23,7 @@ type EngineResponse struct {
 	// PolicyResponse contains the engine policy response
 	PolicyResponse PolicyResponse
 	// Stats contains engine statistics
-	Stats ExecutionStats
+	stats ExecutionStats
 }
 
 func Resource(policyContext PolicyContext) unstructured.Unstructured {
@@ -35,13 +34,11 @@ func Resource(policyContext PolicyContext) unstructured.Unstructured {
 	return resource
 }
 
-func NewEngineResponseFromPolicyContext(policyContext PolicyContext, timestamp time.Time) EngineResponse {
+func NewEngineResponseFromPolicyContext(policyContext PolicyContext) EngineResponse {
 	return NewEngineResponse(
 		Resource(policyContext),
 		policyContext.Policy(),
 		policyContext.NamespaceLabels(),
-		nil,
-		timestamp,
 	)
 }
 
@@ -49,20 +46,13 @@ func NewEngineResponse(
 	resource unstructured.Unstructured,
 	policy kyvernov1.PolicyInterface,
 	namespaceLabels map[string]string,
-	policyResponse *PolicyResponse,
-	timestamp time.Time,
 ) EngineResponse {
-	response := EngineResponse{
+	return EngineResponse{
 		Resource:        resource,
 		Policy:          policy,
 		NamespaceLabels: namespaceLabels,
 		PatchedResource: resource,
-		Stats:           NewExecutionStats(timestamp),
 	}
-	if policyResponse != nil {
-		response = response.WithPolicyResponse(*policyResponse)
-	}
-	return response
 }
 
 func (er EngineResponse) WithPolicyResponse(policyResponse PolicyResponse) EngineResponse {
@@ -70,13 +60,13 @@ func (er EngineResponse) WithPolicyResponse(policyResponse PolicyResponse) Engin
 	return er
 }
 
-func (er EngineResponse) WithPatchedResource(patchedResource unstructured.Unstructured) EngineResponse {
-	er.PatchedResource = patchedResource
-	return er
+func (r EngineResponse) WithStats(stats ExecutionStats) EngineResponse {
+	r.stats = stats
+	return r
 }
 
-func (er EngineResponse) Done(timestamp time.Time) EngineResponse {
-	er.Stats.Done(timestamp)
+func (er EngineResponse) WithPatchedResource(patchedResource unstructured.Unstructured) EngineResponse {
+	er.PatchedResource = patchedResource
 	return er
 }
 

pkg/engine/api/ruleresponse.go

@@ -109,7 +109,6 @@ func (r RuleResponse) WithPatches(patches ...[]byte) *RuleResponse {
 
 func (r RuleResponse) WithStats(stats ExecutionStats) RuleResponse {
 	r.stats = stats
-
 	return r
 }
 

pkg/engine/api/stats.go

@@ -6,19 +6,13 @@ import (
 
 // ExecutionStats stores the statistics for the single policy/rule application
 type ExecutionStats struct {
-	// ProcessingTime is the time required to apply the policy/rule on the resource
+	// processingTime is the time required to apply the policy/rule on the resource
 	processingTime time.Duration
 	// timestamp of the instant the policy/rule got triggered
 	timestamp time.Time
 }
 
-func NewExecutionStats(timestamp time.Time) ExecutionStats {
-	return ExecutionStats{
-		timestamp: timestamp,
-	}
-}
-
-func NewExecutionStatsFull(startTime, endTime time.Time) ExecutionStats {
+func NewExecutionStats(startTime, endTime time.Time) ExecutionStats {
 	return ExecutionStats{
 		timestamp:      startTime,
 		processingTime: endTime.Sub(startTime),
@@ -37,10 +31,6 @@ func (s ExecutionStats) ProcessingTime() time.Duration {
 	return s.processingTime
 }
 
-func (s *ExecutionStats) Done(timestamp time.Time) {
-	s.processingTime = timestamp.Sub(s.timestamp)
-}
-
 // PolicyStats stores statistics for the single policy application
 type PolicyStats struct {
 	// RulesAppliedCount is the count of rules that were applied successfully

pkg/engine/engine.go

@@ -78,13 +78,14 @@ func (e *engine) Validate(
 	ctx context.Context,
 	policyContext engineapi.PolicyContext,
 ) engineapi.EngineResponse {
-	response := engineapi.NewEngineResponseFromPolicyContext(policyContext, time.Now())
+	startTime := time.Now()
+	response := engineapi.NewEngineResponseFromPolicyContext(policyContext)
 	logger := internal.LoggerWithPolicyContext(logging.WithName("engine.validate"), policyContext)
 	if internal.MatchPolicyContext(logger, policyContext, e.configuration) {
 		policyResponse := e.validate(ctx, logger, policyContext)
 		response = response.WithPolicyResponse(policyResponse)
 	}
-	response = response.Done(time.Now())
+	response = response.WithStats(engineapi.NewExecutionStats(startTime, time.Now()))
 	e.reportMetrics(ctx, logger, policyContext.Operation(), policyContext.AdmissionOperation(), response)
 	return response
 }
@@ -93,7 +94,8 @@ func (e *engine) Mutate(
 	ctx context.Context,
 	policyContext engineapi.PolicyContext,
 ) engineapi.EngineResponse {
-	response := engineapi.NewEngineResponseFromPolicyContext(policyContext, time.Now())
+	startTime := time.Now()
+	response := engineapi.NewEngineResponseFromPolicyContext(policyContext)
 	logger := internal.LoggerWithPolicyContext(logging.WithName("engine.mutate"), policyContext)
 	if internal.MatchPolicyContext(logger, policyContext, e.configuration) {
 		policyResponse, patchedResource := e.mutate(ctx, logger, policyContext)
@@ -101,7 +103,7 @@ func (e *engine) Mutate(
 			WithPolicyResponse(policyResponse).
 			WithPatchedResource(patchedResource)
 	}
-	response = response.Done(time.Now())
+	response = response.WithStats(engineapi.NewExecutionStats(startTime, time.Now()))
 	e.reportMetrics(ctx, logger, policyContext.Operation(), policyContext.AdmissionOperation(), response)
 	return response
 }
@@ -110,13 +112,14 @@ func (e *engine) Generate(
 	ctx context.Context,
 	policyContext engineapi.PolicyContext,
 ) engineapi.EngineResponse {
-	response := engineapi.NewEngineResponseFromPolicyContext(policyContext, time.Now())
+	startTime := time.Now()
+	response := engineapi.NewEngineResponseFromPolicyContext(policyContext)
 	logger := internal.LoggerWithPolicyContext(logging.WithName("engine.generate"), policyContext)
 	if internal.MatchPolicyContext(logger, policyContext, e.configuration) {
 		policyResponse := e.generateResponse(ctx, logger, policyContext)
 		response = response.WithPolicyResponse(policyResponse)
 	}
-	response = response.Done(time.Now())
+	response = response.WithStats(engineapi.NewExecutionStats(startTime, time.Now()))
 	e.reportMetrics(ctx, logger, policyContext.Operation(), policyContext.AdmissionOperation(), response)
 	return response
 }
@@ -125,14 +128,15 @@ func (e *engine) VerifyAndPatchImages(
 	ctx context.Context,
 	policyContext engineapi.PolicyContext,
 ) (engineapi.EngineResponse, engineapi.ImageVerificationMetadata) {
-	response := engineapi.NewEngineResponseFromPolicyContext(policyContext, time.Now())
+	startTime := time.Now()
+	response := engineapi.NewEngineResponseFromPolicyContext(policyContext)
 	ivm := engineapi.ImageVerificationMetadata{}
 	logger := internal.LoggerWithPolicyContext(logging.WithName("engine.verify"), policyContext)
 	if internal.MatchPolicyContext(logger, policyContext, e.configuration) {
 		policyResponse, innerIvm := e.verifyAndPatchImages(ctx, logger, policyContext)
 		response, ivm = response.WithPolicyResponse(policyResponse), innerIvm
 	}
-	response = response.Done(time.Now())
+	response = response.WithStats(engineapi.NewExecutionStats(startTime, time.Now()))
 	e.reportMetrics(ctx, logger, policyContext.Operation(), policyContext.AdmissionOperation(), response)
 	return response, ivm
 }
@@ -141,13 +145,14 @@ func (e *engine) ApplyBackgroundChecks(
 	ctx context.Context,
 	policyContext engineapi.PolicyContext,
 ) engineapi.EngineResponse {
-	response := engineapi.NewEngineResponseFromPolicyContext(policyContext, time.Now())
+	startTime := time.Now()
+	response := engineapi.NewEngineResponseFromPolicyContext(policyContext)
 	logger := internal.LoggerWithPolicyContext(logging.WithName("engine.background"), policyContext)
 	if internal.MatchPolicyContext(logger, policyContext, e.configuration) {
 		policyResponse := e.applyBackgroundChecks(ctx, logger, policyContext)
 		response = response.WithPolicyResponse(policyResponse)
 	}
-	response = response.Done(time.Now())
+	response = response.WithStats(engineapi.NewExecutionStats(startTime, time.Now()))
 	e.reportMetrics(ctx, logger, policyContext.Operation(), policyContext.AdmissionOperation(), response)
 	return response
 }

pkg/engine/image_verify.go

@@ -53,7 +53,7 @@ func (e *engine) verifyAndPatchImages(
 			engineapi.ImageVerify,
 		)
 		matchedResource = resource
-		stats := engineapi.NewExecutionStatsFull(startTime, time.Now())
+		stats := engineapi.NewExecutionStats(startTime, time.Now())
 		for _, ruleResp := range ruleResp {
 			resp.Add(stats, ruleResp)
 		}

pkg/engine/mutation.go

@@ -50,7 +50,7 @@ func (e *engine) mutate(
 			engineapi.Mutation,
 		)
 		matchedResource = resource
-		stats := engineapi.NewExecutionStatsFull(startTime, time.Now())
+		stats := engineapi.NewExecutionStats(startTime, time.Now())
 		for _, ruleResp := range ruleResp {
 			resp.Add(stats, ruleResp)
 		}

pkg/engine/validation.go

@@ -68,7 +68,7 @@ func (e *engine) validate(
 			engineapi.Validation,
 		)
 		matchedResource = resource
-		stats := engineapi.NewExecutionStatsFull(startTime, time.Now())
+		stats := engineapi.NewExecutionStats(startTime, time.Now())
 		for _, ruleResp := range ruleResp {
 			resp.Add(stats, ruleResp)
 		}

pkg/utils/annotations_test.go

@@ -2,7 +2,6 @@ package utils
 
 import (
 	"testing"
-	"time"
 
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
 	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
@@ -32,7 +31,7 @@ func newEngineResponse(policy, rule string, patchesStr []string, status engineap
 		},
 	}
 	policyResponse := newPolicyResponse(rule, patchesStr, status)
-	response := engineapi.NewEngineResponse(unstructured.Unstructured{}, p, nil, &policyResponse, time.Now())
+	response := engineapi.NewEngineResponse(unstructured.Unstructured{}, p, nil).WithPolicyResponse(policyResponse)
 	response.PatchedResource = unstructured.Unstructured{
 		Object: map[string]interface{}{
 			"metadata": map[string]interface{}{

pkg/webhooks/utils/block_test.go

@@ -2,7 +2,6 @@ package utils
 
 import (
 	"testing"
-	"time"
 
 	"github.com/go-logr/logr"
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
@@ -85,11 +84,11 @@ func TestBlockRequest(t *testing.T) {
 		name: "failure - enforce",
 		args: args{
 			engineResponses: []engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, enforcePolicy, nil, &engineapi.PolicyResponse{
+				engineapi.NewEngineResponse(resource, enforcePolicy, nil).WithPolicyResponse(engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						*engineapi.RuleFail("rule-fail", engineapi.Validation, "message fail"),
 					},
-				}, time.Now()),
+				}),
 			},
 			failurePolicy: kyvernov1.Fail,
 			log:           logr.Discard(),
@@ -99,11 +98,11 @@ func TestBlockRequest(t *testing.T) {
 		name: "failure - audit",
 		args: args{
 			engineResponses: []engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{
+				engineapi.NewEngineResponse(resource, auditPolicy, nil).WithPolicyResponse(engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						*engineapi.RuleFail("rule-fail", engineapi.Validation, "message fail"),
 					},
-				}, time.Now()),
+				}),
 			},
 			failurePolicy: kyvernov1.Fail,
 			log:           logr.Discard(),
@@ -113,11 +112,11 @@ func TestBlockRequest(t *testing.T) {
 		name: "error - fail",
 		args: args{
 			engineResponses: []engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{
+				engineapi.NewEngineResponse(resource, auditPolicy, nil).WithPolicyResponse(engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						*engineapi.RuleError("rule-error", engineapi.Validation, "message error", nil),
 					},
-				}, time.Now()),
+				}),
 			},
 			failurePolicy: kyvernov1.Fail,
 			log:           logr.Discard(),
@@ -127,11 +126,11 @@ func TestBlockRequest(t *testing.T) {
 		name: "error - ignore",
 		args: args{
 			engineResponses: []engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{
+				engineapi.NewEngineResponse(resource, auditPolicy, nil).WithPolicyResponse(engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						*engineapi.RuleError("rule-error", engineapi.Validation, "message error", nil),
 					},
-				}, time.Now()),
+				}),
 			},
 			failurePolicy: kyvernov1.Ignore,
 			log:           logr.Discard(),
@@ -141,11 +140,11 @@ func TestBlockRequest(t *testing.T) {
 		name: "warning - ignore",
 		args: args{
 			engineResponses: []engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{
+				engineapi.NewEngineResponse(resource, auditPolicy, nil).WithPolicyResponse(engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						*engineapi.NewRuleResponse("rule-warning", engineapi.Validation, "message warning", engineapi.RuleStatusWarn),
 					},
-				}, time.Now()),
+				}),
 			},
 			failurePolicy: kyvernov1.Ignore,
 			log:           logr.Discard(),
@@ -155,11 +154,11 @@ func TestBlockRequest(t *testing.T) {
 		name: "warning - fail",
 		args: args{
 			engineResponses: []engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{
+				engineapi.NewEngineResponse(resource, auditPolicy, nil).WithPolicyResponse(engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						*engineapi.NewRuleResponse("rule-warning", engineapi.Validation, "message warning", engineapi.RuleStatusWarn),
 					},
-				}, time.Now()),
+				}),
 			},
 			failurePolicy: kyvernov1.Fail,
 			log:           logr.Discard(),
@@ -203,11 +202,11 @@ func TestGetBlockedMessages(t *testing.T) {
 		name: "failure - enforce",
 		args: args{
 			engineResponses: []engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, enforcePolicy, nil, &engineapi.PolicyResponse{
+				engineapi.NewEngineResponse(resource, enforcePolicy, nil).WithPolicyResponse(engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						*engineapi.RuleFail("rule-fail", engineapi.Validation, "message fail"),
 					},
-				}, time.Now()),
+				}),
 			},
 		},
 		want: "\n\npolicy foo/bar/baz for resource violation: \n\ntest:\n  rule-fail: message fail\n",
@@ -215,11 +214,11 @@ func TestGetBlockedMessages(t *testing.T) {
 		name: "error - enforce",
 		args: args{
 			engineResponses: []engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, enforcePolicy, nil, &engineapi.PolicyResponse{
+				engineapi.NewEngineResponse(resource, enforcePolicy, nil).WithPolicyResponse(engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						*engineapi.RuleError("rule-error", engineapi.Validation, "message error", nil),
 					},
-				}, time.Now()),
+				}),
 			},
 		},
 		want: "\n\npolicy foo/bar/baz for resource error: \n\ntest:\n  rule-error: message error\n",
@@ -227,12 +226,12 @@ func TestGetBlockedMessages(t *testing.T) {
 		name: "error and failure - enforce",
 		args: args{
 			engineResponses: []engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, enforcePolicy, nil, &engineapi.PolicyResponse{
+				engineapi.NewEngineResponse(resource, enforcePolicy, nil).WithPolicyResponse(engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						*engineapi.RuleFail("rule-fail", engineapi.Validation, "message fail"),
 						*engineapi.RuleError("rule-error", engineapi.Validation, "message error", nil),
 					},
-				}, time.Now()),
+				}),
 			},
 		},
 		want: "\n\npolicy foo/bar/baz for resource violation: \n\ntest:\n  rule-error: message error\n  rule-fail: message fail\n",