Merge pull request #107975 from tkashem/refactor-webhook-duration

refactor: rename webhook duration tracker

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

@@ -265,9 +265,9 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *admiss
 	}
 
 	do := func() { err = r.Do(ctx).Into(response) }
-	if wd, ok := endpointsrequest.WebhookDurationFrom(ctx); ok {
+	if wd, ok := endpointsrequest.LatencyTrackersFrom(ctx); ok {
 		tmp := do
-		do = func() { wd.AdmitTracker.Track(tmp) }
+		do = func() { wd.MutatingWebhookTracker.Track(tmp) }
 	}
 	do()
 	if err != nil {

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

@@ -274,7 +274,7 @@ func TestWebhookDuration(ts *testing.T) {
 		ts.Run(test.Name, func(t *testing.T) {
 			ctx := context.TODO()
 			if test.InitContext {
-				ctx = request.WithWebhookDurationAndCustomClock(ctx, &clk)
+				ctx = request.WithLatencyTrackersAndCustomClock(ctx, &clk)
 			}
 			wh, err := NewMutatingWebhook(nil)
 			if err != nil {
@@ -299,7 +299,7 @@ func TestWebhookDuration(ts *testing.T) {
 			}
 
 			_ = wh.Admit(ctx, webhooktesting.NewAttribute(ns, nil, test.IsDryRun), objectInterfaces)
-			wd, ok := request.WebhookDurationFrom(ctx)
+			wd, ok := request.LatencyTrackersFrom(ctx)
 			if !ok {
 				if test.InitContext {
 					t.Errorf("expected webhook duration to be initialized")
@@ -310,11 +310,11 @@ func TestWebhookDuration(ts *testing.T) {
 				t.Errorf("expected webhook duration to not be initialized")
 				return
 			}
-			if wd.AdmitTracker.GetLatency() != test.ExpectedDurationSum {
+			if wd.MutatingWebhookTracker.GetLatency() != test.ExpectedDurationSum {
-				t.Errorf("expected admit duration %q got %q", test.ExpectedDurationSum, wd.AdmitTracker.GetLatency())
+				t.Errorf("expected admit duration %q got %q", test.ExpectedDurationSum, wd.MutatingWebhookTracker.GetLatency())
 			}
-			if wd.ValidateTracker.GetLatency() != 0 {
+			if wd.ValidatingWebhookTracker.GetLatency() != 0 {
-				t.Errorf("expected validate duraion to be equal to 0 got %q", wd.ValidateTracker.GetLatency())
+				t.Errorf("expected validate duraion to be equal to 0 got %q", wd.ValidatingWebhookTracker.GetLatency())
 			}
 		})
 	}

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

@@ -232,9 +232,9 @@ func (d *validatingDispatcher) callHook(ctx context.Context, h *v1.ValidatingWeb
 	}
 
 	do := func() { err = r.Do(ctx).Into(response) }
-	if wd, ok := endpointsrequest.WebhookDurationFrom(ctx); ok {
+	if wd, ok := endpointsrequest.LatencyTrackersFrom(ctx); ok {
 		tmp := do
-		do = func() { wd.ValidateTracker.Track(tmp) }
+		do = func() { wd.ValidatingWebhookTracker.Track(tmp) }
 	}
 	do()
 	if err != nil {

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

@@ -236,7 +236,7 @@ func TestValidateWebhookDuration(ts *testing.T) {
 		ts.Run(test.Name, func(t *testing.T) {
 			ctx := context.TODO()
 			if test.InitContext {
-				ctx = request.WithWebhookDurationAndCustomClock(ctx, &clk)
+				ctx = request.WithLatencyTrackersAndCustomClock(ctx, &clk)
 			}
 			wh, err := NewValidatingAdmissionWebhook(nil)
 			if err != nil {
@@ -261,7 +261,7 @@ func TestValidateWebhookDuration(ts *testing.T) {
 			}
 
 			_ = wh.Validate(ctx, webhooktesting.NewAttribute(ns, nil, test.IsDryRun), objectInterfaces)
-			wd, ok := request.WebhookDurationFrom(ctx)
+			wd, ok := request.LatencyTrackersFrom(ctx)
 			if !ok {
 				if test.InitContext {
 					t.Errorf("expected webhook duration to be initialized")
@@ -272,11 +272,11 @@ func TestValidateWebhookDuration(ts *testing.T) {
 				t.Errorf("expected webhook duration to not be initialized")
 				return
 			}
-			if wd.AdmitTracker.GetLatency() != 0 {
+			if wd.MutatingWebhookTracker.GetLatency() != 0 {
-				t.Errorf("expected admit duration to be equal to 0 got %q", wd.AdmitTracker.GetLatency())
+				t.Errorf("expected admit duration to be equal to 0 got %q", wd.MutatingWebhookTracker.GetLatency())
 			}
-			if wd.ValidateTracker.GetLatency() < test.ExpectedDurationMax {
+			if wd.ValidatingWebhookTracker.GetLatency() < test.ExpectedDurationMax {
-				t.Errorf("expected validate duraion to be greater or equal to %q got %q", test.ExpectedDurationMax, wd.ValidateTracker.GetLatency())
+				t.Errorf("expected validate duraion to be greater or equal to %q got %q", test.ExpectedDurationMax, wd.ValidatingWebhookTracker.GetLatency())
 			}
 		})
 	}

staging/src/k8s.io/apiserver/pkg/endpoints/filters/webhook_duration.go

@@ -22,12 +22,13 @@ import (
 	"k8s.io/apiserver/pkg/endpoints/request"
 )
 
-// WithWebhookDuration adds WebhookDuration trackers to the
+// WithLatencyTrackers adds a LatencyTrackers instance to the
-// context associated with a request.
+// context associated with a request so that we can measure latency
-func WithWebhookDuration(handler http.Handler) http.Handler {
+// incurred in various components within the apiserver.
+func WithLatencyTrackers(handler http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		ctx := req.Context()
-		req = req.WithContext(request.WithWebhookDuration(ctx))
+		req = req.WithContext(request.WithLatencyTrackers(ctx))
 		handler.ServeHTTP(w, req)
 	})
 }

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

@@ -482,8 +482,8 @@ func MonitorRequest(req *http.Request, verb, group, version, resource, subresour
 		}
 	}
 	requestLatencies.WithContext(req.Context()).WithLabelValues(reportedVerb, dryRun, group, version, resource, subresource, scope, component).Observe(elapsedSeconds)
-	if wd, ok := request.WebhookDurationFrom(req.Context()); ok {
+	if wd, ok := request.LatencyTrackersFrom(req.Context()); ok {
-		sloLatency := elapsedSeconds - (wd.AdmitTracker.GetLatency() + wd.ValidateTracker.GetLatency()).Seconds()
+		sloLatency := elapsedSeconds - (wd.MutatingWebhookTracker.GetLatency() + wd.ValidatingWebhookTracker.GetLatency()).Seconds()
 		requestSloLatencies.WithContext(req.Context()).WithLabelValues(reportedVerb, group, version, resource, subresource, scope, component).Observe(sloLatency)
 	}
 	// We are only interested in response sizes of read requests.

staging/src/k8s.io/apiserver/pkg/endpoints/request/webhook_duration.go

@@ -85,38 +85,43 @@ func newMaxLatencyTracker(c clock.Clock) DurationTracker {
 	}
 }
 
-// WebhookDuration stores trackers used to measure webhook request durations.
+// LatencyTrackers stores trackers used to measure latecny incurred in
-// Since admit webhooks are done sequentially duration is aggregated using
+// components within the apiserver.
-// sum function. Validate webhooks are done in parallel so max function
+type LatencyTrackers struct {
-// is used.
+	// MutatingWebhookTracker tracks the latency incurred in mutating webhook(s).
-type WebhookDuration struct {
+	// Since mutating webhooks are done sequentially, latency
-	AdmitTracker    DurationTracker
+	// is aggregated using sum function.
-	ValidateTracker DurationTracker
+	MutatingWebhookTracker DurationTracker
+
+	// ValidatingWebhookTracker tracks the latency incurred in validating webhook(s).
+	// Validate webhooks are done in parallel, so max function is used.
+	ValidatingWebhookTracker DurationTracker
 }
 
-type webhookDurationKeyType int
+type latencyTrackersKeyType int
 
-// webhookDurationKey is the WebhookDuration (the time the request spent waiting
+// latencyTrackersKey is the key that associates a LatencyTrackers
-// for the webhooks to finish) key for the context.
+// instance with the request context.
-const webhookDurationKey webhookDurationKeyType = iota
+const latencyTrackersKey latencyTrackersKeyType = iota
 
-// WithWebhookDuration returns a copy of parent context to which the
+// WithLatencyTrackers returns a copy of parent context to which an
-// WebhookDuration trackers are added.
+// instance of LatencyTrackers is added.
-func WithWebhookDuration(parent context.Context) context.Context {
+func WithLatencyTrackers(parent context.Context) context.Context {
-	return WithWebhookDurationAndCustomClock(parent, clock.RealClock{})
+	return WithLatencyTrackersAndCustomClock(parent, clock.RealClock{})
 }
 
-// WithWebhookDurationAndCustomClock returns a copy of parent context to which
+// WithLatencyTrackersAndCustomClock returns a copy of parent context to which
-// the WebhookDuration trackers are added. Tracers use given clock.
+// an instance of LatencyTrackers is added. Tracers use given clock.
-func WithWebhookDurationAndCustomClock(parent context.Context, c clock.Clock) context.Context {
+func WithLatencyTrackersAndCustomClock(parent context.Context, c clock.Clock) context.Context {
-	return WithValue(parent, webhookDurationKey, &WebhookDuration{
+	return WithValue(parent, latencyTrackersKey, &LatencyTrackers{
-		AdmitTracker:    newSumLatencyTracker(c),
+		MutatingWebhookTracker:   newSumLatencyTracker(c),
-		ValidateTracker: newMaxLatencyTracker(c),
+		ValidatingWebhookTracker: newMaxLatencyTracker(c),
 	})
 }
 
-// WebhookDurationFrom returns the value of the WebhookDuration key from the specified context.
+// LatencyTrackersFrom returns the associated LatencyTrackers instance
-func WebhookDurationFrom(ctx context.Context) (*WebhookDuration, bool) {
+// from the specified context.
-	wd, ok := ctx.Value(webhookDurationKey).(*WebhookDuration)
+func LatencyTrackersFrom(ctx context.Context) (*LatencyTrackers, bool) {
+	wd, ok := ctx.Value(latencyTrackersKey).(*LatencyTrackers)
 	return wd, ok && wd != nil
 }

staging/src/k8s.io/apiserver/pkg/endpoints/request/webhook_duration_test.go

@@ -24,7 +24,7 @@ import (
 	clocktesting "k8s.io/utils/clock/testing"
 )
 
-func TestWebhookDurationFrom(t *testing.T) {
+func TestLatencyTrackersFrom(t *testing.T) {
 	type testCase struct {
 		Durations    []time.Duration
 		SumDurations time.Duration
@@ -37,37 +37,37 @@ func TestWebhookDurationFrom(t *testing.T) {
 	}
 	t.Run("TestWebhookDurationFrom", func(t *testing.T) {
 		parent := context.TODO()
-		_, ok := WebhookDurationFrom(parent)
+		_, ok := LatencyTrackersFrom(parent)
 		if ok {
-			t.Error("expected WebhookDurationFrom to not be initialized")
+			t.Error("expected LatencyTrackersFrom to not be initialized")
 		}
 
 		clk := clocktesting.FakeClock{}
-		ctx := WithWebhookDurationAndCustomClock(parent, &clk)
+		ctx := WithLatencyTrackersAndCustomClock(parent, &clk)
-		wd, ok := WebhookDurationFrom(ctx)
+		wd, ok := LatencyTrackersFrom(ctx)
 		if !ok {
-			t.Error("expected webhook duration to be initialized")
+			t.Error("expected LatencyTrackersFrom to be initialized")
 		}
-		if wd.AdmitTracker.GetLatency() != 0 || wd.ValidateTracker.GetLatency() != 0 {
+		if wd.MutatingWebhookTracker.GetLatency() != 0 || wd.ValidatingWebhookTracker.GetLatency() != 0 {
 			t.Error("expected values to be initialized to 0")
 		}
 
 		for _, d := range tc.Durations {
-			wd.AdmitTracker.Track(func() { clk.Step(d) })
+			wd.MutatingWebhookTracker.Track(func() { clk.Step(d) })
-			wd.ValidateTracker.Track(func() { clk.Step(d) })
+			wd.ValidatingWebhookTracker.Track(func() { clk.Step(d) })
 		}
 
-		wd, ok = WebhookDurationFrom(ctx)
+		wd, ok = LatencyTrackersFrom(ctx)
 		if !ok {
 			t.Errorf("expected webhook duration to be initialized")
 		}
 
-		if wd.AdmitTracker.GetLatency() != tc.SumDurations {
+		if wd.MutatingWebhookTracker.GetLatency() != tc.SumDurations {
-			t.Errorf("expected admit duration: %q, but got: %q", tc.SumDurations, wd.AdmitTracker.GetLatency())
+			t.Errorf("expected admit duration: %q, but got: %q", tc.SumDurations, wd.MutatingWebhookTracker.GetLatency())
 		}
 
-		if wd.ValidateTracker.GetLatency() != tc.MaxDuration {
+		if wd.ValidatingWebhookTracker.GetLatency() != tc.MaxDuration {
-			t.Errorf("expected validate duration: %q, but got: %q", tc.MaxDuration, wd.ValidateTracker.GetLatency())
+			t.Errorf("expected validate duration: %q, but got: %q", tc.MaxDuration, wd.ValidatingWebhookTracker.GetLatency())
 		}
 	})
 }

staging/src/k8s.io/apiserver/pkg/server/config.go

@@ -762,7 +762,7 @@ func BuildHandlerChainWithStorageVersionPrecondition(apiHandler http.Handler, c
 }
 
 func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) http.Handler {
-	handler := genericapifilters.WithWebhookDuration(apiHandler)
+	handler := genericapifilters.WithLatencyTrackers(apiHandler)
 	handler = filterlatency.TrackCompleted(handler)
 	handler = genericapifilters.WithAuthorization(handler, c.Authorization.Authorizer, c.Serializer)
 	handler = filterlatency.TrackStarted(handler, "authorization")