admission: make metrics compositional and move to metrics sub-package

plugin/pkg/admission/namespace/autoprovision/admission_test.go

@@ -136,7 +136,7 @@ func TestIgnoreAdmission(t *testing.T) {
 		t.Errorf("unexpected error initializing handler: %v", err)
 	}
 	informerFactory.Start(wait.NeverStop)
-	chainHandler := admission.NewChainHandler(admission.NewNamedHandler("ns", handler))
+	chainHandler := admission.NewChainHandler(handler)
 
 	pod := newPod(namespace)
 	err = chainHandler.Admit(admission.NewAttributesRecord(&pod, nil, api.Kind("Pod").WithVersion("version"), pod.Namespace, pod.Name, api.Resource("pods").WithVersion("version"), "", admission.Update, nil))

plugin/pkg/admission/persistentvolume/label/admission_test.go

@@ -78,7 +78,7 @@ func mockVolumeLabels(labels map[string]string) *mockVolumes {
 // TestAdmission
 func TestAdmission(t *testing.T) {
 	pvHandler := NewPersistentVolumeLabel()
-	handler := admission.NewChainHandler(admission.NewNamedHandler("pv", pvHandler))
+	handler := admission.NewChainHandler(pvHandler)
 	ignoredPV := api.PersistentVolume{
 		ObjectMeta: metav1.ObjectMeta{Name: "noncloud", Namespace: "myns"},
 		Spec: api.PersistentVolumeSpec{

staging/src/k8s.io/apiextensions-apiserver/Godeps/Godeps.json

@@ -866,6 +866,10 @@
 			"ImportPath": "k8s.io/apiserver/pkg/admission/initializer",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
 		},
+		{
+			"ImportPath": "k8s.io/apiserver/pkg/admission/metrics",
+			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+		},
 		{
 			"ImportPath": "k8s.io/apiserver/pkg/admission/plugin/initialization",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"

staging/src/k8s.io/apiserver/pkg/admission/BUILD

@@ -13,15 +13,10 @@ go_test(
         "config_test.go",
         "errors_test.go",
         "handler_test.go",
-        "metrics_test.go",
-        "testutil_test.go",
     ],
     importpath = "k8s.io/apiserver/pkg/admission",
     library = ":go_default_library",
     deps = [
-        "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
-        "//vendor/github.com/prometheus/client_model/go:go_default_library",
-        "//vendor/k8s.io/api/admissionregistration/v1alpha1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
@@ -38,15 +33,12 @@ go_library(
         "errors.go",
         "handler.go",
         "interfaces.go",
-        "metrics.go",
         "plugins.go",
     ],
     importpath = "k8s.io/apiserver/pkg/admission",
     deps = [
         "//vendor/github.com/ghodss/yaml:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
-        "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
-        "//vendor/k8s.io/api/admissionregistration/v1alpha1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apimachinery/announced:go_default_library",
@@ -76,6 +68,7 @@ filegroup(
         ":package-srcs",
         "//staging/src/k8s.io/apiserver/pkg/admission/configuration:all-srcs",
         "//staging/src/k8s.io/apiserver/pkg/admission/initializer:all-srcs",
+        "//staging/src/k8s.io/apiserver/pkg/admission/metrics:all-srcs",
         "//staging/src/k8s.io/apiserver/pkg/admission/plugin/initialization:all-srcs",
         "//staging/src/k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle:all-srcs",
         "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/config:all-srcs",

staging/src/k8s.io/apiserver/pkg/admission/chain.go

@@ -16,46 +16,23 @@ limitations under the License.
 
 package admission
 
-import "time"
-
 // chainAdmissionHandler is an instance of admission.NamedHandler that performs admission control using
 // a chain of admission handlers
-type chainAdmissionHandler []NamedHandler
+type chainAdmissionHandler []Interface
 
 // NewChainHandler creates a new chain handler from an array of handlers. Used for testing.
-func NewChainHandler(handlers ...NamedHandler) chainAdmissionHandler {
+func NewChainHandler(handlers ...Interface) chainAdmissionHandler {
 	return chainAdmissionHandler(handlers)
 }
 
-func NewNamedHandler(name string, i Interface) NamedHandler {
-	return &pluginHandler{
-		Interface: i,
-		name:      name,
-	}
-}
-
-const (
-	stepValidate = "validate"
-	stepAdmit    = "admit"
-)
-
 // Admit performs an admission control check using a chain of handlers, and returns immediately on first error
 func (admissionHandler chainAdmissionHandler) Admit(a Attributes) error {
-	start := time.Now()
-	err := admissionHandler.admit(a)
-	Metrics.ObserveAdmissionStep(time.Since(start), err != nil, a, stepAdmit)
-	return err
-}
-
-func (admissionHandler chainAdmissionHandler) admit(a Attributes) error {
 	for _, handler := range admissionHandler {
 		if !handler.Handles(a.GetOperation()) {
 			continue
 		}
 		if mutator, ok := handler.(MutationInterface); ok {
-			t := time.Now()
 			err := mutator.Admit(a)
-			Metrics.ObserveAdmissionController(time.Since(t), err != nil, handler, a, stepAdmit)
 			if err != nil {
 				return err
 			}
@@ -66,21 +43,12 @@ func (admissionHandler chainAdmissionHandler) admit(a Attributes) error {
 
 // Validate performs an admission control check using a chain of handlers, and returns immediately on first error
 func (admissionHandler chainAdmissionHandler) Validate(a Attributes) error {
-	start := time.Now()
-	err := admissionHandler.validate(a)
-	Metrics.ObserveAdmissionStep(time.Since(start), err != nil, a, stepValidate)
-	return err
-}
-
-func (admissionHandler chainAdmissionHandler) validate(a Attributes) (err error) {
 	for _, handler := range admissionHandler {
 		if !handler.Handles(a.GetOperation()) {
 			continue
 		}
 		if validator, ok := handler.(ValidationInterface); ok {
-			t := time.Now()
 			err := validator.Validate(a)
-			Metrics.ObserveAdmissionController(time.Since(t), err != nil, handler, a, stepValidate)
 			if err != nil {
 				return err
 			}

staging/src/k8s.io/apiserver/pkg/admission/chain_test.go

@@ -17,12 +17,45 @@ limitations under the License.
 package admission
 
 import (
+	"fmt"
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
+type FakeHandler struct {
+	*Handler
+	name                     string
+	admit, admitCalled       bool
+	validate, validateCalled bool
+}
+
+func (h *FakeHandler) Admit(a Attributes) (err error) {
+	h.admitCalled = true
+	if h.admit {
+		return nil
+	}
+	return fmt.Errorf("Don't admit")
+}
+
+func (h *FakeHandler) Validate(a Attributes) (err error) {
+	h.validateCalled = true
+	if h.validate {
+		return nil
+	}
+	return fmt.Errorf("Don't validate")
+}
+
+func makeHandler(name string, accept bool, ops ...Operation) *FakeHandler {
+	return &FakeHandler{
+		name:     name,
+		admit:    accept,
+		validate: accept,
+		Handler:  NewHandler(ops...),
+	}
+}
+
 func TestAdmitAndValidate(t *testing.T) {
 	sysns := metav1.NamespaceSystem
 	otherns := "default"
@@ -38,10 +71,10 @@ func TestAdmitAndValidate(t *testing.T) {
 			name:      "all accept",
 			ns:        sysns,
 			operation: Create,
-			chain: []NamedHandler{
+			chain: []Interface{
-				makeNamedHandler("a", true, Update, Delete, Create),
+				makeHandler("a", true, Update, Delete, Create),
-				makeNamedHandler("b", true, Delete, Create),
+				makeHandler("b", true, Delete, Create),
-				makeNamedHandler("c", true, Create),
+				makeHandler("c", true, Create),
 			},
 			calls:  map[string]bool{"a": true, "b": true, "c": true},
 			accept: true,
@@ -50,10 +83,10 @@ func TestAdmitAndValidate(t *testing.T) {
 			name:      "ignore handler",
 			ns:        otherns,
 			operation: Create,
-			chain: []NamedHandler{
+			chain: []Interface{
-				makeNamedHandler("a", true, Update, Delete, Create),
+				makeHandler("a", true, Update, Delete, Create),
-				makeNamedHandler("b", false, Delete),
+				makeHandler("b", false, Delete),
-				makeNamedHandler("c", true, Create),
+				makeHandler("c", true, Create),
 			},
 			calls:  map[string]bool{"a": true, "c": true},
 			accept: true,
@@ -62,10 +95,10 @@ func TestAdmitAndValidate(t *testing.T) {
 			name:      "ignore all",
 			ns:        sysns,
 			operation: Connect,
-			chain: []NamedHandler{
+			chain: []Interface{
-				makeNamedHandler("a", true, Update, Delete, Create),
+				makeHandler("a", true, Update, Delete, Create),
-				makeNamedHandler("b", false, Delete),
+				makeHandler("b", false, Delete),
-				makeNamedHandler("c", true, Create),
+				makeHandler("c", true, Create),
 			},
 			calls:  map[string]bool{},
 			accept: true,
@@ -74,17 +107,16 @@ func TestAdmitAndValidate(t *testing.T) {
 			name:      "reject one",
 			ns:        otherns,
 			operation: Delete,
-			chain: []NamedHandler{
+			chain: []Interface{
-				makeNamedHandler("a", true, Update, Delete, Create),
+				makeHandler("a", true, Update, Delete, Create),
-				makeNamedHandler("b", false, Delete),
+				makeHandler("b", false, Delete),
-				makeNamedHandler("c", true, Create),
+				makeHandler("c", true, Create),
 			},
 			calls:  map[string]bool{"a": true, "b": true},
 			accept: false,
 		},
 	}
 	for _, test := range tests {
-		Metrics.reset()
 		t.Logf("testcase = %s", test.name)
 		// call admit and check that validate was not called at all
 		err := test.chain.Admit(NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, test.ns, "", schema.GroupVersionResource{}, "", test.operation, nil))
@@ -94,25 +126,19 @@ func TestAdmitAndValidate(t *testing.T) {
 		}
 		for _, h := range test.chain {
 			fake := h.(*FakeHandler)
-			_, shouldBeCalled := test.calls[h.Name()]
+			_, shouldBeCalled := test.calls[fake.name]
 			if shouldBeCalled != fake.admitCalled {
-				t.Errorf("admit handler %s not called as expected: %v", h.Name(), fake.admitCalled)
+				t.Errorf("admit handler %s not called as expected: %v", fake.name, fake.admitCalled)
 				continue
 			}
 			if fake.validateCalled {
-				t.Errorf("validate handler %s called during admit", h.Name())
+				t.Errorf("validate handler %s called during admit", fake.name)
 			}
 
 			// reset value for validation test
 			fake.admitCalled = false
 		}
 
-		labelFilter := map[string]string{
-			"type": "admit",
-		}
-
-		checkAdmitAndValidateMetrics(t, labelFilter, test.accept, test.calls)
-		Metrics.reset()
 		// call validate and check that admit was not called at all
 		err = test.chain.Validate(NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, test.ns, "", schema.GroupVersionResource{}, "", test.operation, nil))
 		accepted = (err == nil)
@@ -122,63 +148,24 @@ func TestAdmitAndValidate(t *testing.T) {
 		for _, h := range test.chain {
 			fake := h.(*FakeHandler)
 
-			_, shouldBeCalled := test.calls[h.Name()]
+			_, shouldBeCalled := test.calls[fake.name]
 			if shouldBeCalled != fake.validateCalled {
-				t.Errorf("validate handler %s not called as expected: %v", h.Name(), fake.validateCalled)
+				t.Errorf("validate handler %s not called as expected: %v", fake.name, fake.validateCalled)
 				continue
 			}
 
 			if fake.admitCalled {
-				t.Errorf("mutating handler unexpectedly called: %s", h.Name())
+				t.Errorf("mutating handler unexpectedly called: %s", fake.name)
 			}
 		}
-
-		labelFilter = map[string]string{
-			"type": "validate",
-		}
-
-		checkAdmitAndValidateMetrics(t, labelFilter, test.accept, test.calls)
-	}
-}
-
-func checkAdmitAndValidateMetrics(t *testing.T, labelFilter map[string]string, accept bool, calls map[string]bool) {
-	acceptFilter := map[string]string{"rejected": "false"}
-	for k, v := range labelFilter {
-		acceptFilter[k] = v
-	}
-
-	rejectFilter := map[string]string{"rejected": "true"}
-	for k, v := range labelFilter {
-		rejectFilter[k] = v
-	}
-
-	if accept {
-		// Ensure exactly one admission end-to-end admission accept should have been recorded.
-		expectHistogramCountTotal(t, "apiserver_admission_step_admission_latencies_seconds", acceptFilter, 1)
-
-		// Ensure the expected count of admission controllers have been executed.
-		expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", acceptFilter, len(calls))
-	} else {
-		// When not accepted, ensure exactly one end-to-end rejection has been recorded.
-		expectHistogramCountTotal(t, "apiserver_admission_step_admission_latencies_seconds", rejectFilter, 1)
-		if len(calls) > 0 {
-			if len(calls) > 1 {
-				// When not accepted, ensure that all but the last controller had been accepted, since
-				// the chain stops execution at the first rejection.
-				expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", acceptFilter, len(calls)-1)
-			}
-
-			// When not accepted, ensure exactly one controller has been rejected.
-			expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", rejectFilter, 1)
-		}
 	}
 }
 
 func TestHandles(t *testing.T) {
 	chain := chainAdmissionHandler{
-		makeNamedHandler("a", true, Update, Delete, Create),
+		makeHandler("a", true, Update, Delete, Create),
-		makeNamedHandler("b", true, Delete, Create),
+		makeHandler("b", true, Delete, Create),
-		makeNamedHandler("c", true, Create),
+		makeHandler("c", true, Create),
 	}
 
 	tests := []struct {

staging/src/k8s.io/apiserver/pkg/admission/metrics/BUILD

@@ -0,0 +1,42 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["metrics.go"],
+    importpath = "k8s.io/apiserver/pkg/admission/metrics",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
+        "//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
+    ],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = [
+        "metrics_test.go",
+        "testutil_test.go",
+    ],
+    importpath = "k8s.io/apiserver/pkg/admission/metrics",
+    library = ":go_default_library",
+    deps = [
+        "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
+        "//vendor/github.com/prometheus/client_model/go:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
+        "//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)

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

@@ -14,16 +14,16 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package admission
+package metrics
 
 import (
 	"fmt"
 	"strconv"
 	"time"
 
-	"k8s.io/api/admissionregistration/v1alpha1"
-
 	"github.com/prometheus/client_golang/prometheus"
+
+	"k8s.io/apiserver/pkg/admission"
 )
 
 const (
@@ -39,10 +39,64 @@ var (
 	Metrics = newAdmissionMetrics()
 )
 
-// NamedHandler requires each admission.Interface be named, primarly for metrics tracking purposes.
+// ObserverFunc is a func that emits metrics.
-type NamedHandler interface {
+type ObserverFunc func(elapsed time.Duration, rejected bool, attr admission.Attributes, stepType string, extraLabels ...string)
-	Interface
+
-	Name() string
+const (
+	stepValidate = "validate"
+	stepAdmit    = "admit"
+)
+
+// WithControllerMetrics is a decorator for named admission handlers.
+func WithControllerMetrics(i admission.Interface, name string) admission.Interface {
+	return WithMetrics(i, Metrics.ObserveAdmissionController, name)
+}
+
+// WithStepMetrics is a decorator for a whole admission phase, i.e. admit or validation.admission step.
+func WithStepMetrics(i admission.Interface) admission.Interface {
+	return WithMetrics(i, Metrics.ObserveAdmissionStep)
+}
+
+// WithMetrics is a decorator for admission handlers with a generic observer func.
+func WithMetrics(i admission.Interface, observer ObserverFunc, extraLabels ...string) admission.Interface {
+	return &pluginHandlerWithMetrics{
+		Interface:   i,
+		observer:    observer,
+		extraLabels: extraLabels,
+	}
+}
+
+// pluginHandlerWithMetrics decorates a admission handler with metrics.
+type pluginHandlerWithMetrics struct {
+	admission.Interface
+	observer    ObserverFunc
+	extraLabels []string
+}
+
+// Admit performs a mutating admission control check and emit metrics.
+func (p pluginHandlerWithMetrics) Admit(a admission.Attributes) error {
+	mutatingHandler, ok := p.Interface.(admission.MutationInterface)
+	if !ok {
+		return nil
+	}
+
+	start := time.Now()
+	err := mutatingHandler.Admit(a)
+	p.observer(time.Since(start), err != nil, a, stepAdmit, p.extraLabels...)
+	return err
+}
+
+// Validate performs a non-mutating admission control check and emits metrics.
+func (p pluginHandlerWithMetrics) Validate(a admission.Attributes) error {
+	validatingHandler, ok := p.Interface.(admission.ValidationInterface)
+	if !ok {
+		return nil
+	}
+
+	start := time.Now()
+	err := validatingHandler.Validate(a)
+	p.observer(time.Since(start), err != nil, a, stepValidate, p.extraLabels...)
+	return err
 }
 
 // AdmissionMetrics instruments admission with prometheus metrics.
@@ -83,22 +137,21 @@ func (m *AdmissionMetrics) reset() {
 }
 
 // ObserveAdmissionStep records admission related metrics for a admission step, identified by step type.
-func (m *AdmissionMetrics) ObserveAdmissionStep(elapsed time.Duration, rejected bool, attr Attributes, stepType string) {
+func (m *AdmissionMetrics) ObserveAdmissionStep(elapsed time.Duration, rejected bool, attr admission.Attributes, stepType string, extraLabels ...string) {
 	gvr := attr.GetResource()
-	m.step.observe(elapsed, stepType, string(attr.GetOperation()), gvr.Group, gvr.Version, gvr.Resource, attr.GetSubresource(), strconv.FormatBool(rejected))
+	m.step.observe(elapsed, append(extraLabels, stepType, string(attr.GetOperation()), gvr.Group, gvr.Version, gvr.Resource, attr.GetSubresource(), strconv.FormatBool(rejected))...)
 }
 
 // ObserveAdmissionController records admission related metrics for a built-in admission controller, identified by it's plugin handler name.
-func (m *AdmissionMetrics) ObserveAdmissionController(elapsed time.Duration, rejected bool, handler NamedHandler, attr Attributes, stepType string) {
+func (m *AdmissionMetrics) ObserveAdmissionController(elapsed time.Duration, rejected bool, attr admission.Attributes, stepType string, extraLabels ...string) {
 	gvr := attr.GetResource()
-	m.controller.observe(elapsed, handler.Name(), stepType, string(attr.GetOperation()), gvr.Group, gvr.Version, gvr.Resource, attr.GetSubresource(), strconv.FormatBool(rejected))
+	m.controller.observe(elapsed, append(extraLabels, stepType, string(attr.GetOperation()), gvr.Group, gvr.Version, gvr.Resource, attr.GetSubresource(), strconv.FormatBool(rejected))...)
 }
 
 // ObserveWebhook records admission related metrics for a admission webhook.
-func (m *AdmissionMetrics) ObserveWebhook(elapsed time.Duration, rejected bool, hook *v1alpha1.Webhook, attr Attributes) {
+func (m *AdmissionMetrics) ObserveWebhook(elapsed time.Duration, rejected bool, attr admission.Attributes, stepType string, extraLabels ...string) {
-	t := "admit" // TODO: pass in type (validate|admit) once mutating webhook functionality has been implemented
 	gvr := attr.GetResource()
-	m.webhook.observe(elapsed, hook.Name, t, string(attr.GetOperation()), gvr.Group, gvr.Version, gvr.Resource, attr.GetSubresource(), strconv.FormatBool(rejected))
+	m.webhook.observe(elapsed, append(extraLabels, stepType, string(attr.GetOperation()), gvr.Group, gvr.Version, gvr.Resource, attr.GetSubresource(), strconv.FormatBool(rejected))...)
 }
 
 type metricSet struct {

staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go

@@ -0,0 +1,250 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package metrics
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apiserver/pkg/admission"
+)
+
+var (
+	kind     = schema.GroupVersionKind{Group: "kgroup", Version: "kversion", Kind: "kind"}
+	resource = schema.GroupVersionResource{Group: "rgroup", Version: "rversion", Resource: "resource"}
+	attr     = admission.NewAttributesRecord(nil, nil, kind, "ns", "name", resource, "subresource", admission.Create, nil)
+)
+
+func TestObserveAdmissionStep(t *testing.T) {
+	Metrics.reset()
+	handler := WithStepMetrics(&mutatingAndValidatingFakeHandler{admission.NewHandler(admission.Create), true, true})
+	handler.(admission.MutationInterface).Admit(attr)
+	handler.(admission.ValidationInterface).Validate(attr)
+	wantLabels := map[string]string{
+		"operation":   string(admission.Create),
+		"group":       resource.Group,
+		"version":     resource.Version,
+		"resource":    resource.Resource,
+		"subresource": "subresource",
+		"type":        "admit",
+		"rejected":    "false",
+	}
+	expectHistogramCountTotal(t, "apiserver_admission_step_admission_latencies_seconds", wantLabels, 1)
+	expectFindMetric(t, "apiserver_admission_step_admission_latencies_seconds_summary", wantLabels)
+
+	wantLabels["type"] = "validate"
+	expectHistogramCountTotal(t, "apiserver_admission_step_admission_latencies_seconds", wantLabels, 1)
+	expectFindMetric(t, "apiserver_admission_step_admission_latencies_seconds_summary", wantLabels)
+}
+
+func TestObserveAdmissionController(t *testing.T) {
+	Metrics.reset()
+	handler := WithControllerMetrics(&mutatingAndValidatingFakeHandler{admission.NewHandler(admission.Create), true, true}, "a")
+	handler.(admission.MutationInterface).Admit(attr)
+	handler.(admission.ValidationInterface).Validate(attr)
+	wantLabels := map[string]string{
+		"name":        "a",
+		"operation":   string(admission.Create),
+		"group":       resource.Group,
+		"version":     resource.Version,
+		"resource":    resource.Resource,
+		"subresource": "subresource",
+		"type":        "validate",
+		"rejected":    "false",
+	}
+	expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", wantLabels, 1)
+	expectFindMetric(t, "apiserver_admission_controller_admission_latencies_seconds_summary", wantLabels)
+
+	wantLabels["type"] = "validate"
+	expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", wantLabels, 1)
+	expectFindMetric(t, "apiserver_admission_controller_admission_latencies_seconds_summary", wantLabels)
+}
+
+func TestObserveWebhook(t *testing.T) {
+	Metrics.reset()
+	Metrics.ObserveWebhook(2*time.Second, false, attr, stepAdmit, "x")
+	wantLabels := map[string]string{
+		"name":        "x",
+		"operation":   string(admission.Create),
+		"group":       resource.Group,
+		"version":     resource.Version,
+		"resource":    resource.Resource,
+		"subresource": "subresource",
+		"type":        "admit",
+		"rejected":    "false",
+	}
+	expectHistogramCountTotal(t, "apiserver_admission_webhook_admission_latencies_seconds", wantLabels, 1)
+	expectFindMetric(t, "apiserver_admission_webhook_admission_latencies_seconds_summary", wantLabels)
+}
+
+func TestWithMetrics(t *testing.T) {
+	Metrics.reset()
+
+	type Test struct {
+		name            string
+		ns              string
+		operation       admission.Operation
+		handler         admission.Interface
+		admit, validate bool
+	}
+	for _, test := range []Test{
+		{
+			"both-interfaces-admit-and-validate",
+			"some-ns",
+			admission.Create,
+			&mutatingAndValidatingFakeHandler{admission.NewHandler(admission.Create, admission.Update), true, true},
+			true, true,
+		},
+		{
+			"both-interfaces-dont-admit",
+			"some-ns",
+			admission.Create,
+			&mutatingAndValidatingFakeHandler{admission.NewHandler(admission.Create, admission.Update), false, true},
+			false, true,
+		},
+		{
+			"both-interfaces-admit-dont-validate",
+			"some-ns",
+			admission.Create,
+			&mutatingAndValidatingFakeHandler{admission.NewHandler(admission.Create, admission.Update), true, false},
+			true, false,
+		},
+		{
+			"validate-interfaces-validate",
+			"some-ns",
+			admission.Create,
+			&validatingFakeHandler{admission.NewHandler(admission.Create, admission.Update), true},
+			true, true,
+		},
+		{
+			"validate-interfaces-dont-validate",
+			"some-ns",
+			admission.Create,
+			&validatingFakeHandler{admission.NewHandler(admission.Create, admission.Update), true},
+			true, false,
+		},
+		{
+			"mutating-interfaces-admit",
+			"some-ns",
+			admission.Create,
+			&mutatingFakeHandler{admission.NewHandler(admission.Create, admission.Update), true},
+			true, true,
+		},
+		{
+			"mutating-interfaces-dont-admit",
+			"some-ns",
+			admission.Create,
+			&mutatingFakeHandler{admission.NewHandler(admission.Create, admission.Update), false},
+			true, false,
+		},
+	} {
+		Metrics.reset()
+
+		h := WithMetrics(test.handler, Metrics.ObserveAdmissionController, test.name)
+
+		// test mutation
+		err := h.(admission.MutationInterface).Admit(admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, test.ns, "", schema.GroupVersionResource{}, "", test.operation, nil))
+		if test.admit && err != nil {
+			t.Errorf("expected admit to succeed, but failed: %v", err)
+			continue
+		} else if !test.admit && err == nil {
+			t.Errorf("expected admit to fail, but it succeeded")
+			continue
+		}
+
+		filter := map[string]string{"rejected": "false"}
+		if !test.admit {
+			filter["rejected"] = "true"
+		}
+		if _, mutating := test.handler.(admission.MutationInterface); mutating {
+			expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", filter, 1)
+		} else {
+			expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", filter, 0)
+		}
+
+		if err == nil {
+			// skip validation step if mutation failed
+			continue
+		}
+
+		// test validation
+		err = h.(admission.ValidationInterface).Validate(admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, test.ns, "", schema.GroupVersionResource{}, "", test.operation, nil))
+		if test.validate && err != nil {
+			t.Errorf("expected admit to succeed, but failed: %v", err)
+			continue
+		} else if !test.validate && err == nil {
+			t.Errorf("expected validation to fail, but it succeeded")
+			continue
+		}
+
+		filter = map[string]string{"rejected": "false"}
+		if !test.admit {
+			filter["rejected"] = "true"
+		}
+		if _, validating := test.handler.(admission.ValidationInterface); validating {
+			expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", filter, 1)
+		} else {
+			expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", filter, 0)
+		}
+	}
+}
+
+type mutatingAndValidatingFakeHandler struct {
+	*admission.Handler
+	admit    bool
+	validate bool
+}
+
+func (h *mutatingAndValidatingFakeHandler) Admit(a admission.Attributes) (err error) {
+	if h.admit {
+		return nil
+	}
+	return fmt.Errorf("don't admit")
+}
+
+func (h *mutatingAndValidatingFakeHandler) Validate(a admission.Attributes) (err error) {
+	if h.validate {
+		return nil
+	}
+	return fmt.Errorf("don't validate")
+}
+
+type validatingFakeHandler struct {
+	*admission.Handler
+	validate bool
+}
+
+func (h *validatingFakeHandler) Validate(a admission.Attributes) (err error) {
+	if h.validate {
+		return nil
+	}
+	return fmt.Errorf("don't validate")
+}
+
+type mutatingFakeHandler struct {
+	*admission.Handler
+	admit bool
+}
+
+func (h *mutatingFakeHandler) Amit(a admission.Attributes) (err error) {
+	if h.admit {
+		return nil
+	}
+	return fmt.Errorf("don't admit")
+}

staging/src/k8s.io/apiserver/pkg/admission/metrics/testutil_test.go

@@ -14,93 +14,15 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package admission
+package metrics
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/prometheus/client_golang/prometheus"
 	ptype "github.com/prometheus/client_model/go"
 )
 
-// FakeHandler provide a mock implement both MutationInterface and ValidationInterface that tracks which
-// methods have been called and always returns an error if admit is false.
-type FakeHandler struct {
-	*Handler
-	name           string
-	admit          bool
-	admitCalled    bool
-	validateCalled bool
-}
-
-func (h *FakeHandler) Name() string {
-	return h.name
-}
-
-func (h *FakeHandler) Admit(a Attributes) (err error) {
-	h.admitCalled = true
-	if h.admit {
-		return nil
-	}
-	return fmt.Errorf("Don't admit")
-}
-
-func (h *FakeHandler) Validate(a Attributes) (err error) {
-	h.validateCalled = true
-	if h.admit {
-		return nil
-	}
-	return fmt.Errorf("Don't admit")
-}
-
-func makeHandler(admit bool, ops ...Operation) *FakeHandler {
-	return &FakeHandler{
-		admit:   admit,
-		Handler: NewHandler(ops...),
-	}
-}
-
-func makeNamedHandler(name string, admit bool, ops ...Operation) NamedHandler {
-	return &FakeHandler{
-		name:    name,
-		admit:   admit,
-		Handler: NewHandler(ops...),
-	}
-}
-
-// FakeValidatingHandler provide a mock of ValidationInterface that tracks which
-// methods have been called and always returns an error if validate is false.
-type FakeValidatingHandler struct {
-	*Handler
-	validate, validateCalled bool
-}
-
-func (h *FakeValidatingHandler) Validate(a Attributes) (err error) {
-	h.validateCalled = true
-	if h.validate {
-		return nil
-	}
-	return fmt.Errorf("Don't validate")
-}
-
-func makeValidatingHandler(validate bool, ops ...Operation) *FakeValidatingHandler {
-	return &FakeValidatingHandler{
-		validate: validate,
-		Handler:  NewHandler(ops...),
-	}
-}
-
-func makeValidatingNamedHandler(name string, validate bool, ops ...Operation) NamedHandler {
-	return &pluginHandler{
-		Interface: &FakeValidatingHandler{
-			validate: validate,
-			Handler:  NewHandler(ops...),
-		},
-		name: name,
-	}
-}
-
 func labelsMatch(metric *ptype.Metric, labelFilter map[string]string) bool {
 	for _, lp := range metric.GetLabel() {
 		if value, ok := labelFilter[lp.GetName()]; ok && lp.GetValue() != value {

staging/src/k8s.io/apiserver/pkg/admission/metrics_test.go

@@ -1,84 +0,0 @@
-/*
-Copyright 2017 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package admission
-
-import (
-	"testing"
-	"time"
-
-	"k8s.io/api/admissionregistration/v1alpha1"
-
-	"k8s.io/apimachinery/pkg/runtime/schema"
-)
-
-var (
-	kind     = schema.GroupVersionKind{Group: "kgroup", Version: "kversion", Kind: "kind"}
-	resource = schema.GroupVersionResource{Group: "rgroup", Version: "rversion", Resource: "resource"}
-	attr     = NewAttributesRecord(nil, nil, kind, "ns", "name", resource, "subresource", Create, nil)
-)
-
-func TestObserveAdmissionStep(t *testing.T) {
-	Metrics.reset()
-	Metrics.ObserveAdmissionStep(2*time.Second, false, attr, "admit")
-	wantLabels := map[string]string{
-		"operation":   string(Create),
-		"group":       resource.Group,
-		"version":     resource.Version,
-		"resource":    resource.Resource,
-		"subresource": "subresource",
-		"type":        "admit",
-		"rejected":    "false",
-	}
-	expectHistogramCountTotal(t, "apiserver_admission_step_admission_latencies_seconds", wantLabels, 1)
-	expectFindMetric(t, "apiserver_admission_step_admission_latencies_seconds_summary", wantLabels)
-}
-
-func TestObserveAdmissionController(t *testing.T) {
-	Metrics.reset()
-	handler := makeValidatingNamedHandler("a", true, Create)
-	Metrics.ObserveAdmissionController(2*time.Second, false, handler, attr, "validate")
-	wantLabels := map[string]string{
-		"name":        "a",
-		"operation":   string(Create),
-		"group":       resource.Group,
-		"version":     resource.Version,
-		"resource":    resource.Resource,
-		"subresource": "subresource",
-		"type":        "validate",
-		"rejected":    "false",
-	}
-	expectHistogramCountTotal(t, "apiserver_admission_controller_admission_latencies_seconds", wantLabels, 1)
-	expectFindMetric(t, "apiserver_admission_controller_admission_latencies_seconds_summary", wantLabels)
-}
-
-func TestObserveWebhook(t *testing.T) {
-	Metrics.reset()
-	hook := &v1alpha1.Webhook{Name: "x"}
-	Metrics.ObserveWebhook(2*time.Second, false, hook, attr)
-	wantLabels := map[string]string{
-		"name":        "x",
-		"operation":   string(Create),
-		"group":       resource.Group,
-		"version":     resource.Version,
-		"resource":    resource.Resource,
-		"subresource": "subresource",
-		"type":        "admit",
-		"rejected":    "false",
-	}
-	expectHistogramCountTotal(t, "apiserver_admission_webhook_admission_latencies_seconds", wantLabels, 1)
-	expectFindMetric(t, "apiserver_admission_webhook_admission_latencies_seconds_summary", wantLabels)
-}

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

@@ -23,6 +23,7 @@ go_library(
         "//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/configuration:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library",
+        "//vendor/k8s.io/apiserver/pkg/admission/metrics:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/config:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/errors:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace:go_default_library",

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

@@ -39,6 +39,7 @@ import (
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/admission/configuration"
 	genericadmissioninit "k8s.io/apiserver/pkg/admission/initializer"
+	admissionmetrics "k8s.io/apiserver/pkg/admission/metrics"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook/config"
 	webhookerrors "k8s.io/apiserver/pkg/admission/plugin/webhook/errors"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook/namespace"
@@ -238,7 +239,7 @@ func (a *MutatingWebhook) Admit(attr admission.Attributes) error {
 	for _, hook := range relevantHooks {
 		t := time.Now()
 		err := a.callAttrMutatingHook(ctx, hook, versionedAttr)
-		admission.Metrics.ObserveWebhook(time.Since(t), err != nil, hook, attr)
+		admissionmetrics.Metrics.ObserveWebhook(time.Since(t), err != nil, attr, "admit", hook.Name)
 		if err == nil {
 			continue
 		}

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

@@ -21,6 +21,7 @@ go_library(
         "//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/configuration:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library",
+        "//vendor/k8s.io/apiserver/pkg/admission/metrics:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/config:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/errors:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace:go_default_library",

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

@@ -38,6 +38,7 @@ import (
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/admission/configuration"
 	genericadmissioninit "k8s.io/apiserver/pkg/admission/initializer"
+	admissionmetrics "k8s.io/apiserver/pkg/admission/metrics"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook/config"
 	webhookerrors "k8s.io/apiserver/pkg/admission/plugin/webhook/errors"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook/namespace"
@@ -238,7 +239,7 @@ func (a *ValidatingAdmissionWebhook) Admit(attr admission.Attributes) error {
 
 			t := time.Now()
 			err := a.callHook(ctx, hook, versionedAttr)
-			admission.Metrics.ObserveWebhook(time.Since(t), err != nil, hook, attr)
+			admissionmetrics.Metrics.ObserveWebhook(time.Since(t), err != nil, attr, "validating", hook.Name)
 			if err == nil {
 				return
 			}

staging/src/k8s.io/apiserver/pkg/admission/plugins.go

@@ -39,16 +39,6 @@ type Plugins struct {
 	registry map[string]Factory
 }
 
-// pluginHandler associates name with a admission.Interface handler.
-type pluginHandler struct {
-	Interface
-	name string
-}
-
-func (h *pluginHandler) Name() string {
-	return h.name
-}
-
 // All registered admission options.
 var (
 	// PluginEnabledFn checks whether a plugin is enabled.  By default, if you ask about it, it's enabled.
@@ -128,10 +118,12 @@ func splitStream(config io.Reader) (io.Reader, io.Reader, error) {
 	return bytes.NewBuffer(configBytes), bytes.NewBuffer(configBytes), nil
 }
 
+type Decorator func(handler Interface, name string) Interface
+
 // NewFromPlugins returns an admission.Interface that will enforce admission control decisions of all
 // the given plugins.
-func (ps *Plugins) NewFromPlugins(pluginNames []string, configProvider ConfigProvider, pluginInitializer PluginInitializer) (Interface, error) {
+func (ps *Plugins) NewFromPlugins(pluginNames []string, configProvider ConfigProvider, pluginInitializer PluginInitializer, decorator Decorator) (Interface, error) {
-	handlers := []NamedHandler{}
+	handlers := []Interface{}
 	for _, pluginName := range pluginNames {
 		pluginConfig, err := configProvider.ConfigFor(pluginName)
 		if err != nil {
@@ -143,8 +135,11 @@ func (ps *Plugins) NewFromPlugins(pluginNames []string, configProvider ConfigPro
 			return nil, err
 		}
 		if plugin != nil {
-			handler := &pluginHandler{Interface: plugin, name: pluginName}
+			if decorator != nil {
-			handlers = append(handlers, handler)
+				handlers = append(handlers, decorator(plugin, pluginName))
+			} else {
+				handlers = append(handlers, plugin)
+			}
 		}
 	}
 	return chainAdmissionHandler(handlers), nil

staging/src/k8s.io/apiserver/pkg/server/options/BUILD

@@ -30,6 +30,7 @@ go_library(
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library",
+        "//vendor/k8s.io/apiserver/pkg/admission/metrics:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/plugin/initialization:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating:go_default_library",

staging/src/k8s.io/apiserver/pkg/server/options/admission.go

@@ -24,6 +24,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/admission/initializer"
+	admissionmetrics "k8s.io/apiserver/pkg/admission/metrics"
 	"k8s.io/apiserver/pkg/admission/plugin/initialization"
 	"k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle"
 	mutatingwebhook "k8s.io/apiserver/pkg/admission/plugin/webhook/mutating"
@@ -105,12 +106,12 @@ func (a *AdmissionOptions) ApplyTo(
 	pluginInitializers = append(pluginInitializers, genericInitializer)
 	initializersChain = append(initializersChain, pluginInitializers...)
 
-	admissionChain, err := a.Plugins.NewFromPlugins(pluginNames, pluginsConfigProvider, initializersChain)
+	admissionChain, err := a.Plugins.NewFromPlugins(pluginNames, pluginsConfigProvider, initializersChain, admissionmetrics.WithControllerMetrics)
 	if err != nil {
 		return err
 	}
 
-	c.AdmissionControl = admissionChain
+	c.AdmissionControl = admissionmetrics.WithStepMetrics(admissionChain)
 	return nil
 }
 

staging/src/k8s.io/kube-aggregator/Godeps/Godeps.json

@@ -834,6 +834,10 @@
 			"ImportPath": "k8s.io/apiserver/pkg/admission/initializer",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
 		},
+		{
+			"ImportPath": "k8s.io/apiserver/pkg/admission/metrics",
+			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+		},
 		{
 			"ImportPath": "k8s.io/apiserver/pkg/admission/plugin/initialization",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"

staging/src/k8s.io/sample-apiserver/Godeps/Godeps.json

@@ -830,6 +830,10 @@
 			"ImportPath": "k8s.io/apiserver/pkg/admission/initializer",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
 		},
+		{
+			"ImportPath": "k8s.io/apiserver/pkg/admission/metrics",
+			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+		},
 		{
 			"ImportPath": "k8s.io/apiserver/pkg/admission/plugin/initialization",
 			"Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"