Update: report stats for request (#2584)

* report stats for request Signed-off-by: Kenny Leung <kleung@chainguard.dev> * fix ref Signed-off-by: Kenny Leung <kleung@chainguard.dev> * fix import Signed-off-by: Kenny Leung <kleung@chainguard.dev> * update Signed-off-by: Kenny Leung <kleung@chainguard.dev> * gofmt Signed-off-by: Kenny Leung <kleung@chainguard.dev> * fix test Signed-off-by: Kenny Leung <kleung@chainguard.dev> * fix boiler Signed-off-by: Kenny Leung <kleung@chainguard.dev> Signed-off-by: Kenny Leung <kleung@chainguard.dev>
knative · Aug 26, 2022 · 93b66e6 · 93b66e6
1 parent a650186
commit 93b66e6
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 13 deletions.
diff --git a/webhook/admission.go b/webhook/admission.go
@@ -158,7 +158,7 @@ func admissionHandler(rootLogger *zap.SugaredLogger, stats StatsReporter, c Admi
 
 		if stats != nil {
 			// Only report valid requests
-			stats.ReportRequest(review.Request, response.Response, time.Since(ttStart))
+			stats.ReportAdmissionRequest(review.Request, response.Response, time.Since(ttStart))
 		}
 	}
 }

diff --git a/webhook/conversion.go b/webhook/conversion.go
@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"time"
 
 	"go.uber.org/zap"
 	apixv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -37,8 +38,9 @@ type ConversionController interface {
 	Convert(context.Context, *apixv1.ConversionRequest) *apixv1.ConversionResponse
 }
 
-func conversionHandler(rootLogger *zap.SugaredLogger, _ StatsReporter, c ConversionController) http.HandlerFunc {
+func conversionHandler(rootLogger *zap.SugaredLogger, stats StatsReporter, c ConversionController) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
+		var ttStart = time.Now()
 		logger := rootLogger
 		logger.Infof("Webhook ServeHTTP request=%#v", r)
 
@@ -69,10 +71,9 @@ func conversionHandler(rootLogger *zap.SugaredLogger, _ StatsReporter, c Convers
 			return
 		}
 
-		// TODO(dprotaso) - figure out what metrics we want reported
-		// if stats != nil {
-		// 	// Only report valid requests
-		// 	stats.ReportRequest(review.Request, response.Response, time.Since(ttStart))
-		// }
+		if stats != nil {
+			// Only report valid requests
+			stats.ReportConversionRequest(review.Request, response.Response, time.Since(ttStart))
+		}
 	}
 }
diff --git a/webhook/stats_reporter.go b/webhook/stats_reporter.go
@@ -25,6 +25,7 @@ import (
 	"go.opencensus.io/stats/view"
 	"go.opencensus.io/tag"
 	admissionv1 "k8s.io/api/admission/v1"
+	apixv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"knative.dev/pkg/metrics"
 )
 
@@ -57,11 +58,17 @@ var (
 	resourceResourceKey  = tag.MustNewKey("resource_resource")
 	resourceNamespaceKey = tag.MustNewKey("resource_namespace")
 	admissionAllowedKey  = tag.MustNewKey("admission_allowed")
+
+	desiredAPIVersionKey = tag.MustNewKey("desired_api_version")
+	resultStatusKey      = tag.MustNewKey("result_status")
+	resultReasonKey      = tag.MustNewKey("result_reason")
+	resultCodeKey        = tag.MustNewKey("result_code")
 )
 
 // StatsReporter reports webhook metrics
 type StatsReporter interface {
-	ReportRequest(request *admissionv1.AdmissionRequest, response *admissionv1.AdmissionResponse, d time.Duration) error
+	ReportAdmissionRequest(request *admissionv1.AdmissionRequest, response *admissionv1.AdmissionResponse, d time.Duration) error
+	ReportConversionRequest(request *apixv1.ConversionRequest, response *apixv1.ConversionResponse, d time.Duration) error
 }
 
 // reporter implements StatsReporter interface
@@ -82,7 +89,7 @@ func NewStatsReporter() (StatsReporter, error) {
 }
 
 // Captures req count metric, recording the count and the duration
