Common stats reporting code for validation and mutation

Signed-off-by: mmirecki <mmirecki@redhat.com>

pkg/webhook/common.go

@@ -20,6 +20,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
+type requestResponse string
+
 var log = logf.Log.WithName("webhook")
 
 var (

pkg/webhook/mutation.go

@@ -33,13 +33,11 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
-type mutationResponse string
-
 const (
-	mutationSkipResponse    mutationResponse = "skip"
-	mutationUnknownResponse mutationResponse = "unknown"
-	mutationAllowResponse   mutationResponse = "allow"
-	mutationErrorResponse   mutationResponse = "error"
+	mutationErrorResponse   requestResponse = "error"
+	mutationSuccessResponse requestResponse = "success"
+	mutationSkipResponse    requestResponse = "skip"
+	mutationUnknownResponse requestResponse = "unknown"
 )
 
 func init() {
@@ -121,8 +119,8 @@ func (h *mutationHandler) Handle(ctx context.Context, req admission.Request) adm
 	requestResponse := mutationUnknownResponse
 	defer func() {
 		if h.reporter != nil {
-			if err := h.reporter.ReportMutationRequest(
-				requestResponse, time.Since(timeStart)); err != nil {
+			if err := h.reporter.ReportRequest(
+				requestResponse, mutationStatusKey, mutationResponseTimeInSecM.M(time.Since(timeStart).Seconds())); err != nil {
 				log.Error(err, "failed to report request")
 			}
 		}
@@ -139,7 +137,7 @@ func (h *mutationHandler) Handle(ctx context.Context, req admission.Request) adm
 		requestResponse = mutationErrorResponse
 		return admission.Errored(int32(http.StatusInternalServerError), err)
 	}
-	requestResponse = mutationAllowResponse
+	requestResponse = mutationSuccessResponse
 	return resp
 }
 

pkg/webhook/policy.go

@@ -96,13 +96,11 @@ type validationHandler struct {
 	opa *opa.Client
 }
 
-type admissionReqRes string
-
 const (
-	errorResponse   admissionReqRes = "error"
-	denyResponse    admissionReqRes = "deny"
-	allowResponse   admissionReqRes = "allow"
-	unknownResponse admissionReqRes = "unknown"
+	errorResponse   requestResponse = "error"
+	denyResponse    requestResponse = "deny"
+	allowResponse   requestResponse = "allow"
+	unknownResponse requestResponse = "unknown"
 )
 
 // Handle the validation request
@@ -148,8 +146,12 @@ func (h *validationHandler) Handle(ctx context.Context, req admission.Request) a
 	requestResponse := unknownResponse
 	defer func() {
 		if h.reporter != nil {
-			if err := h.reporter.ReportAdmissionRequest(
-				requestResponse, time.Since(timeStart)); err != nil {
+			if err := h.reporter.ReportRequest(
+				requestResponse, admissionStatusKey, responseTimeInSecM.M(time.Since(timeStart).Seconds())); err != nil {
+				log.Error(err, "failed to report request")
+			}
+			if err := h.reporter.ReportRequest(
+				requestResponse, admissionStatusKey, validationResponseTimeInSecM.M(time.Since(timeStart).Seconds())); err != nil {
 				log.Error(err, "failed to report request")
 			}
 		}

pkg/webhook/stats_reporter.go

@@ -2,7 +2,6 @@ package webhook
 
 import (
 	"context"
-	"time"
 
 	"github.com/open-policy-agent/gatekeeper/pkg/metrics"
 	"go.opencensus.io/stats"
@@ -14,6 +13,9 @@ const (
 	requestCountMetricName    = "request_count"
 	requestDurationMetricName = "request_duration_seconds"
 
+	validationRequestCountMetricName    = "validation_request_count"
+	validationRequestDurationMetricName = "validation_request_duration_seconds"
+
 	mutationRequestCountMetricName    = "mutation_request_count"
 	mutationRequestDurationMetricName = "mutation_request_duration_seconds"
 )
@@ -24,6 +26,11 @@ var (
 		"The response time in seconds",
 		stats.UnitSeconds)
 
+	validationResponseTimeInSecM = stats.Float64(
+		validationRequestDurationMetricName,
+		"The response time in seconds",
+		stats.UnitSeconds)
+
 	mutationResponseTimeInSecM = stats.Float64(
 		mutationRequestDurationMetricName,
 		"The response time in seconds",
@@ -41,8 +48,7 @@ func init() {
 
 // StatsReporter reports webhook metrics
 type StatsReporter interface {
-	ReportAdmissionRequest(response admissionReqRes, d time.Duration) error
-	ReportMutationRequest(response mutationResponse, d time.Duration) error
+	ReportRequest(response requestResponse, statusKey tag.Key, m stats.Measurement) error
 }
 
 // reporter implements StatsReporter interface
@@ -63,29 +69,17 @@ func newStatsReporter() (StatsReporter, error) {
 }
 
 // Captures req count metric, recording the count and the duration
-func (r *reporter) ReportAdmissionRequest(response admissionReqRes, d time.Duration) error {
-	ctx, err := tag.New(
-		r.ctx,
-		tag.Insert(admissionStatusKey, string(response)),
-	)
-	if err != nil {
-		return err
-	}
-
-	return r.report(ctx, responseTimeInSecM.M(d.Seconds()))
-}
-
-// Captures req count metric, recording the count and the duration
-func (r *reporter) ReportMutationRequest(response mutationResponse, d time.Duration) error {
+func (r *reporter) ReportRequest(response requestResponse, statusKey tag.Key, m stats.Measurement) error {
+	//mutationResponseTimeInSecM.M(d.Seconds())
 	ctx, err := tag.New(
 		r.ctx,
-		tag.Insert(mutationStatusKey, string(response)),
+		tag.Insert(statusKey, string(response)),
 	)
 	if err != nil {
 		return err
 	}
 
-	return r.report(ctx, mutationResponseTimeInSecM.M(d.Seconds()))
+	return r.report(ctx, m)
 }
 
 func (r *reporter) report(ctx context.Context, m stats.Measurement) error {
@@ -96,7 +90,7 @@ func register() error {
 	views := []*view.View{
 		{
 			Name:        requestCountMetricName,
-			Description: "The number of requests that are routed to webhook",
+			Description: "The number of requests that are routed to validation webhook",
 			Measure:     responseTimeInSecM,
 			Aggregation: view.Count(),
 			TagKeys:     []tag.Key{admissionStatusKey},
@@ -108,6 +102,20 @@ func register() error {
 			Aggregation: view.Distribution(0.001, 0.002, 0.003, 0.004, 0.005, 0.006, 0.007, 0.008, 0.009, 0.01, 0.02, 0.03, 0.04, 0.05),
 			TagKeys:     []tag.Key{admissionStatusKey},
 		},
+		{
+			Name:        validationRequestCountMetricName,
+			Description: "The number of requests that are routed to validation webhook",
+			Measure:     validationResponseTimeInSecM,
+			Aggregation: view.Count(),
+			TagKeys:     []tag.Key{admissionStatusKey},
+		},
+		{
+			Name:        validationRequestDurationMetricName,
+			Description: validationResponseTimeInSecM.Description(),
+			Measure:     validationResponseTimeInSecM,
+			Aggregation: view.Distribution(0.001, 0.002, 0.003, 0.004, 0.005, 0.006, 0.007, 0.008, 0.009, 0.01, 0.02, 0.03, 0.04, 0.05),
+			TagKeys:     []tag.Key{admissionStatusKey},
+		},
 		{
 			Name:        mutationRequestCountMetricName,
 			Description: "The number of requests that are routed to mutation webhook",
@@ -119,6 +127,7 @@ func register() error {
 			Name:        mutationRequestDurationMetricName,
 			Description: mutationResponseTimeInSecM.Description(),
 			Measure:     mutationResponseTimeInSecM,
+			// TODO: Adjust the distribution once we know what value make sense here
 			Aggregation: view.Distribution(0.001, 0.002, 0.003, 0.004, 0.005, 0.006, 0.007, 0.008, 0.009, 0.01, 0.02, 0.03, 0.04, 0.05),
 			TagKeys:     []tag.Key{mutationStatusKey},
 		},

pkg/webhook/stats_reporter_test.go

@@ -23,17 +23,37 @@ func TestReportRequest(t *testing.T) {
 	if err != nil {
 		t.Errorf("newStatsReporter() error %v", err)
 	}
-	err = r.ReportAdmissionRequest(allowResponse, expectedDurationValueMin)
+	err = r.ReportRequest(allowResponse, admissionStatusKey, responseTimeInSecM.M(expectedDurationValueMin.Seconds()))
 	if err != nil {
 		t.Errorf("ReportRequest error %v", err)
 	}
-	err = r.ReportAdmissionRequest(allowResponse, expectedDurationValueMax)
+	err = r.ReportRequest(allowResponse, admissionStatusKey, responseTimeInSecM.M(expectedDurationValueMax.Seconds()))
 	if err != nil {
 		t.Errorf("ReportRequest error %v", err)
 	}
 	check(t, expectedTags, requestCountMetricName, requestDurationMetricName)
 }
 
+func TestValidationReportRequest(t *testing.T) {
+	expectedTags := map[string]string{
+		"admission_status": "allow",
+	}
+
+	r, err := newStatsReporter()
+	if err != nil {
+		t.Errorf("newStatsReporter() error %v", err)
+	}
+	err = r.ReportRequest(allowResponse, admissionStatusKey, validationResponseTimeInSecM.M(expectedDurationValueMin.Seconds()))
+	if err != nil {
+		t.Errorf("ReportRequest error %v", err)
+	}
+	err = r.ReportRequest(allowResponse, admissionStatusKey, validationResponseTimeInSecM.M(expectedDurationValueMax.Seconds()))
+	if err != nil {
+		t.Errorf("ReportRequest error %v", err)
+	}
+	check(t, expectedTags, validationRequestCountMetricName, validationRequestDurationMetricName)
+}
+
 func TestMutationReportRequest(t *testing.T) {
 	expectedTags := map[string]string{
 		"mutation_status": "allow",
@@ -44,11 +64,11 @@ func TestMutationReportRequest(t *testing.T) {
 	if err != nil {
 		t.Errorf("newStatsReporter() error %v", err)
 	}
-	err = r.ReportMutationRequest(mutationAllowResponse, expectedDurationValueMin)
+	err = r.ReportRequest(mutationSuccessResponse, mutationStatusKey, mutationResponseTimeInSecM.M(expectedDurationValueMin.Seconds()))
 	if err != nil {
 		t.Errorf("ReportRequest error %v", err)
 	}
-	err = r.ReportMutationRequest(mutationAllowResponse, expectedDurationValueMax)
+	err = r.ReportRequest(mutationSuccessResponse, mutationStatusKey, mutationResponseTimeInSecM.M(expectedDurationValueMax.Seconds()))
 	if err != nil {
 		t.Errorf("ReportRequest error %v", err)
 	}