From e7db88b0b65cf685ccae804ff2d073169ed9637e Mon Sep 17 00:00:00 2001 From: Dinghua Li Date: Sat, 17 Apr 2021 00:58:11 +0000 Subject: [PATCH 1/4] Add a namespace label to admission metrics. --- .../apiserver/pkg/admission/metrics/metrics.go | 18 +++++++++--------- .../pkg/admission/metrics/metrics_test.go | 6 ++++++ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go index 82752fe08cc..a5478861d3a 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go @@ -46,7 +46,7 @@ const ( var ( // Use buckets ranging from 5 ms to 2.5 seconds (admission webhooks timeout at 30 seconds by default). - latencyBuckets = []float64{0.005, 0.025, 0.1, 0.5, 2.5} + latencyBuckets = []float64{0.005, 0.025, 0.1, 0.5, 2.5, 5.0, 10.0} latencySummaryMaxAge = 5 * time.Hour // Metrics provides access to all admission metrics. @@ -126,17 +126,17 @@ func newAdmissionMetrics() *AdmissionMetrics { // Admission metrics for a step of the admission flow. The entire admission flow is broken down into a series of steps // Each step is identified by a distinct type label value. step := newMetricSet("step", - []string{"type", "operation", "rejected"}, + []string{"type", "operation", "rejected", "namespace"}, "Admission sub-step %s, broken out for each operation and API resource and step type (validate or admit).", true) // Built-in admission controller metrics. Each admission controller is identified by name. controller := newMetricSet("controller", - []string{"name", "type", "operation", "rejected"}, + []string{"name", "type", "operation", "rejected", "namespace"}, "Admission controller %s, identified by name and broken out for each operation and API resource and type (validate or admit).", false) // Admission webhook metrics. Each webhook is identified by name. webhook := newMetricSet("webhook", - []string{"name", "type", "operation", "rejected"}, + []string{"name", "type", "operation", "rejected", "namespace"}, "Admission webhook %s, identified by name and broken out for each operation and API resource and type (validate or admit).", false) webhookRejection := metrics.NewCounterVec( @@ -147,7 +147,7 @@ func newAdmissionMetrics() *AdmissionMetrics { Help: "Admission webhook rejection count, identified by name and broken out for each admission type (validating or admit) and operation. Additional labels specify an error type (calling_webhook_error or apiserver_internal_error if an error occurred; no_error otherwise) and optionally a non-zero rejection code if the webhook rejects the request with an HTTP status code (honored by the apiserver when the code is greater or equal to 400). Codes greater than 600 are truncated to 600, to keep the metrics cardinality bounded.", StabilityLevel: metrics.ALPHA, }, - []string{"name", "type", "operation", "error_type", "rejection_code"}) + []string{"name", "type", "operation", "error_type", "rejection_code", "namespace"}) step.mustRegister() controller.mustRegister() @@ -164,17 +164,17 @@ func (m *AdmissionMetrics) reset() { // ObserveAdmissionStep records admission related metrics for a admission step, identified by step type. func (m *AdmissionMetrics) ObserveAdmissionStep(ctx context.Context, elapsed time.Duration, rejected bool, attr admission.Attributes, stepType string, extraLabels ...string) { - m.step.observe(ctx, elapsed, append(extraLabels, stepType, string(attr.GetOperation()), strconv.FormatBool(rejected))...) + m.step.observe(ctx, elapsed, append(extraLabels, stepType, string(attr.GetOperation()), strconv.FormatBool(rejected), attr.GetNamespace())...) } // ObserveAdmissionController records admission related metrics for a built-in admission controller, identified by it's plugin handler name. func (m *AdmissionMetrics) ObserveAdmissionController(ctx context.Context, elapsed time.Duration, rejected bool, attr admission.Attributes, stepType string, extraLabels ...string) { - m.controller.observe(ctx, elapsed, append(extraLabels, stepType, string(attr.GetOperation()), strconv.FormatBool(rejected))...) + m.controller.observe(ctx, elapsed, append(extraLabels, stepType, string(attr.GetOperation()), strconv.FormatBool(rejected), attr.GetNamespace())...) } // ObserveWebhook records admission related metrics for a admission webhook. func (m *AdmissionMetrics) ObserveWebhook(ctx context.Context, elapsed time.Duration, rejected bool, attr admission.Attributes, stepType string, extraLabels ...string) { - m.webhook.observe(ctx, elapsed, append(extraLabels, stepType, string(attr.GetOperation()), strconv.FormatBool(rejected))...) + m.webhook.observe(ctx, elapsed, append(extraLabels, stepType, string(attr.GetOperation()), strconv.FormatBool(rejected), attr.GetNamespace())...) } // ObserveWebhookRejection records admission related metrics for an admission webhook rejection. @@ -184,7 +184,7 @@ func (m *AdmissionMetrics) ObserveWebhookRejection(ctx context.Context, name, st if rejectionCode > 600 { rejectionCode = 600 } - m.webhookRejection.WithContext(ctx).WithLabelValues(name, stepType, operation, string(errorType), strconv.Itoa(rejectionCode)).Inc() + m.webhookRejection.WithContext(ctx).WithLabelValues(name, stepType, operation, string(errorType), strconv.Itoa(rejectionCode), attr.GetNamespace()).Inc() } type metricSet struct { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go index a20ce389b7a..6965afbd964 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go @@ -47,6 +47,7 @@ func TestObserveAdmissionStep(t *testing.T) { "operation": string(admission.Create), "type": "admit", "rejected": "false", + "namespace": "ns", } expectHistogramCountTotal(t, "apiserver_admission_step_admission_duration_seconds", wantLabels, 1) expectFindMetric(t, "apiserver_admission_step_admission_duration_seconds_summary", wantLabels) @@ -70,6 +71,7 @@ func TestObserveAdmissionController(t *testing.T) { "operation": string(admission.Create), "type": "admit", "rejected": "false", + "namespace": "ns", } expectHistogramCountTotal(t, "apiserver_admission_controller_admission_duration_seconds", wantLabels, 1) @@ -85,6 +87,7 @@ func TestObserveWebhook(t *testing.T) { "operation": string(admission.Create), "type": "admit", "rejected": "false", + "namespace": "ns", } expectHistogramCountTotal(t, "apiserver_admission_webhook_admission_duration_seconds", wantLabels, 1) } @@ -100,6 +103,7 @@ func TestObserveWebhookRejection(t *testing.T) { "type": "admit", "error_type": "no_error", "rejection_code": "500", + "namespace": "ns", } wantLabels600 := map[string]string{ "name": "x", @@ -107,6 +111,7 @@ func TestObserveWebhookRejection(t *testing.T) { "type": "admit", "error_type": "no_error", "rejection_code": "600", + "namespace": "ns", } wantLabelsCallingWebhookError := map[string]string{ "name": "x", @@ -114,6 +119,7 @@ func TestObserveWebhookRejection(t *testing.T) { "type": "validate", "error_type": "calling_webhook_error", "rejection_code": "0", + "namespace": "ns", } expectCounterValue(t, "apiserver_admission_webhook_rejection_count", wantLabels, 1) expectCounterValue(t, "apiserver_admission_webhook_rejection_count", wantLabels600, 1) From 2dbdfd0902e2625d40f338fdbb814ada63720d32 Mon Sep 17 00:00:00 2001 From: Dinghua Li Date: Sat, 17 Apr 2021 00:59:25 +0000 Subject: [PATCH 2/4] Extend the max of admission latency buckets to 10s. --- staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go index a5478861d3a..d0e65b25ed1 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go @@ -45,7 +45,7 @@ const ( ) var ( - // Use buckets ranging from 5 ms to 2.5 seconds (admission webhooks timeout at 30 seconds by default). + // Use buckets ranging from 5 ms to 10 seconds (admission webhooks timeout at 30 seconds by default). latencyBuckets = []float64{0.005, 0.025, 0.1, 0.5, 2.5, 5.0, 10.0} latencySummaryMaxAge = 5 * time.Hour From fb23e449ab680bc53fc1aae826e377c1153d51e4 Mon Sep 17 00:00:00 2001 From: Dinghua Li Date: Tue, 18 May 2021 17:42:02 +0000 Subject: [PATCH 3/4] Add attr to the argument list of ObserveWebhookRejection, and remove operation, as it is included in attr. --- .../src/k8s.io/apiserver/pkg/admission/metrics/metrics.go | 4 ++-- .../apiserver/pkg/admission/metrics/metrics_test.go | 8 ++++---- .../pkg/admission/plugin/webhook/mutating/dispatcher.go | 6 +++--- .../pkg/admission/plugin/webhook/validating/dispatcher.go | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go index d0e65b25ed1..bdbc0048fe3 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics.go @@ -178,13 +178,13 @@ func (m *AdmissionMetrics) ObserveWebhook(ctx context.Context, elapsed time.Dura } // ObserveWebhookRejection records admission related metrics for an admission webhook rejection. -func (m *AdmissionMetrics) ObserveWebhookRejection(ctx context.Context, name, stepType, operation string, errorType WebhookRejectionErrorType, rejectionCode int) { +func (m *AdmissionMetrics) ObserveWebhookRejection(ctx context.Context, name, stepType string, attr admission.Attributes, errorType WebhookRejectionErrorType, rejectionCode int) { // We truncate codes greater than 600 to keep the cardinality bounded. // This should be rarely done by a malfunctioning webhook server. if rejectionCode > 600 { rejectionCode = 600 } - m.webhookRejection.WithContext(ctx).WithLabelValues(name, stepType, operation, string(errorType), strconv.Itoa(rejectionCode), attr.GetNamespace()).Inc() + m.webhookRejection.WithContext(ctx).WithLabelValues(name, stepType, string(attr.GetOperation()), string(errorType), strconv.Itoa(rejectionCode), attr.GetNamespace()).Inc() } type metricSet struct { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go index 6965afbd964..1cddbcd89ee 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go @@ -94,9 +94,9 @@ func TestObserveWebhook(t *testing.T) { func TestObserveWebhookRejection(t *testing.T) { Metrics.reset() - Metrics.ObserveWebhookRejection(context.TODO(), "x", stepAdmit, string(admission.Create), WebhookRejectionNoError, 500) - Metrics.ObserveWebhookRejection(context.TODO(), "x", stepAdmit, string(admission.Create), WebhookRejectionNoError, 654) - Metrics.ObserveWebhookRejection(context.TODO(), "x", stepValidate, string(admission.Update), WebhookRejectionCallingWebhookError, 0) + Metrics.ObserveWebhookRejection(context.TODO(), "x", stepAdmit, attr, WebhookRejectionNoError, 500) + Metrics.ObserveWebhookRejection(context.TODO(), "x", stepAdmit, attr, WebhookRejectionNoError, 654) + Metrics.ObserveWebhookRejection(context.TODO(), "x", stepValidate, attr, WebhookRejectionCallingWebhookError, 0) wantLabels := map[string]string{ "name": "x", "operation": string(admission.Create), @@ -115,7 +115,7 @@ func TestObserveWebhookRejection(t *testing.T) { } wantLabelsCallingWebhookError := map[string]string{ "name": "x", - "operation": string(admission.Update), + "operation": string(admission.Create), "type": "validate", "error_type": "calling_webhook_error", "rejection_code": "0", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go index 0410d037859..312ff28aabb 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go @@ -142,14 +142,14 @@ func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr admission.Attrib case *webhookutil.ErrCallingWebhook: if !ignoreClientCallFailures { rejected = true - admissionmetrics.Metrics.ObserveWebhookRejection(ctx, hook.Name, "admit", string(versionedAttr.Attributes.GetOperation()), admissionmetrics.WebhookRejectionCallingWebhookError, 0) + admissionmetrics.Metrics.ObserveWebhookRejection(ctx, hook.Name, "admit", versionedAttr.Attributes, admissionmetrics.WebhookRejectionCallingWebhookError, 0) } case *webhookutil.ErrWebhookRejection: rejected = true - admissionmetrics.Metrics.ObserveWebhookRejection(ctx, hook.Name, "admit", string(versionedAttr.Attributes.GetOperation()), admissionmetrics.WebhookRejectionNoError, int(err.Status.ErrStatus.Code)) + admissionmetrics.Metrics.ObserveWebhookRejection(ctx, hook.Name, "admit", versionedAttr.Attributes, admissionmetrics.WebhookRejectionNoError, int(err.Status.ErrStatus.Code)) default: rejected = true - admissionmetrics.Metrics.ObserveWebhookRejection(ctx, hook.Name, "admit", string(versionedAttr.Attributes.GetOperation()), admissionmetrics.WebhookRejectionAPIServerInternalError, 0) + admissionmetrics.Metrics.ObserveWebhookRejection(ctx, hook.Name, "admit", versionedAttr.Attributes, admissionmetrics.WebhookRejectionAPIServerInternalError, 0) } } admissionmetrics.Metrics.ObserveWebhook(ctx, time.Since(t), rejected, versionedAttr.Attributes, "admit", hook.Name) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go index 7a24d52ce57..1c022248bfd 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go @@ -109,14 +109,14 @@ func (d *validatingDispatcher) Dispatch(ctx context.Context, attr admission.Attr case *webhookutil.ErrCallingWebhook: if !ignoreClientCallFailures { rejected = true - admissionmetrics.Metrics.ObserveWebhookRejection(ctx, hook.Name, "validating", string(versionedAttr.Attributes.GetOperation()), admissionmetrics.WebhookRejectionCallingWebhookError, 0) + admissionmetrics.Metrics.ObserveWebhookRejection(ctx, hook.Name, "validating", versionedAttr.Attributes, admissionmetrics.WebhookRejectionCallingWebhookError, 0) } case *webhookutil.ErrWebhookRejection: rejected = true - admissionmetrics.Metrics.ObserveWebhookRejection(ctx, hook.Name, "validating", string(versionedAttr.Attributes.GetOperation()), admissionmetrics.WebhookRejectionNoError, int(err.Status.ErrStatus.Code)) + admissionmetrics.Metrics.ObserveWebhookRejection(ctx, hook.Name, "validating", versionedAttr.Attributes, admissionmetrics.WebhookRejectionNoError, int(err.Status.ErrStatus.Code)) default: rejected = true - admissionmetrics.Metrics.ObserveWebhookRejection(ctx, hook.Name, "validating", string(versionedAttr.Attributes.GetOperation()), admissionmetrics.WebhookRejectionAPIServerInternalError, 0) + admissionmetrics.Metrics.ObserveWebhookRejection(ctx, hook.Name, "validating", versionedAttr.Attributes, admissionmetrics.WebhookRejectionAPIServerInternalError, 0) } } admissionmetrics.Metrics.ObserveWebhook(ctx, time.Since(t), rejected, versionedAttr.Attributes, "validating", hook.Name) From ae90e6b9a1f1e9e7704feeefa8723c15f2afa61e Mon Sep 17 00:00:00 2001 From: Dinghua Li Date: Tue, 18 May 2021 21:13:24 +0000 Subject: [PATCH 4/4] Retain the test coverage of TestObserveWebhookRejection. --- .../k8s.io/apiserver/pkg/admission/metrics/metrics_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go index 1cddbcd89ee..6fe79cd1cd1 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/metrics/metrics_test.go @@ -96,7 +96,7 @@ func TestObserveWebhookRejection(t *testing.T) { Metrics.reset() Metrics.ObserveWebhookRejection(context.TODO(), "x", stepAdmit, attr, WebhookRejectionNoError, 500) Metrics.ObserveWebhookRejection(context.TODO(), "x", stepAdmit, attr, WebhookRejectionNoError, 654) - Metrics.ObserveWebhookRejection(context.TODO(), "x", stepValidate, attr, WebhookRejectionCallingWebhookError, 0) + Metrics.ObserveWebhookRejection(context.TODO(), "x", stepValidate, admission.NewAttributesRecord(nil, nil, kind, "ns", "name", resource, "subresource", admission.Update, &metav1.UpdateOptions{}, false, nil), WebhookRejectionCallingWebhookError, 0) wantLabels := map[string]string{ "name": "x", "operation": string(admission.Create), @@ -115,7 +115,7 @@ func TestObserveWebhookRejection(t *testing.T) { } wantLabelsCallingWebhookError := map[string]string{ "name": "x", - "operation": string(admission.Create), + "operation": string(admission.Update), "type": "validate", "error_type": "calling_webhook_error", "rejection_code": "0",