-func (r *reporter) ReportRequest(req *admissionv1.AdmissionRequest, resp *admissionv1.AdmissionResponse, d time.Duration) error {
+func (r *reporter) ReportAdmissionRequest(req *admissionv1.AdmissionRequest, resp *admissionv1.AdmissionResponse, d time.Duration) error {
 	ctx, err := tag.New(
 		r.ctx,
 		tag.Insert(requestOperationKey, string(req.Operation)),
@@ -105,6 +112,25 @@ func (r *reporter) ReportRequest(req *admissionv1.AdmissionRequest, resp *admiss
 	return nil
 }
 
+// Captures req count metric, recording the count and the duration
+func (r *reporter) ReportConversionRequest(req *apixv1.ConversionRequest, resp *apixv1.ConversionResponse, d time.Duration) error {
+	ctx, err := tag.New(
+		r.ctx,
+		tag.Insert(desiredAPIVersionKey, req.DesiredAPIVersion),
+		tag.Insert(resultStatusKey, resp.Result.Status),
+		tag.Insert(resultReasonKey, string(resp.Result.Reason)),
+		tag.Insert(resultCodeKey, strconv.Itoa(int(resp.Result.Code))),
+	)
+	if err != nil {
+		return err
+	}
+
+	metrics.RecordBatch(ctx, requestCountM.M(1),
+		// Convert time.Duration in nanoseconds to milliseconds
+		responseTimeInMsecM.M(float64(d.Milliseconds())))
+	return nil
+}
+
 func RegisterMetrics() {
 	tagKeys := []tag.Key{
 		requestOperationKey,
@@ -115,7 +141,11 @@ func RegisterMetrics() {
 		resourceVersionKey,
 		resourceResourceKey,
 		resourceNamespaceKey,
-		admissionAllowedKey}
+		admissionAllowedKey,
+		desiredAPIVersionKey,
+		resultStatusKey,
+		resultReasonKey,
+		resultCodeKey}
 
 	if err := view.Register(
 		&view.View{

diff --git a/webhook/stats_reporter_test.go b/webhook/stats_reporter_test.go
@@ -22,12 +22,13 @@ import (
 	"time"
 
 	admissionv1 "k8s.io/api/admission/v1"
+	apixv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/pkg/metrics/metricstest"
 	_ "knative.dev/pkg/metrics/testing"
 )
 
-func TestWebhookStatsReporter(t *testing.T) {
+func TestWebhookStatsReporterAdmission(t *testing.T) {
 	setup()
 	req := &admissionv1.AdmissionRequest{
 		UID:       "705ab4f5-6393-11e8-b7cc-42010a800002",
@@ -58,8 +59,45 @@ func TestWebhookStatsReporter(t *testing.T) {
 		admissionAllowedKey.Name():  strconv.FormatBool(resp.Allowed),
 	}
 
-	r.ReportRequest(req, resp, time.Duration(shortTime)*time.Millisecond)
-	r.ReportRequest(req, resp, time.Duration(longTime)*time.Millisecond)
+	if err := r.ReportAdmissionRequest(req, resp, time.Duration(shortTime)*time.Millisecond); err != nil {
+		t.Fatalf("ReportAdmissionRequest() = %v", err)
+	}
+	if err := r.ReportAdmissionRequest(req, resp, time.Duration(longTime)*time.Millisecond); err != nil {
+		t.Fatalf("ReportAdmissionRequest() = %v", err)
+	}
+
+	metricstest.CheckCountData(t, requestCountName, expectedTags, 2)
+	metricstest.CheckDistributionData(t, requestLatenciesName, expectedTags, 2, shortTime, longTime)
+}
+
+func TestWebhookStatsReporterConversion(t *testing.T) {
+	setup()
+	req := &apixv1.ConversionRequest{
+		UID:               "705ab4f5-6393-11e8-b7cc-42010a800003",
+		DesiredAPIVersion: "knative.dev/v1",
+	}
+
+	resp := &apixv1.ConversionResponse{
+		UID:    req.UID,
+		Result: metav1.Status{Status: "Failure", Reason: metav1.StatusReasonNotFound, Code: 404},
+	}
+
+	r, _ := NewStatsReporter()
+
+	shortTime, longTime := 1100.0, 9100.0
+	expectedTags := map[string]string{
+		desiredAPIVersionKey.Name(): req.DesiredAPIVersion,
+		resultStatusKey.Name():      resp.Result.Status,
+		resultReasonKey.Name():      string(resp.Result.Reason),
+		resultCodeKey.Name():        strconv.Itoa(int(resp.Result.Code)),
+	}
+
+	if err := r.ReportConversionRequest(req, resp, time.Duration(shortTime)*time.Millisecond); err != nil {
+		t.Fatalf("ReportConversionRequest() = %v", err)
+	}
+	if err := r.ReportConversionRequest(req, resp, time.Duration(longTime)*time.Millisecond); err != nil {
+		t.Fatalf("ReportConversionRequest() = %v", err)
+	}
 
 	metricstest.CheckCountData(t, requestCountName, expectedTags, 2)
 	metricstest.CheckDistributionData(t, requestLatenciesName, expectedTags, 2, shortTime, longTime)